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

kubelet runs on master #44

Merged
merged 2 commits into from
May 12, 2020
Merged

kubelet runs on master #44

merged 2 commits into from
May 12, 2020

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?

@@ -175,13 +175,13 @@ func (r *PacketMachineReconciler) reconcile(ctx context.Context, machineScope *s

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

}

// Make sure bootstrap data secret is available and populated.
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

controllers/packetmachine_controller.go Show resolved Hide resolved
pkg/cloud/packet/scope/machine.go Show resolved Hide resolved
pkg/cloud/packet/scope/machine.go Show resolved Hide resolved
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-sigs:v1alpha3 May 12, 2020
@gianarb gianarb deleted the feature/kubelet-works-on-mater branch May 12, 2020 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants