Skip to content
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

Link MachineSets/MachineDeployments to their Cluster via both Labels and OwnerReferences #145

Closed
dgoodwin opened this issue May 9, 2018 · 6 comments

Comments

@dgoodwin
Copy link
Contributor

dgoodwin commented May 9, 2018

Owner references were added to Machines created via a MachineSet in #108

However as it stands today there is no established link required between MachineSet and it's Cluster. Similar for MachineDeployment.

Would it be reasonable to implement two links:

  1. Require (via validation) a label linking MachineSet / MachineDeployment to a Cluster, and Machine to it's MachineSet as well as it's Cluster. This would be useful for querying in a multi-tenant environment where there are multiple Clusters even in a single namespace. During validation the Cluster does not have to exist, the label must simply be set.
  2. Controllers set OwnerReference on MachineSet + Deployment as soon as the Cluster linked in the now manadatory label exists. This would allow for the same cascading deletion we implemented for MachineSet -> Machine.

All of this is possible to add-on but we were wondering if this makes sense for the core of cluster-api?

@staebler
Copy link
Contributor

/dibs

@staebler
Copy link
Contributor

I do not think that adding an OwnerReference to the Cluster on the MachineSet makes sense. Since the user is required to create both the Cluster and the MachineSets, it seems to me that it would be unexpected behavior to a user if the MachineSets were deleted when the Cluster was deleted.

@staebler
Copy link
Contributor

Does anybody have any opinions on whether a LocalObjectReference in the spec of MachineSet that points to the Cluster would be better than a cluster-name label on the MachineSet?

@rsdcastro
Copy link

/cc @k4leung4

@roberthbailey
Copy link
Contributor

Closing in favor of #41.

/close

@k8s-ci-robot
Copy link
Contributor

@roberthbailey: Closing this issue.

In response to this:

Closing in favor of #41.

/close

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/test-infra repository.

chuckha pushed a commit to chuckha/cluster-api that referenced this issue Oct 2, 2019
…_machine_version

modify DockerMachie Api version to v1alpha2
chuckha pushed a commit to chuckha/cluster-api that referenced this issue Oct 2, 2019
…adm-config-template

chore: add KubeadmConfigTemplate type
jayunit100 pushed a commit to jayunit100/cluster-api that referenced this issue Jan 31, 2020
* Vendored k8s.io/cluster-bootstrap to help generate tokens

* Create kubeadm token instead of request from kubeadm

Replaced the method of getting the kubeadm token.  Prior to this, we were
shelling out to kubeadm to create a token.  That proved unreliable in the
latest update to kubeadm (1.13).  Now, we generate the token ourselves and
write the token out as a secret.

Resolves kubernetes-sigs#145
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants