From 92d71a053f88ca82d1c359e910ec6192e3967b62 Mon Sep 17 00:00:00 2001 From: Miciah Masters Date: Fri, 9 Dec 2022 18:50:08 -0500 Subject: [PATCH 1/3] ensureInternalIngressControllerService: Reformat Reformat ensureInternalIngressControllerService to follow the standard pattern for "ensure" functions. * pkg/operator/controller/ingress/controller.go (ensureIngressController): Expect a Boolean return value from ensureInternalIngressControllerService, indicating whether the service exists. * pkg/operator/controller/ingress/internal_service.go (ensureInternalIngressControllerService): Add a Boolean return value. Use a switch statement, and add a to-do comment for handling updates. (currentInternalIngressControllerService): Add a Boolean return value. --- pkg/operator/controller/ingress/controller.go | 4 ++- .../controller/ingress/internal_service.go | 31 ++++++++++--------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/pkg/operator/controller/ingress/controller.go b/pkg/operator/controller/ingress/controller.go index fb9ef7a19..93b203e40 100644 --- a/pkg/operator/controller/ingress/controller.go +++ b/pkg/operator/controller/ingress/controller.go @@ -1036,8 +1036,10 @@ func (r *reconciler) ensureIngressController(ci *operatorv1.IngressController, d errs = append(errs, err) } - if internalSvc, err := r.ensureInternalIngressControllerService(ci, deploymentRef); err != nil { + if haveSvc, internalSvc, err := r.ensureInternalIngressControllerService(ci, deploymentRef); err != nil { errs = append(errs, fmt.Errorf("failed to create internal router service for ingresscontroller %s: %v", ci.Name, err)) + } else if !haveSvc { + errs = append(errs, fmt.Errorf("failed to get internal route service for ingresscontroller %s: %w", ci.Name, err)) } else if err := r.ensureMetricsIntegration(ci, internalSvc, deploymentRef); err != nil { errs = append(errs, fmt.Errorf("failed to integrate metrics with openshift-monitoring for ingresscontroller %s: %v", ci.Name, err)) } diff --git a/pkg/operator/controller/ingress/internal_service.go b/pkg/operator/controller/ingress/internal_service.go index 96c775ebd..888a330fe 100644 --- a/pkg/operator/controller/ingress/internal_service.go +++ b/pkg/operator/controller/ingress/internal_service.go @@ -22,33 +22,36 @@ const ( // ensureInternalRouterServiceForIngress ensures that an internal service exists // for a given IngressController. -func (r *reconciler) ensureInternalIngressControllerService(ic *operatorv1.IngressController, deploymentRef metav1.OwnerReference) (*corev1.Service, error) { +func (r *reconciler) ensureInternalIngressControllerService(ic *operatorv1.IngressController, deploymentRef metav1.OwnerReference) (bool, *corev1.Service, error) { desired := desiredInternalIngressControllerService(ic, deploymentRef) - current, err := r.currentInternalIngressControllerService(ic) + have, current, err := r.currentInternalIngressControllerService(ic) if err != nil { - return nil, err + return false, nil, err } - if current != nil { - return current, nil + switch { + case !have: + if err := r.client.Create(context.TODO(), desired); err != nil { + return false, nil, fmt.Errorf("failed to create internal ingresscontroller service: %w", err) + } + log.Info("created internal ingresscontroller service", "service", desired) + return r.currentInternalIngressControllerService(ic) + case have: + // TODO Handle updates. } - if err := r.client.Create(context.TODO(), desired); err != nil { - return nil, fmt.Errorf("failed to create internal ingresscontroller service: %v", err) - } - log.Info("created internal ingresscontroller service", "service", desired) - return desired, nil + return true, current, nil } -func (r *reconciler) currentInternalIngressControllerService(ic *operatorv1.IngressController) (*corev1.Service, error) { +func (r *reconciler) currentInternalIngressControllerService(ic *operatorv1.IngressController) (bool, *corev1.Service, error) { current := &corev1.Service{} err := r.client.Get(context.TODO(), controller.InternalIngressControllerServiceName(ic), current) if err != nil { if errors.IsNotFound(err) { - return nil, nil + return false, nil, nil } - return nil, err + return false, nil, err } - return current, nil + return true, current, nil } func desiredInternalIngressControllerService(ic *operatorv1.IngressController, deploymentRef metav1.OwnerReference) *corev1.Service { From 44a60f530416bbf93e47a8c7d711177f7008c86d Mon Sep 17 00:00:00 2001 From: Miciah Masters Date: Fri, 9 Dec 2022 19:33:16 -0500 Subject: [PATCH 2/3] ensureInternalIngressControllerService: Updates Add logic to ensureInternalIngressControllerService to update the service when an update is required. * pkg/operator/controller/ingress/internal_service.go (ensureInternalIngressControllerService): Use the new updateInternalService method to update the service as needed. (updateInternalService): Check whether the given ClusterIP service needs to be updated, and update it if so, using the new internalServiceChanged function. (managedInternalServiceAnnotations): New variable with the set of annotation keys for annotations that the operator manages for its internal router services. (internalServiceChanged): New function. Check whether the current internal service needs to be updated, and update it if it does, using the new managedInternalServiceAnnotations variable. * pkg/operator/controller/ingress/internal_service_test.go: New file. (Test_desiredInternalIngressControllerService): New test. Verify that desiredInternalIngressControllerService returns the expected service. (Test_internalServiceChanged): New test. Verify that internalServiceChanged correctly detects changes and performs updates. --- .../controller/ingress/internal_service.go | 95 ++++++++- .../ingress/internal_service_test.go | 199 ++++++++++++++++++ 2 files changed, 293 insertions(+), 1 deletion(-) create mode 100644 pkg/operator/controller/ingress/internal_service_test.go diff --git a/pkg/operator/controller/ingress/internal_service.go b/pkg/operator/controller/ingress/internal_service.go index 888a330fe..8be02bcae 100644 --- a/pkg/operator/controller/ingress/internal_service.go +++ b/pkg/operator/controller/ingress/internal_service.go @@ -4,6 +4,9 @@ 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" @@ -12,6 +15,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" ) const ( @@ -36,7 +40,11 @@ func (r *reconciler) ensureInternalIngressControllerService(ic *operatorv1.Ingre log.Info("created internal ingresscontroller service", "service", desired) return r.currentInternalIngressControllerService(ic) case have: - // TODO Handle updates. + if updated, err := r.updateInternalService(current, desired); err != nil { + return true, current, fmt.Errorf("failed to update internal service: %v", err) + } else if updated { + return r.currentInternalIngressControllerService(ic) + } } return true, current, nil @@ -77,3 +85,88 @@ func desiredInternalIngressControllerService(ic *operatorv1.IngressController, d return s } + +// updateInternalService updates a ClusterIP service. Returns a Boolean +// indicating whether the service was updated, and an error value. +func (r *reconciler) updateInternalService(current, desired *corev1.Service) (bool, error) { + changed, updated := internalServiceChanged(current, desired) + if !changed { + return false, nil + } + + // Diff before updating because the client may mutate the object. + diff := cmp.Diff(current, updated, cmpopts.EquateEmpty()) + if err := r.client.Update(context.TODO(), updated); err != nil { + return false, err + } + log.Info("updated internal service", "namespace", updated.Namespace, "name", updated.Name, "diff", diff) + return true, nil +} + +// managedInternalServiceAnnotations is a set of annotation keys for annotations +// that the operator manages for its internal service. +var managedInternalServiceAnnotations = sets.NewString( + ServingCertSecretAnnotation, +) + +// internalServiceChanged checks if the current internal service annotations and +// spec match the expected annotations and spec and if not returns an updated +// service. +func internalServiceChanged(current, expected *corev1.Service) (bool, *corev1.Service) { + changed := false + + 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", "ClusterIPs", "ExternalIPs", "HealthCheckNodePort"), + cmp.Comparer(cmpServiceAffinity), + cmpopts.EquateEmpty(), + } + if !cmp.Equal(current.Spec, expected.Spec, serviceCmpOpts...) { + changed = true + } + + annotationCmpOpts := []cmp.Option{ + cmpopts.IgnoreMapEntries(func(k, _ string) bool { + return !managedInternalServiceAnnotations.Has(k) + }), + } + if !cmp.Equal(current.Annotations, expected.Annotations, annotationCmpOpts...) { + changed = true + } + + if !changed { + return false, nil + } + + updated := current.DeepCopy() + updated.Spec = expected.Spec + + if updated.Annotations == nil { + updated.Annotations = map[string]string{} + } + for annotation := range managedInternalServiceAnnotations { + currentVal, have := current.Annotations[annotation] + expectedVal, want := expected.Annotations[annotation] + if want && (!have || currentVal != expectedVal) { + updated.Annotations[annotation] = expected.Annotations[annotation] + } else if have && !want { + delete(updated.Annotations, annotation) + } + } + // 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].TargetPort = currentPort.TargetPort + } + } + } + + return true, updated +} diff --git a/pkg/operator/controller/ingress/internal_service_test.go b/pkg/operator/controller/ingress/internal_service_test.go new file mode 100644 index 000000000..37e06cd09 --- /dev/null +++ b/pkg/operator/controller/ingress/internal_service_test.go @@ -0,0 +1,199 @@ +package ingress + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + operatorv1 "github.com/openshift/api/operator/v1" + + corev1 "k8s.io/api/core/v1" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" +) + +// test_desiredInternalIngressControllerService verifies that +// desiredInternalIngressControllerService returns the expected service. +func Test_desiredInternalIngressControllerService(t *testing.T) { + ic := &operatorv1.IngressController{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + }, + } + trueVar := true + deploymentRef := metav1.OwnerReference{ + APIVersion: "apps/v1", + Kind: "Deployment", + Name: "router-default", + UID: "1", + Controller: &trueVar, + } + + svc := desiredInternalIngressControllerService(ic, deploymentRef) + + assert.Equal(t, "ClusterIP", string(svc.Spec.Type)) + assert.Equal(t, map[string]string{ + "service.alpha.openshift.io/serving-cert-secret-name": "router-metrics-certs-default", + }, svc.Annotations) + assert.Equal(t, []corev1.ServicePort{{ + Name: "http", + Protocol: "TCP", + Port: int32(80), + TargetPort: intstr.FromString("http"), + }, { + Name: "https", + Protocol: "TCP", + Port: int32(443), + TargetPort: intstr.FromString("https"), + }, { + Name: "metrics", + Protocol: "TCP", + Port: int32(1936), + TargetPort: intstr.FromInt(1936), + }}, svc.Spec.Ports) +} + +// Test_internalServiceChanged verifies that internalServiceChanged properly +// detects changes and handles updates. +func Test_internalServiceChanged(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" + svc.Spec.ClusterIPs = []string{"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 the service.alpha.openshift.io/serving-cert-secret-name annotation changes", + mutate: func(svc *corev1.Service) { + svc.Annotations["service.alpha.openshift.io/serving-cert-secret-name"] = "x" + }, + expect: true, + }, + { + description: "if the service.alpha.openshift.io/serving-cert-secret-name annotation is deleted", + mutate: func(svc *corev1.Service) { + delete(svc.Annotations, "service.alpha.openshift.io/serving-cert-secret-name") + }, + expect: true, + }, + { + description: "if .spec.ports changes by adding a new port", + 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.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.ServiceTypeLoadBalancer + }, + expect: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + original := corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "service.alpha.openshift.io/serving-cert-secret-name": "router-metrics-certs-default", + }, + Namespace: "openshift-ingress", + Name: "router-internal-default", + UID: "1", + }, + Spec: corev1.ServiceSpec{ + ClusterIP: "1.2.3.4", + Ports: []corev1.ServicePort{ + { + Name: "http", + Port: int32(80), + Protocol: corev1.ProtocolTCP, + TargetPort: intstr.FromString("http"), + }, + { + Name: "https", + Port: int32(443), + Protocol: corev1.ProtocolTCP, + TargetPort: intstr.FromString("https"), + }, + { + Name: "metrics", + Port: int32(1936), + Protocol: corev1.ProtocolTCP, + TargetPort: intstr.FromInt(1936), + }, + }, + Selector: map[string]string{ + "foo": "bar", + }, + Type: corev1.ServiceTypeClusterIP, + }, + } + mutated := original.DeepCopy() + tc.mutate(mutated) + if changed, updated := internalServiceChanged(&original, mutated); changed != tc.expect { + t.Errorf("expected internalServiceChanged to be %t, got %t", tc.expect, changed) + } else if changed { + if changedAgain, _ := internalServiceChanged(mutated, updated); changedAgain { + t.Error("internalServiceChanged does not behave as a fixed point function") + } + } + }) + } +} From 9c6e368c63565d29a45c10f056f7761a53748108 Mon Sep 17 00:00:00 2001 From: Miciah Masters Date: Fri, 9 Dec 2022 19:41:07 -0500 Subject: [PATCH 3/3] service-internal.yaml: Target metrics port by name Use the "metrics" port name instead of port number 1936 for the router internal service's metrics port's target. Before this commit, the router's internal service's metrics port always targeted port 1936 on the router pod. However, the router pod's metrics port can be customized, and so it is not necessarily port 1936. As a consequence, the service's metrics port didn't work when the router pod's metrics port was customized. The router pod's metrics port always has the name "metrics", so this commit changes the service to reference the port by name to avoid breaking when the port number changes. This commit fixes OCPBUGS-4573. https://issues.redhat.com/browse/OCPBUGS-4573 Follow-up to commit af653f9fa7368cf124e11b7ea4666bc40e601165. * assets/router/service-internal.yaml: Use the "metrics" port name instead of port 1936 for the metrics port's target. * pkg/manifests/bindata.go: Regenerate. * pkg/operator/controller/ingress/internal_service_test.go (Test_desiredInternalIngressControllerService): Expect the metrics port to reference the "metrics" target port by name. (Test_internalServiceChanged): Add a test case for changing the "metrics" port's target port from an integer to a string. --- assets/router/service-internal.yaml | 2 +- pkg/manifests/bindata.go | 8 ++++---- .../controller/ingress/internal_service.go | 3 ++- .../controller/ingress/internal_service_test.go | 15 ++++++++++++++- 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/assets/router/service-internal.yaml b/assets/router/service-internal.yaml index 4f5b95adc..675e9d274 100644 --- a/assets/router/service-internal.yaml +++ b/assets/router/service-internal.yaml @@ -17,4 +17,4 @@ spec: - name: metrics port: 1936 protocol: TCP - targetPort: 1936 + targetPort: metrics diff --git a/pkg/manifests/bindata.go b/pkg/manifests/bindata.go index 326b5b400..59de27edb 100644 --- a/pkg/manifests/bindata.go +++ b/pkg/manifests/bindata.go @@ -14,7 +14,7 @@ // assets/router/namespace.yaml (858B) // assets/router/service-account.yaml (213B) // assets/router/service-cloud.yaml (631B) -// assets/router/service-internal.yaml (429B) +// assets/router/service-internal.yaml (432B) // manifests/00-cluster-role.yaml (3.181kB) // manifests/00-custom-resource-definition-internal.yaml (7.756kB) // manifests/00-custom-resource-definition.yaml (121.33kB) @@ -382,7 +382,7 @@ func assetsRouterServiceCloudYaml() (*asset, error) { return a, nil } -var _assetsRouterServiceInternalYaml = []byte("\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\xff\x8c\xcf\x31\x6b\xfb\x30\x10\x05\xf0\x5d\x9f\xe2\x41\xd6\xff\xbf\x34\x24\x94\x56\xab\xa7\x6c\x86\x96\xee\x87\x7c\x49\x8e\xca\x92\xb8\x3b\xbb\xf4\xdb\x97\x38\x0d\xb8\x74\xc9\x22\x90\xee\xe9\xf7\xb8\x0d\xba\x3c\x99\xb3\xe2\x95\x75\x96\xc4\xf8\x14\x3f\x63\xe0\x23\x4d\xd9\x31\x53\x9e\xd8\xc2\x06\x87\x72\x52\x36\x43\x57\x8b\x6b\xcd\x99\x15\xd6\x38\xc9\x51\x12\xa8\x94\xea\xe4\x52\x8b\x81\x94\x41\xad\x65\xe1\x01\xe4\xd0\xa9\xb8\x8c\xfc\x10\x3e\xa4\x0c\xf1\xd6\x11\xa8\xc9\x3b\xab\x49\x2d\x11\xf3\x36\x6c\x50\x68\xe4\x7f\xcb\x69\x8d\x12\x83\xca\xf0\x87\x35\xf6\x5f\xe4\xa5\x3f\x06\xc0\xbf\x1a\xc7\xdb\x1a\x87\x3e\x00\xad\xaa\xdb\x65\xf4\x7f\x21\x23\xce\xee\x2d\x00\xd7\x49\xc4\xf3\xe3\xf5\xa2\xd5\x6b\xaa\x39\xe2\xad\xeb\x97\x17\x27\x3d\xb1\xf7\x4b\xe8\xe7\xcf\x9a\xb0\x95\xb1\xdf\xef\xee\x44\x6c\xa5\x8c\xec\x2a\x69\xed\x6c\x5f\x76\x4f\x77\x40\x4b\xec\x3b\x00\x00\xff\xff\x90\x5e\x33\xca\xad\x01\x00\x00") +var _assetsRouterServiceInternalYaml = []byte("\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\xff\x8c\xcf\x31\x6b\xfb\x30\x10\x05\xf0\x5d\x9f\xe2\x41\xd6\xff\xbf\x34\x24\x94\x56\x6b\xa6\x6c\x86\x96\xee\x87\x7c\x49\x8e\xca\x92\xb8\x3b\xbb\xf4\xdb\x97\x38\x35\xb8\x74\xc9\x22\x90\x4e\xef\xf7\xb8\x0d\x0e\x79\x34\x67\xc5\x2b\xeb\x24\x89\xf1\x29\x7e\x41\xcf\x27\x1a\xb3\x63\xa2\x3c\xb2\x85\x0d\x8e\xe5\xac\x6c\x86\x43\x2d\xae\x35\x67\x56\x58\xe3\x24\x27\x49\xa0\x52\xaa\x93\x4b\x2d\x06\x52\x06\xb5\x96\x85\x7b\x90\x43\xc7\xe2\x32\xf0\x43\xf8\x90\xd2\xc7\xa5\x23\x50\x93\x77\x56\x93\x5a\x22\xa6\x6d\xd8\xa0\xd0\xc0\xff\xe6\xd3\x1a\x25\x06\x95\xfe\x0f\x6b\xec\xbf\xc8\x6b\x7f\x0c\x80\x7f\x35\x8e\xcb\x1a\xc7\x2e\x00\xad\xaa\xdb\x75\xf4\x7f\x26\x23\x2e\xee\x2d\x00\xb7\x49\xc4\xf3\xe3\xed\xa2\xd5\x6b\xaa\x39\xe2\xed\xd0\xcd\x2f\x4e\x7a\x66\xef\xe6\x4f\x3f\x99\x35\x61\x2b\x63\xbf\xdf\xdd\x89\xd8\x4a\x19\xd8\x55\xd2\xda\xd9\xbe\xec\x9e\xee\x80\x96\xe0\x77\x00\x00\x00\xff\xff\x4c\x3f\xad\x47\xb0\x01\x00\x00") func assetsRouterServiceInternalYamlBytes() ([]byte, error) { return bindataRead( @@ -397,8 +397,8 @@ func assetsRouterServiceInternalYaml() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "assets/router/service-internal.yaml", size: 429, mode: os.FileMode(420), modTime: time.Unix(1, 0)} - a := &asset{bytes: bytes, info: info, digest: [32]uint8{0xb8, 0x63, 0x52, 0x85, 0x8b, 0x99, 0xe6, 0xc7, 0xcb, 0x34, 0x3d, 0x8d, 0x43, 0x65, 0x10, 0x63, 0x51, 0x80, 0xc1, 0x29, 0x17, 0xb6, 0x8f, 0x84, 0xdc, 0xf8, 0x33, 0xa1, 0x21, 0xc2, 0x5a, 0x4f}} + info := bindataFileInfo{name: "assets/router/service-internal.yaml", size: 432, mode: os.FileMode(420), modTime: time.Unix(1, 0)} + a := &asset{bytes: bytes, info: info, digest: [32]uint8{0x72, 0x14, 0xe2, 0x6f, 0x8f, 0xec, 0x74, 0xa4, 0xd0, 0x80, 0x68, 0x9f, 0x6e, 0xbf, 0x5e, 0x81, 0x88, 0xae, 0xfe, 0xf6, 0x98, 0x8a, 0xc3, 0x57, 0x9d, 0x68, 0x65, 0x25, 0x6f, 0x10, 0xa5, 0x97}} return a, nil } diff --git a/pkg/operator/controller/ingress/internal_service.go b/pkg/operator/controller/ingress/internal_service.go index 8be02bcae..ee4b224f0 100644 --- a/pkg/operator/controller/ingress/internal_service.go +++ b/pkg/operator/controller/ingress/internal_service.go @@ -25,7 +25,8 @@ const ( ) // ensureInternalRouterServiceForIngress ensures that an internal service exists -// for a given IngressController. +// for a given IngressController. Returns a Boolean indicating whether the +// service exists, the current service if it does exist, and an error value. func (r *reconciler) ensureInternalIngressControllerService(ic *operatorv1.IngressController, deploymentRef metav1.OwnerReference) (bool, *corev1.Service, error) { desired := desiredInternalIngressControllerService(ic, deploymentRef) have, current, err := r.currentInternalIngressControllerService(ic) diff --git a/pkg/operator/controller/ingress/internal_service_test.go b/pkg/operator/controller/ingress/internal_service_test.go index 37e06cd09..8187b9481 100644 --- a/pkg/operator/controller/ingress/internal_service_test.go +++ b/pkg/operator/controller/ingress/internal_service_test.go @@ -50,7 +50,7 @@ func Test_desiredInternalIngressControllerService(t *testing.T) { Name: "metrics", Protocol: "TCP", Port: int32(1936), - TargetPort: intstr.FromInt(1936), + TargetPort: intstr.FromString("metrics"), }}, svc.Spec.Ports) } @@ -116,6 +116,19 @@ func Test_internalServiceChanged(t *testing.T) { }, expect: true, }, + { + description: "if .spec.ports changes by changing the metrics port's target port from an integer to a string", + mutate: func(svc *corev1.Service) { + for i := range svc.Spec.Ports { + if svc.Spec.Ports[i].Name == "metrics" { + svc.Spec.Ports[i].TargetPort = intstr.FromString("metrics") + return + } + } + panic("no metrics port found!") + }, + expect: true, + }, { description: "if .spec.selector changes", mutate: func(svc *corev1.Service) {