crd/ContourDeployment: Add field for set externalTrafficPolicy for se…

…rvice/envoy (#4803) Signed-off-by: Gang Liu <gang.liu@daocloud.io>
projectcontour · Nov 2, 2022 · 4badccc · 4badccc
1 parent 55f5ee5
commit 4badccc
Show file tree

Hide file tree

Showing 15 changed files with 135 additions and 7 deletions.
diff --git a/apis/projectcontour/v1alpha1/contourdeployment.go b/apis/projectcontour/v1alpha1/contourdeployment.go
@@ -204,6 +204,15 @@ type NetworkPublishing struct {
 	// +optional
 	Type NetworkPublishingType `json:"type,omitempty"`
 
+	// ExternalTrafficPolicy describes how nodes distribute service traffic they
+	// receive on one of the Service's "externally-facing" addresses (NodePorts, ExternalIPs,
+	// and LoadBalancer IPs).
+	//
+	// If unset, defaults to "Local".
+	//
+	// +optional
+	ExternalTrafficPolicy corev1.ServiceExternalTrafficPolicyType `json:"externalTrafficPolicy,omitempty"`
+
 	// ServiceAnnotations is the annotations to add to
 	// the provisioned Envoy service.
 	//

diff --git a/changelogs/unreleased/4803-izturn-small.md b/changelogs/unreleased/4803-izturn-small.md
@@ -0,0 +1 @@
+Add Service/Envoy's ExternalTrafficPolicy configurability to ContourDeployment resource.
diff --git a/examples/contour/01-crds.yaml b/examples/contour/01-crds.yaml
@@ -2571,6 +2571,12 @@ spec:
                     description: NetworkPublishing defines how to expose Envoy to
                       a network.
                     properties:
+                      externalTrafficPolicy:
+                        description: "ExternalTrafficPolicy describes how nodes distribute
+                          service traffic they receive on one of the Service's \"externally-facing\"
+                          addresses (NodePorts, ExternalIPs, and LoadBalancer IPs).
+                          \n If unset, defaults to \"Local\"."
+                        type: string
                       serviceAnnotations:
                         additionalProperties:
                           type: string

diff --git a/examples/render/contour-deployment.yaml b/examples/render/contour-deployment.yaml
@@ -2780,6 +2780,12 @@ spec:
                     description: NetworkPublishing defines how to expose Envoy to
                       a network.
                     properties:
+                      externalTrafficPolicy:
+                        description: "ExternalTrafficPolicy describes how nodes distribute
+                          service traffic they receive on one of the Service's \"externally-facing\"
+                          addresses (NodePorts, ExternalIPs, and LoadBalancer IPs).
+                          \n If unset, defaults to \"Local\"."
+                        type: string
                       serviceAnnotations:
                         additionalProperties:
                           type: string

diff --git a/examples/render/contour-gateway-provisioner.yaml b/examples/render/contour-gateway-provisioner.yaml
@@ -2585,6 +2585,12 @@ spec:
                     description: NetworkPublishing defines how to expose Envoy to
                       a network.
                     properties:
+                      externalTrafficPolicy:
+                        description: "ExternalTrafficPolicy describes how nodes distribute
+                          service traffic they receive on one of the Service's \"externally-facing\"
+                          addresses (NodePorts, ExternalIPs, and LoadBalancer IPs).
+                          \n If unset, defaults to \"Local\"."
+                        type: string
                       serviceAnnotations:
                         additionalProperties:
                           type: string

diff --git a/examples/render/contour-gateway.yaml b/examples/render/contour-gateway.yaml
@@ -2786,6 +2786,12 @@ spec:
                     description: NetworkPublishing defines how to expose Envoy to
                       a network.
                     properties:
+                      externalTrafficPolicy:
+                        description: "ExternalTrafficPolicy describes how nodes distribute
+                          service traffic they receive on one of the Service's \"externally-facing\"
+                          addresses (NodePorts, ExternalIPs, and LoadBalancer IPs).
+                          \n If unset, defaults to \"Local\"."
+                        type: string
                       serviceAnnotations:
                         additionalProperties:
                           type: string

diff --git a/examples/render/contour.yaml b/examples/render/contour.yaml
@@ -2780,6 +2780,12 @@ spec:
                     description: NetworkPublishing defines how to expose Envoy to
                       a network.
                     properties:
+                      externalTrafficPolicy:
+                        description: "ExternalTrafficPolicy describes how nodes distribute
+                          service traffic they receive on one of the Service's \"externally-facing\"
+                          addresses (NodePorts, ExternalIPs, and LoadBalancer IPs).
+                          \n If unset, defaults to \"Local\"."
+                        type: string
                       serviceAnnotations:
                         additionalProperties:
                           type: string

diff --git a/internal/provisioner/controller/gateway.go b/internal/provisioner/controller/gateway.go
@@ -284,6 +284,11 @@ func (r *gatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 				if networkPublishing.Type != "" {
 					contourModel.Spec.NetworkPublishing.Envoy.Type = networkPublishing.Type
 				}
+
+				if networkPublishing.ExternalTrafficPolicy != "" {
+					contourModel.Spec.NetworkPublishing.Envoy.ExternalTrafficPolicy = networkPublishing.ExternalTrafficPolicy
+				}
+
 				contourModel.Spec.NetworkPublishing.Envoy.ServiceAnnotations = networkPublishing.ServiceAnnotations
 			}
 

diff --git a/internal/provisioner/controller/gateway_test.go b/internal/provisioner/controller/gateway_test.go
@@ -1015,7 +1015,8 @@ func TestGatewayReconcile(t *testing.T) {
 				Spec: contourv1alpha1.ContourDeploymentSpec{
 					Envoy: &contourv1alpha1.EnvoySettings{
 						NetworkPublishing: &contourv1alpha1.NetworkPublishing{
-							Type: contourv1alpha1.ClusterIPServicePublishingType,
+							Type:                  contourv1alpha1.NodePortServicePublishingType,
+							ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeCluster,
 							ServiceAnnotations: map[string]string{
 								"key-1": "val-1",
 								"key-2": "val-2",
@@ -1050,8 +1051,8 @@ func TestGatewayReconcile(t *testing.T) {
 					},
 				}
 				require.NoError(t, r.client.Get(context.Background(), keyFor(svc), svc))
-
-				assert.Equal(t, corev1.ServiceTypeClusterIP, svc.Spec.Type)
+				assert.Equal(t, corev1.ServiceExternalTrafficPolicyTypeCluster, svc.Spec.ExternalTrafficPolicy)
+				assert.Equal(t, corev1.ServiceTypeNodePort, svc.Spec.Type)
 				require.Len(t, svc.Annotations, 2)
 				assert.Equal(t, "val-1", svc.Annotations["key-1"])
 				assert.Equal(t, "val-2", svc.Annotations["key-2"])

diff --git a/internal/provisioner/controller/gatewayclass.go b/internal/provisioner/controller/gatewayclass.go
@@ -18,9 +18,11 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/go-logr/logr"
 	"github.com/projectcontour/contour/apis/projectcontour/v1alpha1"
 	contour_api_v1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1"
+
+	"github.com/go-logr/logr"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -181,6 +183,14 @@ func (r *gatewayClassReconciler) Reconcile(ctx context.Context, req ctrl.Request
 						params.Spec.Envoy.NetworkPublishing.Type)
 					invalidParamsMessages = append(invalidParamsMessages, msg)
 				}
+
+				switch params.Spec.Envoy.NetworkPublishing.ExternalTrafficPolicy {
+				case "", corev1.ServiceExternalTrafficPolicyTypeCluster, corev1.ServiceExternalTrafficPolicyTypeLocal:
+				default:
+					msg := fmt.Sprintf("invalid ContourDeployment spec.envoy.networkPublishing.externalTrafficPolicy %q, must be Local or Cluster",
+						params.Spec.Envoy.NetworkPublishing.ExternalTrafficPolicy)
+					invalidParamsMessages = append(invalidParamsMessages, msg)
+				}
 			}
 
 			if params.Spec.Envoy.ExtraVolumeMounts != nil {

diff --git a/internal/provisioner/controller/gatewayclass_test.go b/internal/provisioner/controller/gatewayclass_test.go
@@ -350,6 +350,40 @@ func TestGatewayClassReconcile(t *testing.T) {
 				Reason: string(gatewayv1beta1.GatewayClassReasonInvalidParameters),
 			},
 		},
+		"gatewayclass controlled by us with a valid parametersRef but invalid parameter values for ExternalTrafficPolicy gets Accepted: false condition": {
+			gatewayClass: &gatewayv1beta1.GatewayClass{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "gatewayclass-1",
+				},
+				Spec: gatewayv1beta1.GatewayClassSpec{
+					ControllerName: "projectcontour.io/gateway-controller",
+					ParametersRef: &gatewayv1beta1.ParametersReference{
+						Group:     "projectcontour.io",
+						Kind:      "ContourDeployment",
+						Name:      "gatewayclass-params",
+						Namespace: gatewayapi.NamespacePtr("projectcontour"),
+					},
+				},
+			},
+			params: &contourv1alpha1.ContourDeployment{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "projectcontour",
+					Name:      "gatewayclass-params",
+				},
+				Spec: contourv1alpha1.ContourDeploymentSpec{
+					Envoy: &contourv1alpha1.EnvoySettings{
+						NetworkPublishing: &contourv1alpha1.NetworkPublishing{
+							ExternalTrafficPolicy: "invalid-external-traffic-policy",
+						},
+					},
+				},
+			},
+			wantCondition: &metav1.Condition{
+				Type:   string(gatewayv1beta1.GatewayClassConditionStatusAccepted),
+				Status: metav1.ConditionFalse,
+				Reason: string(gatewayv1beta1.GatewayClassReasonInvalidParameters),
+			},
+		},
 	}
 
 	for name, tc := range tests {

diff --git a/internal/provisioner/model/model.go b/internal/provisioner/model/model.go
@@ -41,7 +41,8 @@ func Default(namespace, name string) *Contour {
 			EnvoyLogLevel:     contourv1alpha1.InfoLog,
 			NetworkPublishing: NetworkPublishing{
 				Envoy: EnvoyNetworkPublishing{
-					Type: LoadBalancerServicePublishingType,
+					Type:                  LoadBalancerServicePublishingType,
+					ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeLocal,
 					ContainerPorts: []ContainerPort{
 						{
 							Name:       "http",
@@ -366,6 +367,13 @@ type EnvoyNetworkPublishing struct {
 
 	// ServiceAnnotations is a set of annotations to add to the provisioned Envoy service.
 	ServiceAnnotations map[string]string
+
+	// ExternalTrafficPolicy describes how nodes distribute service traffic they
+	// receive on one of the Service's "externally-facing" addresses (NodePorts, ExternalIPs,
+	// and LoadBalancer IPs).
+	//
+	// If unset, defaults to "Local".
+	ExternalTrafficPolicy corev1.ServiceExternalTrafficPolicyType
 }
 
 type NetworkPublishingType = contourv1alpha1.NetworkPublishingType

diff --git a/internal/provisioner/objects/service/service.go b/internal/provisioner/objects/service/service.go
@@ -250,7 +250,7 @@ func DesiredEnvoyService(contour *model.Contour) *corev1.Service {
 	epType := contour.Spec.NetworkPublishing.Envoy.Type
 	if epType == model.LoadBalancerServicePublishingType ||
 		epType == model.NodePortServicePublishingType {
-		svc.Spec.ExternalTrafficPolicy = corev1.ServiceExternalTrafficPolicyTypeLocal
+		svc.Spec.ExternalTrafficPolicy = contour.Spec.NetworkPublishing.Envoy.ExternalTrafficPolicy
 	}
 	switch epType {
 	case model.LoadBalancerServicePublishingType:

diff --git a/internal/provisioner/objects/service/service_test.go b/internal/provisioner/objects/service/service_test.go
@@ -124,6 +124,13 @@ func checkServiceHasExternalTrafficPolicy(t *testing.T, svc *corev1.Service, pol
 	}
 }
 
+func checkServiceHasNoExternalTrafficPolicy(t *testing.T, svc *corev1.Service) {
+	t.Helper()
+
+	if svc.Spec.ExternalTrafficPolicy != "" {
+		t.Errorf("service has invalid external traffic policy type %s", svc.Spec.ExternalTrafficPolicy)
+	}
+}
 func checkServiceHasLoadBalancerAddress(t *testing.T, svc *corev1.Service, address string) {
 	t.Helper()
 
@@ -177,14 +184,19 @@ func TestDesiredEnvoyService(t *testing.T) {
 	checkServiceHasPortName(t, svc, "https")
 	checkServiceHasPortProtocol(t, svc, corev1.ProtocolTCP)
 
+	cntr.Spec.NetworkPublishing.Envoy.Type = model.ClusterIPServicePublishingType
+	svc = DesiredEnvoyService(cntr)
+	checkServiceHasNoExternalTrafficPolicy(t, svc)
+
 	// Check LB annotations for the different provider types, starting with AWS ELB (the default
 	// if AWS provider params are not passed).
 	cntr.Spec.NetworkPublishing.Envoy.Type = model.LoadBalancerServicePublishingType
+	cntr.Spec.NetworkPublishing.Envoy.ExternalTrafficPolicy = corev1.ServiceExternalTrafficPolicyTypeCluster
 	cntr.Spec.NetworkPublishing.Envoy.LoadBalancer.Scope = model.ExternalLoadBalancer
 	cntr.Spec.NetworkPublishing.Envoy.LoadBalancer.ProviderParameters.Type = model.AWSLoadBalancerProvider
 	svc = DesiredEnvoyService(cntr)
 	checkServiceHasType(t, svc, corev1.ServiceTypeLoadBalancer)
-	checkServiceHasExternalTrafficPolicy(t, svc, corev1.ServiceExternalTrafficPolicyTypeLocal)
+	checkServiceHasExternalTrafficPolicy(t, svc, corev1.ServiceExternalTrafficPolicyTypeCluster)
 	checkServiceHasAnnotations(t, svc, awsLbBackendProtoAnnotation, awsLBProxyProtocolAnnotation)
 
 	// Test proxy protocol for AWS Classic load balancer (when provider params are specified).

diff --git a/site/content/docs/main/config/api-reference.html b/site/content/docs/main/config/api-reference.html
@@ -6998,6 +6998,24 @@ <h3 id="projectcontour.io/v1alpha1.NetworkPublishing">NetworkPublishing
 </tr>
 <tr>
 <td style="white-space:nowrap">
+<code>externalTrafficPolicy</code>
+<br>
+<em>
+<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#serviceexternaltrafficpolicytype-v1-core">
+Kubernetes core/v1.ServiceExternalTrafficPolicyType
+</a>
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>ExternalTrafficPolicy describes how nodes distribute service traffic they
+receive on one of the Service&rsquo;s &ldquo;externally-facing&rdquo; addresses (NodePorts, ExternalIPs,
+and LoadBalancer IPs).</p>
+<p>If unset, defaults to &ldquo;Local&rdquo;.</p>
+</td>
+</tr>
+<tr>
+<td style="white-space:nowrap">
 <code>serviceAnnotations</code>
 <br>
 <em>
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Add Service/Envoy's ExternalTrafficPolicy configurability to ContourDeployment resource.