Skip to content

dynamic link and reconciling between machines/nodes #497

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
enxebre opened this issue Sep 12, 2018 · 10 comments
Closed

dynamic link and reconciling between machines/nodes #497

enxebre opened this issue Sep 12, 2018 · 10 comments
Assignees
Labels
priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone

Comments

@enxebre
Copy link
Member

enxebre commented Sep 12, 2018

Currently the link between machines and nodes relies on something arbitrarily setting an annotation on the node to match a given machine name.
https://github.com/kubernetes-sigs/cluster-api/blob/master/pkg/controller/machine/node.go#L43
https://github.com/kubernetes-sigs/cluster-api-provider-gcp/blob/master/clusterctl/examples/google/provider-components.yaml.template#L294

This is to discuss about a controller mechanism for this to happen dynamically as soon as the node resulting from a given machine comes up. One possible option would be to match both by ip e.g https://github.com/kubernetes-sigs/cluster-api/blob/master/pkg/apis/cluster/v1alpha1/machine_types.go#L143

This would allow more self-contained machine<->node and would facilitate #493, it'd also ensure expected propagation MachineSet --> Machine --> Node for things like #47

@dgoodwin
Copy link
Contributor

+1, we had to do this with our own controller but it feels like it makes good sense in the cluster-api controllers. We have the same node addresses struct used on both the nodes and machine status so IP matching was the best option we could come up with. Curious if there are better ways.

@enxebre
Copy link
Member Author

enxebre commented Sep 12, 2018

@vikaschoudhary16
Copy link
Contributor

/cc @vikaschoudhary16

@derekwaynecarr
Copy link
Contributor

I agree that we should have a mechanism to automatically associate a machine to its corresponding node separate from the existing annotation. The use case I would like to support is creation of a Machine -> creation of a Node -> Node appears to the API server without a role label -> Machine controller assigns a role label to a Node (so identifying labels that steer rather than repel workloads are centrally applied by masters and not the nodes themselves) -> Node dynamic configuration is applied based on label (from machine or other source).

@Lion-Wei
Copy link

I think we may not really care about which exact machine this node linked, we do care about is the machine's flavor, so how about use machineClass?
Then we don't have to distinguish cloud provider, or which machine, just need to choose a type of machine.

@prashanth26
Copy link

@Lion-Wei I think we would care about which exact machine is backing the node object. This especially makes sense when we would try to link the machine health check mechanism where we would like to update machine status based on node conditions. Does that make sense?

@alvaroaleman
Copy link
Member

Why not simple use Machine.Status.NodeRef and in case you need the Machine for a node, just iterate over all machines?

@enxebre
Copy link
Member Author

enxebre commented Nov 5, 2018

Even using machineClasses there's a need for a having a machine<->node map in order to reconcile information and support things like health checks, role reconciliation or draining on delete.
For reference this is a controller https://github.com/openshift/machine-api-operator/blob/master/cmd/nodelink-controller/main.go#L64 we currently use to ensure the right machine label is set on every node so the the core node controller can set the Machine.Status.NodeRef accordingly

@maisem
Copy link

maisem commented Dec 18, 2018

As noted above we currently use an arbitrary annotation on the node object to link the machine and node objects, if we were to merge #565 we could use the ProviderId field instead.
cc @justinsb @roberthbailey

@enxebre enxebre mentioned this issue Dec 18, 2018
@roberthbailey roberthbailey added this to the v1alpha1 milestone Jan 11, 2019
@roberthbailey roberthbailey added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Jan 11, 2019
@roberthbailey
Copy link

Let's consolidate the conversation on #520, which has a bit more detail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

No branches or pull requests

10 participants