From baa7972a07d41b81e8c80b321f4eb81ee6f46b09 Mon Sep 17 00:00:00 2001 From: Yuvaraj Kakaraparthi Date: Wed, 14 Jun 2023 00:13:30 -0700 Subject: [PATCH] optimize machine controller by dropping unwanted get calls --- .../controllers/machine/machine_controller.go | 28 +++- .../machine/machine_controller_node_labels.go | 96 ------------ .../machine_controller_node_labels_test.go | 142 ------------------ .../machine/machine_controller_noderef.go | 28 +++- .../machine_controller_noderef_test.go | 44 ++++++ .../machine/machine_controller_phases.go | 25 +-- .../machine/machine_controller_phases_test.go | 84 ++++++----- 7 files changed, 153 insertions(+), 294 deletions(-) delete mode 100644 internal/controllers/machine/machine_controller_node_labels.go delete mode 100644 internal/controllers/machine/machine_controller_node_labels_test.go diff --git a/internal/controllers/machine/machine_controller.go b/internal/controllers/machine/machine_controller.go index 5b62bbb9a9d5..e0c5630183a7 100644 --- a/internal/controllers/machine/machine_controller.go +++ b/internal/controllers/machine/machine_controller.go @@ -278,19 +278,22 @@ func (r *Reconciler) reconcile(ctx context.Context, cluster *clusterv1.Cluster, })) } - phases := []func(context.Context, *clusterv1.Cluster, *clusterv1.Machine) (ctrl.Result, error){ + phases := []func(context.Context, *scope) (ctrl.Result, error){ r.reconcileBootstrap, r.reconcileInfrastructure, r.reconcileNode, - r.reconcileInterruptibleNodeLabel, r.reconcileCertificateExpiry, } res := ctrl.Result{} errs := []error{} + s := &scope{ + cluster: cluster, + machine: m, + } for _, phase := range phases { // Call the inner reconciliation methods. - phaseResult, err := phase(ctx, cluster, m) + phaseResult, err := phase(ctx, s) if err != nil { errs = append(errs, err) } @@ -302,6 +305,25 @@ func (r *Reconciler) reconcile(ctx context.Context, cluster *clusterv1.Cluster, return res, kerrors.NewAggregate(errs) } +// scope holds the different objects that are read and used during the reconcile. +type scope struct { + // cluster is the Cluster object the Machine belongs to. + // It is set at the beginning of the reconcile function. + cluster *clusterv1.Cluster + + // machine is the Machine object. It is set at the beginning + // of the reconcile function. + machine *clusterv1.Machine + + // infraMachine is the Infrastructure Machine object that is referenced by the + // Machine. It is set after reconcileInfrastructure is called. + infraMachine *unstructured.Unstructured + + // bootstrapConfig is the BootstrapConfig object that is referenced by the + // Machine. It is set after reconcileBootstrap is called. + bootstrapConfig *unstructured.Unstructured +} + func (r *Reconciler) reconcileDelete(ctx context.Context, cluster *clusterv1.Cluster, m *clusterv1.Machine) (ctrl.Result, error) { //nolint:gocyclo log := ctrl.LoggerFrom(ctx) diff --git a/internal/controllers/machine/machine_controller_node_labels.go b/internal/controllers/machine/machine_controller_node_labels.go deleted file mode 100644 index 0364ad1d5125..000000000000 --- a/internal/controllers/machine/machine_controller_node_labels.go +++ /dev/null @@ -1,96 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package machine - -import ( - "context" - - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/klog/v2" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - - clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - "sigs.k8s.io/cluster-api/controllers/external" - "sigs.k8s.io/cluster-api/util" - "sigs.k8s.io/cluster-api/util/patch" -) - -func (r *Reconciler) reconcileInterruptibleNodeLabel(ctx context.Context, cluster *clusterv1.Cluster, machine *clusterv1.Machine) (ctrl.Result, error) { - // Check that the Machine hasn't been deleted or in the process - // and that the Machine has a NodeRef. - if !machine.DeletionTimestamp.IsZero() || machine.Status.NodeRef == nil { - return ctrl.Result{}, nil - } - - // Get the infrastructure object - infra, err := external.Get(ctx, r.Client, &machine.Spec.InfrastructureRef, machine.Namespace) - if err != nil { - return ctrl.Result{}, err - } - - log := ctrl.LoggerFrom(ctx) - - // Get interruptible instance status from the infrastructure provider. - interruptible, _, err := unstructured.NestedBool(infra.Object, "status", "interruptible") - if err != nil { - log.V(1).Error(err, "Failed to get interruptible status from infrastructure provider", "Machine", klog.KObj(machine)) - return ctrl.Result{}, nil - } - if !interruptible { - return ctrl.Result{}, nil - } - - remoteClient, err := r.Tracker.GetClient(ctx, util.ObjectKey(cluster)) - if err != nil { - return ctrl.Result{}, err - } - - if err := r.setInterruptibleNodeLabel(ctx, remoteClient, machine.Status.NodeRef.Name); err != nil { - return ctrl.Result{}, err - } - - log.V(3).Info("Set interruptible label to Machine's Node", "nodename", machine.Status.NodeRef.Name) - r.recorder.Event(machine, corev1.EventTypeNormal, "SuccessfulSetInterruptibleNodeLabel", machine.Status.NodeRef.Name) - - return ctrl.Result{}, nil -} - -func (r *Reconciler) setInterruptibleNodeLabel(ctx context.Context, remoteClient client.Client, nodeName string) error { - node := &corev1.Node{} - if err := remoteClient.Get(ctx, client.ObjectKey{Name: nodeName}, node); err != nil { - return err - } - - if node.Labels == nil { - node.Labels = map[string]string{} - } - - if _, ok := node.Labels[clusterv1.InterruptibleLabel]; ok { - return nil - } - - patchHelper, err := patch.NewHelper(node, remoteClient) - if err != nil { - return err - } - - node.Labels[clusterv1.InterruptibleLabel] = "" - - return patchHelper.Patch(ctx, node) -} diff --git a/internal/controllers/machine/machine_controller_node_labels_test.go b/internal/controllers/machine/machine_controller_node_labels_test.go deleted file mode 100644 index d5b1e9e376e5..000000000000 --- a/internal/controllers/machine/machine_controller_node_labels_test.go +++ /dev/null @@ -1,142 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package machine - -import ( - "context" - "testing" - "time" - - "github.com/go-logr/logr" - . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/tools/record" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/log" - - clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - "sigs.k8s.io/cluster-api/controllers/remote" - "sigs.k8s.io/cluster-api/util/patch" -) - -func TestReconcileInterruptibleNodeLabel(t *testing.T) { - g := NewWithT(t) - - ns, err := env.CreateNamespace(ctx, "test-interruptible-node-label") - g.Expect(err).ToNot(HaveOccurred()) - - infraMachine := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "kind": "GenericInfrastructureMachine", - "apiVersion": "infrastructure.cluster.x-k8s.io/v1beta1", - "metadata": map[string]interface{}{ - "name": "infra-config1", - "namespace": ns.Name, - }, - "status": map[string]interface{}{ - "interruptible": true, - }, - }, - } - - cluster := &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cluster-1", - Namespace: ns.Name, - }, - } - - node := &corev1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - } - - machine := &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "machine-test", - Namespace: ns.Name, - }, - Spec: clusterv1.MachineSpec{ - ClusterName: cluster.Name, - InfrastructureRef: corev1.ObjectReference{ - APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1", - Kind: "GenericInfrastructureMachine", - Name: "infra-config1", - Namespace: ns.Name, - }, - Bootstrap: clusterv1.Bootstrap{ - ConfigRef: &corev1.ObjectReference{ - APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1", - Kind: "BootstrapMachine", - Name: "bootstrap-config1", - }, - }, - }, - Status: clusterv1.MachineStatus{ - NodeRef: &corev1.ObjectReference{ - Name: "node-1", - }, - }, - } - - g.Expect(env.Create(ctx, cluster)).To(Succeed()) - g.Expect(env.Create(ctx, node)).To(Succeed()) - g.Expect(env.Create(ctx, infraMachine)).To(Succeed()) - // Note: We have to DeepCopy the machine, because the Create call clears the status and - // reconcileInterruptibleNodeLabel requires .status.nodeRef to be set. - g.Expect(env.Create(ctx, machine.DeepCopy())).To(Succeed()) - - // Patch infra machine status - patchHelper, err := patch.NewHelper(infraMachine, env) - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(unstructured.SetNestedField(infraMachine.Object, true, "status", "interruptible")).To(Succeed()) - g.Expect(patchHelper.Patch(ctx, infraMachine, patch.WithStatusObservedGeneration{})).To(Succeed()) - - defer func(do ...client.Object) { - g.Expect(env.Cleanup(ctx, do...)).To(Succeed()) - }(cluster, ns, node, infraMachine, machine) - - r := &Reconciler{ - Client: env.Client, - Tracker: remote.NewTestClusterCacheTracker(logr.New(log.NullLogSink{}), env.Client, scheme.Scheme, client.ObjectKey{Name: cluster.Name, Namespace: cluster.Namespace}), - recorder: record.NewFakeRecorder(32), - } - - _, err = r.reconcileInterruptibleNodeLabel(context.Background(), cluster, machine) - g.Expect(err).ToNot(HaveOccurred()) - - // Check if node gets interruptible label - g.Eventually(func() bool { - updatedNode := &corev1.Node{} - err := env.Get(ctx, client.ObjectKey{Name: node.Name}, updatedNode) - if err != nil { - return false - } - - if updatedNode.Labels == nil { - return false - } - - _, ok := updatedNode.Labels[clusterv1.InterruptibleLabel] - - return ok - }, 10*time.Second).Should(BeTrue()) -} diff --git a/internal/controllers/machine/machine_controller_noderef.go b/internal/controllers/machine/machine_controller_noderef.go index 2df5c4da4aa7..7f32c72c48c5 100644 --- a/internal/controllers/machine/machine_controller_noderef.go +++ b/internal/controllers/machine/machine_controller_noderef.go @@ -24,6 +24,7 @@ import ( "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/klog/v2" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -41,8 +42,11 @@ var ( ErrNodeNotFound = errors.New("cannot find node with matching ProviderID") ) -func (r *Reconciler) reconcileNode(ctx context.Context, cluster *clusterv1.Cluster, machine *clusterv1.Machine) (ctrl.Result, error) { +func (r *Reconciler) reconcileNode(ctx context.Context, s *scope) (ctrl.Result, error) { log := ctrl.LoggerFrom(ctx) + cluster := s.cluster + machine := s.machine + infraMachine := s.infraMachine // Create a watch on the nodes in the Cluster. if err := r.watchClusterNodes(ctx, cluster); err != nil { @@ -112,10 +116,32 @@ func (r *Reconciler) reconcileNode(ctx context.Context, cluster *clusterv1.Clust // NOTE: Once we reconcile node labels for the first time, the NodeUninitializedTaint is removed from the node. nodeLabels := getManagedLabels(machine.Labels) + // Get interruptible instance status from the infrastructure provider and set the interruptible label on the node. + interruptible := false + found := false + if infraMachine != nil { + interruptible, found, err = unstructured.NestedBool(infraMachine.Object, "status", "interruptible") + if err != nil { + return ctrl.Result{}, errors.Wrapf(err, "failed to get status interruptible from infra machine %s", klog.KObj(infraMachine)) + } + // If interruptible is set and is true add the interruptible label to the node labels. + if found && interruptible { + nodeLabels[clusterv1.InterruptibleLabel] = "" + } + } + + _, nodeHadInterruptibleLabel := node.Labels[clusterv1.InterruptibleLabel] + // Reconcile node taints if err := r.patchNode(ctx, remoteClient, node, nodeLabels, nodeAnnotations); err != nil { return ctrl.Result{}, errors.Wrapf(err, "failed to reconcile Node %s", klog.KObj(node)) } + if !nodeHadInterruptibleLabel && interruptible { + // If the interruptible label is added to the node then record the event. + // Nb. Only record the event if the node previously did not have the label to avoid recording + // the event during every reconcile. + r.recorder.Event(machine, corev1.EventTypeNormal, "SuccessfulSetInterruptibleNodeLabel", node.Name) + } // Do the remaining node health checks, then set the node health to true if all checks pass. status, message := summarizeNodeConditions(node) diff --git a/internal/controllers/machine/machine_controller_noderef_test.go b/internal/controllers/machine/machine_controller_noderef_test.go index e2ad591ce569..41a24c5bd212 100644 --- a/internal/controllers/machine/machine_controller_noderef_test.go +++ b/internal/controllers/machine/machine_controller_noderef_test.go @@ -25,6 +25,7 @@ import ( . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -173,6 +174,17 @@ func TestNodeLabelSync(t *testing.T) { }, } + defaultInfraMachine := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": "GenericInfrastructureMachine", + "apiVersion": "infrastructure.cluster.x-k8s.io/v1beta1", + "metadata": map[string]interface{}{ + "name": "infra-config1", + "namespace": metav1.NamespaceDefault, + }, + }, + } + defaultMachine := clusterv1.Machine{ ObjectMeta: metav1.ObjectMeta{ Name: "machine-test", @@ -212,6 +224,18 @@ func TestNodeLabelSync(t *testing.T) { cluster := defaultCluster.DeepCopy() cluster.Namespace = ns.Name + infraMachine := defaultInfraMachine.DeepCopy() + infraMachine.SetNamespace(ns.Name) + + interruptibleTrueInfraMachineStatus := map[string]interface{}{ + "interruptible": true, + "ready": true, + } + interruptibleFalseInfraMachineStatus := map[string]interface{}{ + "interruptible": false, + "ready": true, + } + machine := defaultMachine.DeepCopy() machine.Namespace = ns.Name machine.Spec.ProviderID = pointer.String(nodeProviderID) @@ -305,6 +329,13 @@ func TestNodeLabelSync(t *testing.T) { g.Expect(env.Create(ctx, cluster)).To(Succeed()) defaultKubeconfigSecret := kubeconfig.GenerateSecret(cluster, kubeconfig.FromEnvTestConfig(env.Config, cluster)) g.Expect(env.Create(ctx, defaultKubeconfigSecret)).To(Succeed()) + + g.Expect(env.Create(ctx, infraMachine)).To(Succeed()) + // Set InfrastructureMachine .status.interruptible and .status.ready to true. + interruptibleTrueInfraMachine := infraMachine.DeepCopy() + g.Expect(unstructured.SetNestedMap(interruptibleTrueInfraMachine.Object, interruptibleTrueInfraMachineStatus, "status")).Should(Succeed()) + g.Expect(env.Status().Patch(ctx, interruptibleTrueInfraMachine, client.MergeFrom(infraMachine))).Should(Succeed()) + g.Expect(env.Create(ctx, machine)).To(Succeed()) // Validate that the right labels where synced to the Node. @@ -317,6 +348,10 @@ func TestNodeLabelSync(t *testing.T) { for k, v := range managedMachineLabels { g.Expect(node.Labels).To(HaveKeyWithValue(k, v)) } + + // Interruptible label should be set on the node. + g.Expect(node.Labels).To(HaveKey(clusterv1.InterruptibleLabel)) + // Unmanaged Machine labels should not have been synced to the Node. for k, v := range unmanagedMachineLabels { g.Expect(node.Labels).ToNot(HaveKeyWithValue(k, v)) @@ -334,6 +369,11 @@ func TestNodeLabelSync(t *testing.T) { return true }, 10*time.Second).Should(BeTrue()) + // Set InfrastructureMachine .status.interruptible to false. + interruptibleFalseInfraMachine := interruptibleTrueInfraMachine.DeepCopy() + g.Expect(unstructured.SetNestedMap(interruptibleFalseInfraMachine.Object, interruptibleFalseInfraMachineStatus, "status")).Should(Succeed()) + g.Expect(env.Status().Patch(ctx, interruptibleFalseInfraMachine, client.MergeFrom(interruptibleTrueInfraMachine))).Should(Succeed()) + // Remove managed labels from Machine. modifiedMachine := machine.DeepCopy() for k := range managedMachineLabels { @@ -351,6 +391,10 @@ func TestNodeLabelSync(t *testing.T) { for k, v := range managedMachineLabels { g.Expect(node.Labels).ToNot(HaveKeyWithValue(k, v)) } + + // Interruptible label should not be on node. + g.Expect(node.Labels).NotTo(HaveKey(clusterv1.InterruptibleLabel)) + // Unmanaged Machine labels should not have been synced at all to the Node. for k, v := range unmanagedMachineLabels { g.Expect(node.Labels).ToNot(HaveKeyWithValue(k, v)) diff --git a/internal/controllers/machine/machine_controller_phases.go b/internal/controllers/machine/machine_controller_phases.go index 083a3b3ff4a4..abf8a0185e0e 100644 --- a/internal/controllers/machine/machine_controller_phases.go +++ b/internal/controllers/machine/machine_controller_phases.go @@ -166,8 +166,10 @@ func (r *Reconciler) reconcileExternal(ctx context.Context, cluster *clusterv1.C } // reconcileBootstrap reconciles the Spec.Bootstrap.ConfigRef object on a Machine. -func (r *Reconciler) reconcileBootstrap(ctx context.Context, cluster *clusterv1.Cluster, m *clusterv1.Machine) (ctrl.Result, error) { +func (r *Reconciler) reconcileBootstrap(ctx context.Context, s *scope) (ctrl.Result, error) { log := ctrl.LoggerFrom(ctx) + cluster := s.cluster + m := s.machine // If the Bootstrap ref is nil (and so the machine should use user generated data secret), return. if m.Spec.Bootstrap.ConfigRef == nil { @@ -179,6 +181,7 @@ func (r *Reconciler) reconcileBootstrap(ctx context.Context, cluster *clusterv1. if err != nil { return ctrl.Result{}, err } + s.bootstrapConfig = externalResult.Result // If the external object is paused return. if externalResult.Paused { @@ -236,14 +239,17 @@ func (r *Reconciler) reconcileBootstrap(ctx context.Context, cluster *clusterv1. } // reconcileInfrastructure reconciles the Spec.InfrastructureRef object on a Machine. -func (r *Reconciler) reconcileInfrastructure(ctx context.Context, cluster *clusterv1.Cluster, m *clusterv1.Machine) (ctrl.Result, error) { +func (r *Reconciler) reconcileInfrastructure(ctx context.Context, s *scope) (ctrl.Result, error) { log := ctrl.LoggerFrom(ctx) + cluster := s.cluster + m := s.machine // Call generic external reconciler. infraReconcileResult, err := r.reconcileExternal(ctx, cluster, m, &m.Spec.InfrastructureRef) if err != nil { return ctrl.Result{}, err } + s.infraMachine = infraReconcileResult.Result if infraReconcileResult.RequeueAfter > 0 { // Infra object went missing after the machine was up and running if m.Status.InfrastructureReady { @@ -316,7 +322,8 @@ func (r *Reconciler) reconcileInfrastructure(ctx context.Context, cluster *clust return ctrl.Result{}, nil } -func (r *Reconciler) reconcileCertificateExpiry(ctx context.Context, _ *clusterv1.Cluster, m *clusterv1.Machine) (ctrl.Result, error) { +func (r *Reconciler) reconcileCertificateExpiry(_ context.Context, s *scope) (ctrl.Result, error) { + m := s.machine var annotations map[string]string if !util.IsControlPlaneMachine(m) { @@ -337,21 +344,15 @@ func (r *Reconciler) reconcileCertificateExpiry(ctx context.Context, _ *clusterv } expTime := metav1.NewTime(expiryTime) m.Status.CertificatesExpiryDate = &expTime - } else if m.Spec.Bootstrap.ConfigRef != nil { + } else if s.bootstrapConfig != nil { // If the expiry information is not available on the machine annotation // look for it on the bootstrap config. - bootstrapConfig, err := external.Get(ctx, r.Client, m.Spec.Bootstrap.ConfigRef, m.Namespace) - if err != nil { - return ctrl.Result{}, errors.Wrap(err, "failed to reconcile certificates expiry") - } - - // Check for certificate expiry information in the bootstrap config. - annotations = bootstrapConfig.GetAnnotations() + annotations = s.bootstrapConfig.GetAnnotations() if expiry, ok := annotations[clusterv1.MachineCertificatesExpiryDateAnnotation]; ok { expiryInfoFound = true expiryTime, err := time.Parse(time.RFC3339, expiry) if err != nil { - return ctrl.Result{}, errors.Wrapf(err, "failed to reconcile certificates expiry: failed to parse expiry date from annotation on %s", klog.KObj(bootstrapConfig)) + return ctrl.Result{}, errors.Wrapf(err, "failed to reconcile certificates expiry: failed to parse expiry date from annotation on %s", klog.KObj(s.bootstrapConfig)) } expTime := metav1.NewTime(expiryTime) m.Status.CertificatesExpiryDate = &expTime diff --git a/internal/controllers/machine/machine_controller_phases_test.go b/internal/controllers/machine/machine_controller_phases_test.go index b049196da124..b8cf96f59245 100644 --- a/internal/controllers/machine/machine_controller_phases_test.go +++ b/internal/controllers/machine/machine_controller_phases_test.go @@ -909,7 +909,8 @@ func TestReconcileBootstrap(t *testing.T) { ).Build(), } - res, err := r.reconcileBootstrap(ctx, defaultCluster, tc.machine) + s := &scope{cluster: defaultCluster, machine: tc.machine} + res, err := r.reconcileBootstrap(ctx, s) g.Expect(res).To(Equal(tc.expectResult)) if tc.expectError { g.Expect(err).NotTo(BeNil()) @@ -1118,8 +1119,8 @@ func TestReconcileInfrastructure(t *testing.T) { infraConfig, ).Build(), } - - result, err := r.reconcileInfrastructure(ctx, defaultCluster, tc.machine) + s := &scope{cluster: defaultCluster, machine: tc.machine} + result, err := r.reconcileInfrastructure(ctx, s) r.reconcilePhase(ctx, tc.machine) g.Expect(result).To(Equal(tc.expectResult)) if tc.expectError { @@ -1144,41 +1145,46 @@ func TestReconcileCertificateExpiry(t *testing.T) { fakeTime2, _ := time.Parse(time.RFC3339, fakeTimeString2) fakeMetaTime2 := &metav1.Time{Time: fakeTime2} - bootstrapConfigWithExpiry := map[string]interface{}{ - "kind": "GenericBootstrapConfig", - "apiVersion": "bootstrap.cluster.x-k8s.io/v1beta1", - "metadata": map[string]interface{}{ - "name": "bootstrap-config-with-expiry", - "namespace": metav1.NamespaceDefault, - "annotations": map[string]interface{}{ - clusterv1.MachineCertificatesExpiryDateAnnotation: fakeTimeString, + bootstrapConfigWithExpiry := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": "GenericBootstrapConfig", + "apiVersion": "bootstrap.cluster.x-k8s.io/v1beta1", + "metadata": map[string]interface{}{ + "name": "bootstrap-config-with-expiry", + "namespace": metav1.NamespaceDefault, + "annotations": map[string]interface{}{ + clusterv1.MachineCertificatesExpiryDateAnnotation: fakeTimeString, + }, + }, + "spec": map[string]interface{}{}, + "status": map[string]interface{}{ + "ready": true, + "dataSecretName": "secret-data", }, - }, - "spec": map[string]interface{}{}, - "status": map[string]interface{}{ - "ready": true, - "dataSecretName": "secret-data", }, } - bootstrapConfigWithoutExpiry := map[string]interface{}{ - "kind": "GenericBootstrapConfig", - "apiVersion": "bootstrap.cluster.x-k8s.io/v1beta1", - "metadata": map[string]interface{}{ - "name": "bootstrap-config-without-expiry", - "namespace": metav1.NamespaceDefault, - }, - "spec": map[string]interface{}{}, - "status": map[string]interface{}{ - "ready": true, - "dataSecretName": "secret-data", + bootstrapConfigWithoutExpiry := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": "GenericBootstrapConfig", + "apiVersion": "bootstrap.cluster.x-k8s.io/v1beta1", + "metadata": map[string]interface{}{ + "name": "bootstrap-config-without-expiry", + "namespace": metav1.NamespaceDefault, + }, + "spec": map[string]interface{}{}, + "status": map[string]interface{}{ + "ready": true, + "dataSecretName": "secret-data", + }, }, } tests := []struct { - name string - machine *clusterv1.Machine - expected func(g *WithT, m *clusterv1.Machine) + name string + machine *clusterv1.Machine + bootstrapConfig *unstructured.Unstructured + expected func(g *WithT, m *clusterv1.Machine) }{ { name: "worker machine with certificate expiry annotation should not update expiry date", @@ -1236,6 +1242,7 @@ func TestReconcileCertificateExpiry(t *testing.T) { }, }, }, + bootstrapConfig: bootstrapConfigWithoutExpiry, expected: func(g *WithT, m *clusterv1.Machine) { g.Expect(m.Status.CertificatesExpiryDate).To(BeNil()) }, @@ -1260,6 +1267,7 @@ func TestReconcileCertificateExpiry(t *testing.T) { }, }, }, + bootstrapConfig: bootstrapConfigWithExpiry, expected: func(g *WithT, m *clusterv1.Machine) { g.Expect(m.Status.CertificatesExpiryDate).To(Equal(fakeMetaTime)) }, @@ -1287,6 +1295,7 @@ func TestReconcileCertificateExpiry(t *testing.T) { }, }, }, + bootstrapConfig: bootstrapConfigWithoutExpiry, expected: func(g *WithT, m *clusterv1.Machine) { g.Expect(m.Status.CertificatesExpiryDate).To(Equal(fakeMetaTime)) }, @@ -1314,6 +1323,7 @@ func TestReconcileCertificateExpiry(t *testing.T) { }, }, }, + bootstrapConfig: bootstrapConfigWithExpiry, expected: func(g *WithT, m *clusterv1.Machine) { g.Expect(m.Status.CertificatesExpiryDate).To(Equal(fakeMetaTime2)) }, @@ -1341,6 +1351,7 @@ func TestReconcileCertificateExpiry(t *testing.T) { CertificatesExpiryDate: fakeMetaTime, }, }, + bootstrapConfig: bootstrapConfigWithoutExpiry, expected: func(g *WithT, m *clusterv1.Machine) { g.Expect(m.Status.CertificatesExpiryDate).To(BeNil()) }, @@ -1351,16 +1362,9 @@ func TestReconcileCertificateExpiry(t *testing.T) { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) - r := &Reconciler{ - Client: fake.NewClientBuilder(). - WithObjects( - tc.machine, - &unstructured.Unstructured{Object: bootstrapConfigWithExpiry}, - &unstructured.Unstructured{Object: bootstrapConfigWithoutExpiry}, - ).Build(), - } - - _, _ = r.reconcileCertificateExpiry(ctx, nil, tc.machine) + r := &Reconciler{} + s := &scope{machine: tc.machine, bootstrapConfig: tc.bootstrapConfig} + _, _ = r.reconcileCertificateExpiry(ctx, s) if tc.expected != nil { tc.expected(g, tc.machine) }