Add v2v cases about virt-customize options #6618
Conversation
WalkthroughAdded ESX-80 virt_customize test scenarios and Python helpers. New config blocks define Windows and RHEL virt_customize variants (firstboot, upload, command, file, package, permission checkpoints). Test runner now builds new v2v virt-customize options and invokes three new helper checks; static IP check signature changed to accept extracted IP config and MAC. Changes
Sequence Diagram(s)sequenceDiagram
participant CFG as Config (`function_test_esx.cfg`)
participant Runner as Test Runner
participant Builder as v2v Command Builder
participant Helpers as virt_customize Helpers
participant VM as VM / VMCheck
CFG->>Runner: define checkpoints (firstboot_*, upload, pkg_related, file_related, permission_related)
Runner->>Builder: assemble v2v options based on checkpoints
Builder-->>Runner: v2v command with virt-customize options (--upload, --firstboot, --firstboot-command, file/pkg/perm ops)
Runner->>VM: run migration & checkpoints
alt pkg_related present
Runner->>Helpers: virt_customize_pkg_related(vmcheck)
Helpers->>VM: verify package install/run artifacts
end
alt file_related present
Runner->>Helpers: virt_customize_file_related(vmcheck)
Helpers->>VM: verify ssh-inject, file create/edit/scrub
end
alt permission_related present
Runner->>Helpers: virt_customize_permission_related(vmcheck)
Helpers->>VM: verify password/chown effects
end
Runner->>Runner: final VM checks
Runner->>Runner: call check_static_ip_conf(ip_config_list, mac_addr, vmcheck)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
(1/1) type_specific.io-github-autotest-libvirt.function_test_esx.positive_test.windows.mac_ip.3_static_ip.win10.esx_80.libvirt.it_vddk.vpx_uri: PASS (1523.64 s) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
v2v/tests/src/function_test_esx.py (1)
189-197: Add docstring and consider more robust permission checks.Two concerns:
- The function lacks a docstring.
- The regex patterns are fragile:
- Line 192:
r'Access.*0755.*Uid.*1.* Gid.*1.*bin'relies on specific output format and spacing- Line 196:
r'\$5\$'hardcodes the SHA-256 crypt algorithm, but line 1025 setspassword-crypto sha256which should produce$5$Add docstring and consider more defensive checks:
def virt_customize_permission_related(vmcheck): + """ + Check virt-customize permission-related operations (chown, root-password, password-crypto). + + :param vmcheck: VMCheck object for vm checking + """ content1 = vmcheck.session.cmd('stat /home') LOG.info('content1 is %s', content1) - if not re.search(r'Access.*0755.*Uid.*1.* Gid.*1.*bin', content1): + # Check ownership (Uid: 1/bin, Gid: 1/bin) and permissions (0755) + if not (re.search(r'Uid:\s*\(\s*1/\s*bin\s*\)', content1) and + re.search(r'Gid:\s*\(\s*1/\s*bin\s*\)', content1) and + re.search(r'Access:.*0755', content1)): test.error('Fail to test virt-customize chown option') content2 = vmcheck.session.cmd('cat /etc/shadow |grep root') LOG.info('content2 is %s', content2) if not re.search(r'\$5\$', content2): test.error('Fail to test virt-customize password-crypto option')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
v2v/tests/cfg/function_test_esx.cfg(2 hunks)v2v/tests/src/function_test_esx.py(5 hunks)
🧰 Additional context used
🪛 Ruff (0.14.0)
v2v/tests/src/function_test_esx.py
158-158: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
1023-1023: Local variable password is assigned to but never used
Remove assignment to unused variable password
(F841)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Python 3.12
- GitHub Check: Python 3.8
- GitHub Check: Python 3.9
- GitHub Check: Python 3.11
🔇 Additional comments (5)
v2v/tests/src/function_test_esx.py (3)
445-445: Function signature change looks correct.The updated signature
check_static_ip_conf(ip_config_list, mac_addr, vmcheck)properly extracts these parameters earlier in the workflow (lines 664-668 and 671-672), making the function more focused and testable.
663-679: LGTM - Checkpoint handling is well structured.The code properly extracts
ip_config_listandmac_addrfrom either the v2v command or params before callingcheck_static_ip_conf(). The conditional logic correctly handles different firstboot checkpoint variants.
688-693: LGTM - Function invocations are correct.The checkpoint-based invocation of the new virt_customize_* functions follows the existing pattern in the codebase.
v2v/tests/cfg/function_test_esx.cfg (2)
207-220: LGTM - Windows virt_customize test block is well structured.The configuration properly defines two firstboot variants and uses placeholder variables that should be defined elsewhere. The checkpoint names align with the Python test code.
311-325: Verify hardcoded password is test-only.Line 325 sets
password = 123456, which matches the hardcoded password in line 1024 of function_test_esx.py. Ensure this is only used in test environments and not in production.The rest of the configuration is well structured with proper variant organization.
| def virt_customize_pkg_related(vmcheck): | ||
| def pkg_output(): | ||
| for i in range(3): | ||
| try: | ||
| output = vmcheck.session.cmd('rpm -q libvirt virt-v2v nfs-utils kernel') | ||
| except ShellStatusError as e: | ||
| LOG.warning("Caught expected error from rpm command: %s", e) | ||
| output = e.output | ||
| LOG.debug(output) | ||
| if 'not installed' in output: | ||
| time.sleep(10) | ||
| else: | ||
| return output | ||
| return output | ||
|
|
||
| pkg_output = pkg_output() | ||
| if re.search(r'not installed', pkg_output): | ||
| test.error('expected package are not installed') | ||
| if not re.search(r'kernel.*.el9_7', pkg_output): | ||
| test.error('fail to test virt-customise update option') |
There was a problem hiding this comment.
Address function naming and loop variable issues.
Several issues in this function:
- The nested function
pkg_output()(line 157) shadows the variable assignment on line 171, making the code confusing. - Loop variable
iat line 158 is not used within the loop body - rename it to_to indicate it's intentional. - The kernel version check on line 174 is hardcoded to
el9_7, which will break when testing against different kernel versions.
Apply this diff to fix the naming conflict and loop variable:
- def virt_customize_pkg_related(vmcheck):
- def pkg_output():
+def virt_customize_pkg_related(vmcheck):
+ def get_pkg_output():
- for i in range(3):
+ for _ in range(3):
try:
output = vmcheck.session.cmd('rpm -q libvirt virt-v2v nfs-utils kernel')
except ShellStatusError as e:
LOG.warning("Caught expected error from rpm command: %s", e)
output = e.output
LOG.debug(output)
if 'not installed' in output:
time.sleep(10)
else:
return output
return output
- pkg_output = pkg_output()
+ pkg_output = get_pkg_output()
if re.search(r'not installed', pkg_output):
test.error('expected package are not installed')
if not re.search(r'kernel.*.el9_7', pkg_output):
test.error('fail to test virt-customise update option')For the hardcoded kernel version, consider parameterizing it or using a more flexible pattern:
if not re.search(r'kernel.*\.el\d+_\d+', pkg_output):
test.error('fail to test virt-customize update option')🧰 Tools
🪛 Ruff (0.14.0)
158-158: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
🤖 Prompt for AI Agents
In v2v/tests/src/function_test_esx.py around lines 156 to 175, the nested
function pkg_output() shadows the later variable assignment and the loop uses an
unused variable i, and the kernel version check is hardcoded to el9_7; rename
the inner function to e.g. get_pkg_output to avoid shadowing and then assign its
result to pkg_output, change the loop variable i to _ to indicate it is unused,
and replace the hardcoded kernel regex with a flexible one such as matching
kernel.*\.el\d+_\d+ (or accept a passed-in expected kernel pattern) so the
version check works across kernel versions.
| if 'virt_customize_permission_related' in checkpoint: | ||
| password = params_get(params, 'password') | ||
| v2v_params['v2v_opts'] += " --root-password password:123456" \ | ||
| " --password-crypto sha256" \ | ||
| " --chown '1:1:/home'" |
There was a problem hiding this comment.
Remove unused variable.
Line 1023 assigns password but never uses it, as confirmed by static analysis.
Apply this diff:
if 'virt_customize_permission_related' in checkpoint:
- password = params_get(params, 'password')
v2v_params['v2v_opts'] += " --root-password password:123456" \
" --password-crypto sha256" \
" --chown '1:1:/home'"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if 'virt_customize_permission_related' in checkpoint: | |
| password = params_get(params, 'password') | |
| v2v_params['v2v_opts'] += " --root-password password:123456" \ | |
| " --password-crypto sha256" \ | |
| " --chown '1:1:/home'" | |
| if 'virt_customize_permission_related' in checkpoint: | |
| v2v_params['v2v_opts'] += " --root-password password:123456" \ | |
| " --password-crypto sha256" \ | |
| " --chown '1:1:/home'" |
🧰 Tools
🪛 Ruff (0.14.0)
1023-1023: Local variable password is assigned to but never used
Remove assignment to unused variable password
(F841)
🤖 Prompt for AI Agents
v2v/tests/src/function_test_esx.py around lines 1022-1026: the variable
`password` is assigned via `params_get(params, 'password')` but never used;
remove that unused assignment line (delete the `password = ...` statement) so
there is no dead variable left in this block and leave the existing v2v_params
update as-is.
There was a problem hiding this comment.
Please check this one. If password is not used, you can remove this line.
There was a problem hiding this comment.
@xiaodwan, you're correct! The password variable on line 1023 is assigned but never used. The code hardcodes "password:123456" directly in the v2v_params options string instead of using the variable.
The line should be removed as suggested by CodeRabbit. The corrected code would be:
if 'virt_customize_permission_related' in checkpoint:
v2v_params['v2v_opts'] += " --root-password password:123456" \
" --password-crypto sha256" \
" --chown '1:1:/home'"This eliminates the dead code while keeping the functionality intact.
mxie91
force-pushed
the
auto-v2v-case-about-virt-customize-options
branch
from
448ccc9 to
5abd38b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
v2v/tests/src/function_test_esx.py (3)
156-179: Address previously flagged issues: function naming, loop variable, and hardcoded kernel version.The issues identified in the previous review remain unresolved:
- Line 161: The nested function
pkg_output()shadows the variable assignment on line 175, creating confusion.- Line 162: Loop variable
iis unused within the loop body.- Line 178: The hardcoded kernel version pattern
el9_7will fail when testing against different kernel versions.Apply this diff to address all three issues:
def virt_customize_pkg_related(vmcheck): """ Check virt-customize package related options (firstboot-install, run, run-command, update). :param vmcheck: VMCheck object for vm checking """ - def pkg_output(): - for i in range(3): + def get_pkg_output(): + for _ in range(3): try: output = vmcheck.session.cmd('rpm -q libvirt virt-v2v nfs-utils kernel') except ShellStatusError as e: LOG.warning("Caught expected error from rpm command: %s", e) output = e.output LOG.debug(output) if 'not installed' in output: time.sleep(10) else: return output return output - pkg_output = pkg_output() + pkg_output = get_pkg_output() if re.search(r'not installed', pkg_output): test.error('expected package are not installed') - if not re.search(r'kernel.*.el9_7', pkg_output): + if not re.search(r'kernel.*\.el\d+_\d+', pkg_output): test.error('fail to test virt-customise update option')
1022-1033: Fix shell script formatting issues flagged in previous review.The shell script formatting issues identified previously remain unresolved:
- Leading newline: The triple-quoted string starts with a newline.
- Incorrect shebang: Line 1024 has
# !/bin/bash(space after#) instead of#!/bin/bash.- Indentation: The excessive indentation will be written into the script file.
Apply this diff:
if 'virt_customize_pkg_related' in checkpoint: - lines = """ -# !/bin/bash -dnf -y install libvirt -""" + lines = """\ +#!/bin/bash +dnf -y install libvirt +""" script_path = os.path.join(data_dir.get_tmp_dir(), "test.sh") with open(script_path, "w") as f: f.write(lines)
1034-1038: Remove unused variable flagged in previous review.The unused
passwordvariable on line 1035 should be removed as identified in the previous review and confirmed by static analysis.Apply this diff:
if 'virt_customize_permission_related' in checkpoint: - password = params_get(params, 'password') v2v_params['v2v_opts'] += " --root-password password:123456" \ " --password-crypto sha256" \ " --chown '1:1:/home'"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
v2v/tests/cfg/function_test_esx.cfg(2 hunks)v2v/tests/src/function_test_esx.py(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- v2v/tests/cfg/function_test_esx.cfg
🧰 Additional context used
🪛 Ruff (0.14.0)
v2v/tests/src/function_test_esx.py
162-162: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
1035-1035: Local variable password is assigned to but never used
Remove assignment to unused variable password
(F841)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Python 3.9
- GitHub Check: Python 3.8
🔇 Additional comments (4)
v2v/tests/src/function_test_esx.py (4)
180-196: LGTM! Previously flagged issues have been resolved.The docstring has been added and the whitespace issue has been fixed. The function correctly validates virt-customize file operations (ssh-inject, touch, write, edit, scrub).
457-457: LGTM! Function signature refactoring is clean.The signature change for
check_static_ip_confimproves clarity by explicitly passing extracted parameters. The new virt_customize checkpoint handling is well-integrated into the VM check flow.Also applies to: 675-691, 700-705
1006-1014: LGTM! Typo fixed and quoting is correct.The previously flagged typo (
nework→network) has been corrected on line 1013. The quoting strategy (single quotes wrapping the command containing double quotes) is appropriate.
1015-1021: LGTM! File-related options are correctly structured.The virt-customize file operations (ssh-inject, touch, write, scrub, edit) are properly formatted and the regex pattern in the edit command is appropriate.
| def virt_customize_permission_related(vmcheck): | ||
| """ | ||
| Check virt-customize permission related options (root-password, password-crypto, chwon). | ||
| :param vmcheck: VMCheck object for vm checking | ||
| """ | ||
| content1 = vmcheck.session.cmd('stat /home') | ||
| LOG.info('content1 is %s', content1) | ||
| if not re.search(r'Access.*0755.*Uid.*1.* Gid.*1.*bin', content1): | ||
| test.error('Fail to test virt-customize chown option') | ||
| content2 = vmcheck.session.cmd('cat /etc/shadow |grep root') | ||
| LOG.info('content2 is %s', content2) | ||
| if not re.search(r'\$5\$', content2): | ||
| test.error('Fail to test virt-customize password-crypto option') |
There was a problem hiding this comment.
Fix typo in docstring.
Line 199 has a typo: "chwon" should be "chown".
Apply this diff:
def virt_customize_permission_related(vmcheck):
"""
- Check virt-customize permission related options (root-password, password-crypto, chwon).
+ Check virt-customize permission related options (root-password, password-crypto, chown).
:param vmcheck: VMCheck object for vm checking
"""🤖 Prompt for AI Agents
In v2v/tests/src/function_test_esx.py around lines 197 to 209, the docstring
contains a typo: "chwon" should be corrected to "chown"; update the docstring
line to read "Check virt-customize permission related options (root-password,
password-crypto, chown)." and keep the rest of the docstring and function body
unchanged.
There was a problem hiding this comment.
Please update the typo issue. @mxie91
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Signed-off-by: Ming Xie <[email protected]>
mxie91
force-pushed
the
auto-v2v-case-about-virt-customize-options
branch
from
5abd38b to
cb9b21d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
v2v/tests/src/function_test_esx.py (4)
156-179: Resolve shadowing, unused loop var, and brittle kernel regex.Inner function name conflicts with later variable; loop var unused; kernel regex hardcodes el9_7 and will break on version bumps. Apply:
- def pkg_output(): - for i in range(3): + def get_pkg_output(): + for _ in range(3): try: output = vmcheck.session.cmd('rpm -q libvirt virt-v2v nfs-utils kernel') except ShellStatusError as e: LOG.warning("Caught expected error from rpm command: %s", e) output = e.output LOG.debug(output) if 'not installed' in output: time.sleep(10) else: return output return output - pkg_output = pkg_output() + pkg_output = get_pkg_output() if re.search(r'not installed', pkg_output): - test.error('expected package are not installed') - if not re.search(r'kernel.*.el9_7', pkg_output): - test.error('fail to test virt-customise update option') + test.error('expected packages are not installed') + # Allow override; default matches elX_Y across RHEL minors. + expected_kernel_regex = params_get(params, 'expected_kernel_regex', r'kernel.*\.el\d+_\d+') + if not re.search(expected_kernel_regex, pkg_output): + test.error('fail to test virt-customize update option (kernel regex mismatch)')
199-203: Fix typo in docstring ("chwon" → "chown").Small doc text correction.
- Check virt-customize permission related options (root-password, password-crypto, chwon). + Check virt-customize permission related options (root-password, password-crypto, chown).
1026-1036: Fix shebang and indentation in generated shell script.Leading newline and malformed shebang ("# !/bin/bash") will be written verbatim; use a clean, left‑aligned script.
- lines = """ -# !/bin/bash -dnf -y install libvirt -""" + lines = """\ +#!/bin/bash +dnf -y install libvirt +"""
1038-1041: Remove unused variable.
passwordis assigned but never used. Clean it up.- password = params_get(params, 'password') v2v_params['v2v_opts'] += " --root-password password:123456" \ " --password-crypto sha256" \ " --chown '1:1:/home'"
🧹 Nitpick comments (3)
v2v/tests/src/function_test_esx.py (3)
460-530: Harden static-IP check and document new parameters.Add param docs; relax adapter name regex; accept either CIDR prefix or dotted mask to avoid ValueError.
-def check_static_ip_conf(ip_config_list, mac_addr, vmcheck): +def check_static_ip_conf(ip_config_list, mac_addr, vmcheck): """ Check static IP configuration in VM - :param vmcheck: VMCheck object for vm checking + :param ip_config_list: 'ip,gw,prefix[,dns1[,dns2...]]' or dotted mask (e.g. 'ip,gw,255.255.255.0,...') + :param mac_addr: MAC address as shown by ipconfig (e.g. 00-50-56-AC-7A-4D) + :param vmcheck: VMCheck object for vm checking """ def _static_ip_check(): cmd = 'ipconfig /all' _, output = vmcheck.run_cmd(cmd, debug=False) - eth_adapter_ptn = r'Ethernet adapter Ethernet.*?NetBIOS over Tcpip' + # Match any Ethernet adapter block to avoid flakiness when interface is renamed + eth_adapter_ptn = r'Ethernet adapter .*?NetBIOS over Tcpip' try: ipconfig = [ v for v in re.findall( eth_adapter_ptn, - output, - re.S) if mac_addr in v][0] + output, + re.S | re.I) if mac_addr in v][0] except IndexError: return False for i, value in enumerate(ip_config_list.split(',')): if not value: continue @@ - if i == 2: - # convert subnet mask to cidr - bin_mask = '1' * int(value) + '0' * (32 - int(value)) - cidr = '.'.join( - [str(int(bin_mask[i * 8:i * 8 + 8], 2)) for i in range(4)]) - sub_mask = r'Subnet Mask.*?: %s' % cidr + if i == 2: + # Accept either CIDR prefix length or dotted mask + try: + if '.' in value: + mask = value + else: + prefix = int(value) + if prefix < 0 or prefix > 32: + return False + bits = '1' * prefix + '0' * (32 - prefix) + mask = '.'.join(str(int(bits[j*8:(j+1)*8], 2)) for j in range(4)) + except ValueError: + LOG.debug('Invalid subnet mask value: %s', value) + return False + sub_mask = r'Subnet Mask.*?: %s' % mask LOG.info('static subnet mask: %s', re.search(sub_mask, ipconfig, re.S)) if not re.search(sub_mask, ipconfig, re.S): LOG.debug('Found subnet mask failed') return False
678-685: Make MAC/IP extraction regex robust and guard None.Avoid trailing-colon hack; accept standard MAC; handle parse failures.
- mac_ip_pattern = '--mac (([0-9a-zA-Z]{2}:){6})ip:([0-9,.]+)' - ip_config_list = re.search(mac_ip_pattern, v2v_cmd).group(3) - mac_addr = re.search(mac_ip_pattern, v2v_cmd).group(1)[ - 0:-1].upper().replace(':', '-') + mac_ip_pattern = r'--mac\s+(([0-9A-Fa-f]{2}:){5}[0-9A-Fa-f]{2}):ip:([0-9.,]+)' + m = re.search(mac_ip_pattern, v2v_cmd) + if not m: + test.error('Failed to parse --mac ip config from v2v_command') + ip_config_list = m.group(3) + mac_addr = m.group(1).upper().replace(':', '-')
1016-1018: Prefer double quotes around firstboot-command payload.Reduces risk if the command ever contains single quotes. Functional today, but safer consistently.
- v2v_params['v2v_opts'] += " --firstboot-command '%s'" % network_configure_cmd + v2v_params['v2v_opts'] += ' --firstboot-command "%s"' % network_configure_cmd
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
v2v/tests/cfg/function_test_esx.cfg(2 hunks)v2v/tests/src/function_test_esx.py(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- v2v/tests/cfg/function_test_esx.cfg
🧰 Additional context used
🪛 Ruff (0.14.0)
v2v/tests/src/function_test_esx.py
162-162: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
1038-1038: Local variable password is assigned to but never used
Remove assignment to unused variable password
(F841)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Python 3.8
- GitHub Check: Python 3.11
- GitHub Check: Python 3.9
- GitHub Check: Python 3.12
🔇 Additional comments (5)
v2v/tests/src/function_test_esx.py (5)
181-198: LGTM: file-related checks.Docstring and whitespace look good; assertions read clearly.
686-695: LGTM: firstboot + upload flow.Static-IP check reuse looks correct; upload verification matches later dir listing.
703-709: LGTM: helper checks wiring.New virt-customize helper invocations are correctly gated by checkpoint.
1010-1015: LGTM: generate and upload firstboot script.Script is written to tmp and used by both --upload and --firstboot as expected.
1019-1024: LGTM: file-related virt-customize options.Option construction is coherent and matches checks.
|
@xiaodwan pls help review, thanks! |
| v2v_opts = VM_NAME_WIN2025_1_STATIC_IP_MAC_CONF_V2V_EXAMPLE | ||
| - virt_customize: | ||
| only esx_80 | ||
| #Test firsboot(windows), firstboot-command, firstboot-install, ssh-inject, root-password, |
There was a problem hiding this comment.
A space is needed between "#" and the firsht word.
| variants: | ||
| - file_related: | ||
| #Test ssh-inject, touch, write, scrub, edit | ||
| checkpoint = "virt_customize_file_related" |
There was a problem hiding this comment.
The indentation is not 4 spaces.
| for i in range(3): | ||
| try: | ||
| output = vmcheck.session.cmd('rpm -q libvirt virt-v2v nfs-utils kernel') | ||
| except ShellStatusError as e: |
There was a problem hiding this comment.
You didn't do any necessary operattions to the exception. I think you can just add "ignore_all_errors" in session.cmd to ignore the exception. Then in your next check, you can do
if output and 'not installed' in output:
xxxx
| return output | ||
| return output | ||
|
|
||
| pkg_output = pkg_output() |
There was a problem hiding this comment.
don't use the same name as the function.
| pkg_output = pkg_output() | ||
| if re.search(r'not installed', pkg_output): | ||
| test.error('expected package are not installed') | ||
| if not re.search(r'kernel.*.el9_7', pkg_output): |
There was a problem hiding this comment.
As AI mentions here, the el9_7 is hard coded, it may fail in future.
| :param vmcheck: VMCheck object for vm checking | ||
| """ | ||
| def pkg_output(): | ||
| for i in range(3): |
There was a problem hiding this comment.
update this line per AI's suggestion please.
| """ | ||
| content1 = vmcheck.session.cmd('cat /root/.ssh/authorized_keys') | ||
| LOG.info('content1 is %s', content1) | ||
| if not re.search(r'AAtesttesttest', content1): |
There was a problem hiding this comment.
The 'AAtesttesttest' may be changed if the VM is installed in future, it's better to set this special string in ci-share-data.
| LOG.info('content1 is %s', content1) | ||
| if not re.search(r'AAtesttesttest', content1): | ||
| test.error('Fail to test virt-customize ssh-inject option') | ||
| content2 = vmcheck.session.cmd(r'cat /home/testfile1 |grep editoption || true') |
There was a problem hiding this comment.
you can use parameter ignore_all_errors to avoid raising exceptions. Don't need the "|| true" part.
2 participants
Summary by CodeRabbit