From 199af1024311dbbd11ba59edb4d8da0904baa4a9 Mon Sep 17 00:00:00 2001 From: Amulyam24 Date: Wed, 28 Aug 2024 17:05:12 +0530 Subject: [PATCH] Refactor reconcilation of resources --- api/v1beta2/ibmpowervscluster_webhook.go | 12 + cloud/scope/powervs_cluster.go | 209 +++-- cloud/scope/powervs_cluster_test.go | 980 +++++++++++--------- controllers/ibmpowervscluster_controller.go | 14 +- 4 files changed, 687 insertions(+), 528 deletions(-) diff --git a/api/v1beta2/ibmpowervscluster_webhook.go b/api/v1beta2/ibmpowervscluster_webhook.go index 52ce0086c..6326578ac 100644 --- a/api/v1beta2/ibmpowervscluster_webhook.go +++ b/api/v1beta2/ibmpowervscluster_webhook.go @@ -105,8 +105,13 @@ func (r *IBMPowerVSCluster) validateIBMPowerVSClusterNetwork() *field.Error { func (r *IBMPowerVSCluster) validateIBMPowerVSClusterLoadBalancerNames() (allErrs field.ErrorList) { found := make(map[string]bool) for i, loadbalancer := range r.Spec.LoadBalancers { + if loadbalancer.Name == "" { + continue + } + if found[loadbalancer.Name] { allErrs = append(allErrs, field.Duplicate(field.NewPath("spec", fmt.Sprintf("loadbalancers[%d]", i)), map[string]interface{}{"Name": loadbalancer.Name})) + continue } found[loadbalancer.Name] = true } @@ -117,8 +122,12 @@ func (r *IBMPowerVSCluster) validateIBMPowerVSClusterLoadBalancerNames() (allErr func (r *IBMPowerVSCluster) validateIBMPowerVSClusterVPCSubnetNames() (allErrs field.ErrorList) { found := make(map[string]bool) for i, subnet := range r.Spec.VPCSubnets { + if subnet.Name == nil { + continue + } if found[*subnet.Name] { allErrs = append(allErrs, field.Duplicate(field.NewPath("spec", fmt.Sprintf("vpcSubnets[%d]", i)), map[string]interface{}{"Name": *subnet.Name})) + continue } found[*subnet.Name] = true } @@ -130,6 +139,9 @@ func (r *IBMPowerVSCluster) validateIBMPowerVSClusterTransitGateway() *field.Err if r.Spec.Zone == nil && r.Spec.VPC == nil { return nil } + if r.Spec.TransitGateway == nil { + return nil + } if _, globalRouting, _ := genUtil.GetTransitGatewayLocationAndRouting(r.Spec.Zone, r.Spec.VPC.Region); r.Spec.TransitGateway.GlobalRouting != nil && !*r.Spec.TransitGateway.GlobalRouting && globalRouting != nil && *globalRouting { return field.Invalid(field.NewPath("spec.transitGateway.globalRouting"), r.Spec.TransitGateway.GlobalRouting, "global routing is required since PowerVS and VPC region are from different region") } diff --git a/cloud/scope/powervs_cluster.go b/cloud/scope/powervs_cluster.go index af826943a..43cef9754 100644 --- a/cloud/scope/powervs_cluster.go +++ b/cloud/scope/powervs_cluster.go @@ -384,14 +384,8 @@ func (s *PowerVSClusterScope) ServiceInstance() *infrav1beta2.IBMPowerVSResource return s.IBMPowerVSCluster.Spec.ServiceInstance } -// GetServiceInstanceID get the service instance id. +// GetServiceInstanceID returns service instance id set in status field of IBMPowerVSCluster object. If it doesn't exist, returns empty string. func (s *PowerVSClusterScope) GetServiceInstanceID() string { - if s.IBMPowerVSCluster.Spec.ServiceInstanceID != "" { - return s.IBMPowerVSCluster.Spec.ServiceInstanceID - } - if s.IBMPowerVSCluster.Spec.ServiceInstance != nil && s.IBMPowerVSCluster.Spec.ServiceInstance.ID != nil { - return *s.IBMPowerVSCluster.Spec.ServiceInstance.ID - } if s.IBMPowerVSCluster.Status.ServiceInstance != nil && s.IBMPowerVSCluster.Status.ServiceInstance.ID != nil { return *s.IBMPowerVSCluster.Status.ServiceInstance.ID } @@ -470,11 +464,8 @@ func (s *PowerVSClusterScope) Network() *infrav1beta2.IBMPowerVSResourceReferenc return &s.IBMPowerVSCluster.Spec.Network } -// GetDHCPServerID returns the DHCP id from spec or status of IBMPowerVSCluster object. +// GetDHCPServerID returns the DHCP id from status of IBMPowerVSCluster object. If it doesn't exist, returns nil. func (s *PowerVSClusterScope) GetDHCPServerID() *string { - if s.IBMPowerVSCluster.Spec.DHCPServer != nil && s.IBMPowerVSCluster.Spec.DHCPServer.ID != nil { - return s.IBMPowerVSCluster.Spec.DHCPServer.ID - } if s.IBMPowerVSCluster.Status.DHCPServer != nil { return s.IBMPowerVSCluster.Status.DHCPServer.ID } @@ -491,11 +482,8 @@ func (s *PowerVSClusterScope) VPC() *infrav1beta2.VPCResourceReference { return s.IBMPowerVSCluster.Spec.VPC } -// GetVPCID returns the VPC id. +// GetVPCID returns the VPC id set in status field of IBMPowerVSCluster object. If it doesn't exist, returns nil. func (s *PowerVSClusterScope) GetVPCID() *string { - if s.IBMPowerVSCluster.Spec.VPC != nil && s.IBMPowerVSCluster.Spec.VPC.ID != nil { - return s.IBMPowerVSCluster.Spec.VPC.ID - } if s.IBMPowerVSCluster.Status.VPC != nil { return s.IBMPowerVSCluster.Status.VPC.ID } @@ -516,15 +504,6 @@ func (s *PowerVSClusterScope) GetVPCSubnetID(subnetName string) *string { // GetVPCSubnetIDs returns all the VPC subnet ids. func (s *PowerVSClusterScope) GetVPCSubnetIDs() []*string { subnets := []*string{} - // use the vpc subnet id set by user. - for _, subnet := range s.IBMPowerVSCluster.Spec.VPCSubnets { - if subnet.ID != nil { - subnets = append(subnets, subnet.ID) - } - } - if len(subnets) != 0 { - return subnets - } if s.IBMPowerVSCluster.Status.VPCSubnet == nil { return nil } @@ -534,8 +513,8 @@ func (s *PowerVSClusterScope) GetVPCSubnetIDs() []*string { return subnets } -// SetVPCSubnetID set the VPC subnet id. -func (s *PowerVSClusterScope) SetVPCSubnetID(name string, resource infrav1beta2.ResourceReference) { +// SetVPCSubnetStatus set the VPC subnet id. +func (s *PowerVSClusterScope) SetVPCSubnetStatus(name string, resource infrav1beta2.ResourceReference) { s.V(3).Info("Setting status", "name", name, "resource", resource) if s.IBMPowerVSCluster.Status.VPCSubnet == nil { s.IBMPowerVSCluster.Status.VPCSubnet = make(map[string]infrav1beta2.ResourceReference) @@ -572,8 +551,8 @@ func (s *PowerVSClusterScope) GetVPCSecurityGroupByID(securityGroupID string) (* return nil, nil, nil } -// SetVPCSecurityGroup set the VPC security group id. -func (s *PowerVSClusterScope) SetVPCSecurityGroup(name string, resource infrav1beta2.VPCSecurityGroupStatus) { +// SetVPCSecurityGroupStatus set the VPC security group id. +func (s *PowerVSClusterScope) SetVPCSecurityGroupStatus(name string, resource infrav1beta2.VPCSecurityGroupStatus) { s.V(3).Info("Setting VPC security group status", "name", name, "resource", resource) if s.IBMPowerVSCluster.Status.VPCSecurityGroups == nil { s.IBMPowerVSCluster.Status.VPCSecurityGroups = make(map[string]infrav1beta2.VPCSecurityGroupStatus) @@ -599,23 +578,6 @@ func (s *PowerVSClusterScope) GetTransitGatewayID() *string { return nil } -// PublicLoadBalancer returns the cluster public loadBalancer information. -func (s *PowerVSClusterScope) PublicLoadBalancer() *infrav1beta2.VPCLoadBalancerSpec { - // if the user did not specify any loadbalancer then return the public loadbalancer created by the controller. - if len(s.IBMPowerVSCluster.Spec.LoadBalancers) == 0 { - return &infrav1beta2.VPCLoadBalancerSpec{ - Name: *s.GetServiceName(infrav1beta2.ResourceTypeLoadBalancer), - Public: ptr.To(true), - } - } - for _, lb := range s.IBMPowerVSCluster.Spec.LoadBalancers { - if lb.Public != nil && *lb.Public { - return &lb - } - } - return nil -} - // SetLoadBalancerStatus set the loadBalancer id. func (s *PowerVSClusterScope) SetLoadBalancerStatus(name string, loadBalancer infrav1beta2.VPCLoadBalancerStatus) { s.V(3).Info("Setting status", "name", name, "status", loadBalancer) @@ -652,18 +614,45 @@ func (s *PowerVSClusterScope) GetLoadBalancerState(name string) *infrav1beta2.VP return nil } -// GetLoadBalancerHostName will return the hostname of load balancer. -func (s *PowerVSClusterScope) GetLoadBalancerHostName(name string) *string { +// GetLoadBalancerHostName will return the hostname of the public load balancer. +func (s *PowerVSClusterScope) GetPublicLoadBalancerHostName() (*string, error) { if s.IBMPowerVSCluster.Status.LoadBalancers == nil { - return nil + return nil, nil } + + var name string + if len(s.IBMPowerVSCluster.Spec.LoadBalancers) == 0 { + name = *s.GetServiceName(infrav1beta2.ResourceTypeLoadBalancer) + } + + for _, lb := range s.IBMPowerVSCluster.Spec.LoadBalancers { + if !*lb.Public { + continue + } + + if lb.Name != "" { + name = lb.Name + break + } + if lb.ID != nil { + loadBalancer, _, err := s.IBMVPCClient.GetLoadBalancer(&vpcv1.GetLoadBalancerOptions{ + ID: lb.ID, + }) + if err != nil { + return nil, err + } + name = *loadBalancer.Name + break + } + } + if val, ok := s.IBMPowerVSCluster.Status.LoadBalancers[name]; ok { - return val.Hostname + return val.Hostname, nil } - return nil + return nil, nil } -// GetResourceGroupID returns the resource group id if it present under spec or statue filed of IBMPowerVSCluster object +// GetResourceGroupID returns the resource group id if it present under spec or status filed of IBMPowerVSCluster object // or returns empty string. func (s *PowerVSClusterScope) GetResourceGroupID() string { if s.IBMPowerVSCluster.Spec.ResourceGroup != nil && s.IBMPowerVSCluster.Spec.ResourceGroup.ID != nil { @@ -716,7 +705,7 @@ func (s *PowerVSClusterScope) ReconcileResourceGroup() error { // ReconcilePowerVSServiceInstance reconciles Power VS service instance. func (s *PowerVSClusterScope) ReconcilePowerVSServiceInstance() (bool, error) { - // Verify if service instance id is set in spec or status field of IBMPowerVSCluster object. + // Verify if service instance id is set in status field of IBMPowerVSCluster object. serviceInstanceID := s.GetServiceInstanceID() if serviceInstanceID != "" { s.V(3).Info("PowerVS service instance ID is set, fetching details", "id", serviceInstanceID) @@ -745,6 +734,7 @@ func (s *PowerVSClusterScope) ReconcilePowerVSServiceInstance() (bool, error) { } // Set the status of IBMPowerVSCluster object with serviceInstanceID and ControllerCreated to false as PowerVS service instance is already exist in cloud. if serviceInstanceID != "" { + s.V(3).Info("Found PowerVS service instance in IBM Cloud", "id", serviceInstanceID) s.SetStatus(infrav1beta2.ResourceTypeServiceInstance, infrav1beta2.ResourceReference{ID: &serviceInstanceID, ControllerCreated: ptr.To(false)}) return requeue, nil } @@ -782,17 +772,38 @@ func (s *PowerVSClusterScope) checkServiceInstanceState(instance resourcecontrol return false, fmt.Errorf("PowerVS service instance is in %s state", *instance.State) } -// checkServiceInstance checks PowerVS service instance exist in cloud. +// checkServiceInstance checks PowerVS service instance exist in cloud by ID or name. func (s *PowerVSClusterScope) isServiceInstanceExists() (string, bool, error) { s.V(3).Info("Checking for PowerVS service instance in IBM Cloud") - // Fetches service instance by name. - serviceInstance, err := s.getServiceInstance() + var ( + id string + err error + serviceInstance *resourcecontrollerv2.ResourceInstance + ) + + if s.IBMPowerVSCluster.Spec.ServiceInstanceID != "" { + id = s.IBMPowerVSCluster.Spec.ServiceInstanceID + } else if s.IBMPowerVSCluster.Spec.ServiceInstance != nil && s.IBMPowerVSCluster.Spec.ServiceInstance.ID != nil { + id = *s.IBMPowerVSCluster.Spec.ServiceInstance.ID + } + + if id != "" { + // Fetches service instance by ID. + serviceInstance, _, err = s.ResourceClient.GetResourceInstance(&resourcecontrollerv2.GetResourceInstanceOptions{ + ID: &id, + }) + } else { + // Fetches service instance by name. + serviceInstance, err = s.getServiceInstance() + } + if err != nil { s.Error(err, "failed to get PowerVS service instance") return "", false, err } + if serviceInstance == nil { - s.V(3).Info("PowerVS service instance with given name does not exist in IBM Cloud", "name", *s.GetServiceName(infrav1beta2.ResourceTypeServiceInstance)) + s.V(3).Info("PowerVS service instance with given ID or name does not exist in IBM Cloud") return "", false, nil } @@ -871,30 +882,14 @@ func (s *PowerVSClusterScope) ReconcileNetwork() (bool, error) { return false, nil } -// checkNetwork checks the network exist in cloud. +// checkNetwork checks if network exists in cloud with given name or ID mentioned in spec. func (s *PowerVSClusterScope) checkNetwork() (*string, error) { - // get network from cloud. - s.V(3).Info("Checking if PowerVS network exists in IBM Cloud") - networkID, err := s.getNetwork() - if err != nil { - s.Error(err, "failed to get PowerVS network") - return nil, err - } - if networkID == nil { - s.V(3).Info("Unable to find PowerVS network in IBM Cloud", "network", s.IBMPowerVSCluster.Spec.Network) - return nil, nil - } - return networkID, nil -} - -func (s *PowerVSClusterScope) getNetwork() (*string, error) { - // fetch the network associated with network id if s.IBMPowerVSCluster.Spec.Network.ID != nil { + s.V(3).Info("Checking if PowerVS network exists in IBM Cloud with ID", "id", *s.IBMPowerVSCluster.Spec.Network.ID) network, err := s.IBMPowerVSClient.GetNetworkByID(*s.IBMPowerVSCluster.Spec.Network.ID) if err != nil { return nil, err } - s.V(3).Info("Found the PowerVS network", "id", network.NetworkID) return network.NetworkID, nil } @@ -907,25 +902,22 @@ func (s *PowerVSClusterScope) getNetwork() (*string, error) { networkName = *s.GetServiceName(infrav1beta2.ResourceTypeNetwork) } + s.V(3).Info("Checking if PowerVS network exists in IBM Cloud with name", "name", networkName) // fetch the network associated with name network, err := s.IBMPowerVSClient.GetNetworkByName(networkName) if err != nil { return nil, err } - if network == nil { + if network == nil || network.NetworkID == nil { + s.V(3).Info("Unable to find PowerVS network in IBM Cloud", "network", s.IBMPowerVSCluster.Spec.Network) return nil, nil } return network.NetworkID, nil - //TODO: Support regular expression } // isDHCPServerActive checks if the DHCP server status is active. func (s *PowerVSClusterScope) isDHCPServerActive() (bool, error) { - dhcpID := *s.GetDHCPServerID() - if dhcpID == "" { - return false, fmt.Errorf("DHCP ID is empty") - } - dhcpServer, err := s.IBMPowerVSClient.GetDHCPServer(dhcpID) + dhcpServer, err := s.IBMPowerVSClient.GetDHCPServer(*s.GetDHCPServerID()) if err != nil { return false, err } @@ -1021,6 +1013,7 @@ func (s *PowerVSClusterScope) ReconcileVPC() (bool, error) { return false, err } if id != "" { + s.V(3).Info("VPC found in IBM Cloud", "id", id) s.SetStatus(infrav1beta2.ResourceTypeVPC, infrav1beta2.ResourceReference{ID: &id, ControllerCreated: ptr.To(false)}) return false, nil } @@ -1040,7 +1033,18 @@ func (s *PowerVSClusterScope) ReconcileVPC() (bool, error) { // checkVPC checks VPC exist in cloud. func (s *PowerVSClusterScope) checkVPC() (string, error) { - vpcDetails, err := s.getVPCByName() + var ( + err error + vpcDetails *vpcv1.VPC + ) + if s.IBMPowerVSCluster.Spec.VPC != nil && s.IBMPowerVSCluster.Spec.VPC.ID != nil { + vpcDetails, _, err = s.IBMVPCClient.GetVPC(&vpcv1.GetVPCOptions{ + ID: s.IBMPowerVSCluster.Spec.VPC.ID, + }) + } else { + vpcDetails, err = s.getVPCByName() + } + if err != nil { s.Error(err, "failed to get VPC") return "", err @@ -1117,7 +1121,7 @@ func (s *PowerVSClusterScope) ReconcileVPCSubnets() (bool, error) { } } for index, subnet := range s.IBMPowerVSCluster.Spec.VPCSubnets { - if subnet.Name == nil { + if subnet.ID == nil && subnet.Name == nil { subnet.Name = ptr.To(fmt.Sprintf("%s-%d", *s.GetServiceName(infrav1beta2.ResourceTypeSubnet), index)) } subnets = append(subnets, subnet) @@ -1130,6 +1134,7 @@ func (s *PowerVSClusterScope) ReconcileVPCSubnets() (bool, error) { } else { subnetID = s.GetVPCSubnetID(*subnet.Name) } + if subnetID != nil { s.V(3).Info("VPC subnet ID is set, fetching details", "id", *subnetID) subnetDetails, _, err := s.IBMVPCClient.GetSubnet(&vpcv1.GetSubnetOptions{ @@ -1142,6 +1147,7 @@ func (s *PowerVSClusterScope) ReconcileVPCSubnets() (bool, error) { return false, fmt.Errorf("failed to get VPC subnet with ID %s", *subnetID) } // check for next subnet + s.SetVPCSubnetStatus(*subnetDetails.Name, infrav1beta2.ResourceReference{ID: subnetDetails.ID}) continue } @@ -1153,7 +1159,7 @@ func (s *PowerVSClusterScope) ReconcileVPCSubnets() (bool, error) { } if vpcSubnetID != "" { s.V(3).Info("Found VPC subnet in IBM Cloud", "id", vpcSubnetID) - s.SetVPCSubnetID(*subnet.Name, infrav1beta2.ResourceReference{ID: &vpcSubnetID, ControllerCreated: ptr.To(false)}) + s.SetVPCSubnetStatus(*subnet.Name, infrav1beta2.ResourceReference{ID: &vpcSubnetID, ControllerCreated: ptr.To(false)}) // check for next subnet continue } @@ -1165,13 +1171,13 @@ func (s *PowerVSClusterScope) ReconcileVPCSubnets() (bool, error) { return false, err } s.Info("Created VPC subnet", "id", subnetID) - s.SetVPCSubnetID(*subnet.Name, infrav1beta2.ResourceReference{ID: subnetID, ControllerCreated: ptr.To(true)}) + s.SetVPCSubnetStatus(*subnet.Name, infrav1beta2.ResourceReference{ID: subnetID, ControllerCreated: ptr.To(true)}) return true, nil } return false, nil } -// checkVPCSubnet checks VPC subnet exist in cloud. +// checkVPCSubnet checks if VPC subnet by the given name exists in cloud. func (s *PowerVSClusterScope) checkVPCSubnet(subnetName string) (string, error) { vpcSubnet, err := s.IBMVPCClient.GetVPCSubnetByName(subnetName) if err != nil { @@ -1279,7 +1285,7 @@ func (s *PowerVSClusterScope) ReconcileVPCSecurityGroups() error { } if sg != nil { s.V(3).Info("VPC security group already exists", "name", *sg.Name) - s.SetVPCSecurityGroup(*sg.Name, infrav1beta2.VPCSecurityGroupStatus{ + s.SetVPCSecurityGroupStatus(*sg.Name, infrav1beta2.VPCSecurityGroupStatus{ ID: sg.ID, RuleIDs: ruleIDs, ControllerCreated: ptr.To(false), @@ -1292,7 +1298,7 @@ func (s *PowerVSClusterScope) ReconcileVPCSecurityGroups() error { return fmt.Errorf("failed to create VPC security group: %w", err) } s.Info("VPC security group created", "name", *securityGroup.Name) - s.SetVPCSecurityGroup(*securityGroup.Name, infrav1beta2.VPCSecurityGroupStatus{ + s.SetVPCSecurityGroupStatus(*securityGroup.Name, infrav1beta2.VPCSecurityGroupStatus{ ID: securityGroupID, ControllerCreated: ptr.To(true), }) @@ -1431,7 +1437,7 @@ func (s *PowerVSClusterScope) createVPCSecurityGroupRulesAndSetStatus(ogSecurity } s.Info("VPC security group rules created", "security group name", *securityGroupName) - s.SetVPCSecurityGroup(*securityGroupName, infrav1beta2.VPCSecurityGroupStatus{ + s.SetVPCSecurityGroupStatus(*securityGroupName, infrav1beta2.VPCSecurityGroupStatus{ ID: securityGroupID, RuleIDs: ruleIDs, ControllerCreated: ptr.To(true), @@ -1835,7 +1841,6 @@ func (s *PowerVSClusterScope) checkTransitGatewayConnectionStatus(con tgapiv1.Tr s.V(3).Info("Checking the status of transit gateway connection", "name", *con.Name) switch *con.Status { case string(infrav1beta2.TransitGatewayConnectionStateAttached): - s.V(3).Info("Transit gateway connection is in attached state") return false, nil case string(infrav1beta2.TransitGatewayConnectionStateFailed): return false, fmt.Errorf("failed to attach connection to transit gateway, current status: %s", *con.Status) @@ -1887,6 +1892,10 @@ func (s *PowerVSClusterScope) createTransitGateway() error { return fmt.Errorf("failed to fetch resource group ID for resource group %v, ID is empty", s.ResourceGroup()) } + if s.IBMPowerVSCluster.Status.ServiceInstance == nil || s.IBMPowerVSCluster.Status.VPC == nil { + return fmt.Errorf("failed to proeceed with transit gateway creation as either one of VPC or PowerVS service instance reconciliation is not successful") + } + location, globalRouting, err := genUtil.GetTransitGatewayLocationAndRouting(s.Zone(), s.VPC().Region) if err != nil { return fmt.Errorf("failed to get transit gateway location and routing: %w", err) @@ -1894,11 +1903,11 @@ func (s *PowerVSClusterScope) createTransitGateway() error { // throw error when user tries to use local routing where global routing is required. // TODO: Add a webhook validation for below condition. - if s.IBMPowerVSCluster.Spec.TransitGateway.GlobalRouting != nil && !*s.IBMPowerVSCluster.Spec.TransitGateway.GlobalRouting && *globalRouting { + if s.IBMPowerVSCluster.Spec.TransitGateway != nil && s.IBMPowerVSCluster.Spec.TransitGateway.GlobalRouting != nil && !*s.IBMPowerVSCluster.Spec.TransitGateway.GlobalRouting && *globalRouting { return fmt.Errorf("failed to use local routing for transit gateway since powervs and vpc are in different region and requires global routing") } // setting global routing to true when it is set by user. - if s.IBMPowerVSCluster.Spec.TransitGateway.GlobalRouting != nil && *s.IBMPowerVSCluster.Spec.TransitGateway.GlobalRouting { + if s.IBMPowerVSCluster.Spec.TransitGateway != nil && s.IBMPowerVSCluster.Spec.TransitGateway.GlobalRouting != nil && *s.IBMPowerVSCluster.Spec.TransitGateway.GlobalRouting { globalRouting = ptr.To(true) } @@ -1942,7 +1951,7 @@ func (s *PowerVSClusterScope) ReconcileLoadBalancers() (bool, error) { loadBalancers = append(loadBalancers, loadBalancer) } for index, loadBalancer := range s.IBMPowerVSCluster.Spec.LoadBalancers { - if loadBalancer.Name == "" { + if loadBalancer.ID == nil && loadBalancer.Name == "" { loadBalancer.Name = fmt.Sprintf("%s-%d", *s.GetServiceName(infrav1beta2.ResourceTypeLoadBalancer), index) } loadBalancers = append(loadBalancers, loadBalancer) @@ -1983,6 +1992,7 @@ func (s *PowerVSClusterScope) ReconcileLoadBalancers() (bool, error) { return false, err } if loadBalancerStatus != nil { + s.V(3).Info("Found VPC load balancer in IBM Cloud", "id", *loadBalancerStatus.ID) s.SetLoadBalancerStatus(loadBalancer.Name, *loadBalancerStatus) continue } @@ -2033,7 +2043,7 @@ func (s *PowerVSClusterScope) checkLoadBalancerPort(lb infrav1beta2.VPCLoadBalan return nil } -// checkLoadBalancer checks loadBalancer in cloud. +// checkLoadBalancer checks if VPC load balancer by the given name exists in cloud. func (s *PowerVSClusterScope) checkLoadBalancer(lb infrav1beta2.VPCLoadBalancerSpec) (*infrav1beta2.VPCLoadBalancerStatus, error) { loadBalancer, err := s.IBMVPCClient.GetLoadBalancerByName(lb.Name) if err != nil { @@ -2331,7 +2341,9 @@ func (s *PowerVSClusterScope) fetchResourceGroupID() (string, error) { func (s *PowerVSClusterScope) fetchVPCCRN() (*string, error) { vpcID := s.GetVPCID() if vpcID == nil { - return nil, fmt.Errorf("VPC ID is empty") + if s.IBMPowerVSCluster.Spec.VPC != nil && s.IBMPowerVSCluster.Spec.VPC.ID != nil { + vpcID = s.IBMPowerVSCluster.Spec.VPC.ID + } } vpcDetails, _, err := s.IBMVPCClient.GetVPC(&vpcv1.GetVPCOptions{ ID: vpcID, @@ -2345,6 +2357,13 @@ func (s *PowerVSClusterScope) fetchVPCCRN() (*string, error) { // fetchPowerVSServiceInstanceCRN returns Power VS service instance CRN. func (s *PowerVSClusterScope) fetchPowerVSServiceInstanceCRN() (*string, error) { serviceInstanceID := s.GetServiceInstanceID() + if serviceInstanceID == "" { + if s.IBMPowerVSCluster.Spec.ServiceInstanceID != "" { + serviceInstanceID = s.IBMPowerVSCluster.Spec.ServiceInstanceID + } else if s.IBMPowerVSCluster.Spec.ServiceInstance != nil && s.IBMPowerVSCluster.Spec.ServiceInstance.ID != nil { + serviceInstanceID = *s.IBMPowerVSCluster.Spec.ServiceInstance.ID + } + } pvsDetails, _, err := s.ResourceClient.GetResourceInstance(&resourcecontrollerv2.GetResourceInstanceOptions{ ID: &serviceInstanceID, }) diff --git a/cloud/scope/powervs_cluster_test.go b/cloud/scope/powervs_cluster_test.go index 8fd006903..d2997bbfa 100644 --- a/cloud/scope/powervs_cluster_test.go +++ b/cloud/scope/powervs_cluster_test.go @@ -23,6 +23,7 @@ import ( "github.com/IBM/go-sdk-core/v5/core" "github.com/IBM/platform-services-go-sdk/resourcecontrollerv2" + "github.com/IBM/vpc-go-sdk/vpcv1" "go.uber.org/mock/gomock" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -38,6 +39,7 @@ import ( "sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/resourcemanager" "sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/transitgateway" "sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/vpc" + "sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/vpc/mock" . "github.com/onsi/gomega" ) @@ -172,30 +174,6 @@ func TestGetServiceInstanceID(t *testing.T) { IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{}, }, }, - { - name: "Service Instance ID is set in spec.ServiceInstanceID", - clusterScope: PowerVSClusterScope{ - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ - Spec: infrav1beta2.IBMPowerVSClusterSpec{ - ServiceInstanceID: "specServiceInstanceID", - }, - }, - }, - expectedID: "specServiceInstanceID", - }, - { - name: "Service Instance ID is set in spec.ServiceInstance.ID", - clusterScope: PowerVSClusterScope{ - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ - Spec: infrav1beta2.IBMPowerVSClusterSpec{ - ServiceInstance: &infrav1beta2.IBMPowerVSResourceReference{ - ID: ptr.To("specServiceInstanceID"), - }, - }, - }, - }, - expectedID: "specServiceInstanceID", - }, { name: "Service Instance ID is set in status.ServiceInstanceID", clusterScope: PowerVSClusterScope{ @@ -209,22 +187,6 @@ func TestGetServiceInstanceID(t *testing.T) { }, expectedID: "statusServiceInstanceID", }, - { - name: "Spec Service Instance ID takes precedence over status Service Instance ID", - clusterScope: PowerVSClusterScope{ - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ - Spec: infrav1beta2.IBMPowerVSClusterSpec{ - ServiceInstanceID: "specServiceInstanceID", - }, - Status: infrav1beta2.IBMPowerVSClusterStatus{ - ServiceInstance: &infrav1beta2.ResourceReference{ - ID: ptr.To("statusServiceInstanceID"), - }, - }, - }, - }, - expectedID: "specServiceInstanceID", - }, } for _, tc := range testCases { @@ -248,17 +210,6 @@ func TestGetDHCPServerID(t *testing.T) { IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{}, }, }, - { - name: "DHCP server ID is set in spec", - clusterScope: PowerVSClusterScope{ - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ - Spec: infrav1beta2.IBMPowerVSClusterSpec{ - DHCPServer: &infrav1beta2.DHCPServer{ID: ptr.To("dhcpserverid")}, - }, - }, - }, - expectedID: ptr.To("dhcpserverid"), - }, { name: "DHCP server ID is set in status", clusterScope: PowerVSClusterScope{ @@ -272,22 +223,6 @@ func TestGetDHCPServerID(t *testing.T) { }, expectedID: ptr.To("dhcpserverid"), }, - { - name: "Spec DHCP server ID takes precedence over status DHCP Server ID", - clusterScope: PowerVSClusterScope{ - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ - Spec: infrav1beta2.IBMPowerVSClusterSpec{ - DHCPServer: &infrav1beta2.DHCPServer{ID: ptr.To("dhcpserverid")}, - }, - Status: infrav1beta2.IBMPowerVSClusterStatus{ - DHCPServer: &infrav1beta2.ResourceReference{ - ID: ptr.To("dhcpserveridstatus"), - }, - }, - }, - }, - expectedID: ptr.To("dhcpserverid"), - }, } for _, tc := range testCases { @@ -311,17 +246,6 @@ func TestGetVPCID(t *testing.T) { IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{}, }, }, - { - name: "VPC ID is set in spec", - clusterScope: PowerVSClusterScope{ - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ - Spec: infrav1beta2.IBMPowerVSClusterSpec{ - VPC: &infrav1beta2.VPCResourceReference{ID: ptr.To("vpcID")}, - }, - }, - }, - expectedID: ptr.To("vpcID"), - }, { name: "VPC ID is set in status", clusterScope: PowerVSClusterScope{ @@ -335,22 +259,6 @@ func TestGetVPCID(t *testing.T) { }, expectedID: ptr.To("vpcID"), }, - { - name: "spec VPC ID takes precedence over status VPC ID", - clusterScope: PowerVSClusterScope{ - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ - Spec: infrav1beta2.IBMPowerVSClusterSpec{ - VPC: &infrav1beta2.VPCResourceReference{ID: ptr.To("vpcID")}, - }, - Status: infrav1beta2.IBMPowerVSClusterStatus{ - VPC: &infrav1beta2.ResourceReference{ - ID: ptr.To("vpcID1"), - }, - }, - }, - }, - expectedID: ptr.To("vpcID"), - }, } for _, tc := range testCases { @@ -453,24 +361,6 @@ func TestGetVPCSubnetIDs(t *testing.T) { IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{}, }, }, - { - name: "VPC subnet id is set in spec", - clusterScope: PowerVSClusterScope{ - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ - Spec: infrav1beta2.IBMPowerVSClusterSpec{ - VPCSubnets: []infrav1beta2.Subnet{ - { - ID: ptr.To("subnet1"), - }, - { - ID: ptr.To("subnet2"), - }, - }, - }, - }, - }, - expectedIDs: []*string{ptr.To("subnet1"), ptr.To("subnet2")}, - }, { name: "VPC subnet id is set in status", clusterScope: PowerVSClusterScope{ @@ -868,166 +758,337 @@ func TestGetLoadBalancerState(t *testing.T) { } func TestGetLoadBalancerHostName(t *testing.T) { - testCases := []struct { - name string - lbName string - expectedHostName *string - clusterScope PowerVSClusterScope - }{ - { - name: "LoadBalancer status is not set", - clusterScope: PowerVSClusterScope{ - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{}, + var ( + mockVPC *mock.MockVpc + mockCtrl *gomock.Controller + ) + setup := func(t *testing.T) { + t.Helper() + mockCtrl = gomock.NewController(t) + mockVPC = mock.NewMockVpc(mockCtrl) + } + teardown := func() { + mockCtrl.Finish() + } + + t.Run("Load balancer status is nil", func(t *testing.T) { + g := NewWithT(t) + setup(t) + t.Cleanup(teardown) + + clusterScope := PowerVSClusterScope{ + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Status: infrav1beta2.IBMPowerVSClusterStatus{}, }, - }, - { - name: "LoadBalancer status is empty", - clusterScope: PowerVSClusterScope{ - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ - Status: infrav1beta2.IBMPowerVSClusterStatus{ - LoadBalancers: make(map[string]infrav1beta2.VPCLoadBalancerStatus), + } + + hostName, err := clusterScope.GetPublicLoadBalancerHostName() + g.Expect(hostName).To(BeNil()) + g.Expect(err).To(BeNil()) + }) + t.Run("Load balancer name is not set in IBMPowerVSCluster spec", func(t *testing.T) { + g := NewWithT(t) + setup(t) + t.Cleanup(teardown) + + clusterScope := PowerVSClusterScope{ + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{}, + Status: infrav1beta2.IBMPowerVSClusterStatus{ + LoadBalancers: map[string]infrav1beta2.VPCLoadBalancerStatus{ + "-loadbalancer": { + Hostname: ptr.To("lb-hostname"), + }, }, }, }, - }, - { - name: "empty LoadBalancer name is passed", - clusterScope: PowerVSClusterScope{ - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ - Status: infrav1beta2.IBMPowerVSClusterStatus{ - LoadBalancers: map[string]infrav1beta2.VPCLoadBalancerStatus{ - "lb": { - Hostname: ptr.To("hostname"), - }, + } + + hostName, err := clusterScope.GetPublicLoadBalancerHostName() + g.Expect(hostName).To(Equal(ptr.To("lb-hostname"))) + g.Expect(err).To(BeNil()) + }) + t.Run("Invalid load balancer name is set in IBMPowerVSCluster spec", func(t *testing.T) { + g := NewWithT(t) + setup(t) + t.Cleanup(teardown) + + clusterScope := PowerVSClusterScope{ + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + LoadBalancers: []infrav1beta2.VPCLoadBalancerSpec{ + { + Name: "lb", + Public: core.BoolPtr(true), }, }, }, - }, - }, - { - name: "invalid LoadBalancer name is passed", - clusterScope: PowerVSClusterScope{ - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ - Status: infrav1beta2.IBMPowerVSClusterStatus{ - LoadBalancers: map[string]infrav1beta2.VPCLoadBalancerStatus{ - "lb": { - Hostname: ptr.To("hostname"), - }, + Status: infrav1beta2.IBMPowerVSClusterStatus{ + LoadBalancers: map[string]infrav1beta2.VPCLoadBalancerStatus{ + "loadbalancer": { + Hostname: ptr.To("lb-hostname"), }, }, }, }, - lbName: "lb2", - }, - { - name: "valid LoadBalancer name is passed", - clusterScope: PowerVSClusterScope{ - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ - Status: infrav1beta2.IBMPowerVSClusterStatus{ - LoadBalancers: map[string]infrav1beta2.VPCLoadBalancerStatus{ - "lb": { - Hostname: ptr.To("hostname"), - }, + } + + hostName, err := clusterScope.GetPublicLoadBalancerHostName() + g.Expect(hostName).To(BeNil()) + g.Expect(err).To(BeNil()) + }) + t.Run("Valid load balancer name is set in IBMPowerVSCluster spec", func(t *testing.T) { + g := NewWithT(t) + setup(t) + t.Cleanup(teardown) + + clusterScope := PowerVSClusterScope{ + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + LoadBalancers: []infrav1beta2.VPCLoadBalancerSpec{ + { + Name: "loadbalancer", + Public: core.BoolPtr(true), + }, + }, + }, + Status: infrav1beta2.IBMPowerVSClusterStatus{ + LoadBalancers: map[string]infrav1beta2.VPCLoadBalancerStatus{ + "loadbalancer": { + Hostname: ptr.To("lb-hostname"), }, }, }, }, - lbName: "lb", - expectedHostName: ptr.To("hostname"), - }, - } + } - for _, tc := range testCases { + hostName, err := clusterScope.GetPublicLoadBalancerHostName() + g.Expect(hostName).To(Equal(ptr.To("lb-hostname"))) + g.Expect(err).To(BeNil()) + }) + t.Run("Both public and private load balancer name is set in IBMPowerVSCluster spec", func(t *testing.T) { g := NewWithT(t) - t.Run(tc.name, func(_ *testing.T) { - lbName := tc.clusterScope.GetLoadBalancerHostName(tc.lbName) - g.Expect(utils.DereferencePointer(lbName)).To(Equal(utils.DereferencePointer(tc.expectedHostName))) - }) - } -} + setup(t) + t.Cleanup(teardown) -func TestPublicLoadBalancer(t *testing.T) { - testCases := []struct { - name string - expectedLB *infrav1beta2.VPCLoadBalancerSpec - clusterScope PowerVSClusterScope - }{ - { - name: "IBMPowerVSCluster spec is empty", - clusterScope: PowerVSClusterScope{ - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{}, - }, - expectedLB: &infrav1beta2.VPCLoadBalancerSpec{ - Name: "-loadbalancer", - Public: ptr.To(true), - }, - }, - { - name: "one public loadbalancer is configured", - clusterScope: PowerVSClusterScope{ - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ - Spec: infrav1beta2.IBMPowerVSClusterSpec{ - LoadBalancers: []infrav1beta2.VPCLoadBalancerSpec{ - { - Name: "lb", - Public: ptr.To(true), - }, + clusterScope := PowerVSClusterScope{ + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + LoadBalancers: []infrav1beta2.VPCLoadBalancerSpec{ + { + Name: "lb1", + Public: core.BoolPtr(false), + }, + { + Name: "lb2", + Public: core.BoolPtr(true), + }, + }, + }, + Status: infrav1beta2.IBMPowerVSClusterStatus{ + LoadBalancers: map[string]infrav1beta2.VPCLoadBalancerStatus{ + "lb1": { + Hostname: ptr.To("lb1-hostname"), + }, + "lb2": { + Hostname: ptr.To("lb2-hostname"), }, }, }, }, - expectedLB: &infrav1beta2.VPCLoadBalancerSpec{ - Name: "lb", - Public: ptr.To(true), + } + + hostName, err := clusterScope.GetPublicLoadBalancerHostName() + g.Expect(hostName).To(Equal(ptr.To("lb2-hostname"))) + g.Expect(err).To(BeNil()) + }) + t.Run("Multiple public load balancer names are set in IBMPowerVSCluster spec", func(t *testing.T) { + g := NewWithT(t) + setup(t) + t.Cleanup(teardown) + + clusterScope := PowerVSClusterScope{ + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + LoadBalancers: []infrav1beta2.VPCLoadBalancerSpec{ + { + Name: "lb1", + Public: core.BoolPtr(true), + }, + { + Name: "lb2", + Public: core.BoolPtr(true), + }, + }, + }, + Status: infrav1beta2.IBMPowerVSClusterStatus{ + LoadBalancers: map[string]infrav1beta2.VPCLoadBalancerStatus{ + "lb1": { + Hostname: ptr.To("lb1-hostname"), + }, + "lb2": { + Hostname: ptr.To("lb2-hostname"), + }, + }, + }, }, - }, - { - name: "multiple public loadbalancer is configured", - clusterScope: PowerVSClusterScope{ - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ - Spec: infrav1beta2.IBMPowerVSClusterSpec{ - LoadBalancers: []infrav1beta2.VPCLoadBalancerSpec{ - { - Name: "lb", - Public: ptr.To(true), - }, - { - Name: "lb2", - Public: ptr.To(true), - }, + } + + hostName, err := clusterScope.GetPublicLoadBalancerHostName() + g.Expect(hostName).To(Equal(ptr.To("lb1-hostname"))) + g.Expect(err).To(BeNil()) + }) + t.Run("Valid load balancer ID is set in IBMPowerVSCluster spec", func(t *testing.T) { + g := NewWithT(t) + setup(t) + t.Cleanup(teardown) + + clusterScope := PowerVSClusterScope{ + IBMVPCClient: mockVPC, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + LoadBalancers: []infrav1beta2.VPCLoadBalancerSpec{ + { + ID: ptr.To("loadbalancer-id"), + Public: core.BoolPtr(true), + }, + }, + }, + Status: infrav1beta2.IBMPowerVSClusterStatus{ + LoadBalancers: map[string]infrav1beta2.VPCLoadBalancerStatus{ + "loadbalancer": { + Hostname: ptr.To("lb-hostname"), }, }, }, }, - expectedLB: &infrav1beta2.VPCLoadBalancerSpec{ - Name: "lb", - Public: ptr.To(true), + } + lb := &vpcv1.LoadBalancer{ + Name: ptr.To("loadbalancer"), + } + mockVPC.EXPECT().GetLoadBalancer(gomock.Any()).Return(lb, &core.DetailedResponse{}, nil) + + hostName, err := clusterScope.GetPublicLoadBalancerHostName() + g.Expect(hostName).To(Equal(ptr.To("lb-hostname"))) + g.Expect(err).To(BeNil()) + }) + t.Run("Invalid load balancer ID is set in IBMPowerVSCluster spec", func(t *testing.T) { + g := NewWithT(t) + setup(t) + t.Cleanup(teardown) + + clusterScope := PowerVSClusterScope{ + IBMVPCClient: mockVPC, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + LoadBalancers: []infrav1beta2.VPCLoadBalancerSpec{ + { + ID: ptr.To("loadbalancer-id1"), + Public: core.BoolPtr(true), + }, + }, + }, + Status: infrav1beta2.IBMPowerVSClusterStatus{ + LoadBalancers: map[string]infrav1beta2.VPCLoadBalancerStatus{ + "loadbalancer": { + Hostname: ptr.To("lb-hostname"), + }, + }, + }, }, - }, - { - name: "only private loadbalancer is configured", - clusterScope: PowerVSClusterScope{ - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ - Spec: infrav1beta2.IBMPowerVSClusterSpec{ - LoadBalancers: []infrav1beta2.VPCLoadBalancerSpec{ - { - Name: "lb", - Public: ptr.To(false), - }, + } + + mockVPC.EXPECT().GetLoadBalancer(gomock.Any()).Return(nil, &core.DetailedResponse{}, errors.New("failed to get the load balancer")) + + hostName, err := clusterScope.GetPublicLoadBalancerHostName() + g.Expect(hostName).To(BeNil()) + g.Expect(err).ToNot(BeNil()) + }) + t.Run("Multiple public load balancer IDs are set in IBMPowerVSCluster spec", func(t *testing.T) { + g := NewWithT(t) + setup(t) + t.Cleanup(teardown) + + clusterScope := PowerVSClusterScope{ + IBMVPCClient: mockVPC, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + LoadBalancers: []infrav1beta2.VPCLoadBalancerSpec{ + { + ID: ptr.To("lb1"), + Public: core.BoolPtr(true), + }, + { + ID: ptr.To("lb2"), + Public: core.BoolPtr(true), + }, + }, + }, + Status: infrav1beta2.IBMPowerVSClusterStatus{ + LoadBalancers: map[string]infrav1beta2.VPCLoadBalancerStatus{ + "loadbalancer1": { + Hostname: ptr.To("lb1-hostname"), + }, + "loadbalancer2": { + Hostname: ptr.To("lb2-hostname"), }, }, }, }, - }, - } + } - for _, tc := range testCases { + lb := &vpcv1.LoadBalancer{ + Name: ptr.To("loadbalancer1"), + } + mockVPC.EXPECT().GetLoadBalancer(gomock.Any()).Return(lb, &core.DetailedResponse{}, nil) + + hostName, err := clusterScope.GetPublicLoadBalancerHostName() + g.Expect(hostName).To(Equal(ptr.To("lb1-hostname"))) + g.Expect(err).To(BeNil()) + }) + + t.Run("Both private and public load balancer IDs are set in IBMPowerVSCluster spec", func(t *testing.T) { g := NewWithT(t) - t.Run(tc.name, func(_ *testing.T) { - lbSpec := tc.clusterScope.PublicLoadBalancer() - g.Expect(lbSpec).To(Equal(tc.expectedLB)) - }) - } + setup(t) + t.Cleanup(teardown) + + clusterScope := PowerVSClusterScope{ + IBMVPCClient: mockVPC, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + LoadBalancers: []infrav1beta2.VPCLoadBalancerSpec{ + { + ID: ptr.To("lb1"), + Public: core.BoolPtr(false), + }, + { + ID: ptr.To("lb2"), + Public: core.BoolPtr(true), + }, + }, + }, + Status: infrav1beta2.IBMPowerVSClusterStatus{ + LoadBalancers: map[string]infrav1beta2.VPCLoadBalancerStatus{ + "loadbalancer1": { + Hostname: ptr.To("lb1-hostname"), + }, + "loadbalancer2": { + Hostname: ptr.To("lb2-hostname"), + }, + }, + }, + }, + } + + lb := &vpcv1.LoadBalancer{ + Name: ptr.To("loadbalancer2"), + } + mockVPC.EXPECT().GetLoadBalancer(gomock.Any()).Return(lb, &core.DetailedResponse{}, nil) + + hostName, err := clusterScope.GetPublicLoadBalancerHostName() + g.Expect(hostName).To(Equal(ptr.To("lb2-hostname"))) + g.Expect(err).To(BeNil()) + }) } func TestGetResourceGroupID(t *testing.T) { @@ -1107,125 +1168,151 @@ func TestReconcilePowerVSServiceInstance(t *testing.T) { teardown := func() { mockCtrl.Finish() } - t.Run("When service instance id is set and GetResourceInstance returns error", func(t *testing.T) { + t.Run("When service instance id is set in status and GetResourceInstance returns error", func(t *testing.T) { g := NewWithT(t) setup(t) t.Cleanup(teardown) - clusterScopeParams := getPowerVSClusterScopeParams() - clusterScopeParams.ResourceControllerFactory = func() (resourcecontroller.ResourceController, error) { - mockResourceController.EXPECT().GetResourceInstance(gomock.Any()).Return(nil, nil, errors.New("failed to get resource instance")) - return mockResourceController, nil + clusterScope := PowerVSClusterScope{ + ResourceClient: mockResourceController, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Status: infrav1beta2.IBMPowerVSClusterStatus{ + ServiceInstance: &infrav1beta2.ResourceReference{ + ID: ptr.To(serviceInstanceID), + }, + }, + }, } - clusterScope, err := NewPowerVSClusterScope(clusterScopeParams) - g.Expect(err).To(BeNil()) - clusterScope.IBMPowerVSCluster.Spec.ServiceInstanceID = serviceInstanceID + + mockResourceController.EXPECT().GetResourceInstance(gomock.Any()).Return(nil, nil, errors.New("failed to get resource instance")) requeue, err := clusterScope.ReconcilePowerVSServiceInstance() g.Expect(err).ToNot(BeNil()) g.Expect(requeue).To(BeFalse()) }) - t.Run("When service instance id is set and GetResourceInstance returns nil", func(t *testing.T) { + t.Run("When service instance id is set in status and GetResourceInstance returns nil", func(t *testing.T) { g := NewWithT(t) setup(t) t.Cleanup(teardown) - clusterScopeParams := getPowerVSClusterScopeParams() - clusterScopeParams.ResourceControllerFactory = func() (resourcecontroller.ResourceController, error) { - mockResourceController.EXPECT().GetResourceInstance(gomock.Any()).Return(nil, nil, nil) - return mockResourceController, nil + clusterScope := PowerVSClusterScope{ + ResourceClient: mockResourceController, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Status: infrav1beta2.IBMPowerVSClusterStatus{ + ServiceInstance: &infrav1beta2.ResourceReference{ + ID: ptr.To(serviceInstanceID), + }, + }, + }, } - clusterScope, err := NewPowerVSClusterScope(clusterScopeParams) - g.Expect(err).To(BeNil()) - clusterScope.IBMPowerVSCluster.Spec.ServiceInstanceID = serviceInstanceID + + mockResourceController.EXPECT().GetResourceInstance(gomock.Any()).Return(nil, nil, nil) requeue, err := clusterScope.ReconcilePowerVSServiceInstance() g.Expect(err).ToNot(BeNil()) g.Expect(requeue).To(BeFalse()) }) - t.Run("When service instance id is set and instance in failed state", func(t *testing.T) { + t.Run("When service instance id is set in status and instance is in failed state", func(t *testing.T) { g := NewWithT(t) setup(t) t.Cleanup(teardown) - clusterScopeParams := getPowerVSClusterScopeParams() - clusterScopeParams.ResourceControllerFactory = func() (resourcecontroller.ResourceController, error) { - instance := &resourcecontrollerv2.ResourceInstance{ - Name: ptr.To("test-instance"), - State: ptr.To("failed"), - } - mockResourceController.EXPECT().GetResourceInstance(gomock.Any()).Return(instance, nil, nil) - return mockResourceController, nil + clusterScope := PowerVSClusterScope{ + ResourceClient: mockResourceController, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Status: infrav1beta2.IBMPowerVSClusterStatus{ + ServiceInstance: &infrav1beta2.ResourceReference{ + ID: ptr.To(serviceInstanceID), + }, + }, + }, } - clusterScope, err := NewPowerVSClusterScope(clusterScopeParams) - g.Expect(err).To(BeNil()) - clusterScope.IBMPowerVSCluster.Spec.ServiceInstanceID = serviceInstanceID + + instance := &resourcecontrollerv2.ResourceInstance{ + Name: ptr.To("test-instance"), + State: ptr.To("failed"), + } + mockResourceController.EXPECT().GetResourceInstance(gomock.Any()).Return(instance, nil, nil) requeue, err := clusterScope.ReconcilePowerVSServiceInstance() g.Expect(err).ToNot(BeNil()) g.Expect(requeue).To(BeFalse()) }) - t.Run("When service instance id is set and instance in active state", func(t *testing.T) { + t.Run("When service instance id is set in status and instance is in active state", func(t *testing.T) { g := NewWithT(t) setup(t) t.Cleanup(teardown) - clusterScopeParams := getPowerVSClusterScopeParams() - clusterScopeParams.ResourceControllerFactory = func() (resourcecontroller.ResourceController, error) { - instance := &resourcecontrollerv2.ResourceInstance{ - Name: ptr.To("test-instance"), - State: ptr.To("active"), - } - mockResourceController.EXPECT().GetResourceInstance(gomock.Any()).Return(instance, nil, nil) - return mockResourceController, nil + clusterScope := PowerVSClusterScope{ + ResourceClient: mockResourceController, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Status: infrav1beta2.IBMPowerVSClusterStatus{ + ServiceInstance: &infrav1beta2.ResourceReference{ + ID: ptr.To(serviceInstanceID), + }, + }, + }, } - clusterScope, err := NewPowerVSClusterScope(clusterScopeParams) - g.Expect(err).To(BeNil()) - clusterScope.IBMPowerVSCluster.Spec.ServiceInstanceID = serviceInstanceID + + instance := &resourcecontrollerv2.ResourceInstance{ + Name: ptr.To("test-instance"), + State: ptr.To("active"), + } + mockResourceController.EXPECT().GetResourceInstance(gomock.Any()).Return(instance, nil, nil) requeue, err := clusterScope.ReconcilePowerVSServiceInstance() g.Expect(err).To(BeNil()) g.Expect(requeue).To(BeFalse()) }) - t.Run("When isServiceInstanceExists returns error", func(t *testing.T) { + t.Run("When service instance id is set in spec and instance does not exist", func(t *testing.T) { g := NewWithT(t) setup(t) t.Cleanup(teardown) - clusterScopeParams := getPowerVSClusterScopeParams() - clusterScopeParams.ResourceControllerFactory = func() (resourcecontroller.ResourceController, error) { - mockResourceController.EXPECT().GetServiceInstance(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("failed to get service instance")) - return mockResourceController, nil + clusterScope := PowerVSClusterScope{ + ResourceClient: mockResourceController, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + ServiceInstance: &infrav1beta2.IBMPowerVSResourceReference{ + ID: ptr.To(serviceInstanceID), + }, + }, + }, } - clusterScope, err := NewPowerVSClusterScope(clusterScopeParams) - g.Expect(err).To(BeNil()) + + mockResourceController.EXPECT().GetResourceInstance(gomock.Any()).Return(nil, nil, errors.New("failed to get resource instance")) requeue, err := clusterScope.ReconcilePowerVSServiceInstance() g.Expect(err).ToNot(BeNil()) g.Expect(requeue).To(BeFalse()) }) - t.Run("When service instance exists in cloud and in active state", func(t *testing.T) { + t.Run("When service instance id is set in spec and instance exists in active state", func(t *testing.T) { g := NewWithT(t) setup(t) t.Cleanup(teardown) - clusterScopeParams := getPowerVSClusterScopeParams() - clusterScopeParams.ResourceControllerFactory = func() (resourcecontroller.ResourceController, error) { - instance := &resourcecontrollerv2.ResourceInstance{ - GUID: ptr.To("instance-GUID"), - Name: ptr.To("test-instance"), - State: ptr.To("active"), - } - mockResourceController.EXPECT().GetServiceInstance(gomock.Any(), gomock.Any(), gomock.Any()).Return(instance, nil) - return mockResourceController, nil + clusterScope := PowerVSClusterScope{ + ResourceClient: mockResourceController, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + ServiceInstance: &infrav1beta2.IBMPowerVSResourceReference{ + ID: ptr.To(serviceInstanceID), + }, + }, + }, } - clusterScope, err := NewPowerVSClusterScope(clusterScopeParams) - g.Expect(err).To(BeNil()) + + instance := &resourcecontrollerv2.ResourceInstance{ + Name: ptr.To("test-instance"), + State: ptr.To("active"), + GUID: ptr.To("instance-GUID"), + } + mockResourceController.EXPECT().GetResourceInstance(gomock.Any()).Return(instance, nil, nil) requeue, err := clusterScope.ReconcilePowerVSServiceInstance() g.Expect(err).To(BeNil()) @@ -1234,20 +1321,51 @@ func TestReconcilePowerVSServiceInstance(t *testing.T) { g.Expect(*clusterScope.IBMPowerVSCluster.Status.ServiceInstance.ControllerCreated).To(BeFalse()) }) - t.Run("When create service instance return error", func(t *testing.T) { + t.Run("When service instance id is set in both spec and status, ID from status takes precedence", func(t *testing.T) { g := NewWithT(t) setup(t) t.Cleanup(teardown) - clusterScopeParams := getPowerVSClusterScopeParams() - clusterScopeParams.ResourceControllerFactory = func() (resourcecontroller.ResourceController, error) { - mockResourceController.EXPECT().GetServiceInstance(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil) - mockResourceController.EXPECT().CreateResourceInstance(gomock.Any()).Return(nil, nil, errors.New("failed to create resource instance")) - return mockResourceController, nil + clusterScope := PowerVSClusterScope{ + ResourceClient: mockResourceController, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + ServiceInstance: &infrav1beta2.IBMPowerVSResourceReference{ + ID: ptr.To(serviceInstanceID), + }, + }, + Status: infrav1beta2.IBMPowerVSClusterStatus{ + ServiceInstance: &infrav1beta2.ResourceReference{ + ID: ptr.To(serviceInstanceID), + }, + }, + }, + } + + instance := &resourcecontrollerv2.ResourceInstance{ + Name: ptr.To("test-instance"), + State: ptr.To("active"), } - clusterScope, err := NewPowerVSClusterScope(clusterScopeParams) + mockResourceController.EXPECT().GetResourceInstance(gomock.Any()).Return(instance, nil, nil) + + requeue, err := clusterScope.ReconcilePowerVSServiceInstance() g.Expect(err).To(BeNil()) - clusterScope.IBMPowerVSCluster.Spec.ResourceGroup = &infrav1beta2.IBMPowerVSResourceReference{ID: ptr.To("resourceGroupID")} + g.Expect(requeue).To(BeFalse()) + }) + + t.Run("When create service instance returns error", func(t *testing.T) { + g := NewWithT(t) + setup(t) + t.Cleanup(teardown) + + clusterScope := PowerVSClusterScope{ + ResourceClient: mockResourceController, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{}, + }, + } + + mockResourceController.EXPECT().GetServiceInstance(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil) requeue, err := clusterScope.ReconcilePowerVSServiceInstance() g.Expect(err).ToNot(BeNil()) @@ -1259,40 +1377,50 @@ func TestReconcilePowerVSServiceInstance(t *testing.T) { setup(t) t.Cleanup(teardown) - clusterScopeParams := getPowerVSClusterScopeParams() - clusterScopeParams.ResourceControllerFactory = func() (resourcecontroller.ResourceController, error) { - mockResourceController.EXPECT().GetServiceInstance(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil) - mockResourceController.EXPECT().CreateResourceInstance(gomock.Any()).Return(nil, nil, nil) - return mockResourceController, nil + clusterScope := PowerVSClusterScope{ + ResourceClient: mockResourceController, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + ResourceGroup: &infrav1beta2.IBMPowerVSResourceReference{ + ID: ptr.To("resource-group-id"), + }, + Zone: ptr.To("zone1"), + }, + }, } - clusterScope, err := NewPowerVSClusterScope(clusterScopeParams) - g.Expect(err).To(BeNil()) - clusterScope.IBMPowerVSCluster.Spec.ResourceGroup = &infrav1beta2.IBMPowerVSResourceReference{ID: ptr.To("resourceGroupID")} + + mockResourceController.EXPECT().GetServiceInstance(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil) + mockResourceController.EXPECT().CreateResourceInstance(gomock.Any()).Return(nil, nil, nil) requeue, err := clusterScope.ReconcilePowerVSServiceInstance() g.Expect(err).ToNot(BeNil()) g.Expect(requeue).To(BeFalse()) }) - t.Run("When successfully created new service instance", func(t *testing.T) { + t.Run("When successfully created a new service instance", func(t *testing.T) { g := NewWithT(t) setup(t) t.Cleanup(teardown) - clusterScopeParams := getPowerVSClusterScopeParams() - clusterScopeParams.ResourceControllerFactory = func() (resourcecontroller.ResourceController, error) { - instance := &resourcecontrollerv2.ResourceInstance{ - GUID: ptr.To("instance-GUID"), - Name: ptr.To("test-instance"), - } + clusterScope := PowerVSClusterScope{ + ResourceClient: mockResourceController, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + ResourceGroup: &infrav1beta2.IBMPowerVSResourceReference{ + ID: ptr.To("resource-group-id"), + }, + Zone: ptr.To("zone1"), + }, + }, + } - mockResourceController.EXPECT().GetServiceInstance(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil) - mockResourceController.EXPECT().CreateResourceInstance(gomock.Any()).Return(instance, nil, nil) - return mockResourceController, nil + instance := &resourcecontrollerv2.ResourceInstance{ + GUID: ptr.To("instance-GUID"), + Name: ptr.To("test-instance"), } - clusterScope, err := NewPowerVSClusterScope(clusterScopeParams) - g.Expect(err).To(BeNil()) - clusterScope.IBMPowerVSCluster.Spec.ResourceGroup = &infrav1beta2.IBMPowerVSResourceReference{ID: ptr.To("resourceGroupID")} + + mockResourceController.EXPECT().GetServiceInstance(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil) + mockResourceController.EXPECT().CreateResourceInstance(gomock.Any()).Return(instance, nil, nil) requeue, err := clusterScope.ReconcilePowerVSServiceInstance() g.Expect(err).To(BeNil()) @@ -1358,90 +1486,116 @@ func TestIsServiceInstanceExists(t *testing.T) { teardown := func() { mockCtrl.Finish() } - t.Run("When get service instance returns error", func(t *testing.T) { + + t.Run("When ServiceInstanceID is set in spec and GetResourceInstance returns error", func(t *testing.T) { g := NewWithT(t) setup(t) t.Cleanup(teardown) - clusterScopeParams := getPowerVSClusterScopeParams() - clusterScopeParams.ResourceControllerFactory = func() (resourcecontroller.ResourceController, error) { - mockResourceController.EXPECT().GetServiceInstance(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("failed to get service instance")) - return mockResourceController, nil + clusterScope := PowerVSClusterScope{ + ResourceClient: mockResourceController, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + ServiceInstanceID: "instance-id", + }, + }, } - clusterScope, err := NewPowerVSClusterScope(clusterScopeParams) - g.Expect(err).To(BeNil()) + + mockResourceController.EXPECT().GetResourceInstance(gomock.Any()).Return(nil, nil, errors.New("failed to get resource instance")) instanceID, requeue, err := clusterScope.isServiceInstanceExists() g.Expect(instanceID).To(Equal("")) g.Expect(requeue).To(BeFalse()) g.Expect(err).NotTo(BeNil()) }) - t.Run("When get service instance returns nil", func(t *testing.T) { + t.Run("When ServiceInstance.ID is set in spec and GetResourceInstance returns error", func(t *testing.T) { g := NewWithT(t) setup(t) t.Cleanup(teardown) - clusterScopeParams := getPowerVSClusterScopeParams() - clusterScopeParams.ResourceControllerFactory = func() (resourcecontroller.ResourceController, error) { - mockResourceController.EXPECT().GetServiceInstance(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil) - return mockResourceController, nil + clusterScope := PowerVSClusterScope{ + ResourceClient: mockResourceController, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + ServiceInstance: &infrav1beta2.IBMPowerVSResourceReference{ + ID: ptr.To("instance-id"), + }, + }, + }, } - clusterScope, err := NewPowerVSClusterScope(clusterScopeParams) - g.Expect(err).To(BeNil()) + + mockResourceController.EXPECT().GetResourceInstance(gomock.Any()).Return(nil, nil, errors.New("failed to get resource instance")) instanceID, requeue, err := clusterScope.isServiceInstanceExists() g.Expect(instanceID).To(Equal("")) g.Expect(requeue).To(BeFalse()) - g.Expect(err).To(BeNil()) + g.Expect(err).NotTo(BeNil()) }) - t.Run("When checkServiceInstanceState returns error", func(t *testing.T) { + t.Run("When ServiceInstance.Name is set in spec and GetResourceInstance returns nil", func(t *testing.T) { g := NewWithT(t) setup(t) t.Cleanup(teardown) - clusterScopeParams := getPowerVSClusterScopeParams() - clusterScopeParams.ResourceControllerFactory = func() (resourcecontroller.ResourceController, error) { - mockResourceController.EXPECT().GetServiceInstance(gomock.Any(), gomock.Any(), gomock.Any()).Return(&resourcecontrollerv2.ResourceInstance{Name: ptr.To("instance"), State: ptr.To("unknown")}, nil) - return mockResourceController, nil + clusterScope := PowerVSClusterScope{ + ResourceClient: mockResourceController, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + ServiceInstance: &infrav1beta2.IBMPowerVSResourceReference{ + Name: ptr.To("instance-name"), + }, + }, + }, } - clusterScope, err := NewPowerVSClusterScope(clusterScopeParams) - g.Expect(err).To(BeNil()) + + mockResourceController.EXPECT().GetServiceInstance(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil) instanceID, requeue, err := clusterScope.isServiceInstanceExists() g.Expect(instanceID).To(Equal("")) g.Expect(requeue).To(BeFalse()) - g.Expect(err).ToNot(BeNil()) + g.Expect(err).To(BeNil()) }) + t.Run("When checkServiceInstanceState returns error", func(t *testing.T) { g := NewWithT(t) setup(t) t.Cleanup(teardown) - clusterScopeParams := getPowerVSClusterScopeParams() - clusterScopeParams.ResourceControllerFactory = func() (resourcecontroller.ResourceController, error) { - mockResourceController.EXPECT().GetServiceInstance(gomock.Any(), gomock.Any(), gomock.Any()).Return(&resourcecontrollerv2.ResourceInstance{Name: ptr.To("instance"), State: ptr.To("unknown")}, nil) - return mockResourceController, nil + clusterScope := PowerVSClusterScope{ + ResourceClient: mockResourceController, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + ServiceInstance: &infrav1beta2.IBMPowerVSResourceReference{ + Name: ptr.To("instance"), + }, + }, + }, } - clusterScope, err := NewPowerVSClusterScope(clusterScopeParams) - g.Expect(err).To(BeNil()) + + mockResourceController.EXPECT().GetServiceInstance(gomock.Any(), gomock.Any(), gomock.Any()).Return(&resourcecontrollerv2.ResourceInstance{Name: ptr.To("instance"), State: ptr.To("unknown")}, nil) instanceID, requeue, err := clusterScope.isServiceInstanceExists() g.Expect(instanceID).To(Equal("")) g.Expect(requeue).To(BeFalse()) g.Expect(err).ToNot(BeNil()) }) + t.Run("When isServiceInstanceExists returns success", func(t *testing.T) { g := NewWithT(t) setup(t) t.Cleanup(teardown) - clusterScopeParams := getPowerVSClusterScopeParams() - clusterScopeParams.ResourceControllerFactory = func() (resourcecontroller.ResourceController, error) { - mockResourceController.EXPECT().GetServiceInstance(gomock.Any(), gomock.Any(), gomock.Any()).Return(&resourcecontrollerv2.ResourceInstance{GUID: ptr.To("guid"), Name: ptr.To("instance"), State: ptr.To("active")}, nil) - return mockResourceController, nil + clusterScope := PowerVSClusterScope{ + ResourceClient: mockResourceController, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + ServiceInstance: &infrav1beta2.IBMPowerVSResourceReference{ + Name: ptr.To("instance"), + }, + }, + }, } - clusterScope, err := NewPowerVSClusterScope(clusterScopeParams) - g.Expect(err).To(BeNil()) + + mockResourceController.EXPECT().GetServiceInstance(gomock.Any(), gomock.Any(), gomock.Any()).Return(&resourcecontrollerv2.ResourceInstance{GUID: ptr.To("guid"), Name: ptr.To("instance"), State: ptr.To("active")}, nil) instanceID, requeue, err := clusterScope.isServiceInstanceExists() g.Expect(instanceID).To(Equal("guid")) @@ -1468,10 +1622,12 @@ func TestCreateServiceInstance(t *testing.T) { setup(t) t.Cleanup(teardown) - clusterScopeParams := getPowerVSClusterScopeParams() - clusterScopeParams.IBMPowerVSCluster.Spec.ResourceGroup = nil - clusterScope, err := NewPowerVSClusterScope(clusterScopeParams) - g.Expect(err).To(BeNil()) + clusterScope := PowerVSClusterScope{ + ResourceClient: mockResourceController, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{}, + }, + } instance, err := clusterScope.createServiceInstance() g.Expect(instance).To(BeNil()) @@ -1482,10 +1638,16 @@ func TestCreateServiceInstance(t *testing.T) { setup(t) t.Cleanup(teardown) - clusterScopeParams := getPowerVSClusterScopeParams() - clusterScope, err := NewPowerVSClusterScope(clusterScopeParams) - g.Expect(err).To(BeNil()) - clusterScope.IBMPowerVSCluster.Spec.Zone = nil + clusterScope := PowerVSClusterScope{ + ResourceClient: mockResourceController, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + ResourceGroup: &infrav1beta2.IBMPowerVSResourceReference{ + ID: ptr.To("resource-group-id"), + }, + }, + }, + } instance, err := clusterScope.createServiceInstance() g.Expect(instance).To(BeNil()) @@ -1496,13 +1658,19 @@ func TestCreateServiceInstance(t *testing.T) { setup(t) t.Cleanup(teardown) - clusterScopeParams := getPowerVSClusterScopeParams() - clusterScopeParams.ResourceControllerFactory = func() (resourcecontroller.ResourceController, error) { - mockResourceController.EXPECT().CreateResourceInstance(gomock.Any()).Return(nil, nil, errors.New("failed to create resource instance")) - return mockResourceController, nil + clusterScope := PowerVSClusterScope{ + ResourceClient: mockResourceController, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + ResourceGroup: &infrav1beta2.IBMPowerVSResourceReference{ + ID: ptr.To("resource-group-id"), + }, + Zone: ptr.To("zone1"), + }, + }, } - clusterScope, err := NewPowerVSClusterScope(clusterScopeParams) - g.Expect(err).To(BeNil()) + + mockResourceController.EXPECT().CreateResourceInstance(gomock.Any()).Return(nil, nil, errors.New("failed to create resource instance")) instance, err := clusterScope.createServiceInstance() g.Expect(instance).To(BeNil()) @@ -1513,60 +1681,22 @@ func TestCreateServiceInstance(t *testing.T) { setup(t) t.Cleanup(teardown) - clusterScopeParams := getPowerVSClusterScopeParams() - clusterScopeParams.ResourceControllerFactory = func() (resourcecontroller.ResourceController, error) { - mockResourceController.EXPECT().CreateResourceInstance(gomock.Any()).Return(&resourcecontrollerv2.ResourceInstance{}, nil, nil) - return mockResourceController, nil + clusterScope := PowerVSClusterScope{ + ResourceClient: mockResourceController, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + ResourceGroup: &infrav1beta2.IBMPowerVSResourceReference{ + ID: ptr.To("resource-group-id"), + }, + Zone: ptr.To("zone1"), + }, + }, } - clusterScope, err := NewPowerVSClusterScope(clusterScopeParams) - g.Expect(err).To(BeNil()) + + mockResourceController.EXPECT().CreateResourceInstance(gomock.Any()).Return(&resourcecontrollerv2.ResourceInstance{}, nil, nil) instance, err := clusterScope.createServiceInstance() g.Expect(instance).ToNot(BeNil()) g.Expect(err).To(BeNil()) }) } - -func getPowerVSClusterScopeParams() PowerVSClusterScopeParams { - return PowerVSClusterScopeParams{ - Client: testEnv.Client, - Cluster: newCluster(clusterName), - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"powervs.cluster.x-k8s.io/create-infra": "true"}, - GenerateName: "powervs-test-", - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: capiv1beta1.GroupVersion.String(), - Kind: "Cluster", - Name: "capi-test", - UID: "1", - }}}, - Spec: infrav1beta2.IBMPowerVSClusterSpec{ - Zone: ptr.To("zone"), - VPC: &infrav1beta2.VPCResourceReference{Region: ptr.To("eu-gb")}, - ResourceGroup: &infrav1beta2.IBMPowerVSResourceReference{ID: ptr.To("rg-id")}, - }, - }, - ClientFactory: ClientFactory{ - AuthenticatorFactory: func() (core.Authenticator, error) { - return nil, nil - }, - PowerVSClientFactory: func() (powervs.PowerVS, error) { - return nil, nil - }, - VPCClientFactory: func() (vpc.Vpc, error) { - return nil, nil - }, - TransitGatewayFactory: func() (transitgateway.TransitGateway, error) { - return nil, nil - }, - ResourceControllerFactory: func() (resourcecontroller.ResourceController, error) { - return nil, nil - }, - ResourceManagerFactory: func() (resourcemanager.ResourceManager, error) { - return nil, nil - }, - }, - } -} diff --git a/controllers/ibmpowervscluster_controller.go b/controllers/ibmpowervscluster_controller.go index 59131a988..6021492e3 100644 --- a/controllers/ibmpowervscluster_controller.go +++ b/controllers/ibmpowervscluster_controller.go @@ -330,20 +330,18 @@ func (r *IBMPowerVSClusterReconciler) reconcile(clusterScope *scope.PowerVSClust return ctrl.Result{RequeueAfter: 30 * time.Second}, nil } - // update cluster object with loadbalancer host - loadBalancer := clusterScope.PublicLoadBalancer() - if loadBalancer == nil { - return reconcile.Result{}, fmt.Errorf("failed to fetch public loadbalancer") - } - clusterScope.Info("Getting load balancer host") - hostName := clusterScope.GetLoadBalancerHostName(loadBalancer.Name) + hostName, err := clusterScope.GetPublicLoadBalancerHostName() + if err != nil { + return reconcile.Result{}, fmt.Errorf("failed to fetch public loadbalancer: %w", err) + } if hostName == nil || *hostName == "" { clusterScope.Info("LoadBalancer hostname is not yet available, requeuing") return reconcile.Result{RequeueAfter: time.Minute}, nil } - clusterScope.IBMPowerVSCluster.Spec.ControlPlaneEndpoint.Host = *clusterScope.GetLoadBalancerHostName(loadBalancer.Name) + // update cluster object with loadbalancer host name + clusterScope.IBMPowerVSCluster.Spec.ControlPlaneEndpoint.Host = *hostName clusterScope.IBMPowerVSCluster.Spec.ControlPlaneEndpoint.Port = clusterScope.APIServerPort() clusterScope.IBMPowerVSCluster.Status.Ready = true return ctrl.Result{}, nil