Cluster-api: Cluster references should be required on child objects

Created on 10 Jul 2019  路  10Comments  路  Source: kubernetes-sigs/cluster-api

/kind feature

Describe the solution you'd like
In v1alpha1 we allow Cluster references to be empty in the Machine object.

  • Code in multiple places has to check if the Machine label is specified and returns a <nil> Cluster reference.
  • Provider implementations have to check if the Cluster reference passed in is not <nil>.
  • NodeRef controller doesn't work without a Cluster reference given the need of a kubeconfig secret.
  • MachineSet and MachineDeployment is broken without a Cluster reference, when the controller runs in a management cluster.

The Cluster object in v1alpha2 can be made very thin and light, providers that don't want to reconcile the Cluster infrastructure should be able to do so without any issues.

areapi kinfeature lifecyclactive prioritimportant-longterm

Most helpful comment

+1

This will simplify the conceptual model and may make the Cluster API easier to use.

All 10 comments

@vincepri I can pick this up if you help me with pointers/guidance on how to start

/assign

@girikuncoro We might need to discuss about this issue a little more in community meetings to reach some consensus. Today, we rely on labels to link objects to a cluster and those are completely optional, this proposal is a breaking change that could impact some consumers.

@vincepri got it! I'll wait for the consensus and try to pick up some other issues

/unassign

/area api
/assign

Now that we're in v1alpha3 planning, we should prioritize the discussion of making the Cluster object reference required in any object that's currently referencing it via labels.

+1

This will simplify the conceptual model and may make the Cluster API easier to use.

Lazy consensus until next week meeting

~### Proposal for v1alpha2~

Given that there have been no comments or pushbacks, what do we think about adding a field to the v1alpha2 types which then becomes required in the next version of API types?

        // Cluster represents the name of the cluster associated with the object.
    // +optional
    Cluster *string `json:"cluster,omitempty"`

The field should be added to Machine, MachineSet, and MachineDeployment. The reconcilers will look at this field and set the appropriate label where needed. If the field isn't set, but there is a label, the field is populated.

/lifecycle active

I think Vince & I chatted about this but didn't record our conversation - my suggestion is to leave v1alpha2 alone, and make this a required field for v1alpha3.

Yep -- that's what I'll be working on :)

Was this page helpful?
0 / 5 - 0 ratings