diff --git a/cloud/scope/load_balancer_reconciler.go b/cloud/scope/load_balancer_reconciler.go index ffad11b2..5aeba192 100644 --- a/cloud/scope/load_balancer_reconciler.go +++ b/cloud/scope/load_balancer_reconciler.go @@ -37,6 +37,9 @@ func (s *ClusterScope) ReconcileApiServerLB(ctx context.Context) error { return err } if lb != nil { + if lb.LifecycleState != loadbalancer.LoadBalancerLifecycleStateActive { + return errors.New(fmt.Sprintf("load balancer is in %s state. Waiting for ACTIVE state.", lb.LifecycleState)) + } lbIP, err := s.getLoadbalancerIp(*lb) if err != nil { return err diff --git a/cloud/scope/load_balancer_reconciler_test.go b/cloud/scope/load_balancer_reconciler_test.go index 929ecc6f..6804d946 100644 --- a/cloud/scope/load_balancer_reconciler_test.go +++ b/cloud/scope/load_balancer_reconciler_test.go @@ -97,11 +97,12 @@ func TestLBReconciliation(t *testing.T) { })). Return(loadbalancer.GetLoadBalancerResponse{ LoadBalancer: loadbalancer.LoadBalancer{ - Id: common.String("lb-id"), - FreeformTags: tags, - DefinedTags: make(map[string]map[string]interface{}), - IsPrivate: common.Bool(false), - DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")), + Id: common.String("lb-id"), + LifecycleState: loadbalancer.LoadBalancerLifecycleStateActive, + FreeformTags: tags, + DefinedTags: make(map[string]map[string]interface{}), + IsPrivate: common.Bool(false), + DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")), IpAddresses: []loadbalancer.IpAddress{ { IpAddress: common.String("2.2.2.2"), @@ -123,11 +124,12 @@ func TestLBReconciliation(t *testing.T) { })). Return(loadbalancer.GetLoadBalancerResponse{ LoadBalancer: loadbalancer.LoadBalancer{ - Id: common.String("lb-id"), - FreeformTags: tags, - DefinedTags: make(map[string]map[string]interface{}), - IsPrivate: common.Bool(false), - DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")), + Id: common.String("lb-id"), + LifecycleState: loadbalancer.LoadBalancerLifecycleStateActive, + FreeformTags: tags, + DefinedTags: make(map[string]map[string]interface{}), + IsPrivate: common.Bool(false), + DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")), }, }, nil) }, @@ -143,11 +145,12 @@ func TestLBReconciliation(t *testing.T) { })). Return(loadbalancer.GetLoadBalancerResponse{ LoadBalancer: loadbalancer.LoadBalancer{ - Id: common.String("lb-id"), - FreeformTags: tags, - DefinedTags: make(map[string]map[string]interface{}), - IsPrivate: common.Bool(false), - DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")), + Id: common.String("lb-id"), + LifecycleState: loadbalancer.LoadBalancerLifecycleStateActive, + FreeformTags: tags, + DefinedTags: make(map[string]map[string]interface{}), + IsPrivate: common.Bool(false), + DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")), IpAddresses: []loadbalancer.IpAddress{ { IpAddress: common.String("2.2.2.2"), @@ -189,11 +192,12 @@ func TestLBReconciliation(t *testing.T) { })). Return(loadbalancer.GetLoadBalancerResponse{ LoadBalancer: loadbalancer.LoadBalancer{ - Id: common.String("lb-id"), - FreeformTags: tags, - DefinedTags: make(map[string]map[string]interface{}), - IsPrivate: common.Bool(false), - DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")), + Id: common.String("lb-id"), + LifecycleState: loadbalancer.LoadBalancerLifecycleStateActive, + FreeformTags: tags, + DefinedTags: make(map[string]map[string]interface{}), + IsPrivate: common.Bool(false), + DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")), IpAddresses: []loadbalancer.IpAddress{ { IpAddress: common.String("2.2.2.2"), @@ -537,11 +541,12 @@ func TestLBReconciliation(t *testing.T) { })). Return(loadbalancer.GetLoadBalancerResponse{ LoadBalancer: loadbalancer.LoadBalancer{ - Id: common.String("lb-id"), - FreeformTags: tags, - DefinedTags: make(map[string]map[string]interface{}), - IsPrivate: common.Bool(false), - DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver-test")), + Id: common.String("lb-id"), + LifecycleState: loadbalancer.LoadBalancerLifecycleStateActive, + FreeformTags: tags, + DefinedTags: make(map[string]map[string]interface{}), + IsPrivate: common.Bool(false), + DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver-test")), IpAddresses: []loadbalancer.IpAddress{ { IpAddress: common.String("2.2.2.2"), @@ -570,6 +575,34 @@ func TestLBReconciliation(t *testing.T) { }, nil) }, }, + { + name: "lb not active", + errorExpected: true, + errorSubStringMatch: true, + matchError: errors.New(fmt.Sprintf("load balancer is in %s state. Waiting for ACTIVE state.", loadbalancer.LoadBalancerLifecycleStateCreating)), + testSpecificSetup: func(clusterScope *ClusterScope, lbClient *mock_lb.MockLoadBalancerClient) { + ociClusterAccessor.OCICluster.Spec.NetworkSpec.APIServerLB.LoadBalancerId = common.String("lb-id") + lbClient.EXPECT().GetLoadBalancer(gomock.Any(), gomock.Eq(loadbalancer.GetLoadBalancerRequest{ + LoadBalancerId: common.String("lb-id"), + })). + Return(loadbalancer.GetLoadBalancerResponse{ + LoadBalancer: loadbalancer.LoadBalancer{ + Id: common.String("lb-id"), + LifecycleState: loadbalancer.LoadBalancerLifecycleStateCreating, + FreeformTags: tags, + DefinedTags: make(map[string]map[string]interface{}), + IsPrivate: common.Bool(false), + DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver-test")), + IpAddresses: []loadbalancer.IpAddress{ + { + IpAddress: common.String("2.2.2.2"), + IsPublic: common.Bool(true), + }, + }, + }, + }, nil) + }, + }, { name: "lb update request failed", errorExpected: true, @@ -582,11 +615,12 @@ func TestLBReconciliation(t *testing.T) { })). Return(loadbalancer.GetLoadBalancerResponse{ LoadBalancer: loadbalancer.LoadBalancer{ - Id: common.String("lb-id"), - FreeformTags: tags, - DefinedTags: make(map[string]map[string]interface{}), - IsPrivate: common.Bool(false), - DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver-test")), + Id: common.String("lb-id"), + LifecycleState: loadbalancer.LoadBalancerLifecycleStateActive, + FreeformTags: tags, + DefinedTags: make(map[string]map[string]interface{}), + IsPrivate: common.Bool(false), + DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver-test")), IpAddresses: []loadbalancer.IpAddress{ { IpAddress: common.String("2.2.2.2"), diff --git a/cloud/scope/machine.go b/cloud/scope/machine.go index 651c68fa..fe684436 100644 --- a/cloud/scope/machine.go +++ b/cloud/scope/machine.go @@ -520,33 +520,29 @@ func (m *MachineScope) ReconcileCreateInstanceOnLB(ctx context.Context) error { backendName := instanceIp + ":" + strconv.Itoa(int(m.OCICluster.Spec.ControlPlaneEndpoint.Port)) if !m.containsLBBackend(backendSet, backendName) { logger := m.Logger.WithValues("backend-set", *backendSet.Name) - workRequest := m.OCIMachine.Status.CreateBackendWorkRequestId logger.Info("Checking work request status for create backend") - if workRequest != "" { - _, err = ociutil.AwaitLBWorkRequest(ctx, m.LoadBalancerClient, &workRequest) - if err != nil { - return err - } - } else { - resp, err := m.LoadBalancerClient.CreateBackend(ctx, loadbalancer.CreateBackendRequest{ - LoadBalancerId: loadbalancerId, - BackendSetName: backendSet.Name, - CreateBackendDetails: loadbalancer.CreateBackendDetails{ - IpAddress: common.String(instanceIp), - Port: common.Int(int(m.OCICluster.Spec.ControlPlaneEndpoint.Port)), - }, - OpcRetryToken: ociutil.GetOPCRetryToken("%s-%s", "create-backend", string(m.OCIMachine.UID)), - }) - if err != nil { - return err - } - m.OCIMachine.Status.CreateBackendWorkRequestId = *resp.OpcWorkRequestId - logger.Info("Add instance to LB backend-set", "WorkRequestId", resp.OpcWorkRequestId) - logger.Info("Waiting for LB work request to be complete") - _, err = ociutil.AwaitLBWorkRequest(ctx, m.LoadBalancerClient, resp.OpcWorkRequestId) - if err != nil { - return err - } + // we always try to create the backend if it exists during a reconcile loop and wait for the work request + // to complete. If there is a work request in progress, in the rare case, CAPOCI pod restarts during the + // work request, the create backend call may throw an error which is ok, as reconcile will go into + // an exponential backoff + resp, err := m.LoadBalancerClient.CreateBackend(ctx, loadbalancer.CreateBackendRequest{ + LoadBalancerId: loadbalancerId, + BackendSetName: backendSet.Name, + CreateBackendDetails: loadbalancer.CreateBackendDetails{ + IpAddress: common.String(instanceIp), + Port: common.Int(int(m.OCICluster.Spec.ControlPlaneEndpoint.Port)), + }, + OpcRetryToken: ociutil.GetOPCRetryToken("%s-%s", "create-backend", string(m.OCIMachine.UID)), + }) + if err != nil { + return err + } + m.OCIMachine.Status.CreateBackendWorkRequestId = *resp.OpcWorkRequestId + logger.Info("Add instance to LB backend-set", "WorkRequestId", resp.OpcWorkRequestId) + logger.Info("Waiting for LB work request to be complete") + _, err = ociutil.AwaitLBWorkRequest(ctx, m.LoadBalancerClient, resp.OpcWorkRequestId) + if err != nil { + return err } } @@ -561,37 +557,32 @@ func (m *MachineScope) ReconcileCreateInstanceOnLB(ctx context.Context) error { if !m.containsNLBBackend(backendSet, m.Name()) { logger := m.Logger.WithValues("backend-set", *backendSet.Name) logger.Info("Checking work request status for create backend") - workRequest := m.OCIMachine.Status.CreateBackendWorkRequestId - if workRequest != "" { - _, err = ociutil.AwaitNLBWorkRequest(ctx, m.NetworkLoadBalancerClient, &workRequest) - if err != nil { - return err - } - } else { - resp, err := m.NetworkLoadBalancerClient.CreateBackend(ctx, networkloadbalancer.CreateBackendRequest{ - NetworkLoadBalancerId: loadbalancerId, - BackendSetName: backendSet.Name, - CreateBackendDetails: networkloadbalancer.CreateBackendDetails{ - IpAddress: common.String(instanceIp), - Port: common.Int(int(m.OCICluster.Spec.ControlPlaneEndpoint.Port)), - Name: common.String(m.Name()), - }, - OpcRetryToken: ociutil.GetOPCRetryToken("%s-%s", "create-backend", string(m.OCIMachine.UID)), - }) - if err != nil { - return err - } - m.OCIMachine.Status.CreateBackendWorkRequestId = *resp.OpcWorkRequestId - logger.Info("Add instance to NLB backend-set", "WorkRequestId", resp.OpcWorkRequestId) - logger.Info("Waiting for NLB work request to be complete") - _, err = ociutil.AwaitNLBWorkRequest(ctx, m.NetworkLoadBalancerClient, resp.OpcWorkRequestId) - if err != nil { - return err - } - logger.Info("NLB Backend addition work request is complete") + // we always try to create the backend if it exists during a reconcile loop and wait for the work request + // to complete. If there is a work request in progress, in the rare case, CAPOCI pod restarts during the + // work request, the create backend call may throw an error which is ok, as reconcile will go into + // an exponential backoff + resp, err := m.NetworkLoadBalancerClient.CreateBackend(ctx, networkloadbalancer.CreateBackendRequest{ + NetworkLoadBalancerId: loadbalancerId, + BackendSetName: backendSet.Name, + CreateBackendDetails: networkloadbalancer.CreateBackendDetails{ + IpAddress: common.String(instanceIp), + Port: common.Int(int(m.OCICluster.Spec.ControlPlaneEndpoint.Port)), + Name: common.String(m.Name()), + }, + OpcRetryToken: ociutil.GetOPCRetryToken("%s-%s", "create-backend", string(m.OCIMachine.UID)), + }) + if err != nil { + return err + } + m.OCIMachine.Status.CreateBackendWorkRequestId = *resp.OpcWorkRequestId + logger.Info("Add instance to NLB backend-set", "WorkRequestId", resp.OpcWorkRequestId) + logger.Info("Waiting for NLB work request to be complete") + _, err = ociutil.AwaitNLBWorkRequest(ctx, m.NetworkLoadBalancerClient, resp.OpcWorkRequestId) + if err != nil { + return err } + logger.Info("NLB Backend addition work request is complete") } - } return nil } @@ -628,36 +619,31 @@ func (m *MachineScope) ReconcileDeleteInstanceOnLB(ctx context.Context) error { backendName := instanceIp + ":" + strconv.Itoa(int(m.OCICluster.Spec.ControlPlaneEndpoint.Port)) if m.containsLBBackend(backendSet, backendName) { logger := m.Logger.WithValues("backend-set", *backendSet.Name) - workRequest := m.OCIMachine.Status.DeleteBackendWorkRequestId - if workRequest != "" { - _, err = ociutil.AwaitLBWorkRequest(ctx, m.LoadBalancerClient, &workRequest) - if err != nil { - return err - } - } else { - // in OCI CLI, the colon in the backend name is replaced by %3A - // replace the colon in the backend name by %3A to avoid the error in PCA - escapedBackendName := url.QueryEscape(backendName) - - resp, err := m.LoadBalancerClient.DeleteBackend(ctx, loadbalancer.DeleteBackendRequest{ - LoadBalancerId: loadbalancerId, - BackendSetName: backendSet.Name, - BackendName: common.String(escapedBackendName), - }) - if err != nil { - logger.Error(err, "Delete instance from LB backend-set failed", - "backendSetName", *backendSet.Name, - "backendName", escapedBackendName, - ) - return err - } - m.OCIMachine.Status.DeleteBackendWorkRequestId = *resp.OpcWorkRequestId - logger.Info("Delete instance from LB backend-set", "WorkRequestId", resp.OpcWorkRequestId) - logger.Info("Waiting for LB work request to be complete") - _, err = ociutil.AwaitLBWorkRequest(ctx, m.LoadBalancerClient, resp.OpcWorkRequestId) - if err != nil { - return err - } + // in OCI CLI, the colon in the backend name is replaced by %3A + // replace the colon in the backend name by %3A to avoid the error in PCA + escapedBackendName := url.QueryEscape(backendName) + // we always try to delete the backend if it exists during a reconcile loop and wait for the work request + // to complete. If there is a work request in progress, in the rare case, CAPOCI pod restarts during the + // work request, the delete backend call may throw an error which is ok, as reconcile will go into + // an exponential backoff + resp, err := m.LoadBalancerClient.DeleteBackend(ctx, loadbalancer.DeleteBackendRequest{ + LoadBalancerId: loadbalancerId, + BackendSetName: backendSet.Name, + BackendName: common.String(escapedBackendName), + }) + if err != nil { + logger.Error(err, "Delete instance from LB backend-set failed", + "backendSetName", *backendSet.Name, + "backendName", escapedBackendName, + ) + return err + } + m.OCIMachine.Status.DeleteBackendWorkRequestId = *resp.OpcWorkRequestId + logger.Info("Delete instance from LB backend-set", "WorkRequestId", resp.OpcWorkRequestId) + logger.Info("Waiting for LB work request to be complete") + _, err = ociutil.AwaitLBWorkRequest(ctx, m.LoadBalancerClient, resp.OpcWorkRequestId) + if err != nil { + return err } logger.Info("LB Backend addition work request is complete") } @@ -671,28 +657,24 @@ func (m *MachineScope) ReconcileDeleteInstanceOnLB(ctx context.Context) error { backendSet := lb.BackendSets[APIServerLBBackendSetName] if m.containsNLBBackend(backendSet, m.Name()) { logger := m.Logger.WithValues("backend-set", *backendSet.Name) - workRequest := m.OCIMachine.Status.DeleteBackendWorkRequestId - if workRequest != "" { - _, err = ociutil.AwaitNLBWorkRequest(ctx, m.NetworkLoadBalancerClient, &workRequest) - if err != nil { - return err - } - } else { - resp, err := m.NetworkLoadBalancerClient.DeleteBackend(ctx, networkloadbalancer.DeleteBackendRequest{ - NetworkLoadBalancerId: loadbalancerId, - BackendSetName: backendSet.Name, - BackendName: common.String(m.Name()), - }) - if err != nil { - return err - } - m.OCIMachine.Status.DeleteBackendWorkRequestId = *resp.OpcWorkRequestId - logger.Info("Delete instance from LB backend-set", "WorkRequestId", resp.OpcWorkRequestId) - logger.Info("Waiting for LB work request to be complete") - _, err = ociutil.AwaitNLBWorkRequest(ctx, m.NetworkLoadBalancerClient, resp.OpcWorkRequestId) - if err != nil { - return err - } + // we always try to delete the backend if it exists during a reconcile loop and wait for the work request + // to complete. If there is a work request in progress, in the rare case, CAPOCI pod restarts during the + // work request, the delete backend call may throw an error which is ok, as reconcile will go into + // an exponential backoff + resp, err := m.NetworkLoadBalancerClient.DeleteBackend(ctx, networkloadbalancer.DeleteBackendRequest{ + NetworkLoadBalancerId: loadbalancerId, + BackendSetName: backendSet.Name, + BackendName: common.String(m.Name()), + }) + if err != nil { + return err + } + m.OCIMachine.Status.DeleteBackendWorkRequestId = *resp.OpcWorkRequestId + logger.Info("Delete instance from LB backend-set", "WorkRequestId", resp.OpcWorkRequestId) + logger.Info("Waiting for LB work request to be complete") + _, err = ociutil.AwaitNLBWorkRequest(ctx, m.NetworkLoadBalancerClient, resp.OpcWorkRequestId) + if err != nil { + return err } } } diff --git a/cloud/scope/machine_test.go b/cloud/scope/machine_test.go index 3d0c53fe..a62f43c6 100644 --- a/cloud/scope/machine_test.go +++ b/cloud/scope/machine_test.go @@ -1292,7 +1292,7 @@ func TestNLBReconciliationCreation(t *testing.T) { }, }, { - name: "work request exists", + name: "work request exists, will retry", errorExpected: false, testSpecificSetup: func(machineScope *MachineScope, nlbClient *mock_nlb.MockNetworkLoadBalancerClient) { machineScope.OCIMachine.Status.Addresses = []clusterv1.MachineAddress{ @@ -1314,45 +1314,26 @@ func TestNLBReconciliationCreation(t *testing.T) { }, }, nil) machineScope.OCIMachine.Status.CreateBackendWorkRequestId = "wrid" - nlbClient.EXPECT().GetWorkRequest(gomock.Any(), gomock.Eq( - networkloadbalancer.GetWorkRequestRequest{ - WorkRequestId: common.String("wrid"), - })).Return(networkloadbalancer.GetWorkRequestResponse{ - WorkRequest: networkloadbalancer.WorkRequest{ - Status: networkloadbalancer.OperationStatusSucceeded, - }}, nil) - }, - }, - { - name: "work request exists error", - errorExpected: true, - matchError: errors.Errorf("WorkRequest %s failed", "wrid"), - testSpecificSetup: func(machineScope *MachineScope, nlbClient *mock_nlb.MockNetworkLoadBalancerClient) { - machineScope.OCIMachine.Status.Addresses = []clusterv1.MachineAddress{ - { - Type: clusterv1.MachineInternalIP, - Address: "1.1.1.1", - }, - } - machineScope.OCIMachine.Status.CreateBackendWorkRequestId = "wrid" - nlbClient.EXPECT().GetNetworkLoadBalancer(gomock.Any(), gomock.Eq(networkloadbalancer.GetNetworkLoadBalancerRequest{ - NetworkLoadBalancerId: common.String("nlbid"), - })).Return(networkloadbalancer.GetNetworkLoadBalancerResponse{ - NetworkLoadBalancer: networkloadbalancer.NetworkLoadBalancer{ - BackendSets: map[string]networkloadbalancer.BackendSet{ - APIServerLBBackendSetName: { - Name: common.String(APIServerLBBackendSetName), - Backends: []networkloadbalancer.Backend{}, - }, + nlbClient.EXPECT().CreateBackend(gomock.Any(), gomock.Eq( + networkloadbalancer.CreateBackendRequest{ + NetworkLoadBalancerId: common.String("nlbid"), + BackendSetName: common.String(APIServerLBBackendSetName), + CreateBackendDetails: networkloadbalancer.CreateBackendDetails{ + IpAddress: common.String("1.1.1.1"), + Port: common.Int(6443), + Name: common.String("test"), }, - }, + OpcRetryToken: ociutil.GetOPCRetryToken("%s-%s", "create-backend", "uid"), + })).Return(networkloadbalancer.CreateBackendResponse{ + OpcWorkRequestId: common.String("wrid-1"), }, nil) + nlbClient.EXPECT().GetWorkRequest(gomock.Any(), gomock.Eq( networkloadbalancer.GetWorkRequestRequest{ - WorkRequestId: common.String("wrid"), + WorkRequestId: common.String("wrid-1"), })).Return(networkloadbalancer.GetWorkRequestResponse{ WorkRequest: networkloadbalancer.WorkRequest{ - Status: networkloadbalancer.OperationStatusFailed, + Status: networkloadbalancer.OperationStatusSucceeded, }}, nil) }, }, @@ -1628,7 +1609,7 @@ func TestNLBReconciliationDeletion(t *testing.T) { }, }, { - name: "work request exists", + name: "work request exists, still delete should be called", errorExpected: false, matchError: errors.New("could not get nlb"), testSpecificSetup: func(machineScope *MachineScope, nlbClient *mock_nlb.MockNetworkLoadBalancerClient) { @@ -1649,6 +1630,14 @@ func TestNLBReconciliationDeletion(t *testing.T) { }, }, }, nil) + nlbClient.EXPECT().DeleteBackend(gomock.Any(), gomock.Eq(networkloadbalancer.DeleteBackendRequest{ + NetworkLoadBalancerId: common.String("nlbid"), + BackendSetName: common.String(APIServerLBBackendSetName), + BackendName: common.String("test"), + })).Return(networkloadbalancer.DeleteBackendResponse{ + OpcWorkRequestId: common.String("wrid"), + }, nil) + nlbClient.EXPECT().GetWorkRequest(gomock.Any(), gomock.Eq( networkloadbalancer.GetWorkRequestRequest{ WorkRequestId: common.String("wrid"), @@ -1853,7 +1842,7 @@ func TestLBReconciliationCreation(t *testing.T) { }, }, { - name: "work request exists", + name: "work request exists, will retry", errorExpected: false, testSpecificSetup: func(machineScope *MachineScope, lbClient *mock_lb.MockLoadBalancerClient) { machineScope.OCIMachine.Status.Addresses = []clusterv1.MachineAddress{ @@ -1875,45 +1864,25 @@ func TestLBReconciliationCreation(t *testing.T) { }, }, nil) machineScope.OCIMachine.Status.CreateBackendWorkRequestId = "wrid" - lbClient.EXPECT().GetWorkRequest(gomock.Any(), gomock.Eq( - loadbalancer.GetWorkRequestRequest{ - WorkRequestId: common.String("wrid"), - })).Return(loadbalancer.GetWorkRequestResponse{ - WorkRequest: loadbalancer.WorkRequest{ - LifecycleState: loadbalancer.WorkRequestLifecycleStateSucceeded, - }}, nil) - }, - }, - { - name: "work request exists error", - errorExpected: true, - matchError: errors.Errorf("WorkRequest %s failed", "wrid"), - testSpecificSetup: func(machineScope *MachineScope, lbClient *mock_lb.MockLoadBalancerClient) { - machineScope.OCIMachine.Status.Addresses = []clusterv1.MachineAddress{ - { - Type: clusterv1.MachineInternalIP, - Address: "1.1.1.1", - }, - } - machineScope.OCIMachine.Status.CreateBackendWorkRequestId = "wrid" - lbClient.EXPECT().GetLoadBalancer(gomock.Any(), gomock.Eq(loadbalancer.GetLoadBalancerRequest{ - LoadBalancerId: common.String("lbid"), - })).Return(loadbalancer.GetLoadBalancerResponse{ - LoadBalancer: loadbalancer.LoadBalancer{ - BackendSets: map[string]loadbalancer.BackendSet{ - APIServerLBBackendSetName: { - Name: common.String(APIServerLBBackendSetName), - Backends: []loadbalancer.Backend{}, - }, + lbClient.EXPECT().CreateBackend(gomock.Any(), gomock.Eq( + loadbalancer.CreateBackendRequest{ + LoadBalancerId: common.String("lbid"), + BackendSetName: common.String(APIServerLBBackendSetName), + CreateBackendDetails: loadbalancer.CreateBackendDetails{ + IpAddress: common.String("1.1.1.1"), + Port: common.Int(6443), }, - }, + OpcRetryToken: ociutil.GetOPCRetryToken("%s-%s", "create-backend", "uid"), + })).Return(loadbalancer.CreateBackendResponse{ + OpcWorkRequestId: common.String("wrid-1"), }, nil) + lbClient.EXPECT().GetWorkRequest(gomock.Any(), gomock.Eq( loadbalancer.GetWorkRequestRequest{ - WorkRequestId: common.String("wrid"), + WorkRequestId: common.String("wrid-1"), })).Return(loadbalancer.GetWorkRequestResponse{ WorkRequest: loadbalancer.WorkRequest{ - LifecycleState: loadbalancer.WorkRequestLifecycleStateFailed, + LifecycleState: loadbalancer.WorkRequestLifecycleStateSucceeded, }}, nil) }, }, @@ -2217,7 +2186,7 @@ func TestLBReconciliationDeletion(t *testing.T) { }, }, { - name: "work request exists", + name: "work request exists, still delete should be called", errorExpected: false, matchError: errors.New("could not get lb"), testSpecificSetup: func(machineScope *MachineScope, lbClient *mock_lb.MockLoadBalancerClient) { @@ -2244,6 +2213,14 @@ func TestLBReconciliationDeletion(t *testing.T) { }, }, }, nil) + lbClient.EXPECT().DeleteBackend(gomock.Any(), gomock.Eq(loadbalancer.DeleteBackendRequest{ + LoadBalancerId: common.String("lbid"), + BackendSetName: common.String(APIServerLBBackendSetName), + BackendName: common.String("1.1.1.1%3A6443"), + })).Return(loadbalancer.DeleteBackendResponse{ + OpcWorkRequestId: common.String("wrid"), + }, nil) + lbClient.EXPECT().GetWorkRequest(gomock.Any(), gomock.Eq( loadbalancer.GetWorkRequestRequest{ WorkRequestId: common.String("wrid"), diff --git a/cloud/scope/network_load_balancer_reconciler.go b/cloud/scope/network_load_balancer_reconciler.go index 238cdde2..b10fc758 100644 --- a/cloud/scope/network_load_balancer_reconciler.go +++ b/cloud/scope/network_load_balancer_reconciler.go @@ -19,7 +19,6 @@ package scope import ( "context" "fmt" - infrastructurev1beta2 "github.com/oracle/cluster-api-provider-oci/api/v1beta2" "github.com/oracle/cluster-api-provider-oci/cloud/ociutil" "github.com/oracle/oci-go-sdk/v65/common" @@ -37,6 +36,9 @@ func (s *ClusterScope) ReconcileApiServerNLB(ctx context.Context) error { return err } if nlb != nil { + if nlb.LifecycleState != networkloadbalancer.LifecycleStateActive { + return errors.New(fmt.Sprintf("network load balancer is in %s state. Waiting for ACTIVE state.", nlb.LifecycleState)) + } lbIP, err := s.getNetworkLoadbalancerIp(*nlb) if err != nil { return err diff --git a/cloud/scope/network_load_balancer_reconciler_test.go b/cloud/scope/network_load_balancer_reconciler_test.go index 2802505a..7d8baca7 100644 --- a/cloud/scope/network_load_balancer_reconciler_test.go +++ b/cloud/scope/network_load_balancer_reconciler_test.go @@ -97,11 +97,12 @@ func TestNLBReconciliation(t *testing.T) { })). Return(networkloadbalancer.GetNetworkLoadBalancerResponse{ NetworkLoadBalancer: networkloadbalancer.NetworkLoadBalancer{ - Id: common.String("nlb-id"), - FreeformTags: tags, - DefinedTags: make(map[string]map[string]interface{}), - IsPrivate: common.Bool(false), - DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")), + LifecycleState: networkloadbalancer.LifecycleStateActive, + Id: common.String("nlb-id"), + FreeformTags: tags, + DefinedTags: make(map[string]map[string]interface{}), + IsPrivate: common.Bool(false), + DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")), IpAddresses: []networkloadbalancer.IpAddress{ { IpAddress: common.String("2.2.2.2"), @@ -123,11 +124,12 @@ func TestNLBReconciliation(t *testing.T) { })). Return(networkloadbalancer.GetNetworkLoadBalancerResponse{ NetworkLoadBalancer: networkloadbalancer.NetworkLoadBalancer{ - Id: common.String("nlb-id"), - FreeformTags: tags, - DefinedTags: make(map[string]map[string]interface{}), - IsPrivate: common.Bool(false), - DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")), + LifecycleState: networkloadbalancer.LifecycleStateActive, + Id: common.String("nlb-id"), + FreeformTags: tags, + DefinedTags: make(map[string]map[string]interface{}), + IsPrivate: common.Bool(false), + DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")), }, }, nil) }, @@ -143,11 +145,12 @@ func TestNLBReconciliation(t *testing.T) { })). Return(networkloadbalancer.GetNetworkLoadBalancerResponse{ NetworkLoadBalancer: networkloadbalancer.NetworkLoadBalancer{ - Id: common.String("nlb-id"), - FreeformTags: tags, - DefinedTags: make(map[string]map[string]interface{}), - IsPrivate: common.Bool(false), - DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")), + LifecycleState: networkloadbalancer.LifecycleStateActive, + Id: common.String("nlb-id"), + FreeformTags: tags, + DefinedTags: make(map[string]map[string]interface{}), + IsPrivate: common.Bool(false), + DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")), IpAddresses: []networkloadbalancer.IpAddress{ { IpAddress: common.String("2.2.2.2"), @@ -191,11 +194,12 @@ func TestNLBReconciliation(t *testing.T) { })). Return(networkloadbalancer.GetNetworkLoadBalancerResponse{ NetworkLoadBalancer: networkloadbalancer.NetworkLoadBalancer{ - Id: common.String("nlb-id"), - FreeformTags: tags, - DefinedTags: make(map[string]map[string]interface{}), - IsPrivate: common.Bool(false), - DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")), + LifecycleState: networkloadbalancer.LifecycleStateActive, + Id: common.String("nlb-id"), + FreeformTags: tags, + DefinedTags: make(map[string]map[string]interface{}), + IsPrivate: common.Bool(false), + DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")), IpAddresses: []networkloadbalancer.IpAddress{ { IpAddress: common.String("2.2.2.2"), @@ -466,11 +470,12 @@ func TestNLBReconciliation(t *testing.T) { })). Return(networkloadbalancer.GetNetworkLoadBalancerResponse{ NetworkLoadBalancer: networkloadbalancer.NetworkLoadBalancer{ - Id: common.String("nlb-id"), - FreeformTags: tags, - DefinedTags: make(map[string]map[string]interface{}), - IsPrivate: common.Bool(false), - DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver-test")), + LifecycleState: networkloadbalancer.LifecycleStateActive, + Id: common.String("nlb-id"), + FreeformTags: tags, + DefinedTags: make(map[string]map[string]interface{}), + IsPrivate: common.Bool(false), + DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver-test")), IpAddresses: []networkloadbalancer.IpAddress{ { IpAddress: common.String("2.2.2.2"), @@ -497,6 +502,34 @@ func TestNLBReconciliation(t *testing.T) { }, nil) }, }, + { + name: "nlb not active", + errorExpected: true, + errorSubStringMatch: true, + matchError: errors.New(fmt.Sprintf("network load balancer is in %s state. Waiting for ACTIVE state.", networkloadbalancer.LifecycleStateCreating)), + testSpecificSetup: func(clusterScope *ClusterScope, nlbClient *mock_nlb.MockNetworkLoadBalancerClient) { + ociClusterAccessor.OCICluster.Spec.NetworkSpec.APIServerLB.LoadBalancerId = common.String("nlb-id") + nlbClient.EXPECT().GetNetworkLoadBalancer(gomock.Any(), gomock.Eq(networkloadbalancer.GetNetworkLoadBalancerRequest{ + NetworkLoadBalancerId: common.String("nlb-id"), + })). + Return(networkloadbalancer.GetNetworkLoadBalancerResponse{ + NetworkLoadBalancer: networkloadbalancer.NetworkLoadBalancer{ + LifecycleState: networkloadbalancer.LifecycleStateCreating, + Id: common.String("nlb-id"), + FreeformTags: tags, + DefinedTags: make(map[string]map[string]interface{}), + IsPrivate: common.Bool(false), + DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver-test")), + IpAddresses: []networkloadbalancer.IpAddress{ + { + IpAddress: common.String("2.2.2.2"), + IsPublic: common.Bool(true), + }, + }, + }, + }, nil) + }, + }, { name: "nlb update request failed", errorExpected: true, @@ -509,11 +542,12 @@ func TestNLBReconciliation(t *testing.T) { })). Return(networkloadbalancer.GetNetworkLoadBalancerResponse{ NetworkLoadBalancer: networkloadbalancer.NetworkLoadBalancer{ - Id: common.String("nlb-id"), - FreeformTags: tags, - DefinedTags: make(map[string]map[string]interface{}), - IsPrivate: common.Bool(false), - DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver-test")), + LifecycleState: networkloadbalancer.LifecycleStateActive, + Id: common.String("nlb-id"), + FreeformTags: tags, + DefinedTags: make(map[string]map[string]interface{}), + IsPrivate: common.Bool(false), + DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver-test")), IpAddresses: []networkloadbalancer.IpAddress{ { IpAddress: common.String("2.2.2.2"),