fix: use strconv.ParseBool for force-deletion label
#999
Conversation
|
@Kumm-Kai Thank you for your contribution. |
gardener-robot
added
needs/review
|
Thank you @Kumm-Kai for your contribution. Before I can start building your PR, a member of the organization must set the required label(s) {'reviewed/ok-to-test'}. Once started, you can check the build status in the PR checks section below. |
| ) | ||
|
|
||
| forceDeleteLabelPresent, _ = strconv.ParseBool(machine.Labels["force-deletion"]) |
There was a problem hiding this comment.
Would it make sense to check for an error and log a warning? sort of like a heads up about a misconfigured label
There was a problem hiding this comment.
I don't really think that would be beneficial. It's also not done in other parts of gardener where a label or annotation is used to trigger something.
There was a problem hiding this comment.
Yes i would tend to agree that error in parsing an invalid bool value should not cause a re-queue, thus the error should be ignored. However you should at least log that the label has an invalid value and thus no action has been taken. If the intent was to force delete a machine then that would not be achieved with the current value. This will be good for diagnostic as well especially when the developers get involved late and the machines are no longer available. So RCAs become a bit easier as we will have a log to refer to.
There was a problem hiding this comment.
good points 🙂
I've pushed a new commit.
aaronfern
added
the
reviewed/ok-to-test
gardener-robot-ci-1
added
needs/ok-to-test
What this PR does / why we need it:
Uses
strconv.ParseBoolinstead of"True"to check theforce-deletionMachine label.