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

Add a field on Cluster Status indicating the control plane is ready #1243

Closed
chuckha opened this issue Aug 12, 2019 · 9 comments · Fixed by #1331
Closed

Add a field on Cluster Status indicating the control plane is ready #1243

chuckha opened this issue Aug 12, 2019 · 9 comments · Fixed by #1331
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.

Comments

@chuckha
Copy link
Contributor

chuckha commented Aug 12, 2019

/kind feature

Describe the solution you'd like
CAPI uses an annotation to mark a cluster with a control plane ready for joining new nodes. Instead of using an annotation we should use a field on Status.

Anything else you would like to add:
See kubernetes-retired/cluster-api-bootstrap-provider-kubeadm#99

/cc @detiber @fabriziopandini @ykakarap

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 12, 2019
@ykakarap
Copy link
Contributor

ykakarap commented Aug 12, 2019

Since this is related to an issue I am working on in CABPK (#99), I would like to take this up.

/assign @ykakarap

@vincepri
Copy link
Member

@chuckha I don't think CAPI uses any annotations today to mark a control plane ready, were you referring to CAPA?

@ykakarap Before starting implementing Status.ControlPlaneReady field, we should first update the cluster proposal document to reflect this change and include details such as: how will we determine if a Cluster's control plane is ready, how can we make sure we can re-calculate readiness given that the value is going to be in Status.

@chuckha
Copy link
Contributor Author

chuckha commented Aug 12, 2019

@vincepri https://github.com/kubernetes-sigs/cluster-api/blob/master/pkg/controller/machine/machine_controller_phases.go#L279

is what I'm referencing

@vincepri
Copy link
Member

Things past me wrote and I forgot 😂

@moshloop
Copy link
Contributor

This will need to consider #1197 and #1250 which could potentially influence this.

@timothysc timothysc added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Aug 14, 2019
@timothysc timothysc added this to the v1alpha2 milestone Aug 14, 2019
@vincepri
Copy link
Member

/cc @ncdc

@ncdc
Copy link
Contributor

ncdc commented Aug 19, 2019

I am in favor of making this change. We have it as an annotation in v1alpha1, mainly because we didn't want to modify the API. I think it makes sense to promote this to a regular field for v1alpha2 so it is more visible.

As far as #1197 and #1250, there is not time for either of those to land in v1alpha2, so (pending lazy consensus) this should be able to go in by itself.

@ykakarap
Copy link
Contributor

ykakarap commented Aug 19, 2019

I am guessing we only want to add the field for now and considering refactoring the code to use this field instead of the annotation in a follow-up issue, correct?

@ncdc
Copy link
Contributor

ncdc commented Aug 19, 2019

I'd rather do it in a single PR. But we should open a PR to modify the proposal first.

I'd actually like to propose we consider naming it ControlPlaneInitialized instead of ControlPlaneReady. I don't think we necessarily need to constantly monitor the health of the control plane and update this bool. I'd rather use it as a gate that only ever changes from false to true a single time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. 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

Successfully merging a pull request may close this issue.

7 participants