oVirt: remove tmp template VM on bootstrap destroy phase #3949
Conversation
Gal-Zaidman
changed the title
|
nice patch @Gal-Zaidman |
|
/lgtm |
|
verified locally as well. Worked like a charm, waiting oVirt CI pass for merge. |
|
@openshift-ci-robot |
|
/approve |
|
/assign @abhinavdahiya |
|
I do not understand why you need to do this? Can you please explain why you need additional step in deleting bootstrap.. outside of just deleting the bootstrap module |
Sure, On ovirt before the bootstrap, masters and workers start they need to be created from a template. |
Gal-Zaidman
force-pushed
the
remove-tmp-template-vm-datasource
branch
from
6f26961
to
3c59625
Compare
|
Let's make sure this ends up in the commit message and PR description. Secondly, other platforms that need it using a variable to provide the API so that this is not tied to resource name like it is right now but more to an API provided by the tf templates. |
This patch breaks the dependency between terraform resources: releaseimage_template and tmp_import_vm On ovirt before the bootstrap, masters and workers start they need to be created from a template. The installer creates this template with the terraform template module. As part of the process for creating the template, we create an oVirt tmp VM which should be sealed into a template for the bootstrap, masters and workers machines and then removed. Until this PR the tmp VM will just stay in the oVirt cluster until cluster destroy is called, this is not good because it is just garbage in the oVirt cluster that the user will probably remove on its own once the installation is complete. This PR makes bootstrap destroy command to destroy the tmp VM as well, which make sense because it is created for bootstrapping the cluster. Signed-off-by: Gal-Zaidman <gzaidman@redhat.com>
Gal-Zaidman
force-pushed
the
remove-tmp-template-vm-datasource
branch
from
3c59625
to
79c7425
Compare
Edited the commit message and the PR description
I guess we can do something similar in the future but for now, I think this PR fixes the issue according to our implementation. |
|
/retest |
|
/lgtm @abhinavdahiya is there a better way other than the usage of |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavdahiya, dougsland The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@Gal-Zaidman: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
This patch breaks the dependency between terraform resources:
releaseimage_template and tmp_import_vm
On ovirt before the bootstrap, masters and workers start they need to be created from a template.
The installer creates this template with the terraform template module.
As part of the process for creating the template, we create an oVirt tmp VM which should be sealed into a template for the bootstrap, masters and workers machines and then removed.
Until this PR the tmp VM will just stay in the oVirt cluster until cluster destroy is called, this is not good because it is just garbage in the oVirt cluster that the user will probably remove on its own once the installation is complete.
This PR makes bootstrap destroy command to destroy the tmp VM as well, which makes sense because it is created for bootstrapping the cluster.
Signed-off-by: Gal-Zaidman gzaidman@redhat.com