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

🌱 Kcp use one workload cluster for reconcile #8900

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion controlplane/kubeadm/internal/control_plane.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,13 @@ type ControlPlane struct {
// See discussion on https://github.com/kubernetes-sigs/cluster-api/pull/3405
KubeadmConfigs map[string]*bootstrapv1.KubeadmConfig
InfraResources map[string]*unstructured.Unstructured

managementCluster ManagementCluster
workloadCluster WorkloadCluster
}

// NewControlPlane returns an instantiated ControlPlane.
func NewControlPlane(ctx context.Context, client client.Client, cluster *clusterv1.Cluster, kcp *controlplanev1.KubeadmControlPlane, ownedMachines collections.Machines) (*ControlPlane, error) {
func NewControlPlane(ctx context.Context, managementCluster ManagementCluster, client client.Client, cluster *clusterv1.Cluster, kcp *controlplanev1.KubeadmControlPlane, ownedMachines collections.Machines) (*ControlPlane, error) {
infraObjects, err := getInfraResources(ctx, client, ownedMachines)
if err != nil {
return nil, err
Expand All @@ -80,6 +83,7 @@ func NewControlPlane(ctx context.Context, client client.Client, cluster *cluster
KubeadmConfigs: kubeadmConfigs,
InfraResources: infraObjects,
reconciliationTime: metav1.Now(),
managementCluster: managementCluster,
}, nil
}

Expand Down Expand Up @@ -268,3 +272,28 @@ func (c *ControlPlane) SetPatchHelpers(patchHelpers map[string]*patch.Helper) {
c.machinesPatchHelpers[machineName] = patchHelper
}
}

// GetWorkloadCluster builds a cluster object.
// The cluster comes with an etcd client generator to connect to any etcd pod living on a managed machine.
func (c *ControlPlane) GetWorkloadCluster(ctx context.Context) (WorkloadCluster, error) {
if c.workloadCluster != nil {
return c.workloadCluster, nil
}

workloadCluster, err := c.managementCluster.GetWorkloadCluster(ctx, client.ObjectKeyFromObject(c.Cluster))
if err != nil {
return nil, err
}
c.workloadCluster = workloadCluster
return c.workloadCluster, nil
}

// InjectTestManagementCluster allows to inject a test ManagementCluster during tests.
// NOTE: This approach allows to keep the managementCluster field private, which will
// prevent people from using managementCluster.GetWorkloadCluster because it creates a new
// instance of WorkloadCluster at every call. People instead should use ControlPlane.GetWorkloadCluster
// that creates only a single instance of WorkloadCluster for each reconcile.
func (c *ControlPlane) InjectTestManagementCluster(managementCluster ManagementCluster) {
c.managementCluster = managementCluster
c.workloadCluster = nil
}
221 changes: 130 additions & 91 deletions controlplane/kubeadm/internal/controllers/controller.go

Large diffs are not rendered by default.

95 changes: 71 additions & 24 deletions controlplane/kubeadm/internal/controllers/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,9 @@ func TestKubeadmControlPlaneReconciler_adoption(t *testing.T) {
managementClusterUncached: fmc,
}

g.Expect(r.reconcile(ctx, cluster, kcp)).To(Equal(ctrl.Result{}))
_, adoptableMachineFound, err := r.initControlPlaneScope(ctx, cluster, kcp)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(adoptableMachineFound).To(BeTrue())

machineList := &clusterv1.MachineList{}
g.Expect(fakeClient.List(ctx, machineList, client.InNamespace(cluster.Namespace))).To(Succeed())
Expand Down Expand Up @@ -614,7 +616,9 @@ func TestKubeadmControlPlaneReconciler_adoption(t *testing.T) {
managementClusterUncached: fmc,
}

g.Expect(r.reconcile(ctx, cluster, kcp)).To(Equal(ctrl.Result{}))
_, adoptableMachineFound, err := r.initControlPlaneScope(ctx, cluster, kcp)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(adoptableMachineFound).To(BeTrue())

machineList := &clusterv1.MachineList{}
g.Expect(fakeClient.List(ctx, machineList, client.InNamespace(cluster.Namespace))).To(Succeed())
Expand Down Expand Up @@ -698,10 +702,9 @@ func TestKubeadmControlPlaneReconciler_adoption(t *testing.T) {
managementClusterUncached: fmc,
}

result, err := r.reconcile(ctx, cluster, kcp)
g.Expect(result).To(Equal(ctrl.Result{}))
g.Expect(err).To(HaveOccurred())
g.Expect(err.Error()).To(ContainSubstring("has just been deleted"))
_, adoptableMachineFound, err := r.initControlPlaneScope(ctx, cluster, kcp)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(adoptableMachineFound).To(BeFalse())

machineList := &clusterv1.MachineList{}
g.Expect(fakeClient.List(ctx, machineList, client.InNamespace(cluster.Namespace))).To(Succeed())
Expand All @@ -711,7 +714,7 @@ func TestKubeadmControlPlaneReconciler_adoption(t *testing.T) {
}
})

t.Run("refuses to adopt Machines that are more than one version old", func(t *testing.T) {
t.Run("Do not adopt Machines that are more than one version old", func(t *testing.T) {
g := NewWithT(t)

cluster, kcp, tmpl := createClusterWithControlPlane(metav1.NamespaceDefault)
Expand Down Expand Up @@ -753,7 +756,10 @@ func TestKubeadmControlPlaneReconciler_adoption(t *testing.T) {
managementClusterUncached: fmc,
}

g.Expect(r.reconcile(ctx, cluster, kcp)).To(Equal(ctrl.Result{}))
_, adoptableMachineFound, err := r.initControlPlaneScope(ctx, cluster, kcp)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(adoptableMachineFound).To(BeTrue())

// Message: Warning AdoptionFailed Could not adopt Machine test/test0: its version ("v1.15.0") is outside supported +/- one minor version skew from KCP's ("v1.17.0")
g.Expect(recorder.Events).To(Receive(ContainSubstring("minor version")))

Expand Down Expand Up @@ -818,7 +824,8 @@ func TestKubeadmControlPlaneReconciler_ensureOwnerReferences(t *testing.T) {

fakeClient := newFakeClient(objs...)

err = ensureCertificatesOwnerRef(ctx, fakeClient, client.ObjectKeyFromObject(cluster), certificates, kcpOwner)
r := KubeadmControlPlaneReconciler{Client: fakeClient}
err = r.ensureCertificatesOwnerRef(ctx, client.ObjectKeyFromObject(cluster), certificates, kcpOwner)
g.Expect(err).To(BeNil())

secrets := &corev1.SecretList{}
Expand Down Expand Up @@ -858,7 +865,9 @@ func TestKubeadmControlPlaneReconciler_ensureOwnerReferences(t *testing.T) {
}

fakeClient := newFakeClient(objs...)
err := ensureCertificatesOwnerRef(ctx, fakeClient, client.ObjectKeyFromObject(cluster), certificates, kcpOwner)

r := KubeadmControlPlaneReconciler{Client: fakeClient}
err := r.ensureCertificatesOwnerRef(ctx, client.ObjectKeyFromObject(cluster), certificates, kcpOwner)
g.Expect(err).To(BeNil())

secrets := &corev1.SecretList{}
Expand Down Expand Up @@ -901,7 +910,9 @@ func TestKubeadmControlPlaneReconciler_ensureOwnerReferences(t *testing.T) {
}

fakeClient := newFakeClient(objs...)
err := ensureCertificatesOwnerRef(ctx, fakeClient, client.ObjectKeyFromObject(cluster), certificates, kcpOwner)

r := KubeadmControlPlaneReconciler{Client: fakeClient}
err := r.ensureCertificatesOwnerRef(ctx, client.ObjectKeyFromObject(cluster), certificates, kcpOwner)
g.Expect(err).To(BeNil())

secrets := &corev1.SecretList{}
Expand Down Expand Up @@ -1088,18 +1099,20 @@ func TestReconcileCertificateExpiries(t *testing.T) {
machineWithoutNodeRefKubeadmConfig,
)

controlPlane, err := internal.NewControlPlane(ctx, fakeClient, cluster, kcp, ownedMachines)
g.Expect(err).ToNot(HaveOccurred())
managementCluster := &fakeManagementCluster{
Workload: fakeWorkloadCluster{
APIServerCertificateExpiry: &detectedExpiry,
},
}

r := &KubeadmControlPlaneReconciler{
Client: fakeClient,
managementCluster: &fakeManagementCluster{
Workload: fakeWorkloadCluster{
APIServerCertificateExpiry: &detectedExpiry,
},
},
Client: fakeClient,
managementCluster: managementCluster,
}

controlPlane, err := internal.NewControlPlane(ctx, managementCluster, fakeClient, cluster, kcp, ownedMachines)
g.Expect(err).ToNot(HaveOccurred())

_, err = r.reconcileCertificateExpiries(ctx, controlPlane)
g.Expect(err).NotTo(HaveOccurred())

Expand Down Expand Up @@ -2014,9 +2027,11 @@ func TestKubeadmControlPlaneReconciler_reconcileDelete(t *testing.T) {
controllerutil.AddFinalizer(kcp, controlplanev1.KubeadmControlPlaneFinalizer)
initObjs := []client.Object{cluster.DeepCopy(), kcp.DeepCopy()}

machines := collections.New()
for i := 0; i < 3; i++ {
m, _ := createMachineNodePair(fmt.Sprintf("test-%d", i), cluster, kcp, true)
initObjs = append(initObjs, m)
machines.Insert(m)
}

fakeClient := newFakeClient(initObjs...)
Expand All @@ -2031,7 +2046,13 @@ func TestKubeadmControlPlaneReconciler_reconcileDelete(t *testing.T) {
recorder: record.NewFakeRecorder(32),
}

result, err := r.reconcileDelete(ctx, cluster, kcp)
controlPlane := &internal.ControlPlane{
KCP: kcp,
Cluster: cluster,
Machines: machines,
}

result, err := r.reconcileDelete(ctx, controlPlane)
g.Expect(result).To(Equal(ctrl.Result{RequeueAfter: deleteRequeueAfter}))
g.Expect(err).ToNot(HaveOccurred())
g.Expect(kcp.Finalizers).To(ContainElement(controlplanev1.KubeadmControlPlaneFinalizer))
Expand All @@ -2040,7 +2061,12 @@ func TestKubeadmControlPlaneReconciler_reconcileDelete(t *testing.T) {
g.Expect(fakeClient.List(ctx, &controlPlaneMachines)).To(Succeed())
g.Expect(controlPlaneMachines.Items).To(BeEmpty())

result, err = r.reconcileDelete(ctx, cluster, kcp)
controlPlane = &internal.ControlPlane{
KCP: kcp,
Cluster: cluster,
}

result, err = r.reconcileDelete(ctx, controlPlane)
g.Expect(result).To(Equal(ctrl.Result{}))
g.Expect(err).NotTo(HaveOccurred())
g.Expect(kcp.Finalizers).To(BeEmpty())
Expand All @@ -2064,9 +2090,11 @@ func TestKubeadmControlPlaneReconciler_reconcileDelete(t *testing.T) {

initObjs := []client.Object{cluster.DeepCopy(), kcp.DeepCopy(), workerMachine.DeepCopy()}

machines := collections.New()
for i := 0; i < 3; i++ {
m, _ := createMachineNodePair(fmt.Sprintf("test-%d", i), cluster, kcp, true)
initObjs = append(initObjs, m)
machines.Insert(m)
}

fakeClient := newFakeClient(initObjs...)
Expand All @@ -2080,7 +2108,13 @@ func TestKubeadmControlPlaneReconciler_reconcileDelete(t *testing.T) {
recorder: record.NewFakeRecorder(32),
}

result, err := r.reconcileDelete(ctx, cluster, kcp)
controlPlane := &internal.ControlPlane{
KCP: kcp,
Cluster: cluster,
Machines: machines,
}

result, err := r.reconcileDelete(ctx, controlPlane)
g.Expect(result).To(Equal(ctrl.Result{RequeueAfter: deleteRequeueAfter}))
g.Expect(err).ToNot(HaveOccurred())

Expand Down Expand Up @@ -2113,9 +2147,11 @@ func TestKubeadmControlPlaneReconciler_reconcileDelete(t *testing.T) {

initObjs := []client.Object{cluster.DeepCopy(), kcp.DeepCopy(), workerMachinePool.DeepCopy()}

machines := collections.New()
for i := 0; i < 3; i++ {
m, _ := createMachineNodePair(fmt.Sprintf("test-%d", i), cluster, kcp, true)
initObjs = append(initObjs, m)
machines.Insert(m)
}

fakeClient := newFakeClient(initObjs...)
Expand All @@ -2129,7 +2165,13 @@ func TestKubeadmControlPlaneReconciler_reconcileDelete(t *testing.T) {
recorder: record.NewFakeRecorder(32),
}

result, err := r.reconcileDelete(ctx, cluster, kcp)
controlPlane := &internal.ControlPlane{
KCP: kcp,
Cluster: cluster,
Machines: machines,
}

result, err := r.reconcileDelete(ctx, controlPlane)
g.Expect(result).To(Equal(ctrl.Result{RequeueAfter: deleteRequeueAfter}))
g.Expect(err).ToNot(HaveOccurred())

Expand Down Expand Up @@ -2160,7 +2202,12 @@ func TestKubeadmControlPlaneReconciler_reconcileDelete(t *testing.T) {
recorder: record.NewFakeRecorder(32),
}

result, err := r.reconcileDelete(ctx, cluster, kcp)
controlPlane := &internal.ControlPlane{
KCP: kcp,
Cluster: cluster,
}

result, err := r.reconcileDelete(ctx, controlPlane)
g.Expect(result).To(Equal(ctrl.Result{}))
g.Expect(err).NotTo(HaveOccurred())
g.Expect(kcp.Finalizers).To(BeEmpty())
Expand Down
12 changes: 6 additions & 6 deletions controlplane/kubeadm/internal/controllers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,16 @@ import (
"sigs.k8s.io/cluster-api/util/secret"
)

func (r *KubeadmControlPlaneReconciler) reconcileKubeconfig(ctx context.Context, cluster *clusterv1.Cluster, kcp *controlplanev1.KubeadmControlPlane) (ctrl.Result, error) {
func (r *KubeadmControlPlaneReconciler) reconcileKubeconfig(ctx context.Context, controlPlane *internal.ControlPlane) (ctrl.Result, error) {
log := ctrl.LoggerFrom(ctx)

endpoint := cluster.Spec.ControlPlaneEndpoint
endpoint := controlPlane.Cluster.Spec.ControlPlaneEndpoint
if endpoint.IsZero() {
return ctrl.Result{}, nil
}

controllerOwnerRef := *metav1.NewControllerRef(kcp, controlplanev1.GroupVersion.WithKind(kubeadmControlPlaneKind))
clusterName := util.ObjectKey(cluster)
controllerOwnerRef := *metav1.NewControllerRef(controlPlane.KCP, controlplanev1.GroupVersion.WithKind(kubeadmControlPlaneKind))
clusterName := util.ObjectKey(controlPlane.Cluster)
configSecret, err := secret.GetFromNamespacedName(ctx, r.Client, clusterName, secret.Kubeconfig)
switch {
case apierrors.IsNotFound(err):
Expand All @@ -76,12 +76,12 @@ func (r *KubeadmControlPlaneReconciler) reconcileKubeconfig(ctx context.Context,
return ctrl.Result{}, errors.Wrap(err, "failed to retrieve kubeconfig Secret")
}

if err := r.adoptKubeconfigSecret(ctx, configSecret, kcp); err != nil {
if err := r.adoptKubeconfigSecret(ctx, configSecret, controlPlane.KCP); err != nil {
return ctrl.Result{}, err
}

// only do rotation on owned secrets
if !util.IsControlledBy(configSecret, kcp) {
if !util.IsControlledBy(configSecret, controlPlane.KCP) {
return ctrl.Result{}, nil
}

Expand Down
30 changes: 26 additions & 4 deletions controlplane/kubeadm/internal/controllers/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1"
"sigs.k8s.io/cluster-api/controllers/external"
controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
"sigs.k8s.io/cluster-api/controlplane/kubeadm/internal"
"sigs.k8s.io/cluster-api/util/conditions"
"sigs.k8s.io/cluster-api/util/kubeconfig"
"sigs.k8s.io/cluster-api/util/secret"
Expand Down Expand Up @@ -77,7 +78,12 @@ func TestReconcileKubeconfigEmptyAPIEndpoints(t *testing.T) {
recorder: record.NewFakeRecorder(32),
}

result, err := r.reconcileKubeconfig(ctx, cluster, kcp)
controlPlane := &internal.ControlPlane{
KCP: kcp,
Cluster: cluster,
}

result, err := r.reconcileKubeconfig(ctx, controlPlane)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(result).To(BeZero())

Expand Down Expand Up @@ -126,7 +132,12 @@ func TestReconcileKubeconfigMissingCACertificate(t *testing.T) {
recorder: record.NewFakeRecorder(32),
}

result, err := r.reconcileKubeconfig(ctx, cluster, kcp)
controlPlane := &internal.ControlPlane{
KCP: kcp,
Cluster: cluster,
}

result, err := r.reconcileKubeconfig(ctx, controlPlane)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(result).To(Equal(ctrl.Result{RequeueAfter: dependentCertRequeueAfter}))

Expand Down Expand Up @@ -192,7 +203,12 @@ func TestReconcileKubeconfigSecretDoesNotAdoptsUserSecrets(t *testing.T) {
recorder: record.NewFakeRecorder(32),
}

result, err := r.reconcileKubeconfig(ctx, cluster, kcp)
controlPlane := &internal.ControlPlane{
KCP: kcp,
Cluster: cluster,
}

result, err := r.reconcileKubeconfig(ctx, controlPlane)
g.Expect(err).To(Succeed())
g.Expect(result).To(BeZero())

Expand Down Expand Up @@ -251,7 +267,13 @@ func TestKubeadmControlPlaneReconciler_reconcileKubeconfig(t *testing.T) {
Client: fakeClient,
recorder: record.NewFakeRecorder(32),
}
result, err := r.reconcileKubeconfig(ctx, cluster, kcp)

controlPlane := &internal.ControlPlane{
KCP: kcp,
Cluster: cluster,
}

result, err := r.reconcileKubeconfig(ctx, controlPlane)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(result).To(Equal(ctrl.Result{}))

Expand Down
Loading