diff --git a/pkg/reconciler/knativeserving/ingress/ingress_test.go b/pkg/reconciler/knativeserving/ingress/ingress_test.go index 800601d5eb..fe3747c858 100644 --- a/pkg/reconciler/knativeserving/ingress/ingress_test.go +++ b/pkg/reconciler/knativeserving/ingress/ingress_test.go @@ -58,7 +58,7 @@ func TestTransformers(t *testing.T) { }, }, }, - expected: 4, + expected: 3, }, { name: "Available contour ingress", instance: servingv1beta1.KnativeServing{ @@ -94,7 +94,7 @@ func TestTransformers(t *testing.T) { }, }, }, - expected: 5, + expected: 4, }} for _, tt := range tests { diff --git a/pkg/reconciler/knativeserving/ingress/kourier.go b/pkg/reconciler/knativeserving/ingress/kourier.go index 6988e88316..e2e200713d 100644 --- a/pkg/reconciler/knativeserving/ingress/kourier.go +++ b/pkg/reconciler/knativeserving/ingress/kourier.go @@ -40,16 +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), - configureGWServiceLoadBalancerIP(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{} @@ -75,19 +74,19 @@ 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.ServiceTypeNodePort, v1.ServiceTypeLoadBalancer: @@ -97,34 +96,18 @@ func configureGWServiceType(instance *v1beta1.KnativeServing) mf.Transformer { default: return fmt.Errorf("unknown service type %q", serviceType) } - - if err := scheme.Scheme.Convert(svc, u, nil); err != nil { - return err - } } - return nil - } -} -// configureGWServiceLoadBalancerIP configures Kourier GW's service loadBalancerIP. -func configureGWServiceLoadBalancerIP(instance *v1beta1.KnativeServing) mf.Transformer { - return func(u *unstructured.Unstructured) error { - if u.GetKind() == "Service" && u.GetName() == kourierGatewayServiceName { - if instance.Spec.Ingress.Kourier.ServiceLoadBalancerIP == "" { - // Do nothing if ServiceLoadBalancerIP is not configured. - return nil - } - svc := &v1.Service{} - if err := scheme.Scheme.Convert(u, svc, nil); err != nil { - return err + // Then 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 - if err := scheme.Scheme.Convert(svc, u, nil); err != nil { - return err - } } - return nil + + // Return any conversion error or nil + return scheme.Scheme.Convert(svc, u, nil) } } diff --git a/pkg/reconciler/knativeserving/ingress/kourier_test.go b/pkg/reconciler/knativeserving/ingress/kourier_test.go index 3599c2746e..7252e1f37c 100644 --- a/pkg/reconciler/knativeserving/ingress/kourier_test.go +++ b/pkg/reconciler/knativeserving/ingress/kourier_test.go @@ -83,22 +83,30 @@ func TestTransformKourierManifest(t *testing.T) { 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 unsupported service type", instance: servingInstance(servingNamespace, "ExternalName", "", ""), expNamespace: servingNamespace, expServiceType: "ExternalName", expConfigMapName: kourierDefaultVolumeName, - expError: fmt.Errorf("unsupported service type \"ExternalName\""), expServiceLoadBalancerIP: "", + expError: fmt.Errorf("unsupported service type \"ExternalName\""), }, { name: "Use unknown service type", instance: servingInstance(servingNamespace, "Foo", "", ""), expNamespace: servingNamespace, expServiceType: "Foo", expConfigMapName: kourierDefaultVolumeName, - expError: fmt.Errorf("unknown service type \"Foo\""), expServiceLoadBalancerIP: "", + expError: fmt.Errorf("unknown service type \"Foo\""), }} for _, tt := range tests { @@ -109,12 +117,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 { @@ -126,11 +134,6 @@ func TestTransformKourierManifest(t *testing.T) { t.Fatalf("Failed to transform manifest: %v", err) } - manifest, err = manifest.Transform(configureGWServiceLoadBalancerIP(tt.instance)) - if err != nil { - t.Fatalf("Failed to transform manifest: %v", err) - } - for _, u := range manifest.Resources() { verifyControllerNamespace(t, &u, tt.expNamespace) verifyGatewayServiceType(t, &u, tt.expServiceType)