kubernetes-retired / cluster-api-provider-nested

Cluster API Provider for Nested Clusters
Apache License 2.0
301 stars 67 forks source link

Who will fill out the ownerreference of the component CR. #14

Closed charleszheng44 closed 3 years ago

charleszheng44 commented 3 years ago

As discussed in the PR #11 , we all agree that there will be multiple CRs, NestedAPIServer, NestedEtcd and NestedControllerManager, to associate them, we plan to set each component CR's metav1.OwnerReference as the owner NCP. But who will fill out the OwnerReference for component CRs?

We can let the end user do it, but the metav1.OwnerReference are normally filled out by the controller/operator (as the OwnerReference contains field like ObjectUID that is normally unknonw in advance). I think a more conventional method would be grouping the CRs using the label, and let the NCP controller fill out the metav1.OwnerReference for the component CRs.

For example, we have an NCP CR named NCP1, and for each of its component CR, end user will need to set metav1.Labels[ownerNCP] = NCP1, then after user applying them, the NCP controller will add OwnerReference for them.

What do you think? @christopherhein @vincepri @Fei-Guo @brightzheng100

christopherhein commented 3 years ago

Agree with all except the labels, I think the NCP should have the fields from your proposal as references and then when NCP finds the associated Nested* it will add the metav1.OwnerReferences.

charleszheng44 commented 3 years ago

@christopherhein Do you mean we can add ObjectReference in NesterControlPlane.Spec?

type NestedControlPlaneSpec struct {
    // other fields ..

    Etcd               corev1.ObjectReference
    APIServer          corev1.ObjectReference
    ControllerManager  corev1.ObjectReference
}

That would be great if we can have these fields in the NCP spec.

christopherhein commented 3 years ago

Yeah, that is what I'm thinking, that would setup the connection from NCP to component CR, and then ownerReferences would setup the bidirectional connection for component -> CR.

charleszheng44 commented 3 years ago

will add object reference to the NCP spec.