Skip to content
This repository was archived by the owner on Aug 12, 2025. It is now read-only.

Conversation

@gianarb
Copy link
Contributor

@gianarb gianarb commented May 11, 2020

with the userdata currently it place in this PR the master gets a
running kubelet.

As reported on Jira, the kubelet is not running because we do not
install a CNI yet, but this is not crucial at the moment.

The userdata has some fixed values that we have to remove, like the
ROLE. We will get there moving forward.

I would like to fix the certificate issue first

with the userdata currently it place in this PR the master gets a
running kubelet.

As reported on Jira, the kubelet is not running because we do not
install a CNI yet, but this is not crucial at the moment.

The userdata has some fixed values that we have to remove, like the
ROLE. We will get there moving forward.

I would like to fix the certificate issue first
@gianarb gianarb requested a review from deitch May 11, 2020 17:02
@deitch
Copy link
Contributor

deitch commented May 12, 2020

I have a few questions with this.

  • This seems to go in an opposite direction of the standard bootstrap mechanism in v1alpha3, where they attempted to standardize this. A big part of v1alpha2/3 was that each provider had to do all of this userdata on their own, lots of repetitive boilerplate, that should be replaced by a bootstrap provider. This appears to be a (possibly temporary) measure to get it up and running "the old way", and eventually much of this will be removed once we get the formal Bootstrap provider wired in. Is that correct?
  • We are adding two vars - notably ${CLUSTER_DNS_SERVER} and ${PRIVATEIP} - to the cluster example template. Will those survive passing through generate-examples.sh? And how are they set?

Copy link
Contributor

@deitch deitch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks good, really nice tracking all of this down. The only outstanding questions I had were about removing some comments, some typos, and the requeue?

if !machineScope.Cluster.Status.InfrastructureReady {
machineScope.Info("Cluster infrastructure is not ready yet")
return ctrl.Result{Requeue: true, RequeueAfter: 30 * time.Second}, nil
return ctrl.Result{}, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we no longer requeue? Don't we need to tell it to come back and queue up the request again? Or does the Machine controller change something on the PacketMachine when the cluster is ready? Something has to do it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copied it from aws. It works

if machineScope.Machine.Spec.Bootstrap.DataSecretName == nil {
machineScope.Info("Bootstrap data secret is not yet available")
return ctrl.Result{Requeue: true, RequeueAfter: 30 * time.Second}, nil
return ctrl.Result{}, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question as above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copied it from aws. It works

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw. But how?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have an answer even for the previous code

secret := &corev1.Secret{}
key := types.NamespacedName{Namespace: m.Namespace(), Name: *m.Machine.Spec.Bootstrap.DataSecretName}
if err := m.client.Get(context.TODO(), key, secret); err != nil {
return nil, errors.Wrapf(err, "failed to retrieve bootstrap data secret for AWSMachine %s/%s", m.Namespace(), m.Name())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you meant "PacketMachine"

@gianarb gianarb requested a review from deitch May 12, 2020 11:02
@gianarb gianarb merged commit 6dff23a into kubernetes-retired:v1alpha3 May 12, 2020
@gianarb gianarb deleted the feature/kubelet-works-on-mater branch May 12, 2020 11:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants