feat: add in cloudprovider disruption methods #1577
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: njtran The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
| return Command{}, scheduling.Results{}, nil | ||
| } | ||
|
|
||
| func (cp *CloudProvider) Reason() v1.DisruptionReason { |
There was a problem hiding this comment.
Why not just use the actual reason that was passed by the CloudProvider?
|
|
||
| // CloudProvider is a subreconciler that deletes candidates according to cloud provider specific reasons. | ||
| // This should be methods that are | ||
| type CloudProvider struct { |
There was a problem hiding this comment.
I'm not convinced that we need a separate method definition here. Can't we just combine this with the drift implementation so that drift is just another one of these "reasons" that we pull from the CloudProvider. I'm asking because this looks exactly the same as the drift implementation at this point.
| @@ -79,6 +79,8 @@ func NewController(clk clock.Clock, kubeClient client.Client, provisioner *provi | |||
| cloudProvider: cp, | |||
| lastRun: map[string]time.Time{}, | |||
| methods: []Method{ | |||
| // Terminate any NodeClaims with StatusConditions specific to the CloudProvider implementation. For | |||
| NewCloudProvider(kubeClient, cluster, c.cloudProvider, recorder, provisioner), | |||
There was a problem hiding this comment.
What about appending NewStatusConditionDisruption() with new methods gen-ed based on a combination of the CloudProvider reason and the Drift reason?
|
/close in favor of #1588 |
k8s-ci-robot
added
the
needs-rebase
|
PR needs rebase. 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-sigs/prow repository. |
|
/close |
|
@jonathan-innis: Closed this PR. In 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-sigs/prow repository. |
Fixes #N/A
Description
Adds in cloudprovider disruption methods - Will be taken over by @engedaam
TODO: add testing for cloudprovider methods. Either requires removing the CRD enum, or adding in the enum for each cloud provider installation, and ingesting a new CRD for the test environment
How was this change tested?
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.