Only start CloudNodeController when nessesary #261
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/M
pkg/controller/controller.go
Outdated
| klog.Fatalf("error getting node: %v", err) | ||
| cancel() | ||
| } | ||
| _, exists := node.ObjectMeta.Annotations[cloudproviderapi.AnnotationAlphaProvidedIPAddr] |
There was a problem hiding this comment.
this annotation is to publish the ip set by the --node-ip flag, I do not know if we can depend on that convention for kind clusters, I think we always set it now, but we may not do it in the future
pkg/controller/controller.go
Outdated
| clustersInterface, ok := cloud.Clusters() | ||
| if !ok { | ||
| klog.Fatalf("Unable to use clusters interface: %v", err) | ||
| } | ||
|
|
||
| masterName, err := clustersInterface.Master(ctx, clusterName) |
There was a problem hiding this comment.
This seems overly complicated to get the masterName. I could change it to:
masterName := clusterName+'-control-plane'
Thoughts?
pkg/controller/controller.go
Outdated
| klog.Fatalf("Unable to use clusters interface: %v", err) | ||
| } | ||
|
|
||
| masterName, err := clustersInterface.Master(ctx, clusterName) |
There was a problem hiding this comment.
you need to identify the need of this controller by the Taint https://github.com/kubernetes/kubernetes/blob/3d13f39f24e5533286278f04702c69fbed448c35/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go#L277C1-L278C1
just create a function that gets the clientset and list all the nodes and returns tur if any of the nodes has the cloud provider taint
There was a problem hiding this comment.
Aaahhhh, thank you!
I was looking at https://github.com/kubernetes/kubernetes/blob/49a220699d0ac9df5dbd2b7a59844fab4e4710c3/pkg/kubelet/nodestatus/setters.go#L97 and made assumptions about how it worked.
adrianmoisey
force-pushed
the
node-controller
branch
from
1a93136 to
94f3237
Compare
Only start the CloudNodeController when the cloud provider taint exists on any node
adrianmoisey
force-pushed
the
node-controller
branch
from
94f3237 to
df74813
Compare
|
is this working fine now ? have yout tested it? |
I've tested the "default" case (where the taint doesn't exist, and CloudNodeController is not started). |
@adrianmoisey https://github.com/kubernetes-sigs/cloud-provider-kind/blob/main/kind.yaml |
|
Perfect, thanks! I'll give it a more thorough test |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adrianmoisey, aojea 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
the
approved
Only start the CloudNodeController when the cloud provider taint exists on any node.
Fixes #260
I don't really know if this is the right solution. Happy to take suggestions.