From c42751f958891c9e75462b864b36042fb8c85867 Mon Sep 17 00:00:00 2001 From: Theodore Messinezis <7229472+theomessin@users.noreply.github.com> Date: Tue, 12 Sep 2023 03:33:15 +0100 Subject: [PATCH] Add Kourier LoadBalancerIP Config (#1343) * Add Kourier LoadBalancerIP Config * Refactor configureGWServiceType to configureGatewayService * Rename service-loadBalancerIP to service-load-balancer-ip * Update pkg/reconciler/knativeserving/ingress/kourier_test.go Co-authored-by: Kenjiro Nakayama * Can we please get this merged? --------- Co-authored-by: Theodore Messinezis Co-authored-by: Kenjiro Nakayama --- .../operator.knative.dev_knativeservings.yaml | 2 + .../operator/base/ingressconfiguration.go | 3 + .../knativeserving/ingress/kourier.go | 55 +++++++++------ .../knativeserving/ingress/kourier_test.go | 70 ++++++++++++------- 4 files changed, 84 insertions(+), 46 deletions(-) diff --git a/config/crd/bases/operator.knative.dev_knativeservings.yaml b/config/crd/bases/operator.knative.dev_knativeservings.yaml index e59e59f147..db51f012a1 100644 --- a/config/crd/bases/operator.knative.dev_knativeservings.yaml +++ b/config/crd/bases/operator.knative.dev_knativeservings.yaml @@ -2310,6 +2310,8 @@ spec: type: boolean service-type: type: string + service-load-balancer-ip: + type: string bootstrap-configmap: type: string http-port: diff --git a/pkg/apis/operator/base/ingressconfiguration.go b/pkg/apis/operator/base/ingressconfiguration.go index 7c98a8f399..9da7337dcd 100644 --- a/pkg/apis/operator/base/ingressconfiguration.go +++ b/pkg/apis/operator/base/ingressconfiguration.go @@ -41,6 +41,9 @@ type KourierIngressConfiguration struct { // ServiceType specifies the service type for kourier gateway. ServiceType v1.ServiceType `json:"service-type,omitempty"` + // ServiceLoadBalancerIP specifies the service load balancer IP. + ServiceLoadBalancerIP string `json:"service-load-balancer-ip,omitempty"` + // HTTPPort specifies the port used in case of ServiceType = "NodePort" for http traffic HTTPPort int32 `json:"http-port,omitempty"` diff --git a/pkg/reconciler/knativeserving/ingress/kourier.go b/pkg/reconciler/knativeserving/ingress/kourier.go index a76bee20ef..ad95d9e20b 100644 --- a/pkg/reconciler/knativeserving/ingress/kourier.go +++ b/pkg/reconciler/knativeserving/ingress/kourier.go @@ -40,15 +40,15 @@ var kourierControllerDeploymentNames = sets.NewString("3scale-kourier-control", func kourierTransformers(ctx context.Context, instance *v1beta1.KnativeServing) []mf.Transformer { return []mf.Transformer{ - replaceGWNamespace(), - configureGWServiceType(instance), + replaceGatewayNamespace(), + configureGatewayService(instance), configureBootstrapConfigMap(instance), } } -// replaceGWNamespace replace the environment variable KOURIER_GATEWAY_NAMESPACE with the +// replaceGatewayNamespace replace the environment variable KOURIER_GATEWAY_NAMESPACE with the // namespace of the deployment its set on. -func replaceGWNamespace() mf.Transformer { +func replaceGatewayNamespace() mf.Transformer { return func(u *unstructured.Unstructured) error { if u.GetKind() == "Deployment" && kourierControllerDeploymentNames.Has(u.GetName()) { deployment := &appsv1.Deployment{} @@ -74,39 +74,50 @@ func replaceGWNamespace() mf.Transformer { } } -// configureGWServiceType configures Kourier GW's service type such as ClusterIP, LoadBalancer and NodePort. -func configureGWServiceType(instance *v1beta1.KnativeServing) mf.Transformer { +func configureGatewayService(instance *v1beta1.KnativeServing) mf.Transformer { return func(u *unstructured.Unstructured) error { - if u.GetKind() == "Service" && u.GetName() == kourierGatewayServiceName { - if instance.Spec.Ingress.Kourier.ServiceType == "" { - // Do nothing if ServiceType is not configured. - return nil - } - svc := &v1.Service{} - if err := scheme.Scheme.Convert(u, svc, nil); err != nil { - return err - } + if !(u.GetKind() == "Service" && u.GetName() == kourierGatewayServiceName) { + return nil + } + + svc := &v1.Service{} + if err := scheme.Scheme.Convert(u, svc, nil); err != nil { + return err + } + // First configure the Service Type. + if instance.Spec.Ingress.Kourier.ServiceType != "" { serviceType := instance.Spec.Ingress.Kourier.ServiceType switch serviceType { case v1.ServiceTypeClusterIP, v1.ServiceTypeLoadBalancer: svc.Spec.Type = serviceType case v1.ServiceTypeNodePort: svc.Spec.Type = serviceType - if instance.Spec.Ingress.Kourier.HTTPPort > 0 || instance.Spec.Ingress.Kourier.HTTPSPort > 0 { - configureGWServiceTypeNodePort(instance, svc) - } case v1.ServiceTypeExternalName: return fmt.Errorf("unsupported service type %q", serviceType) default: return fmt.Errorf("unknown service type %q", serviceType) } + } - if err := scheme.Scheme.Convert(svc, u, nil); err != nil { - return err + // Configure LoadBalancerIP if set. + if instance.Spec.Ingress.Kourier.ServiceLoadBalancerIP != "" { + if svc.Spec.Type != v1.ServiceTypeLoadBalancer { + return fmt.Errorf("cannot configure LoadBalancerIP for service type %q", svc.Spec.Type) } + svc.Spec.LoadBalancerIP = instance.Spec.Ingress.Kourier.ServiceLoadBalancerIP } - return nil + + // Configure HTTPPort/HTTPSPort if set. + if instance.Spec.Ingress.Kourier.HTTPPort > 0 || instance.Spec.Ingress.Kourier.HTTPSPort > 0 { + if svc.Spec.Type != v1.ServiceTypeNodePort { + return fmt.Errorf("cannot configure HTTP(S)Port for service type %q", svc.Spec.Type) + } + configureGatewayServiceTypeNodePort(instance, svc) + } + + // Return any conversion error or nil + return scheme.Scheme.Convert(svc, u, nil) } } @@ -144,7 +155,7 @@ func configureBootstrapConfigMap(instance *v1beta1.KnativeServing) mf.Transforme } } -func configureGWServiceTypeNodePort(instance *v1beta1.KnativeServing, svc *v1.Service) { +func configureGatewayServiceTypeNodePort(instance *v1beta1.KnativeServing, svc *v1.Service) { for i, v := range svc.Spec.Ports { if v.Name != "https" && instance.Spec.Ingress.Kourier.HTTPPort > 0 { v.NodePort = instance.Spec.Ingress.Kourier.HTTPPort diff --git a/pkg/reconciler/knativeserving/ingress/kourier_test.go b/pkg/reconciler/knativeserving/ingress/kourier_test.go index 9a38127a85..5dacfd4066 100644 --- a/pkg/reconciler/knativeserving/ingress/kourier_test.go +++ b/pkg/reconciler/knativeserving/ingress/kourier_test.go @@ -34,7 +34,7 @@ import ( const servingNamespace = "knative-serving" -func servingInstance(ns string, serviceType v1.ServiceType, bootstrapConfigmapName string) *servingv1beta1.KnativeServing { +func servingInstance(ns string, serviceType v1.ServiceType, bootstrapConfigmapName string, serviceLoadBalancerIP string) *servingv1beta1.KnativeServing { return &servingv1beta1.KnativeServing{ ObjectMeta: metav1.ObjectMeta{ Name: "test-instance", @@ -45,6 +45,7 @@ func servingInstance(ns string, serviceType v1.ServiceType, bootstrapConfigmapNa Kourier: base.KourierIngressConfiguration{ Enabled: true, ServiceType: serviceType, + ServiceLoadBalancerIP: serviceLoadBalancerIP, BootstrapConfigmapName: bootstrapConfigmapName, }, }, @@ -74,36 +75,46 @@ func servingInstanceNodePorts(ns string, bootstrapConfigmapName string, httpPort func TestTransformKourierManifest(t *testing.T) { tests := []struct { - name string - instance *servingv1beta1.KnativeServing - expNamespace string - expServiceType string - expConfigMapName string - expNodePortsHTTP int32 - expNodePortsHTTPS int32 - expError error + name string + instance *servingv1beta1.KnativeServing + expNamespace string + expServiceType string + expServiceLoadBalancerIP string + expConfigMapName string + expNodePortsHTTP int32 + expNodePortsHTTPS int32 + expError error }{{ name: "Replaces Kourier Gateway Namespace, ServiceType and bootstrap cm", - instance: servingInstance(servingNamespace, "ClusterIP", "my-bootstrap"), + instance: servingInstance(servingNamespace, "ClusterIP", "my-bootstrap", ""), expNamespace: servingNamespace, expConfigMapName: "my-bootstrap", expServiceType: "ClusterIP", }, { - name: "Use Kourier default service type", - instance: servingInstance(servingNamespace, "" /* empty service type */, ""), - expNamespace: servingNamespace, - expConfigMapName: kourierDefaultVolumeName, - expServiceType: "LoadBalancer", // kourier GW default service type + name: "Use Kourier default service type", + instance: servingInstance(servingNamespace, "" /* empty service type */, "", ""), + expNamespace: servingNamespace, + expConfigMapName: kourierDefaultVolumeName, + expServiceType: "LoadBalancer", // kourier GW default service type + expServiceLoadBalancerIP: "", }, { - name: "Use unsupported service type", - instance: servingInstance(servingNamespace, "ExternalName", ""), - expNamespace: servingNamespace, - expServiceType: "ExternalName", - expConfigMapName: kourierDefaultVolumeName, - expError: fmt.Errorf("unsupported service type \"ExternalName\""), + name: "Sets Kourier Gateway ServiceLoadBalancerIP", + instance: servingInstance(servingNamespace, "" /* empty service type */, "", "1.2.3.4"), + expNamespace: servingNamespace, + expConfigMapName: kourierDefaultVolumeName, + expServiceType: "LoadBalancer", + expServiceLoadBalancerIP: "1.2.3.4", + }, { + name: "Use ServiceLoadBalancerIP with unsupported service type", + instance: servingInstance(servingNamespace, "ClusterIP", "", "1.2.3.4"), + expNamespace: servingNamespace, + expConfigMapName: kourierDefaultVolumeName, + expServiceType: "ClusterIP", + expServiceLoadBalancerIP: "1.2.3.4", + expError: fmt.Errorf("cannot configure LoadBalancerIP for service type \"ClusterIP\""), }, { name: "Use unknown service type", - instance: servingInstance(servingNamespace, "Foo", ""), + instance: servingInstance(servingNamespace, "Foo", "", ""), expNamespace: servingNamespace, expServiceType: "Foo", expConfigMapName: kourierDefaultVolumeName, @@ -142,12 +153,12 @@ func TestTransformKourierManifest(t *testing.T) { t.Fatalf("Failed to read manifest: %v", err) } - manifest, err = manifest.Transform(replaceGWNamespace()) + manifest, err = manifest.Transform(replaceGatewayNamespace()) if err != nil { t.Fatalf("Failed to transform manifest: %v", err) } - manifest, err = manifest.Transform(configureGWServiceType(tt.instance)) + manifest, err = manifest.Transform(configureGatewayService(tt.instance)) if err != nil { util.AssertEqual(t, err.Error(), tt.expError.Error()) } else { @@ -162,6 +173,7 @@ func TestTransformKourierManifest(t *testing.T) { for _, u := range manifest.Resources() { verifyControllerNamespace(t, &u, tt.expNamespace) verifyGatewayServiceType(t, &u, tt.expServiceType) + verifyGatewayServiceLoadBalancerIP(t, &u, tt.expServiceLoadBalancerIP) verifyGatewayServiceTypeNodePortHTTP(t, &u, tt.expNodePortsHTTP) verifyGatewayServiceTypeNodePortHTTPS(t, &u, tt.expNodePortsHTTPS) verifyBootstrapVolumeName(t, &u, tt.expConfigMapName) @@ -238,6 +250,16 @@ func verifyGatewayServiceType(t *testing.T, u *unstructured.Unstructured, expSer } } +func verifyGatewayServiceLoadBalancerIP(t *testing.T, u *unstructured.Unstructured, expServiceLoadBalancerIP string) { + if u.GetKind() == "Service" && u.GetName() == kourierGatewayServiceName { + svc := &v1.Service{} + err := scheme.Scheme.Convert(u, svc, nil) + util.AssertEqual(t, err, nil) + svcLoadBalancerIP := svc.Spec.LoadBalancerIP + util.AssertDeepEqual(t, svcLoadBalancerIP, expServiceLoadBalancerIP) + } +} + // removeProviderLabels removes labels. This util is used for tests without provider label. func removeLabels() mf.Transformer { return func(u *unstructured.Unstructured) error {