🌱Add condition indicating deletion timestamp is set #1284
Merged
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
janiskemper
reviewed
Apr 30, 2024
guettli
reviewed
May 8, 2024
janiskemper
requested changes
May 16, 2024
janiskemper
requested changes
May 16, 2024
There was a problem hiding this comment.
now you have a problem with "needsUpdate". You currently don't check that for the condition. I would just say remove the "needsUpdate code and just always update it. If you want to make it 100% efficient in terms of API calls, you need to check whether the condition is set before.
You can do that with some utility function of the conditions package (something like HasReason)
yrs147
force-pushed
the
yrs/deleting-condition
branch
from
e42bac5 to
8a134e3
Compare
janiskemper
requested changes
May 17, 2024
janiskemper
reviewed
May 17, 2024
yrs147
force-pushed
the
yrs/deleting-condition
branch
from
c85823c to
d117e35
Compare
syself-bot
bot
added
area/code
janiskemper
requested changes
May 20, 2024
There was a problem hiding this comment.
sorry I just realized something.. You don't want to check this only for hosts that are in state none - but for all that are getting deleted! You actually have to move this outside of this. Maybe even outside of the reconcileSelectedStates one. Because it is not related to a certain provisioning state
syself-bot
bot
added
the
size/S
janiskemper
requested changes
May 21, 2024
yrs147
force-pushed
the
yrs/deleting-condition
branch
from
42bd5ca to
3af97a6
Compare
janiskemper
reviewed
May 21, 2024
syself-bot
bot
added
the
size/XS
syself-bot
bot
removed
the
size/S
janiskemper
reviewed
May 22, 2024
yrs147
force-pushed
the
yrs/deleting-condition
branch
from
597d90d to
43696e1
Compare
janiskemper
approved these changes
May 22, 2024
yrs147
force-pushed
the
yrs/deleting-condition
branch
from
43696e1 to
8a7d2bf
Compare
🌱Fix lint errors 🌱Fix gofmt lint error 🐛Fix condition 🌱Remove Deletion condition 🐛Fix condition errors 🌱Remove condition and split if statements 🐛Fix lint error and remove needsUpdate 🌱Remove if statement 🌱Move set condition outside the switch Add being deleted condition in host 🌱Remove deleting condition from reconcileSelectedStates method 🌱Minor change 🌱Revert changes made to hbmh controller 🌱Change severity
yrs147
force-pushed
the
yrs/deleting-condition
branch
from
8a7d2bf to
97547ac
Compare
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.
What this PR does / why we need it:
Add condition indicating deletion timestamp is set
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #1229
Special notes for your reviewer:
Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.
TODOs: