🐛 Make custom deploy equivalent to image deploy #2048
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/cc @dtantsur |
metal3-io-bot
added
the
do-not-merge/hold
zaneb
force-pushed
the
custom-deploy-equivalence
branch
from
aba54a5
to
02c793e
Compare
metal3-io-bot
added
the
size/S
zaneb
force-pushed
the
custom-deploy-equivalence
branch
from
02c793e
to
97f8bea
Compare
Since c0373bb we always set the instance_info in ValidateManagementAccess() whether or not we are re-registering a fresh node, so the code to check whether the node has been re-registered or is transitioning from a clean failure is not accurate; we have always been adopting when there is an image URL available, even after a cleaning failed and the node has always existed. For this code to have had its original effect, we would have to limit the setting of the instance_info to the Enroll state in ValidateManagementAccess(). However, since d78fb4d we always move the state back to available during deprovisioning, so adopting during deprovisioning is not needed anyway. The Deprovision() function will always clean the node. Therefore, never adopt in Adopt() during deprovisioning, allow Deprovision() to do the cleaning in both cases. Signed-off-by: Zane Bitter <[email protected]>
This change ensures that custom deploy methods behave comparably to image deploys in respect of calling the provisioner's Adopt method. In practice, the ironic provisioner does not take any action during Adopt() when the state is deprovisioning, but conceptually it is up to the provisioner to decide that regardless of the deploy mode. Signed-off-by: Zane Bitter <[email protected]>
If an externally provisioned host has a provisioning image set and then the externallyProvisioned flag is removed, we do not begin an inspection (which would involve rebooting the host), but instead move directly to the provisioned state. This change ensures that the same behaviour occurs when the host has a custom deploy method set, since this should be equivalent to an image deploy. Signed-off-by: Zane Bitter <[email protected]>
zaneb
force-pushed
the
custom-deploy-equivalence
branch
from
97f8bea
to
dd67b17
Compare
|
/hold cancel |
metal3-io-bot
removed
the
do-not-merge/hold
|
/retest |
|
It seems Prow is having issues. Failures are non PR related. |
|
/approve Looks valid to me. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dtantsur 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 |
metal3-io-bot
added
the
approved
|
/test generate |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If we have to recreate the provisioner's state during deprovisioning, we must
first call Adopt() to ensure that something will actually happen to deprovision it.
This is the case regardless of provisioning method, but previously only
happened when an image was provisioned. In practice, the ironic provisioner
was attempting to adopt in cases where it was not originally intended to, and
subsequent changes mean it is never necessary.
If an externally provisioned host has a provisioning image set and then
the externallyProvisioned flag is removed, we should not begin an inspection
(which would involve rebooting the host), but instead move directly to
the provisioned state. This is also the case regardless of
provisioning method, but previously only happened when an image was
provisioned.