From 92c83f281ba5fb6a1d91ecc3beaa4bcf2647a729 Mon Sep 17 00:00:00 2001 From: Miciah Masters Date: Tue, 8 Dec 2020 12:31:38 -0500 Subject: [PATCH] Revert "Support changing ingresscontroller load balancer scope" This reverts commit 04d3d074796457a1a17f80496686b30d7e3461b4. This commit fixes bug 1905490. https://bugzilla.redhat.com/show_bug.cgi?id=1905490 --- pkg/operator/controller/ingress/controller.go | 6 - .../ingress/load_balancer_service.go | 247 ++---------------- .../ingress/load_balancer_service_test.go | 202 -------------- test/e2e/operator_test.go | 127 --------- 4 files changed, 27 insertions(+), 555 deletions(-) diff --git a/pkg/operator/controller/ingress/controller.go b/pkg/operator/controller/ingress/controller.go index ee9e6001b1..0642819f18 100644 --- a/pkg/operator/controller/ingress/controller.go +++ b/pkg/operator/controller/ingress/controller.go @@ -359,12 +359,6 @@ func setDefaultPublishingStrategy(ic *operatorv1.IngressController, infraConfig ic.Status.EndpointPublishingStrategy = effectiveStrategy return true } - // Detect changes to LB scope, which is something we can safely roll out. - statusLB := ic.Status.EndpointPublishingStrategy.LoadBalancer - specLB := effectiveStrategy.LoadBalancer - if specLB != nil && statusLB != nil && specLB.Scope != statusLB.Scope { - ic.Status.EndpointPublishingStrategy.LoadBalancer.Scope = effectiveStrategy.LoadBalancer.Scope - } return false } diff --git a/pkg/operator/controller/ingress/load_balancer_service.go b/pkg/operator/controller/ingress/load_balancer_service.go index 9aeeeee783..2c4de654d0 100644 --- a/pkg/operator/controller/ingress/load_balancer_service.go +++ b/pkg/operator/controller/ingress/load_balancer_service.go @@ -4,11 +4,7 @@ import ( "context" "fmt" - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" - operatorv1 "github.com/openshift/api/operator/v1" - "github.com/openshift/cluster-ingress-operator/pkg/manifests" "github.com/openshift/cluster-ingress-operator/pkg/operator/controller" oputil "github.com/openshift/cluster-ingress-operator/pkg/util" @@ -20,8 +16,6 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - crclient "sigs.k8s.io/controller-runtime/pkg/client" ) const ( @@ -92,7 +86,7 @@ const ( ) var ( - // InternalLBAnnotations maps platform to the annotation name and value + // internalLBAnnotations maps platform to the annotation name and value // that tell the cloud provider that is associated with the platform // that the load balancer is internal. // @@ -124,16 +118,6 @@ var ( iksLBScopeAnnotation: iksLBScopePrivate, }, } - // externalLBAnnotations maps platform to the annotation name and value - // that tell the cloud provider that is associated with the platform - // that the load balancer is external. This is the default for most - // platforms; only platforms for which it is not the default need - // entries in this map. - externalLBAnnotations = map[configv1.PlatformType]map[string]string{ - configv1.IBMCloudPlatformType: { - iksLBScopeAnnotation: iksLBScopePublic, - }, - } ) // ensureLoadBalancerService creates an LB service if one is desired but absent. @@ -157,46 +141,16 @@ func (r *reconciler) ensureLoadBalancerService(ci *operatorv1.IngressController, if err != nil { return false, nil, err } - - switch { - case !wantLBS && !haveLBS: - return false, nil, nil - case !wantLBS && haveLBS: - if deletedFinalizer, err := r.deleteLoadBalancerServiceFinalizer(currentLBService); err != nil { - return true, currentLBService, fmt.Errorf("failed to remove finalizer from load balancer service: %v", err) - } else if deletedFinalizer { - haveLBS, currentLBService, err = r.currentLoadBalancerService(ci) - if err != nil { - return haveLBS, currentLBService, err - } - } - if err := r.deleteLoadBalancerService(currentLBService, &crclient.DeleteOptions{}); err != nil { - return true, currentLBService, err - } - return false, nil, nil - case wantLBS && !haveLBS: - if err := r.createLoadBalancerService(desiredLBService); err != nil { - return false, nil, err - } - return r.currentLoadBalancerService(ci) - case wantLBS && haveLBS: - if deletedFinalizer, err := r.deleteLoadBalancerServiceFinalizer(currentLBService); err != nil { - return true, currentLBService, fmt.Errorf("failed to remove finalizer from load balancer service: %v", err) - } else if deletedFinalizer { - haveLBS, currentLBService, err = r.currentLoadBalancerService(ci) - if err != nil { - return haveLBS, currentLBService, err - } - } - if updated, err := r.updateLoadBalancerService(currentLBService, desiredLBService, platform); err != nil { - return true, currentLBService, fmt.Errorf("failed to update load balancer service: %v", err) - } else if updated { - log.Info("updated load balancer service", "namespace", desiredLBService.Namespace, "name", desiredLBService.Name) - return r.currentLoadBalancerService(ci) + if wantLBS && !haveLBS { + if err := r.client.Create(context.TODO(), desiredLBService); err != nil { + return false, nil, fmt.Errorf("failed to create load balancer service %s/%s: %v", desiredLBService.Namespace, desiredLBService.Name, err) } + log.Info("created load balancer service", "namespace", desiredLBService.Namespace, "name", desiredLBService.Name) + return true, desiredLBService, nil } - - return true, currentLBService, nil + // return haveLBS instead of forcing true here since + // there is no guarantee that currentLBService != nil + return haveLBS, currentLBService, nil } // desiredLoadBalancerService returns the desired LB service for a @@ -238,11 +192,6 @@ func desiredLoadBalancerService(ci *operatorv1.IngressController, deploymentRef for name, value := range annotation { service.Annotations[name] = value } - } else { - annotation := externalLBAnnotations[platform.Type] - for name, value := range annotation { - service.Annotations[name] = value - } } switch platform.Type { case configv1.AWSPlatformType: @@ -258,12 +207,17 @@ func desiredLoadBalancerService(ci *operatorv1.IngressController, deploymentRef service.Annotations[awsLBHealthCheckTimeoutAnnotation] = awsLBHealthCheckTimeoutDefault service.Annotations[awsLBHealthCheckUnhealthyThresholdAnnotation] = awsLBHealthCheckUnhealthyThresholdDefault service.Annotations[awsLBHealthCheckHealthyThresholdAnnotation] = awsLBHealthCheckHealthyThresholdDefault + case configv1.IBMCloudPlatformType: + if !isInternal { + service.Annotations[iksLBScopeAnnotation] = iksLBScopePublic + } } // Azure load balancers are not customizable and are set to (2 fail @ 5s interval, 2 healthy) // GCP load balancers are not customizable and are set to (3 fail @ 8s interval, 1 healthy) } service.SetOwnerReferences([]metav1.OwnerReference{deploymentRef}) + service.Finalizers = []string{manifests.LoadBalancerServiceFinalizer} return true, service, nil } @@ -280,168 +234,11 @@ func (r *reconciler) currentLoadBalancerService(ci *operatorv1.IngressController return true, service, nil } -// createLoadBalancerService creates a load balancer service. -func (r *reconciler) createLoadBalancerService(service *corev1.Service) error { - if err := r.client.Create(context.TODO(), service); err != nil { - return fmt.Errorf("failed to create load balancer service %s/%s: %v", service.Namespace, service.Name, err) - } - log.Info("created load balancer service", "namespace", service.Namespace, "name", service.Name) - return nil -} - -// deleteLoadBalancerService deletes a load balancer service. -func (r *reconciler) deleteLoadBalancerService(service *corev1.Service, options *crclient.DeleteOptions) error { - if err := r.client.Delete(context.TODO(), service, options); err != nil { - if errors.IsNotFound(err) { - return nil - } - return fmt.Errorf("failed to delete load balancer service %s/%s: %v", service.Namespace, service.Name, err) - } - log.Info("deleted load balancer service", "namespace", service.Namespace, "name", service.Name) - return nil -} - -// updateLoadBalancerService updates a load balancer service. Returns a Boolean -// indicating whether the service was updated, and an error value. -func (r *reconciler) updateLoadBalancerService(current, desired *corev1.Service, platform *configv1.PlatformStatus) (bool, error) { - changed, updated, needRecreate := loadBalancerServiceChanged(current, desired, platform) - if !changed { - return false, nil - } - - if needRecreate { - log.Info("load balancer scope changed, which requires deleting and recreating the load balancer", "namespace", updated.Namespace, "name", updated.Name, "platform", platform.Type) - foreground := metav1.DeletePropagationForeground - deleteOptions := crclient.DeleteOptions{PropagationPolicy: &foreground} - if err := r.deleteLoadBalancerService(current, &deleteOptions); err != nil { - return false, err - } - if err := r.createLoadBalancerService(updated); err != nil { - return false, err - } - return true, nil - } - - if err := r.client.Update(context.TODO(), updated); err != nil { - return false, err - } - return true, nil -} - -// loadBalancerServiceScopeChanged checks if the load balancer scope changed. -func loadBalancerServiceScopeChanged(current, expected *corev1.Service, platform *configv1.PlatformStatus) bool { - currentAnnotations := current.Annotations - if currentAnnotations == nil { - currentAnnotations = map[string]string{} - } - expectedAnnotations := expected.Annotations - if expectedAnnotations == nil { - expectedAnnotations = map[string]string{} - } - for name := range InternalLBAnnotations[platform.Type] { - if currentAnnotations[name] != expectedAnnotations[name] { - return true - } - } - return false -} - -// loadBalancerServiceChanged checks if the current load balancer service spec -// matches the expected spec and if not returns an updated one. -func loadBalancerServiceChanged(current, expected *corev1.Service, platform *configv1.PlatformStatus) (bool, *corev1.Service, bool) { - scopeChanged := loadBalancerServiceScopeChanged(current, expected, platform) - - serviceCmpOpts := []cmp.Option{ - // Ignore fields that the API, other controllers, or user may - // have modified. - cmpopts.IgnoreFields(corev1.ServicePort{}, "NodePort"), - cmpopts.IgnoreFields(corev1.ServiceSpec{}, "ClusterIP", "ExternalIPs", "HealthCheckNodePort"), - cmp.Comparer(cmpServiceAffinity), - cmpopts.EquateEmpty(), - } - if !scopeChanged && cmp.Equal(current.Spec, expected.Spec, serviceCmpOpts...) { - return false, nil, false - } - - updated := current.DeepCopy() - if updated.Annotations == nil { - updated.Annotations = map[string]string{} - } - for name := range InternalLBAnnotations[platform.Type] { - if v, ok := expected.Annotations[name]; ok { - updated.Annotations[name] = v - } else { - delete(updated.Annotations, name) - } - } - - updated.Spec = expected.Spec - - // Preserve fields that the API, other controllers, or user may have - // modified. - updated.Spec.ClusterIP = current.Spec.ClusterIP - updated.Spec.ExternalIPs = current.Spec.ExternalIPs - updated.Spec.HealthCheckNodePort = current.Spec.HealthCheckNodePort - for i, updatedPort := range updated.Spec.Ports { - for _, currentPort := range current.Spec.Ports { - if currentPort.Name == updatedPort.Name { - updated.Spec.Ports[i].NodePort = currentPort.NodePort - } - } - } - - // When switching between internal scope and external scope, it may be - // necessary to delete and recreate the service, depending on the cloud - // provider implementation: - // - // * Azure and GCE can handle changing scope, so updating the annotation - // on the service suffices. - // - // * AWS cannot handle changing scope, so the service needs to be - // recreated. - // - // * IBM Cloud may or may not handle scope change, so recreate the - // service to be safe. - needsRecreate := false - if scopeChanged { - switch platform.Type { - case configv1.AWSPlatformType, configv1.IBMCloudPlatformType: - needsRecreate = true - } - } - - return true, updated, needsRecreate -} - -// deleteLoadBalancerServiceFinalizer removes the -// "ingress.openshift.io/operator" finalizer from the provided load balancer -// service. This finalizer used to be needed for helping with DNS cleanup, but -// that's no longer necessary. We just need to clear the finalizer which might -// exist on existing resources. -// -// TODO: Delete this method and all calls to it after 4.7. -func (r *reconciler) deleteLoadBalancerServiceFinalizer(service *corev1.Service) (bool, error) { - if !slice.ContainsString(service.Finalizers, manifests.LoadBalancerServiceFinalizer) { - return false, nil - } - - // Mutate a copy to avoid assuming we know where the current one came from - // (i.e. it could have been from a cache). - updated := service.DeepCopy() - updated.Finalizers = slice.RemoveString(updated.Finalizers, manifests.LoadBalancerServiceFinalizer) - if err := r.client.Update(context.TODO(), updated); err != nil { - return false, fmt.Errorf("failed to remove finalizer from service %s/%s: %v", service.Namespace, service.Name, err) - } - - log.Info("removed finalizer from load balancer service", "namespace", service.Namespace, "name", service.Name) - - return true, nil -} - // finalizeLoadBalancerService removes the "ingress.openshift.io/operator" finalizer // from the load balancer service of ci. This was for helping with DNS cleanup, but // that's no longer necessary. We just need to clear the finalizer which might exist // on existing resources. +// TODO: How can we delete this code? func (r *reconciler) finalizeLoadBalancerService(ci *operatorv1.IngressController) (bool, error) { haveLBS, service, err := r.currentLoadBalancerService(ci) if err != nil { @@ -450,6 +247,16 @@ func (r *reconciler) finalizeLoadBalancerService(ci *operatorv1.IngressControlle if !haveLBS { return false, nil } - _, err = r.deleteLoadBalancerServiceFinalizer(service) - return true, err + // Mutate a copy to avoid assuming we know where the current one came from + // (i.e. it could have been from a cache). + updated := service.DeepCopy() + if slice.ContainsString(updated.Finalizers, manifests.LoadBalancerServiceFinalizer) { + updated.Finalizers = slice.RemoveString(updated.Finalizers, manifests.LoadBalancerServiceFinalizer) + if err := r.client.Update(context.TODO(), updated); err != nil { + return true, fmt.Errorf("failed to remove finalizer from service %s/%s for ingress %s/%s: %v", + service.Namespace, service.Name, ci.Namespace, ci.Name, err) + } + } + log.Info("finalized load balancer service for ingress", "namespace", ci.Namespace, "name", ci.Name) + return true, nil } diff --git a/pkg/operator/controller/ingress/load_balancer_service_test.go b/pkg/operator/controller/ingress/load_balancer_service_test.go index 2f035a0815..f23177a3e5 100644 --- a/pkg/operator/controller/ingress/load_balancer_service_test.go +++ b/pkg/operator/controller/ingress/load_balancer_service_test.go @@ -9,7 +9,6 @@ import ( operatorv1 "github.com/openshift/api/operator/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/intstr" ) func TestDesiredLoadBalancerService(t *testing.T) { @@ -330,204 +329,3 @@ func checkServiceHasAnnotation(svc *corev1.Service, name string, expectValue boo return nil } } - -func TestLoadBalancerServiceChanged(t *testing.T) { - testCases := []struct { - description string - mutate func(*corev1.Service) - expect bool - }{ - { - description: "if nothing changes", - mutate: func(_ *corev1.Service) {}, - expect: false, - }, - { - description: "if .uid changes", - mutate: func(svc *corev1.Service) { - svc.UID = "2" - }, - expect: false, - }, - { - description: "if .spec.clusterIP changes", - mutate: func(svc *corev1.Service) { - svc.Spec.ClusterIP = "2.3.4.5" - }, - expect: false, - }, - { - description: "if .spec.externalIPs changes", - mutate: func(svc *corev1.Service) { - svc.Spec.ExternalIPs = []string{"3.4.5.6"} - }, - expect: false, - }, - { - description: "if .spec.externalTrafficPolicy changes", - mutate: func(svc *corev1.Service) { - svc.Spec.ExternalTrafficPolicy = corev1.ServiceExternalTrafficPolicyTypeCluster - }, - expect: true, - }, - { - description: "if .spec.healthCheckNodePort changes", - mutate: func(svc *corev1.Service) { - svc.Spec.HealthCheckNodePort = int32(34566) - }, - expect: false, - }, - { - description: "if .spec.ports changes", - mutate: func(svc *corev1.Service) { - newPort := corev1.ServicePort{ - Name: "foo", - Protocol: corev1.ProtocolTCP, - Port: int32(8080), - TargetPort: intstr.FromString("foo"), - } - svc.Spec.Ports = append(svc.Spec.Ports, newPort) - }, - expect: true, - }, - { - description: "if .spec.ports[*].nodePort changes", - mutate: func(svc *corev1.Service) { - svc.Spec.Ports[0].NodePort = int32(33337) - svc.Spec.Ports[1].NodePort = int32(33338) - }, - expect: false, - }, - { - description: "if .spec.selector changes", - mutate: func(svc *corev1.Service) { - svc.Spec.Selector = nil - }, - expect: true, - }, - { - description: "if .spec.sessionAffinity is defaulted", - mutate: func(service *corev1.Service) { - service.Spec.SessionAffinity = corev1.ServiceAffinityNone - }, - expect: false, - }, - { - description: "if .spec.sessionAffinity is set to a non-default value", - mutate: func(service *corev1.Service) { - service.Spec.SessionAffinity = corev1.ServiceAffinityClientIP - }, - expect: true, - }, - { - description: "if .spec.type changes", - mutate: func(svc *corev1.Service) { - svc.Spec.Type = corev1.ServiceTypeNodePort - }, - expect: true, - }, - } - - for _, tc := range testCases { - platformStatus := &configv1.PlatformStatus{ - Type: configv1.AWSPlatformType, - } - original := corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "openshift-ingress", - Name: "router-original", - UID: "1", - }, - Spec: corev1.ServiceSpec{ - ClusterIP: "1.2.3.4", - ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeLocal, - HealthCheckNodePort: int32(33333), - Ports: []corev1.ServicePort{ - { - Name: "http", - NodePort: int32(33334), - Port: int32(80), - Protocol: corev1.ProtocolTCP, - TargetPort: intstr.FromString("http"), - }, - { - Name: "https", - NodePort: int32(33335), - Port: int32(443), - Protocol: corev1.ProtocolTCP, - TargetPort: intstr.FromString("https"), - }, - }, - Selector: map[string]string{ - "foo": "bar", - }, - Type: corev1.ServiceTypeLoadBalancer, - }, - } - mutated := original.DeepCopy() - tc.mutate(mutated) - if changed, updated, needsRecreate := loadBalancerServiceChanged(&original, mutated, platformStatus); changed != tc.expect { - t.Errorf("%s, expect loadBalancerServiceChanged to be %t, got %t", tc.description, tc.expect, changed) - } else if needsRecreate { - t.Errorf("%s, loadBalancerServiceChanged returned needsRecreate=true", tc.description) - } else if changed { - if changedAgain, _, _ := loadBalancerServiceChanged(mutated, updated, platformStatus); changedAgain { - t.Errorf("%s, loadBalancerServiceChanged does not behave as a fixed point function", tc.description) - } - } - } -} - -func TestLoadBalancerServiceChangedScopeNeedsRecreate(t *testing.T) { - testCases := []struct { - platform configv1.PlatformType - expect bool - }{ - {platform: configv1.AWSPlatformType, expect: true}, - {platform: configv1.AzurePlatformType, expect: false}, - {platform: configv1.GCPPlatformType, expect: false}, - {platform: configv1.IBMCloudPlatformType, expect: true}, - } - - for _, tc := range testCases { - platformStatus := &configv1.PlatformStatus{Type: tc.platform} - original := corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{}, - Namespace: "openshift-ingress", - Name: "router-original", - }, - } - internal := original.DeepCopy() - extAnnotations := externalLBAnnotations[platformStatus.Type] - for name, value := range extAnnotations { - original.Annotations[name] = value - } - intAnnotations := InternalLBAnnotations[platformStatus.Type] - for name, value := range intAnnotations { - internal.Annotations[name] = value - } - if changed, updated, needsRecreate := loadBalancerServiceChanged(&original, internal, platformStatus); needsRecreate != tc.expect { - t.Errorf("on platform %s, when setting internal scope, expected loadBalancerServiceChanged to return needsRecreate=%t, got %t", tc.platform, tc.expect, needsRecreate) - } else if !changed { - t.Errorf("on platform %s, when setting internal scope, expected loadBalancerServiceChanged to return changed=%t, got %t", tc.platform, true, changed) - } else { - for name, value := range intAnnotations { - if err := checkServiceHasAnnotation(updated, name, true, value); err != nil { - t.Errorf("on platform %s, when setting internal scope, %v", tc.platform, err) - } - } - } - if changed, updated, needsRecreate := loadBalancerServiceChanged(internal, &original, platformStatus); needsRecreate != tc.expect { - t.Errorf("on platform %s, when setting external scope, expected loadBalancerServiceChanged to return needsRecreate=%t, got %t", tc.platform, tc.expect, needsRecreate) - } else if !changed { - t.Errorf("on platform %s, when setting external scope, expected loadBalancerServiceChanged to return changed=%t, got %t", tc.platform, true, changed) - } else { - for name, value := range extAnnotations { - if err := checkServiceHasAnnotation(updated, name, true, value); err != nil { - t.Errorf("on platform %s, when setting external scope, %v", tc.platform, err) - } - } - } - } -} diff --git a/test/e2e/operator_test.go b/test/e2e/operator_test.go index b2e79a418c..024746b70a 100644 --- a/test/e2e/operator_test.go +++ b/test/e2e/operator_test.go @@ -587,133 +587,6 @@ func TestInternalLoadBalancer(t *testing.T) { } } -// isServiceInternal returns a Boolean indicating whether the provided service -// is annotated to request an internal load balancer. -func isServiceInternal(service *corev1.Service) bool { - for dk, dv := range service.Annotations { - for _, annotations := range ingresscontroller.InternalLBAnnotations { - for ik, iv := range annotations { - if dk == ik && dv == iv { - return true - } - } - } - } - return false -} - -// TestScopeChange creates an ingresscontroller with the -// "LoadBalancerServiceName" endpoint publishing strategy type and verifies that -// the operator correctly updates the associated load balancer when the -// ingresscontroller's scope is mutated. -func TestScopeChange(t *testing.T) { - platform := infraConfig.Status.Platform - - supportedPlatforms := map[configv1.PlatformType]struct{}{ - configv1.AWSPlatformType: {}, - configv1.AzurePlatformType: {}, - configv1.GCPPlatformType: {}, - configv1.IBMCloudPlatformType: {}, - } - if _, supported := supportedPlatforms[platform]; !supported { - t.Skipf("test skipped on platform %q", platform) - } - - name := types.NamespacedName{Namespace: operatorNamespace, Name: "scope"} - ic := newLoadBalancerController(name, name.Name+"."+dnsConfig.Spec.BaseDomain) - ic.Spec.EndpointPublishingStrategy.LoadBalancer = &operatorv1.LoadBalancerStrategy{ - Scope: operatorv1.ExternalLoadBalancer, - } - if err := kclient.Create(context.TODO(), ic); err != nil { - t.Fatalf("failed to create ingresscontroller: %v", err) - } - defer assertIngressControllerDeleted(t, kclient, ic) - - // Wait for the load balancer and DNS to be ready. - if err := waitForIngressControllerCondition(t, kclient, 5*time.Minute, name, defaultAvailableConditions...); err != nil { - t.Fatalf("failed to observe expected conditions: %v", err) - } - - lbService := &corev1.Service{} - if err := kclient.Get(context.TODO(), controller.LoadBalancerServiceName(ic), lbService); err != nil { - t.Fatalf("failed to get LoadBalancer service: %v", err) - } - - if isServiceInternal(lbService) { - t.Fatalf("load balancer is internal but should be external") - } - - // Change the scope to internal and wait for everything to come back to - // normal. - if err := kclient.Get(context.TODO(), name, ic); err != nil { - t.Fatalf("failed to get ingresscontroller %s: %v", name, err) - } - ic.Spec.EndpointPublishingStrategy.LoadBalancer.Scope = operatorv1.InternalLoadBalancer - - if err := kclient.Update(context.TODO(), ic); err != nil { - t.Fatal(err) - } - - err := wait.PollImmediate(5*time.Second, 5*time.Minute, func() (bool, error) { - service := &corev1.Service{} - if err := kclient.Get(context.TODO(), controller.LoadBalancerServiceName(ic), service); err != nil { - if errors.IsNotFound(err) { - return false, nil - } else { - t.Logf("failed to get ingresscontroller %s: %v", name.Name, err) - return false, nil - } - } - if isServiceInternal(service) { - return true, nil - } - t.Logf("service is still external: %#v\n", service) - return false, nil - }) - if err != nil { - t.Fatalf("expected load balancer to become internal") - } - - if err := waitForIngressControllerCondition(t, kclient, 5*time.Minute, name, defaultAvailableConditions...); err != nil { - t.Fatalf("failed to observe expected conditions: %v", err) - } - - // Change the scope back to external and wait for everything to come - // back to normal. - if err := kclient.Get(context.TODO(), name, ic); err != nil { - t.Fatalf("failed to get ingresscontroller %s: %v", name, err) - } - ic.Spec.EndpointPublishingStrategy.LoadBalancer.Scope = operatorv1.ExternalLoadBalancer - - if err := kclient.Update(context.TODO(), ic); err != nil { - t.Fatal(err) - } - - err = wait.PollImmediate(5*time.Second, 5*time.Minute, func() (bool, error) { - service := &corev1.Service{} - if err := kclient.Get(context.TODO(), controller.LoadBalancerServiceName(ic), service); err != nil { - if errors.IsNotFound(err) { - return false, nil - } else { - t.Logf("failed to get ingresscontroller %s: %v", name.Name, err) - return false, nil - } - } - if isServiceInternal(service) { - t.Logf("service is still internal: %#v\n", service) - return false, nil - } - return true, nil - }) - if err != nil { - t.Fatalf("expected load balancer to become external: %v", err) - } - - if err := waitForIngressControllerCondition(t, kclient, 5*time.Minute, name, defaultAvailableConditions...); err != nil { - t.Fatalf("failed to observe expected conditions: %v", err) - } -} - // TestNodePortServiceEndpointPublishingStrategy creates an ingresscontroller // with the "NodePortService" endpoint publishing strategy type and verifies // that the operator creates a router and that the router becomes available.