Skip to content
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

testlib: when provisioning VMs allow to keep the original machine_class #19510

Merged

Conversation

KKoukiou
Copy link
Contributor

This use case is becoming now valid in the anaconda repository, where we overwrite the machine_class for running the VMs from ISO, however we will need to start provisioning 'normal' cloud VMs, to act as servers for network storage; see iscsi.

test/common/testlib.py Outdated Show resolved Hide resolved
@KKoukiou KKoukiou force-pushed the testlib-provision-no-machine_class branch from c4aea73 to 54b32e0 Compare October 20, 2023 07:15
@KKoukiou
Copy link
Contributor Author

@martinpitt re-check please :) ?

Comment on lines 1280 to 1283
if inherit_machine_class:
machine_class = inherit_machine_class and self.machine_class or testvm.VirtMachine
else:
machine_class = testvm.VirtMachine
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please drop the if: and else: branch. Line 1281 is sufficient to cover all cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops thanks.

This use case is becoming now valid in the anaconda repository, where we
overwrite the machine_class for running the VMs from ISO, however we
will need to start provisioning 'normal' cloud VMs, to act as servers
for network storage; see iscsi.
@KKoukiou KKoukiou force-pushed the testlib-provision-no-machine_class branch from 54b32e0 to 5774a3f Compare October 24, 2023 06:37
Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Epharisto! Still a bit of an ad-hoc API, but we can change this in the future.

@KKoukiou
Copy link
Contributor Author

Epharisto! Still a bit of an ad-hoc API, but we can change this in the future.

Ideally I would like to be able to provision only VMs as extra, and let the the default VM that the test spawns out of the provision list.
This will allow me to have non-destructive tests which provision helper VMs and also we could obsolete this ad-hoc option, as there is not use-case yet where provisioned machines need to have a different parent class.

I did not see this as straightforward therefore I went with this easy alternative.

@martinpitt martinpitt merged commit 7baca28 into cockpit-project:main Oct 24, 2023
98 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants