Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

scaleway_compute: When removing node, wait for transition #444

Conversation

olof
Copy link
Contributor

@olof olof commented Jun 2, 2020

SUMMARY

To remove a scaleway compute node, one needs to stop it first. This is
handled internally within the module by shutting down before removing.
Shutting down the node transitions it to a "stopping" state, which is
not the "stopped" state we expect. We thus need the transition to
complete so that we can put it in the actual target state (absent, i.e.
delete it).

The mechanism for waiting for such transitions today is controlled by
module parameters, with default to not being enabled at all, which
includes the transition from ([running] -(stopping)-> [stopped]).

Without this chage, in case of a running node, we would shut it down
(transition it to "stopping"), not wait for it complete the transition,
realize that it's not yet stopped and issue a second shut down command
to the api. This would fail with a 400 Bad Request error, "already
stopped".

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

scaleway_compute

ADDITIONAL INFORMATION

This was originally reported as ansible/ansible#45740, and proposed in ansible/ansible#68392.

(Edit: initially wrong component name. Updated.)

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added affects_2.10 bug This issue/PR relates to a bug module module new_contributor Help guide this first time contributor labels Jun 2, 2020
@olof olof force-pushed the scaleway_compute_state_transition_stopping branch from bb1b171 to 276789a Compare June 2, 2020 11:48
@olof
Copy link
Contributor Author

olof commented Jun 2, 2020

The third-party-check failed, but unclear why. Both of my two concurrent pull requests (and others) failed in the same way. Based on that, and that I couldn't figure out what went wrong (Getting doc strings via ansible-doc ERROR: Error running ansible-doc), I drew the conclusion that this is not my fault :), but let me know if that conclusion was premature.

Edit: found #402, which describes this issue.

@gundalow
Copy link
Contributor

gundalow commented Jun 7, 2020

recheck

@ansibullbot ansibullbot removed needs_triage new_contributor Help guide this first time contributor labels Jun 7, 2020
To remove a scaleway compute node, one needs to stop it first. This is
handled internally within the module by shutting down before removing.
Shutting down the node transitions it to a "stopping" state, which is
not the "stopped" state we expect. We thus need the transition to
complete so that we can put it in the actual target state (absent, i.e.
delete it).

The mechanism for waiting for such transitions today is controlled by
module parameters, with default to not being enabled at all, which
includes the transition from ([running] -(stopping)-> [stopped]).

Without this chage, in case of a running node, we would shut it down
(transition it to "stopping"), not wait for it complete the transition,
realize that it's not yet stopped and issue a second shut down command
to the api. This would fail with a 400 Bad Request error, "already
stopped".

Reference: ansible/ansible#45740
Reported-by: zwindler
@olof olof force-pushed the scaleway_compute_state_transition_stopping branch from 276789a to 1bbca26 Compare June 9, 2020 22:34
@Andersson007
Copy link
Contributor

@sieben , as the module's author, could you please take a look?

@remyleone
Copy link
Contributor

shipit

@Andersson007
Copy link
Contributor

@remyleone thanks for fast reply!
I didn't see and just emailed opensource@scaleway.com about the pr, sorry

@Andersson007 Andersson007 merged commit 3044c02 into ansible-collections:master Jun 11, 2020
@Andersson007
Copy link
Contributor

@olof thanks for the contribution!
@remyleone thanks for reviewing!

@Andersson007
Copy link
Contributor

merged #444 into master

@olof
Copy link
Contributor Author

olof commented Jun 11, 2020

Super, thanks everybody involved :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug module module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants