Bug 2073398: Fix InstanceCreate port & trunk cleanup #232
Conversation
openshift-ci
bot
added
the
bugzilla/severity-medium
openshift-ci
bot
added
the
bugzilla/invalid-bug
|
@EmilienM: This pull request references Bugzilla bug 2073398, which is invalid:
Comment DetailsIn response to this:
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. |
|
/bugzilla refresh |
openshift-ci
bot
added
bugzilla/valid-bug
|
@EmilienM: This pull request references Bugzilla bug 2073398, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
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. |
|
@openshift-ci[bot]: GitHub didn't allow me to request PR reviews from the following users: eurijon. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn response to this:
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. |
mdbooth
left a comment
mdbooth
left a comment
There was a problem hiding this comment.
Approach looks good to me, and is consistent with volume cleanup in this repo, and port and trunk cleanup in upstream CAPO.
Unfortunately, unlike upstream, we don't have any test coverage which can exercise this. It would good to get at least a manual test. I'm not sure it's worth adding downstream tests just for this given that:
- MAPO (4.11+) has this test coverage
- Adding this test coverage was extremely invasive upstream
- It's not super-critical
|
For manual test run: Feel free to remove this when it's tested. |
openshift-ci
bot
added
the
do-not-merge/hold
With this change, the port and trunk cleanups are triggered by any early failure of InstanceCreate. When InstanceCreate creates ports and if it creates trunks, these resources will be deleted if any of the next steps fail and cause the server never to be created. Note: the trunk will be deleted first, then the ports.
|
/retest |
|
@EmilienM: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
/cherry-pick release-4.10 |
|
@EmilienM: once the present PR merges, I will cherry-pick it on top of release-4.10 in a new PR and assign it to you. DetailsIn response to this:
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. |
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
|
/cherry-pick release-4.9 |
|
@EmilienM: once the present PR merges, I will cherry-pick it on top of release-4.9 in a new PR and assign it to you. DetailsIn response to this:
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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mdbooth The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
@EmilienM: All pull requests linked via external trackers have merged: Bugzilla bug 2073398 has been moved to the MODIFIED state. DetailsIn response to this:
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. |
|
@EmilienM: new pull request created: #233 DetailsIn response to this:
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. |
|
@EmilienM: new pull request created: #234 DetailsIn response to this:
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. |
Signed-off-by: Alex Yang <[email protected]>
4 participants
With this change, the port and trunk cleanups are triggered by any
early failure of InstanceCreate.
When InstanceCreate creates ports and if it creates trunks, these
resources will be deleted if any of the next steps fail and cause the
server never to be created.
Note: the trunk will be deleted first, then the ports.