-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make SONiC the default flavor. #158
Conversation
@Gerrit91 sonic-vs.img is missing on the integration platform. |
8b098a4
to
31dd55c
Compare
@Gerrit91 What is your opinion to use both flavors for cleanup? The leaves of one flavor cannot be removed by the other flavor.
|
Not yet decided, maybe we can just destroy for all flavors in cleanup. |
Makefile
Outdated
|
||
# destroying the sonic lab requires the image to exist, otherwise it fails with bind path verification | ||
touch sonic-vs.img | ||
sudo $(CONTAINERLAB) destroy --topo mini-lab.sonic.yaml | ||
rm -f sonic-vs.img |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a conditional?
ifneq ($(wildcard sonic-vs.img),)
sudo $(CONTAINERLAB) destroy --topo mini-lab.sonic.yaml
endif
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this has the issue that on repeated run of the CI job on the same runner after checkout the image file will not be present but the SONiC VMs might still be running.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe its easier to create a container image that contains the VM image. Let me try something next week.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you come up with something better? Release pipeline is currently broken due to cleanup problems so we should find something soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will look into it during my journey.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We prioritized fixing the pipeline for now. If you find some time to improve the cleanup please feel free to raise a new pull request.
Closes #154.