Don't annotate nodes with `NotManagedByMCM` annotation

gardener / machine-controller-manager

Declarative way of managing machines for Kubernetes cluster

Apache License 2.0

256 stars 117 forks source link

Don't annotate nodes with `NotManagedByMCM` annotation #774

Open himanshu-kun opened 1 year ago

himanshu-kun commented 1 year ago

How to categorize this issue?

/area usability /kind enhancement /priority 3

What would you like to be added: Currently any node which is added to the cluster and is not managed by MCM is annotated with node.machine.sapcloud.io/notManagedByMCM": "1" This is a problem when there are two MCM's acting on the same target cluster from different control namespaces. This is usually the scenario in the IT we perform today. In such a case because one MCM couldn't see the machine objects for other MCM which is in another namespace, it marks the node of the other MCM with this annotation. And this way all the machines end up getting marked with the annotation.

Why is this needed:

To not mark every node with the annotation in case multiple MCM's are operating on the cluster
To enable customers to run another MCM deployment from their target cluster itself without being annotated.

himanshu-kun commented 1 year ago

Proposed solution

We can instead annotate the nodes which are managed by MCM with the annotation node.machine.sapcloud.io/managedByMCM: <place-holder>

<place-holder> needs to be unique for every MC which is running . One solution is to use pod-name of the MCM pod where the MC is running.

but this has the drawback of not being able to name for the case when MC is running locally as a process.

Another solution is to use process-id

this has the issue of change in process-id name with a crash of the process.

A mix of the two solutions could be used as well.

cc @unmarshall @elankath for more comments

unmarshall commented 1 year ago

but this has the drawback of not being able to name for the case when MC is running locally as a process.

You can pass a command line flag. If it detects that it is running in a K8S environment then it will default to pod-name. If it detects that it is not running in a pod then it will fail if the command line flag (name) is not provided.

elankath commented 1 year ago

but this has the drawback of not being able to name for the case when MC is running locally as a process.

For local cases, one can just fallback to the local host name.

himanshu-kun commented 1 year ago

but this has the drawback of not being able to name for the case when MC is running locally as a process.

You can pass a command line flag. If it detects that it is running in a K8S environment then it will default to pod-name. If it detects that it is not running in a pod then it will fail if the command line flag (name) is not provided.

panicing for not providing the hostname ? I think falling back to local host name , and printing log for it , should be good enough.

unmarshall commented 1 year ago

Yes hostname should serve as long as you are going to start a single process locally. If you wish to start more than one process locally then that will again not work.

panicing for not providing the hostname ?

Well its like any other required parameter that you pass to the command line. How is this going to be any different?

himanshu-kun commented 1 year ago

Yes multiple case would be a problem, then making it required in local case is the only way to go.

himanshu-kun commented 1 year ago

a solution proposed in issue https://github.com/gardener/machine-controller-manager/issues/718 could also be used.