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

feat(destroy): Add a flag to be able to skip the disable of destroy stage #3858

Merged
merged 6 commits into from
Aug 13, 2020

Conversation

marchello2000
Copy link
Contributor

Add an experimental flag forceSkipDisable to the DestroyServerGroupStage so that applications that don't participate in discovery can skip disable step which should speed up their destruction operation.

…tage

Add an experimental flag `forceSkipDisable` to the `DestroyServerGroupStage` so that applications that don't participate in discovery can skip disable step which should speed up their destruction operation.
it.name = "disableServerGroup"
it.type = getType(DisableServerGroupStage)
it.context.putAll(context)
boolean skipDisable = (boolean)context.getOrDefault("forceSkipDisable", false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit ... maybe shouldDisableBeforeDestroy (default to true) vs forceSkipDisable (default to false).

Otherwise I think this is a useful flag!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer props that aren't there to be false. But in this very limited (from code perspective) use case it feels less consequential. I also thought having more strong language e.g. force would be good since most people shouldn't be setting the flag so when it's set, hopefully it stands out a bit

Copy link
Contributor

@ajordens ajordens Aug 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with the default-to-false idea, makes good sense to me.

Going to merge this as we're following up with RTDI today ... I added a log message and comments (and s/forceSkipDisable/skipDisableBeforeDestroy 🌺 ).

@ajordens ajordens merged commit 03bb515 into spinnaker:master Aug 13, 2020
KathrynLewis pushed a commit to KathrynLewis/orca that referenced this pull request Jan 31, 2021
…tage (spinnaker#3858)

Add an experimental flag `skipDisableBeforeDestroy` to the `DestroyServerGroupStage` so that applications that don't participate in discovery can skip disable step which should speed up their destruction operation.

Co-authored-by: Adam Jordens <adam@jordens.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants