diff --git a/controllers/infrastructure/byomachine_controller.go b/controllers/infrastructure/byomachine_controller.go index 4cb3429d7..4a5601294 100644 --- a/controllers/infrastructure/byomachine_controller.go +++ b/controllers/infrastructure/byomachine_controller.go @@ -42,7 +42,7 @@ import ( const ( ProviderIDPrefix = "byoh://" - providerIDSuffixLength = 6 + ProviderIDSuffixLength = 6 RequeueForbyohost = 10 * time.Second ) @@ -259,27 +259,25 @@ func (r *ByoMachineReconciler) reconcileNormal(ctx context.Context, machineScope r.Recorder.Eventf(machineScope.ByoMachine, corev1.EventTypeNormal, "ByoHostAttachSucceeded", "Attached ByoHost %s", machineScope.ByoHost.Name) } - if machineScope.ByoMachine.Spec.ProviderID == "" { - logger.Info("Updating Node with ProviderID") - remoteClient, err := r.getRemoteClient(ctx, machineScope.ByoMachine) - if err != nil { - logger.Error(err, "failed to get remote client") - return ctrl.Result{}, err - } - - providerID, err := r.setNodeProviderID(ctx, remoteClient, machineScope.ByoHost) - if err != nil { - logger.Error(err, "failed to set node providerID") - r.Recorder.Eventf(machineScope.ByoMachine, corev1.EventTypeWarning, "SetNodeProviderFailed", "Node %s does not exist", machineScope.ByoHost.Name) - return ctrl.Result{}, err - } + logger.Info("Updating Node with ProviderID") + remoteClient, err := r.getRemoteClient(ctx, machineScope.ByoMachine) + if err != nil { + logger.Error(err, "failed to get remote client") + return ctrl.Result{}, err + } - machineScope.ByoMachine.Spec.ProviderID = providerID - machineScope.ByoMachine.Status.Ready = true - conditions.MarkTrue(machineScope.ByoMachine, infrav1.BYOHostReady) - r.Recorder.Eventf(machineScope.ByoMachine, corev1.EventTypeNormal, "NodeProvisionedSucceeded", "Provisioned Node %s", machineScope.ByoHost.Name) + providerID, err := r.setNodeProviderID(ctx, remoteClient, machineScope.ByoHost) + if err != nil { + logger.Error(err, "failed to set node providerID") + r.Recorder.Eventf(machineScope.ByoMachine, corev1.EventTypeWarning, "SetNodeProviderFailed", "Node %s does not exist", machineScope.ByoHost.Name) + return ctrl.Result{}, err } + machineScope.ByoMachine.Spec.ProviderID = providerID + machineScope.ByoMachine.Status.Ready = true + conditions.MarkTrue(machineScope.ByoMachine, infrav1.BYOHostReady) + r.Recorder.Eventf(machineScope.ByoMachine, corev1.EventTypeNormal, "NodeProvisionedSucceeded", "Provisioned Node %s", machineScope.ByoHost.Name) + return ctrl.Result{}, nil } @@ -375,7 +373,7 @@ func (r *ByoMachineReconciler) setNodeProviderID(ctx context.Context, remoteClie return "", err } - node.Spec.ProviderID = fmt.Sprintf("%s%s/%s", ProviderIDPrefix, host.Name, util.RandomString(providerIDSuffixLength)) + node.Spec.ProviderID = fmt.Sprintf("%s%s/%s", ProviderIDPrefix, host.Name, util.RandomString(ProviderIDSuffixLength)) return node.Spec.ProviderID, helper.Patch(ctx, node) } diff --git a/controllers/infrastructure/byomachine_controller_test.go b/controllers/infrastructure/byomachine_controller_test.go index 348e8f380..d73c77721 100644 --- a/controllers/infrastructure/byomachine_controller_test.go +++ b/controllers/infrastructure/byomachine_controller_test.go @@ -18,6 +18,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/annotations" "sigs.k8s.io/cluster-api/util/conditions" "sigs.k8s.io/cluster-api/util/patch" @@ -117,6 +118,37 @@ var _ = Describe("Controllers/ByomachineController", func() { Expect(err).To(MatchError("nodes \"" + byoHost.Name + "\" not found")) }) + Context("When node.Spec.ProviderID is already set", func() { + + BeforeEach(func() { + byoHost = builder.ByoHost(defaultNamespace, "test-node-providerid-host").Build() + Expect(k8sClientUncached.Create(ctx, byoHost)).Should(Succeed()) + WaitForObjectsToBePopulatedInCache(byoHost) + }) + + AfterEach(func() { + Expect(k8sClientUncached.Delete(ctx, byoHost)).ToNot(HaveOccurred()) + }) + + It("should not return error when node.Spec.ProviderID is with correct value", func() { + node := builder.Node(defaultNamespace, byoHost.Name). + WithProviderID(fmt.Sprintf("%s%s/%s", controllers.ProviderIDPrefix, byoHost.Name, util.RandomString(controllers.ProviderIDSuffixLength))). + Build() + Expect(clientFake.Create(ctx, node)).Should(Succeed()) + _, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: byoMachineLookupKey}) + Expect(err).ToNot(HaveOccurred()) + }) + + It("should return error when node.Spec.ProviderID has stale value", func() { + node := builder.Node(defaultNamespace, byoHost.Name). + WithProviderID(fmt.Sprintf("%sanother-host/%s", controllers.ProviderIDPrefix, util.RandomString(controllers.ProviderIDSuffixLength))). + Build() + Expect(clientFake.Create(ctx, node)).Should(Succeed()) + _, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: byoMachineLookupKey}) + Expect(err).To(MatchError("invalid format for node.Spec.ProviderID")) + }) + }) + Context("When BYO Hosts are not available", func() { It("should mark BYOHostReady as False", func() { _, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: byoMachineLookupKey}) diff --git a/test/builder/builders.go b/test/builder/builders.go index b5c55dc2b..a92b521a2 100644 --- a/test/builder/builders.go +++ b/test/builder/builders.go @@ -332,8 +332,9 @@ func (s *SecretBuilder) Build() *corev1.Secret { // NodeBuilder holds the variables and objects required to build a corev1.Node type NodeBuilder struct { - namespace string - name string + namespace string + name string + providerID string } // Node returns a NodeBuilder with the given name and namespace @@ -344,6 +345,12 @@ func Node(namespace, name string) *NodeBuilder { } } +// WithProviderID adds the passed providerID to the NodeBuilder +func (n *NodeBuilder) WithProviderID(providerID string) *NodeBuilder { + n.providerID = providerID + return n +} + // Build returns a Node with the attributes added to the NodeBuilder func (n *NodeBuilder) Build() *corev1.Node { node := &corev1.Node{ @@ -355,7 +362,9 @@ func (n *NodeBuilder) Build() *corev1.Node { Name: n.name, Namespace: n.namespace, }, - Spec: corev1.NodeSpec{}, + Spec: corev1.NodeSpec{ + ProviderID: n.providerID, + }, Status: corev1.NodeStatus{}, }