Skip to content

Commit

Permalink
Add Kourier LoadBalancerIP Config (knative#1343)
Browse files Browse the repository at this point in the history 
* 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 <[email protected]>

* Can we please get this merged?

---------

Co-authored-by: Theodore Messinezis <[email protected]>
Co-authored-by: Kenjiro Nakayama <[email protected]>
  • Loading branch information
@theomessin @nak3
3 people authored Sep 12, 2023
1 parent d5e977a commit c42751f
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 46 deletions.
2 changes: 2 additions & 0 deletions config/crd/bases/operator.knative.dev_knativeservings.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2310,6 +2310,8 @@ spec:
type: boolean
service-type:
type: string
service-load-balancer-ip:
type: string
bootstrap-configmap:
type: string
http-port:
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/operator/base/ingressconfiguration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"`

Expand Down
55 changes: 33 additions & 22 deletions pkg/reconciler/knativeserving/ingress/kourier.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -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)
}
}

Expand Down Expand Up @@ -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
Expand Down
70 changes: 46 additions & 24 deletions pkg/reconciler/knativeserving/ingress/kourier_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -45,6 +45,7 @@ func servingInstance(ns string, serviceType v1.ServiceType, bootstrapConfigmapNa
Kourier: base.KourierIngressConfiguration{
Enabled: true,
ServiceType: serviceType,
ServiceLoadBalancerIP: serviceLoadBalancerIP,
BootstrapConfigmapName: bootstrapConfigmapName,
},
},
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit c42751f

Please sign in to comment.