From dd989c401c6298af1420898d2b4e6d1eaa35035d Mon Sep 17 00:00:00 2001 From: Karthik Bhat Date: Tue, 5 Nov 2024 15:31:00 +0530 Subject: [PATCH] Use ignition only when Ignition is set in IBMPowerVSCluster --- cloud/scope/powervs_machine.go | 33 ++++++++----------- controllers/ibmpowervsmachine_controller.go | 4 ++- .../ibmpowervsmachine_controller_test.go | 6 ++++ 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/cloud/scope/powervs_machine.go b/cloud/scope/powervs_machine.go index 2b685f7aa..89ae4724c 100644 --- a/cloud/scope/powervs_machine.go +++ b/cloud/scope/powervs_machine.go @@ -131,6 +131,7 @@ func NewPowerVSMachineScope(params PowerVSMachineScopeParams) (scope *PowerVSMac scope.IBMPowerVSMachine = params.IBMPowerVSMachine scope.IBMPowerVSCluster = params.IBMPowerVSCluster scope.IBMPowerVSImage = params.IBMPowerVSImage + scope.ServiceEndpoint = params.ServiceEndpoint if params.Logger == (logr.Logger{}) { params.Logger = klog.Background() @@ -168,6 +169,7 @@ func NewPowerVSMachineScope(params PowerVSMachineScopeParams) (scope *PowerVSMac } scope.Logger.V(3).Info("Overriding the default resource controller endpoint") } + scope.ResourceClient = rc var serviceInstanceID, serviceInstanceName string if params.IBMPowerVSMachine.Spec.ServiceInstanceID != "" { @@ -221,10 +223,6 @@ func NewPowerVSMachineScope(params PowerVSMachineScopeParams) (scope *PowerVSMac scope.IBMPowerVSClient = c scope.DHCPIPCacheStore = params.DHCPIPCacheStore - if !CheckCreateInfraAnnotation(*params.IBMPowerVSCluster) { - return scope, nil - } - var vpcRegion string if params.IBMPowerVSCluster.Spec.VPC == nil || params.IBMPowerVSCluster.Spec.VPC.Region == nil { vpcRegion, err = regionUtil.VPCRegionForPowerVSRegion(scope.GetRegion()) @@ -239,10 +237,7 @@ func NewPowerVSMachineScope(params PowerVSMachineScopeParams) (scope *PowerVSMac if err != nil { return nil, fmt.Errorf("failed to create IBM VPC client: %w", err) } - scope.IBMVPCClient = vpcClient - scope.ResourceClient = rc - scope.ServiceEndpoint = params.ServiceEndpoint return scope, nil } @@ -354,11 +349,11 @@ func (m *PowerVSMachineScope) CreateMachine() (*models.PVMInstanceReference, err } func (m *PowerVSMachineScope) resolveUserData() (string, error) { - userData, userDataFormat, err := m.GetRawBootstrapDataWithFormat() + userData, err := m.GetRawBootstrapDataWithFormat() if err != nil { return "", err } - if m.UseIgnition(userDataFormat) { + if m.UseIgnition() { data, err := m.ignitionUserData(userData) if err != nil { return "", err @@ -512,9 +507,9 @@ func (m *PowerVSMachineScope) ignitionUserData(userData []byte) ([]byte, error) } } -// UseIgnition returns true if user data format is of type 'ignition', else returns false. -func (m *PowerVSMachineScope) UseIgnition(userDataFormat string) bool { - return userDataFormat == "ignition" || (m.IBMPowerVSCluster.Spec.Ignition != nil) +// UseIgnition returns true if Ignition is set in IBMPowerVSCluster. +func (m *PowerVSMachineScope) UseIgnition() bool { + return m.IBMPowerVSCluster.Spec.Ignition != nil } // Close closes the current scope persisting the cluster configuration and status. @@ -539,11 +534,11 @@ func (m *PowerVSMachineScope) DeleteMachine() error { // DeleteMachineIgnition deletes the ignition associated with machine. func (m *PowerVSMachineScope) DeleteMachineIgnition() error { - _, userDataFormat, err := m.GetRawBootstrapDataWithFormat() + _, err := m.GetRawBootstrapDataWithFormat() if err != nil { return err } - if !m.UseIgnition(userDataFormat) { + if !m.UseIgnition() { m.V(3).Info("Machine is not using user data of type ignition") return nil } @@ -633,23 +628,23 @@ func (m *PowerVSMachineScope) createCOSClient() (*cos.Service, error) { } // GetRawBootstrapDataWithFormat returns the bootstrap data if present. -func (m *PowerVSMachineScope) GetRawBootstrapDataWithFormat() ([]byte, string, error) { +func (m *PowerVSMachineScope) GetRawBootstrapDataWithFormat() ([]byte, error) { if m.Machine == nil || m.Machine.Spec.Bootstrap.DataSecretName == nil { - return nil, "", errors.New("failed to retrieve bootstrap data: linked Machine's bootstrap.dataSecretName is nil") + return nil, errors.New("failed to retrieve bootstrap data: linked Machine's bootstrap.dataSecretName is nil") } secret := &corev1.Secret{} key := types.NamespacedName{Namespace: m.Machine.Namespace, Name: *m.Machine.Spec.Bootstrap.DataSecretName} if err := m.Client.Get(context.TODO(), key, secret); err != nil { - return nil, "", fmt.Errorf("failed to retrieve bootstrap data secret for IBMPowerVSMachine %s/%s: %w", m.Machine.Namespace, m.Machine.Name, err) + return nil, fmt.Errorf("failed to retrieve bootstrap data secret for IBMPowerVSMachine %s/%s: %w", m.Machine.Namespace, m.Machine.Name, err) } value, ok := secret.Data["value"] if !ok { - return nil, "", errors.New("failed to retrieve bootstrap data: secret value key is missing") + return nil, errors.New("failed to retrieve bootstrap data: secret value key is missing") } - return value, string(secret.Data["format"]), nil + return value, nil } func getImageID(image *infrav1beta2.IBMPowerVSResourceReference, m *PowerVSMachineScope) (*string, error) { diff --git a/controllers/ibmpowervsmachine_controller.go b/controllers/ibmpowervsmachine_controller.go index 24beb6a51..69ff1e4f0 100644 --- a/controllers/ibmpowervsmachine_controller.go +++ b/controllers/ibmpowervsmachine_controller.go @@ -289,9 +289,11 @@ func (r *IBMPowerVSMachineReconciler) reconcileNormal(machineScope *scope.PowerV return ctrl.Result{RequeueAfter: 2 * time.Minute}, nil } - if !scope.CheckCreateInfraAnnotation(*machineScope.IBMPowerVSCluster) { + if machineScope.IBMPowerVSCluster.Spec.VPC == nil || machineScope.IBMPowerVSCluster.Spec.VPC.Region == nil { + machineScope.Info("Skipping configuring machine to loadbalancer as VPC is not set") return ctrl.Result{}, nil } + // Register instance with load balancer machineScope.Info("updating loadbalancer for machine", "name", machineScope.IBMPowerVSMachine.Name) internalIP := machineScope.GetMachineInternalIP() diff --git a/controllers/ibmpowervsmachine_controller_test.go b/controllers/ibmpowervsmachine_controller_test.go index 806c3fda5..84cb2fc70 100644 --- a/controllers/ibmpowervsmachine_controller_test.go +++ b/controllers/ibmpowervsmachine_controller_test.go @@ -512,6 +512,9 @@ func TestIBMPowerVSMachineReconciler_ReconcileOperations(t *testing.T) { }, }, Spec: infrav1beta2.IBMPowerVSClusterSpec{ + VPC: &infrav1beta2.VPCResourceReference{ + Region: ptr.To("us-south"), + }, LoadBalancers: []infrav1beta2.VPCLoadBalancerSpec{ { Name: "capi-test-lb", @@ -603,6 +606,9 @@ func TestIBMPowerVSMachineReconciler_ReconcileOperations(t *testing.T) { }, }, Spec: infrav1beta2.IBMPowerVSClusterSpec{ + VPC: &infrav1beta2.VPCResourceReference{ + Region: ptr.To("us-south"), + }, LoadBalancers: []infrav1beta2.VPCLoadBalancerSpec{ { Name: "capi-test-lb",