[vm_set]: Add fallback default for VM_targets when VMs are not required#23361
Merged
auspham merged 1 commit intosonic-net:masterfrom Mar 27, 2026
Merged
Conversation
When ptf_imagename is 'docker-keysight-api-server', vm_required is set to false, which skips the block that defines VM_targets. However, remove_topo.yml and add_topo.yml reference VM_targets unconditionally, causing 'VM_targets is undefined' errors during topology teardown. Add a top-level fallback that sets VM_targets to an empty list when it is not defined. Signed-off-by: Edi Wibowo <[email protected]>
Collaborator
|
/azp run |
github-actions
bot
requested review from
lolyu,
wangxin and
yutongzhang-microsoft
sdszhang
approved these changes
Mar 26, 2026
auspham
approved these changes
Mar 26, 2026
wangxinbot
approved these changes
Mar 27, 2026
wangxin
approved these changes
Mar 27, 2026
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
ediwibowo-msft
added
Request for 202311 branch
Request for 202511 branch
Contributor
Author
|
ADO PBI# 37299108 |
mssonicbld
pushed a commit
to mssonicbld/sonic-mgmt
that referenced
this pull request
Mar 27, 2026
…ed (sonic-net#23361) When ptf_imagename is 'docker-keysight-api-server', vm_required is set to false, which skips the block that defines VM_targets. However, remove_topo.yml and add_topo.yml reference VM_targets unconditionally, causing 'VM_targets is undefined' errors during topology teardown. Add a top-level fallback that sets VM_targets to an empty list when it is not defined. Signed-off-by: Edi Wibowo <[email protected]> Signed-off-by: mssonicbld <[email protected]>
Collaborator
|
Cherry-pick PR to 202511: #23377 |
Open
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of PR
Summary:
When
ptf_imagenameisdocker-keysight-api-server,vm_requiredis set tofalse, which skips the block that definesVM_targets. However,remove_topo.ymlandadd_topo.ymlreferenceVM_targetsunconditionally, causingVM_targets is undefinederrors during topology teardown.This change adds a top-level fallback that sets
VM_targetsto an empty list when it is not defined.Type of change
Back port request
Approach
What is the motivation for this PR?
Topology setup and teardown should not fail when VMs are intentionally not required. For the
docker-keysight-api-serverPTF image, VM creation is skipped by design, but downstream tasks still assumeVM_targetsexists. That leads to avoidable teardown failures caused by an undefined variable rather than a real topology issue.How did you do it?
Added a top-level fallback in
ansible/roles/vm_set/tasks/main.ymlto initializeVM_targetsto an empty list when it is not defined. This matches the existing defensive pattern already used fordpu_targetsand allowsadd_topo.ymlandremove_topo.ymlto evaluate safely even when VM setup is skipped.How did you verify/test it?
Verified the change by reviewing the execution flow for the case where
ptf_imagename == 'docker-keysight-api-server'and confirming that:vm_requiredcan befalseVM_targetsis still defined afterwardVM_targets is undefinedAny platform specific information?
This change is primarily relevant to environments using
docker-keysight-api-server, where VMs are not required. It is otherwise low risk because it only provides a default empty list whenVM_targetswas previously undefined.Supported testbed topology if it's a new test case?
Not applicable. This is not a new test case.
Documentation
No documentation update is required for this bug fix.