Add paused annotation for MachineHealthCheck#1046
Conversation
| const ( | ||
| // PausedAnnotation is an annotation that can be applied to MachineHealthCheck objects to prevent the MHC controller | ||
| // from processing it. | ||
| MHCPausedAnnotation = "cluster.x-k8s.io/paused" |
There was a problem hiding this comment.
Do we want to call this MHCPausedAnnotation? I know it's only used for MHC at the moment but that does restrict us in the future if we wanted to use this elsewhere, WDYT?
There was a problem hiding this comment.
I named it MHCPausedAnnotation because no other Machine API components support paused state, a more generic name can be confusing now.
There was a problem hiding this comment.
I guess in the future we can rename it to something more generic, but we will have to keep this constant around, deprecate it, etc. I'm fine with this for now so long as API folks are
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: alexander-demichev, JoelSpeed The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| const ( | ||
| // PausedAnnotation is an annotation that can be applied to MachineHealthCheck objects to prevent the MHC controller | ||
| // from processing it. | ||
| MHCPausedAnnotation = "cluster.x-k8s.io/paused" |
There was a problem hiding this comment.
is the machine health check controller an upstream controller or downstream one? I thought it was a downstream controller that watched MachineHealthCheck.machine.openshift.io. If it is a downstream controller and a downstream API, this seems like a good choice for either
- a spec field
- handling by simply deleting the machinehealthcheck
There was a problem hiding this comment.
The machinehealthcheck is a fork of the upstream controller which is currently feature paired. We do have our own CRDs but they match the upstream identically at the moment.
The paused feature is something we picked from their implementation and we decided, to avoid potential future discrepancies between OCP and HyperShift, to keep the same annotation.
This was shipped in 4.9 and so I don't think we want to make a change to this now, though I appreciate based on our prior conversations about API compatibility that we should have made this a first class field when we backported it
|
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
|
Stale issues rot after 30d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle rotten |
|
Rotten issues close after 30d of inactivity. Reopen the issue by commenting /close |
|
@openshift-bot: Closed this PR. DetailsIn 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. |
No description provided.