machine-controller-manager icon indicating copy to clipboard operation
machine-controller-manager copied to clipboard

Don't annotate nodes with `NotManagedByMCM` annotation

Open himanshu-kun opened this issue 2 years ago • 7 comments

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 avatar Feb 01 '23 06:02 himanshu-kun

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

himanshu-kun avatar Feb 01 '23 06:02 himanshu-kun

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.

unmarshall avatar Feb 01 '23 06:02 unmarshall

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.

elankath avatar Feb 01 '23 06:02 elankath

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.

himanshu-kun avatar Feb 01 '23 08:02 himanshu-kun

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?

unmarshall avatar Feb 01 '23 09:02 unmarshall

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

himanshu-kun avatar Feb 01 '23 13:02 himanshu-kun

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

himanshu-kun avatar Feb 20 '23 12:02 himanshu-kun