Bug 1925276: Set provisioning phase after attempting create #793
Conversation
Setting the phase (or anything else) that causes a machine object to be patched prior to recording the instance ID can result in a double create. This is because the requeue from the initial phase will process before the cache is updated with the instance ID.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
requested review from
alexander-demicev and
Danil-Grigorev
|
/test unit |
michaelgugino
changed the title
There was a problem hiding this comment.
this makes sense to me, i just have a question about how the operations are done. is it possible to have the machine be created with the provisioning state? (just curious about the update action and sequence)
/lgtm
edit:
got an answer to my question out of band, updating the status takes an extra call to the api due to the status subresource. so, creating a machine with the state set is complicated to say the least.
|
/bugzilla refresh |
|
@JoelSpeed: Bugzilla bug 1920770 is in a bug group that is not in the allowed groups for this repo.
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. |
|
/retest |
|
This change means that we won't set the phase to provisioning if the machine actuator create fails right? As an alternative, what about checking if the phase is empty, setting it to provisioned and then requeueing straight away, so we only start creating when we have read the object as having provisioning already? I think that would solve the same issue and get around the possibility of cache issues /create failure right? |
|
@michaelgugino: The following tests 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. |
michaelgugino
changed the title
openshift-ci-robot
added
the
bugzilla/severity-medium
|
@michaelgugino: This pull request references Bugzilla bug 1925276, 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
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-robot
added
the
bugzilla/valid-bug
|
@michaelgugino: PR needs rebase. 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-robot
added
the
needs-rebase
|
@JoelSpeed you've been working on phases and whatnot, can you take this over? |
|
/close This has been covered by #857 |
|
/close |
|
@JoelSpeed: Closed this PR. 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. |
|
@michaelgugino: This pull request references Bugzilla bug 1925276. The bug has been updated to no longer refer to the pull request using the external bug tracker. 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. |
Setting the phase (or anything else) that causes a machine
object to be patched prior to recording the instance ID
can result in a double create. This is because the requeue
from the initial phase will process before the cache is
updated with the instance ID.