search

kubernetes-retired / cluster-api-bootstrap-provider-kubeadm

LEGACY REPO. NEW CODE IS https://github.com/kubernetes-sigs/cluster-api/tree/master/bootstrap/kubeadm
Apache License 2.0
62 stars 67 forks source link

should return without an error if no cluster is set on the machine #212

Closed chuckha closed 4 years ago

chuckha commented 4 years ago

/kind bug

https://github.com/kubernetes-sigs/cluster-api-bootstrap-provider-kubeadm/blob/cc2b1e48885f2f7bda4b8a3048b1c33367322a0f/controllers/kubeadmconfig_controller.go#L128-L132

What steps did you take and what happened: As of today the reconciler returns an error if no cluster can be found for a machine. Instead, we should either requeue or simply return and try again later.

What did you expect to happen: If there is no label utils.GetClusterFromMetadata returns a specific error. In that case, we should return no error. Perhaps there should be a log line indicating there is no associated cluster yet? The reason we should not error is that If the machine gets updated with a cluster the associated KubeadmConfig will get reconciled anyway and can proceed past this point.

If there is a label but the cluster does not exist in etcd yet we should also return. This assumes that the cluster will be created. When the cluster is created the associated KubeadmConfigs will all reconcile and the reconciler can proceed past this point.

The only time this should return an error is when there is a real error getting data from the API server.

Thoughts on this one @detiber ?

detiber commented 4 years ago

I think this behavior definitely sounds reasonable. If we have watches on Machines and Clusters then logging and returning without an error would be good, since we'll re-reconcile once the Machine and/or Cluster are updated/created.

chuckha commented 4 years ago

/assign @kashifest

k8s-ci-robot commented 4 years ago

@chuckha: GitHub didn't allow me to assign the following users: kashifest.

Note that only kubernetes-sigs members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. For more information please see the contributor guide

In response to [this](https://github.com/kubernetes-sigs/cluster-api-bootstrap-provider-kubeadm/issues/212#issuecomment-530531749): >/assign @kashifest Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
chuckha commented 4 years ago

/lifecycle active

@kashifest is working on this