Bug 2022627: Fix nodelink and CSR approval when a machine has multiple addresses #210
Conversation
openshift-ci
bot
added
do-not-merge/hold
|
@mdbooth: This pull request references Bugzilla bug 2022627, 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: In 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. In 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. |
actuator.Update() could in certain circumstances cause a server to be recreated. This is always an error and can only result in data loss. This change removes all functionality from CAPO which relates to recreating OpenStack resources. Instead, when we get an Update we simply synchronise the Machine object with the current state of the OpenStack resource. Additionally, we are no longer setting the instance-status annotation on the machine object, as this is no longer relevant. Instead we detect a deleted OpenStack resource by setting the providerID on the machine. This also has the advantage of making nodelink matching more robust.
It was dead code. Move the OpenstackIdAnnotationKey constant to machine, which is the only place it is used. Remove the OpenstackIPAnnotationKey and no longer set the corresponding annotation, because it is no longer used.
We were previously only reporting a single 'primary' address. This would cause CSRs for the machine not to be approved if additional ports were added. This removes the last use of code for determining the 'primary' IP, so we also remove all the dead code. Fixes: rhbz#2022627
mdbooth
force-pushed
the
actuator-update
branch
from
4745d28
to
67b4aa8
Compare
|
/test e2e-openstack |
1 similar comment
|
/test e2e-openstack |
|
/test e2e-openstack |
|
/hold cancel |
|
@mdbooth: This pull request references Bugzilla bug 2022627, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In 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
removed
the
do-not-merge/hold
|
@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. In 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: 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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: EmilienM 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
bot
added
the
approved
|
The proxy job isn't happy for a few tests that failed, but this is a well known issue and should be fixed soon via openshift/release#24212 |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
2 similar comments
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
@mdbooth: All pull requests linked via external trackers have merged: Bugzilla bug 2022627 has been moved to the MODIFIED state. In 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. |
using --vm-dirver lead to error and --bootstrap-flags vm-driver=kvm2 works
What this PR does / why we need it:
This PR does 2 things:
Immutable OpenStack resources
It is never correct to attempt to recreate an OpenStack server hosting an OCP node. Attempting to do so will always fail, and if we ever automatically delete the previous resource this will always cause data loss. We delete everything associated with this functionality.
Report all node address
We already made this change upstream in ✨Return all NodeAddresses in OpenStackMachine.Status.Addresses kubernetes-sigs/cluster-api-provider-openstack#1004. This fixes a failure to approve a CSR when the machine has multiple addresses.
Release note: