From 3c879df6ce7b3097cd222d0078f1a59ebe7e699b Mon Sep 17 00:00:00 2001 From: Kevin Shelaga <65769106+kevin-shelaga@users.noreply.github.com> Date: Tue, 31 Dec 2024 11:11:00 -0500 Subject: [PATCH 01/10] backport to 1.18 - add external traffic policy to gwp --- .../v1.18.1/external-traffic-policy.yaml | 6 +++++ .../v1.18.4/external-traffic-policy.yaml | 6 +++++ .../api/v1alpha1/gateway_parameters.md | 7 +++++ docs/content/reference/values.txt | 5 ++-- ...ateway.gloo.solo.io_gatewayparameters.yaml | 2 ++ install/helm/gloo/generate/values.go | 9 ++++--- .../gloo/templates/43-gatewayparameters.yaml | 3 +++ install/test/k8sgateway_test.go | 4 +++ projects/gateway2/api/v1alpha1/kube_types.go | 12 +++++++++ .../api/v1alpha1/zz_generated.deepcopy.go | 5 ++++ projects/gateway2/deployer/deployer_test.go | 6 +++++ projects/gateway2/deployer/merge.go | 4 +++ projects/gateway2/deployer/merge_test.go | 26 +++++++++++++++++++ projects/gateway2/deployer/values.go | 9 ++++--- projects/gateway2/deployer/values_helpers.go | 13 +++++++--- .../templates/gateway/proxy-deployment.yaml | 3 +++ .../kubernetes/e2e/features/deployer/suite.go | 9 +++++++ 17 files changed, 115 insertions(+), 14 deletions(-) create mode 100644 changelog/v1.18.1/external-traffic-policy.yaml create mode 100644 changelog/v1.18.4/external-traffic-policy.yaml diff --git a/changelog/v1.18.1/external-traffic-policy.yaml b/changelog/v1.18.1/external-traffic-policy.yaml new file mode 100644 index 00000000000..557ca0a4fd0 --- /dev/null +++ b/changelog/v1.18.1/external-traffic-policy.yaml @@ -0,0 +1,6 @@ +changelog: + - type: NEW_FEATURE + issueLink: https://github.com/k8sgateway/k8sgateway/issues/9879 + resolvesIssue: true + description: >- + Add ability to configure proxy service External Traffic Policy via Gateway Params \ No newline at end of file diff --git a/changelog/v1.18.4/external-traffic-policy.yaml b/changelog/v1.18.4/external-traffic-policy.yaml new file mode 100644 index 00000000000..557ca0a4fd0 --- /dev/null +++ b/changelog/v1.18.4/external-traffic-policy.yaml @@ -0,0 +1,6 @@ +changelog: + - type: NEW_FEATURE + issueLink: https://github.com/k8sgateway/k8sgateway/issues/9879 + resolvesIssue: true + description: >- + Add ability to configure proxy service External Traffic Policy via Gateway Params \ No newline at end of file diff --git a/docs/content/reference/api/github.com/solo-io/gloo/projects/gateway2/api/v1alpha1/gateway_parameters.md b/docs/content/reference/api/github.com/solo-io/gloo/projects/gateway2/api/v1alpha1/gateway_parameters.md index bf0913a0af0..42e0be911d5 100644 --- a/docs/content/reference/api/github.com/solo-io/gloo/projects/gateway2/api/v1alpha1/gateway_parameters.md +++ b/docs/content/reference/api/github.com/solo-io/gloo/projects/gateway2/api/v1alpha1/gateway_parameters.md @@ -7657,6 +7657,13 @@ Resource Types:
false + + externalTrafficPolicy + string + +
+ + false extraAnnotations map[string]string diff --git a/docs/content/reference/values.txt b/docs/content/reference/values.txt index f0c00e7e30b..5db8b9bca08 100644 --- a/docs/content/reference/values.txt +++ b/docs/content/reference/values.txt @@ -42,6 +42,7 @@ |kubeGateway.gatewayParameters.glooGateway.service.type|string|LoadBalancer|K8s service type. If set to null, a default of LoadBalancer will be imposed.| |kubeGateway.gatewayParameters.glooGateway.service.extraLabels.NAME|string||Extra labels to add to the service.| |kubeGateway.gatewayParameters.glooGateway.service.extraAnnotations.NAME|string||Extra annotations to add to the service.| +|kubeGateway.gatewayParameters.glooGateway.service.externalTrafficPolicy|string||Set the external traffic policy on the provisioned service.| |kubeGateway.gatewayParameters.glooGateway.serviceAccount.extraLabels.NAME|string||Extra labels to add to the service account.| |kubeGateway.gatewayParameters.glooGateway.serviceAccount.extraAnnotations.NAME|string||Extra annotations to add to the service account.| |kubeGateway.gatewayParameters.glooGateway.sdsContainer.image.tag|string||The image tag for the container.| @@ -1002,7 +1003,7 @@ |gatewayProxies.NAME.service.httpsNodePort|int||HTTPS nodeport for the gateway service if using type NodePort| |gatewayProxies.NAME.service.clusterIP|string||static clusterIP (or `None`) when `gatewayProxies[].gatewayProxy.service.type` is `ClusterIP`| |gatewayProxies.NAME.service.extraAnnotations.NAME|string||| -|gatewayProxies.NAME.service.externalTrafficPolicy|string||| +|gatewayProxies.NAME.service.externalTrafficPolicy|string||Set the external traffic policy on the provisioned service.| |gatewayProxies.NAME.service.name|string||Custom name override for the service resource of the proxy| |gatewayProxies.NAME.service.httpsFirst|bool||List HTTPS port before HTTP| |gatewayProxies.NAME.service.loadBalancerIP|string||IP address of the load balancer| @@ -1255,7 +1256,7 @@ |gatewayProxies.gatewayProxy.service.httpsNodePort|int||HTTPS nodeport for the gateway service if using type NodePort| |gatewayProxies.gatewayProxy.service.clusterIP|string||static clusterIP (or `None`) when `gatewayProxies[].gatewayProxy.service.type` is `ClusterIP`| |gatewayProxies.gatewayProxy.service.extraAnnotations.NAME|string||| -|gatewayProxies.gatewayProxy.service.externalTrafficPolicy|string||| +|gatewayProxies.gatewayProxy.service.externalTrafficPolicy|string||Set the external traffic policy on the provisioned service.| |gatewayProxies.gatewayProxy.service.name|string||Custom name override for the service resource of the proxy| |gatewayProxies.gatewayProxy.service.httpsFirst|bool||List HTTPS port before HTTP| |gatewayProxies.gatewayProxy.service.loadBalancerIP|string||IP address of the load balancer| diff --git a/install/helm/gloo/crds/gateway.gloo.solo.io_gatewayparameters.yaml b/install/helm/gloo/crds/gateway.gloo.solo.io_gatewayparameters.yaml index 7e7fb5fbe5f..48c47ad909e 100644 --- a/install/helm/gloo/crds/gateway.gloo.solo.io_gatewayparameters.yaml +++ b/install/helm/gloo/crds/gateway.gloo.solo.io_gatewayparameters.yaml @@ -2075,6 +2075,8 @@ spec: properties: clusterIP: type: string + externalTrafficPolicy: + type: string extraAnnotations: additionalProperties: type: string diff --git a/install/helm/gloo/generate/values.go b/install/helm/gloo/generate/values.go index 2aa2f193b5f..d045b3cdf9f 100644 --- a/install/helm/gloo/generate/values.go +++ b/install/helm/gloo/generate/values.go @@ -369,9 +369,10 @@ type ProvisionedDeployment struct { } type ProvisionedService struct { - Type *string `json:"type,omitempty" desc:"K8s service type. If set to null, a default of LoadBalancer will be imposed."` - ExtraLabels map[string]string `json:"extraLabels,omitempty" desc:"Extra labels to add to the service."` - ExtraAnnotations map[string]string `json:"extraAnnotations,omitempty" desc:"Extra annotations to add to the service."` + Type *string `json:"type,omitempty" desc:"K8s service type. If set to null, a default of LoadBalancer will be imposed."` + ExtraLabels map[string]string `json:"extraLabels,omitempty" desc:"Extra labels to add to the service."` + ExtraAnnotations map[string]string `json:"extraAnnotations,omitempty" desc:"Extra annotations to add to the service."` + ExternalTrafficPolicy *string `json:"externalTrafficPolicy,omitempty" desc:"Set the external traffic policy on the provisioned service."` } type ProvisionedServiceAccount struct { @@ -717,7 +718,7 @@ type GatewayProxyService struct { HttpsNodePort *int `json:"httpsNodePort,omitempty" desc:"HTTPS nodeport for the gateway service if using type NodePort"` ClusterIP *string "json:\"clusterIP,omitempty\" desc:\"static clusterIP (or `None`) when `gatewayProxies[].gatewayProxy.service.type` is `ClusterIP`\"" ExtraAnnotations map[string]string `json:"extraAnnotations,omitempty"` - ExternalTrafficPolicy *string `json:"externalTrafficPolicy,omitempty"` + ExternalTrafficPolicy *string `json:"externalTrafficPolicy,omitempty" desc:"Set the external traffic policy on the provisioned service."` Name *string `json:"name,omitempty" desc:"Custom name override for the service resource of the proxy"` HttpsFirst *bool `json:"httpsFirst,omitempty" desc:"List HTTPS port before HTTP"` LoadBalancerIP *string `json:"loadBalancerIP,omitempty" desc:"IP address of the load balancer"` diff --git a/install/helm/gloo/templates/43-gatewayparameters.yaml b/install/helm/gloo/templates/43-gatewayparameters.yaml index b7cb803fa48..b1583d5db69 100644 --- a/install/helm/gloo/templates/43-gatewayparameters.yaml +++ b/install/helm/gloo/templates/43-gatewayparameters.yaml @@ -31,6 +31,9 @@ spec: {{- end }}{{/* if $gg.service */}} service: type: {{ $serviceType }} + {{- with ($gg.service).externalTrafficPolicy }} + externalTrafficPolicy: {{ . }} + {{- end }} {{- with ($gg.service).extraLabels }} extraLabels: {{- toYaml . | nindent 8 }} diff --git a/install/test/k8sgateway_test.go b/install/test/k8sgateway_test.go index b5a322d4670..fdcb4e2b20c 100644 --- a/install/test/k8sgateway_test.go +++ b/install/test/k8sgateway_test.go @@ -153,6 +153,7 @@ var _ = Describe("Kubernetes Gateway API integration", func() { fmt.Sprintf("kubeGateway.gatewayParameters.glooGateway.envoyContainer.resources.limits.cpu=%s", envoyLimits["cpu"].ToUnstructured()), "kubeGateway.gatewayParameters.glooGateway.proxyDeployment.replicas=5", "kubeGateway.gatewayParameters.glooGateway.service.type=ClusterIP", + "kubeGateway.gatewayParameters.glooGateway.service.externalTrafficPolicy=Local", "kubeGateway.gatewayParameters.glooGateway.service.extraLabels.svclabel1=x", "kubeGateway.gatewayParameters.glooGateway.service.extraAnnotations.svcanno1=y", "kubeGateway.gatewayParameters.glooGateway.serviceAccount.extraLabels.label1=a", @@ -246,6 +247,7 @@ var _ = Describe("Kubernetes Gateway API integration", func() { Expect(gwpKube.GetSdsContainer().GetResources().Limits).To(matchers.ContainMapElements(sdsLimits)) Expect(*gwpKube.GetService().GetType()).To(Equal(corev1.ServiceTypeClusterIP)) + Expect(gwpKube.GetService().GetExternalTrafficPolicy()).To(HaveValue(Equal(corev1.ServiceExternalTrafficPolicyLocal))) Expect(gwpKube.GetService().GetExtraLabels()).To(matchers.ContainMapElements(map[string]string{"svclabel1": "x"})) Expect(gwpKube.GetService().GetExtraAnnotations()).To(matchers.ContainMapElements(map[string]string{"svcanno1": "y"})) @@ -282,6 +284,7 @@ var _ = Describe("Kubernetes Gateway API integration", func() { "kubeGateway.gatewayParameters.glooGateway.envoyContainer.image.pullPolicy=Always", "kubeGateway.gatewayParameters.glooGateway.proxyDeployment.replicas=5", "kubeGateway.gatewayParameters.glooGateway.service.type=ClusterIP", + "kubeGateway.gatewayParameters.glooGateway.service.externalTrafficPolicy=Local", "kubeGateway.gatewayParameters.glooGateway.service.extraLabels.svclabel1=a", "kubeGateway.gatewayParameters.glooGateway.service.extraAnnotations.svcanno1=b", "kubeGateway.gatewayParameters.glooGateway.serviceAccount.extraLabels.label1=a", @@ -324,6 +327,7 @@ var _ = Describe("Kubernetes Gateway API integration", func() { Expect(*gwpKube.GetSdsContainer().GetBootstrap().GetLogLevel()).To(Equal("debug")) Expect(*gwpKube.GetService().GetType()).To(Equal(corev1.ServiceTypeClusterIP)) + Expect(gwpKube.GetService().GetExternalTrafficPolicy()).To(HaveValue(Equal(corev1.ServiceExternalTrafficPolicyLocal))) Expect(gwpKube.GetService().GetExtraLabels()).To(matchers.ContainMapElements(map[string]string{"svclabel1": "a"})) Expect(gwpKube.GetService().GetExtraAnnotations()).To(matchers.ContainMapElements(map[string]string{"svcanno1": "b"})) diff --git a/projects/gateway2/api/v1alpha1/kube_types.go b/projects/gateway2/api/v1alpha1/kube_types.go index 64028b3e39a..a7efeaf3380 100644 --- a/projects/gateway2/api/v1alpha1/kube_types.go +++ b/projects/gateway2/api/v1alpha1/kube_types.go @@ -96,6 +96,11 @@ type Service struct { // // +kubebuilder:validation:Optional ExtraAnnotations map[string]string `json:"extraAnnotations,omitempty"` + + // External Traffic Policy on the Service object. + // + // +kubebuilder:validation:Optional + ExternalTrafficPolicy *corev1.ServiceExternalTrafficPolicy `json:"externalTrafficPolicy,omitempty"` } func (in *Service) GetType() *corev1.ServiceType { @@ -126,6 +131,13 @@ func (in *Service) GetExtraAnnotations() map[string]string { return in.ExtraAnnotations } +func (in *Service) GetExternalTrafficPolicy() *corev1.ServiceExternalTrafficPolicy { + if in == nil { + return nil + } + return in.ExternalTrafficPolicy +} + type ServiceAccount struct { // Additional labels to add to the ServiceAccount object metadata. // diff --git a/projects/gateway2/api/v1alpha1/zz_generated.deepcopy.go b/projects/gateway2/api/v1alpha1/zz_generated.deepcopy.go index 212c08e2b49..74f482b295d 100644 --- a/projects/gateway2/api/v1alpha1/zz_generated.deepcopy.go +++ b/projects/gateway2/api/v1alpha1/zz_generated.deepcopy.go @@ -782,6 +782,11 @@ func (in *Service) DeepCopyInto(out *Service) { (*out)[key] = val } } + if in.ExternalTrafficPolicy != nil { + in, out := &in.ExternalTrafficPolicy, &out.ExternalTrafficPolicy + *out = new(v1.ServiceExternalTrafficPolicy) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Service. diff --git a/projects/gateway2/deployer/deployer_test.go b/projects/gateway2/deployer/deployer_test.go index b898de16a9c..10f5f56562d 100644 --- a/projects/gateway2/deployer/deployer_test.go +++ b/projects/gateway2/deployer/deployer_test.go @@ -216,6 +216,7 @@ var _ = Describe("Deployer", func() { ExtraAnnotations: map[string]string{ "foo": "bar", }, + ExternalTrafficPolicy: ptr.To(corev1.ServiceExternalTrafficPolicyCluster), }, ServiceAccount: &gw2_v1alpha1.ServiceAccount{ ExtraLabels: map[string]string{ @@ -598,6 +599,7 @@ var _ = Describe("Deployer", func() { ExtraAnnotations: map[string]string{ "override-foo": "override-bar", }, + ExternalTrafficPolicy: ptr.To(corev1.ServiceExternalTrafficPolicyLocal), }, ServiceAccount: &gw2_v1alpha1.ServiceAccount{ ExtraLabels: map[string]string{ @@ -665,6 +667,7 @@ var _ = Describe("Deployer", func() { "foo": "bar", "override-foo": "override-bar", }, + ExternalTrafficPolicy: ptr.To(corev1.ServiceExternalTrafficPolicyLocal), }, ServiceAccount: &gw2_v1alpha1.ServiceAccount{ ExtraLabels: map[string]string{ @@ -832,6 +835,7 @@ var _ = Describe("Deployer", func() { Expect(svc.GetLabels()).To(matchers.ContainMapElements(expectedGwp.Service.ExtraLabels)) Expect(svc.Spec.Type).To(Equal(*expectedGwp.Service.Type)) Expect(svc.Spec.ClusterIP).To(Equal(*expectedGwp.Service.ClusterIP)) + Expect(svc.Spec.ExternalTrafficPolicy).To(Equal(*expectedGwp.Service.ExternalTrafficPolicy)) sa := objs.findServiceAccount(defaultNamespace, defaultServiceAccountName) Expect(sa).ToNot(BeNil()) @@ -948,6 +952,7 @@ var _ = Describe("Deployer", func() { Expect(svc.GetLabels()).To(matchers.ContainMapElements(expectedGwp.Service.ExtraLabels)) Expect(svc.Spec.Type).To(Equal(*expectedGwp.Service.Type)) Expect(svc.Spec.ClusterIP).To(Equal(*expectedGwp.Service.ClusterIP)) + Expect(svc.Spec.ExternalTrafficPolicy).To(Equal(*expectedGwp.Service.ExternalTrafficPolicy)) sa := objs.findServiceAccount(defaultNamespace, defaultServiceAccountName) Expect(sa).ToNot(BeNil()) @@ -1552,6 +1557,7 @@ func fullyDefinedGatewayParameters(name, namespace string) *gw2_v1alpha1.Gateway ExtraLabels: map[string]string{ "service-label": "foo", }, + ExternalTrafficPolicy: ptr.To(corev1.ServiceExternalTrafficPolicyLocal), }, ServiceAccount: &gw2_v1alpha1.ServiceAccount{ ExtraLabels: map[string]string{ diff --git a/projects/gateway2/deployer/merge.go b/projects/gateway2/deployer/merge.go index 608c0b36e7b..ee8c210ebda 100644 --- a/projects/gateway2/deployer/merge.go +++ b/projects/gateway2/deployer/merge.go @@ -443,6 +443,10 @@ func deepMergeService(dst, src *v1alpha1.Service) *v1alpha1.Service { dst.ExtraLabels = deepMergeMaps(dst.GetExtraLabels(), src.GetExtraLabels()) dst.ExtraAnnotations = deepMergeMaps(dst.GetExtraAnnotations(), src.GetExtraAnnotations()) + if src.GetExternalTrafficPolicy() != nil { + dst.ExternalTrafficPolicy = src.GetExternalTrafficPolicy() + } + return dst } diff --git a/projects/gateway2/deployer/merge_test.go b/projects/gateway2/deployer/merge_test.go index 0c44ba65f66..648b9865983 100644 --- a/projects/gateway2/deployer/merge_test.go +++ b/projects/gateway2/deployer/merge_test.go @@ -4,6 +4,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" gw2_v1alpha1 "github.com/solo-io/gloo/projects/gateway2/api/v1alpha1" + corev1 "k8s.io/api/core/v1" "k8s.io/utils/ptr" ) @@ -134,4 +135,29 @@ var _ = Describe("deepMergeGatewayParameters", func() { Expect(out.Spec.Kube.ServiceAccount.ExtraLabels).To(Equal(expectedMap)) Expect(out.Spec.Kube.ServiceAccount.ExtraAnnotations).To(Equal(expectedMap)) }) + + It("merges service strings", func() { + + dst := &gw2_v1alpha1.GatewayParameters{ + Spec: gw2_v1alpha1.GatewayParametersSpec{ + Kube: &gw2_v1alpha1.KubernetesProxyConfig{ + Service: &gw2_v1alpha1.Service{ + ExternalTrafficPolicy: ptr.To(corev1.ServiceExternalTrafficPolicyLocal), + }, + }, + }, + } + src := &gw2_v1alpha1.GatewayParameters{ + Spec: gw2_v1alpha1.GatewayParametersSpec{ + Kube: &gw2_v1alpha1.KubernetesProxyConfig{ + Service: &gw2_v1alpha1.Service{ + ExternalTrafficPolicy: ptr.To(corev1.ServiceExternalTrafficPolicyCluster), + }, + }, + }, + } + + out := deepMergeGatewayParameters(dst, src) + Expect(out.Spec.Kube.Service.ExternalTrafficPolicy).To(Equal(ptr.To(corev1.ServiceExternalTrafficPolicyCluster))) + }) }) diff --git a/projects/gateway2/deployer/values.go b/projects/gateway2/deployer/values.go index 4fc0f2a6621..06fc99b960e 100644 --- a/projects/gateway2/deployer/values.go +++ b/projects/gateway2/deployer/values.go @@ -85,10 +85,11 @@ type helmImage struct { } type helmService struct { - Type *string `json:"type,omitempty"` - ClusterIP *string `json:"clusterIP,omitempty"` - ExtraAnnotations map[string]string `json:"extraAnnotations,omitempty"` - ExtraLabels map[string]string `json:"extraLabels,omitempty"` + Type *string `json:"type,omitempty"` + ClusterIP *string `json:"clusterIP,omitempty"` + ExtraAnnotations map[string]string `json:"extraAnnotations,omitempty"` + ExtraLabels map[string]string `json:"extraLabels,omitempty"` + ExternalTrafficPolicy *string `json:"externalTrafficPolicy,omitempty"` } type helmServiceAccount struct { diff --git a/projects/gateway2/deployer/values_helpers.go b/projects/gateway2/deployer/values_helpers.go index c27423f7ce2..491cd37fc25 100644 --- a/projects/gateway2/deployer/values_helpers.go +++ b/projects/gateway2/deployer/values_helpers.go @@ -77,11 +77,16 @@ func getServiceValues(svcConfig *v1alpha1.Service) *helmService { if svcConfig.GetType() != nil { svcType = ptr.To(string(*svcConfig.GetType())) } + var svcExternalTrafficPolicy *string + if svcConfig.GetExternalTrafficPolicy() != nil { + svcExternalTrafficPolicy = ptr.To(string(*svcConfig.GetExternalTrafficPolicy())) + } return &helmService{ - Type: svcType, - ClusterIP: svcConfig.GetClusterIP(), - ExtraAnnotations: svcConfig.GetExtraAnnotations(), - ExtraLabels: svcConfig.GetExtraLabels(), + Type: svcType, + ClusterIP: svcConfig.GetClusterIP(), + ExtraAnnotations: svcConfig.GetExtraAnnotations(), + ExtraLabels: svcConfig.GetExtraLabels(), + ExternalTrafficPolicy: svcExternalTrafficPolicy, } } diff --git a/projects/gateway2/helm/gloo-gateway/templates/gateway/proxy-deployment.yaml b/projects/gateway2/helm/gloo-gateway/templates/gateway/proxy-deployment.yaml index 3637bf647ff..c92216ae755 100644 --- a/projects/gateway2/helm/gloo-gateway/templates/gateway/proxy-deployment.yaml +++ b/projects/gateway2/helm/gloo-gateway/templates/gateway/proxy-deployment.yaml @@ -380,6 +380,9 @@ metadata: {{- end }} spec: type: {{ $gateway.service.type }} + {{- if $gateway.service.externalTrafficPolicy }} + externalTrafficPolicy: {{ $gateway.service.externalTrafficPolicy }} + {{- end }} {{- with $gateway.service.clusterIP }} clusterIP: {{ . }} {{- end }} diff --git a/test/kubernetes/e2e/features/deployer/suite.go b/test/kubernetes/e2e/features/deployer/suite.go index 0d8ccb0ac93..05d1e0130cf 100644 --- a/test/kubernetes/e2e/features/deployer/suite.go +++ b/test/kubernetes/e2e/features/deployer/suite.go @@ -141,6 +141,10 @@ func (s *testingSuite) TestConfigureProxiesFromGatewayParameters() { s.testInstallation.Assertions.Gomega.Expect(svc.GetAnnotations()).To( gomega.HaveKeyWithValue("svc-anno-key", "svc-anno-val")) + // check that external traffic policy got passwed through from GatewayParameters to the Service + s.testInstallation.Assertions.Gomega.Expect(svc.Spec.ExternalTrafficPolicy).To( + gomega.Equal(corev1.ServiceExternalTrafficPolicyCluster)) + // Update the Gateway to use the custom GatewayParameters gwName := types.NamespacedName{Name: gw.Name, Namespace: gw.Namespace} err = s.testInstallation.ClusterContext.Client.Get(s.ctx, gwName, gw) @@ -181,6 +185,11 @@ func (s *testingSuite) TestProvisionResourcesUpdatedWithValidParameters() { // the GatewayParameters modification should cause the deployer to re-run and update the // deployment to have 2 replicas s.testInstallation.Assertions.EventuallyRunningReplicas(s.ctx, proxyDeployment.ObjectMeta, gomega.Equal(2)) + + // modify the external traffic policy in the GatewayParameters + s.patchGatewayParameters(gwParamsDefault.ObjectMeta, func(parameters *v1alpha1.GatewayParameters) { + parameters.Spec.Kube.Service.ExternalTrafficPolicy = ptr.To(corev1.ServiceExternalTrafficPolicyLocal) + }) } func (s *testingSuite) TestProvisionResourcesNotUpdatedWithInvalidParameters() { From 64df728fe3110b151fa7557110ed33ebdce98198 Mon Sep 17 00:00:00 2001 From: Kevin Shelaga Date: Tue, 7 Jan 2025 06:47:59 -0500 Subject: [PATCH 02/10] remove wrong changelog --- changelog/v1.18.1/external-traffic-policy.yaml | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 changelog/v1.18.1/external-traffic-policy.yaml diff --git a/changelog/v1.18.1/external-traffic-policy.yaml b/changelog/v1.18.1/external-traffic-policy.yaml deleted file mode 100644 index 557ca0a4fd0..00000000000 --- a/changelog/v1.18.1/external-traffic-policy.yaml +++ /dev/null @@ -1,6 +0,0 @@ -changelog: - - type: NEW_FEATURE - issueLink: https://github.com/k8sgateway/k8sgateway/issues/9879 - resolvesIssue: true - description: >- - Add ability to configure proxy service External Traffic Policy via Gateway Params \ No newline at end of file From 1c1735fb83ebf0a0117a3de1bdcc6c4e3aaffe04 Mon Sep 17 00:00:00 2001 From: Kevin Shelaga <65769106+kevin-shelaga@users.noreply.github.com> Date: Tue, 7 Jan 2025 20:56:57 -0500 Subject: [PATCH 03/10] Update changelog/v1.18.4/external-traffic-policy.yaml Co-authored-by: Jenny Shu <28537278+jenshu@users.noreply.github.com> --- changelog/v1.18.4/external-traffic-policy.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/v1.18.4/external-traffic-policy.yaml b/changelog/v1.18.4/external-traffic-policy.yaml index 557ca0a4fd0..8c97b371686 100644 --- a/changelog/v1.18.4/external-traffic-policy.yaml +++ b/changelog/v1.18.4/external-traffic-policy.yaml @@ -1,5 +1,5 @@ changelog: - - type: NEW_FEATURE + - type: FIX issueLink: https://github.com/k8sgateway/k8sgateway/issues/9879 resolvesIssue: true description: >- From 5ea47f2d1cf83461791820d962051ac0a1cde235 Mon Sep 17 00:00:00 2001 From: Kevin Shelaga Date: Wed, 8 Jan 2025 06:11:13 -0500 Subject: [PATCH 04/10] fix tests --- pkg/utils/kubeutils/service.go | 26 +++++++++++++++++++ .../kubernetes/e2e/features/deployer/suite.go | 5 ++++ .../testutils/assertions/service.go | 26 +++++++++++++++++++ 3 files changed, 57 insertions(+) create mode 100644 pkg/utils/kubeutils/service.go create mode 100644 test/kubernetes/testutils/assertions/service.go diff --git a/pkg/utils/kubeutils/service.go b/pkg/utils/kubeutils/service.go new file mode 100644 index 00000000000..fc901757416 --- /dev/null +++ b/pkg/utils/kubeutils/service.go @@ -0,0 +1,26 @@ +package kubeutils + +import ( + "context" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/kubernetes" +) + +// GetPodsForService gets all pods backing a deployment +func GetService( + ctx context.Context, + kubeClient *kubernetes.Clientset, + serviceName string, + serviceNamespace string, +) (*corev1.Service, error) { + + service, err := kubeClient.CoreV1().Services(serviceNamespace).Get(ctx, serviceName, metav1.GetOptions{}) + if err != nil { + return nil, err + } + + return service, nil +} diff --git a/test/kubernetes/e2e/features/deployer/suite.go b/test/kubernetes/e2e/features/deployer/suite.go index 05d1e0130cf..3d9cdbe628d 100644 --- a/test/kubernetes/e2e/features/deployer/suite.go +++ b/test/kubernetes/e2e/features/deployer/suite.go @@ -190,6 +190,11 @@ func (s *testingSuite) TestProvisionResourcesUpdatedWithValidParameters() { s.patchGatewayParameters(gwParamsDefault.ObjectMeta, func(parameters *v1alpha1.GatewayParameters) { parameters.Spec.Kube.Service.ExternalTrafficPolicy = ptr.To(corev1.ServiceExternalTrafficPolicyLocal) }) + + // the GatewayParameters modification should cause the deployer to re-run and update the + // service to have ExternalTrafficPolicy = Local + s.testInstallation.Assertions.ExternalTrafficPolicy(s.ctx, *proxyService, gomega.Equal(corev1.ServiceExternalTrafficPolicyLocal)) + } func (s *testingSuite) TestProvisionResourcesNotUpdatedWithInvalidParameters() { diff --git a/test/kubernetes/testutils/assertions/service.go b/test/kubernetes/testutils/assertions/service.go new file mode 100644 index 00000000000..25ecf5e2cc5 --- /dev/null +++ b/test/kubernetes/testutils/assertions/service.go @@ -0,0 +1,26 @@ +package assertions + +import ( + "context" + "time" + + . "github.com/onsi/gomega" + "github.com/onsi/gomega/types" + "github.com/solo-io/gloo/pkg/utils/kubeutils" + corev1 "k8s.io/api/core/v1" +) + +func (p *Provider) ExternalTrafficPolicy(ctx context.Context, service corev1.Service, externalTrafficPolicyMatcher types.GomegaMatcher) { + p.Gomega.Eventually(func(innerG Gomega) { + // We intentionally rely only on Pods that have marked themselves as ready as a way of defining more explicit assertions + service, err := kubeutils.GetService(ctx, p.clusterContext.Clientset, service.Name, service.Namespace) + innerG.Expect(err).NotTo(HaveOccurred(), "can get service") + innerG.Expect(service.Spec.ExternalTrafficPolicy).To(externalTrafficPolicyMatcher, "externalTrafficPolicy to match") + }). + WithContext(ctx). + // It may take some time for pods to initialize and pull images from remote registries. + // Therefore, we set a longer timeout, to account for latency that may exist. + WithTimeout(time.Second * 30). + WithPolling(time.Millisecond * 200). + Should(Succeed()) +} From 8967d10864ce232b2c4af8bb6a3ce5816a9d9b32 Mon Sep 17 00:00:00 2001 From: Shashank Ram <21697719+shashankram@users.noreply.github.com> Date: Tue, 7 Jan 2025 07:31:32 -0800 Subject: [PATCH 05/10] [backport] gateway2: allow child policies to always set fields unset on the parent (#10550) --- changelog/v1.18.4/policy-prefixrewrite.yaml | 14 +++++++ .../routeoptions/route_options_plugin_test.go | 38 ++++++++++++++++++- ...ti_level_inheritance_override_partial.yaml | 6 +++ ...ti_level_inheritance_override_partial.yaml | 1 + projects/gloo/pkg/utils/merge.go | 5 ++- 5 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 changelog/v1.18.4/policy-prefixrewrite.yaml diff --git a/changelog/v1.18.4/policy-prefixrewrite.yaml b/changelog/v1.18.4/policy-prefixrewrite.yaml new file mode 100644 index 00000000000..66757a2d04f --- /dev/null +++ b/changelog/v1.18.4/policy-prefixrewrite.yaml @@ -0,0 +1,14 @@ +changelog: + - type: FIX + issueLink: https://github.com/solo-io/solo-projects/issues/7601 + resolvesIssue: false + description: | + When merging parent-child policies, the merging should allow child + policies to augment parent policies such that fields unset on the + parent can be set by the child. There is a bug when using policy + override capability with route delegation that disallows this when + the annotation specifies non-wildcard fields, such that even if + a field is unset by the parent only the fields specified in the + override annotation are merged in - which is incorrect because + the annotation only applies to fields that are being overriden + (set by the parent). This change fixes the bug. diff --git a/projects/gateway2/translator/plugins/routeoptions/route_options_plugin_test.go b/projects/gateway2/translator/plugins/routeoptions/route_options_plugin_test.go index a0497cb53e4..85172b5ccf2 100644 --- a/projects/gateway2/translator/plugins/routeoptions/route_options_plugin_test.go +++ b/projects/gateway2/translator/plugins/routeoptions/route_options_plugin_test.go @@ -5,9 +5,11 @@ import ( "errors" "time" + "github.com/google/go-cmp/cmp" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/testing/protocmp" "google.golang.org/protobuf/types/known/durationpb" "google.golang.org/protobuf/types/known/wrapperspb" "istio.io/istio/pkg/kube/krt" @@ -737,7 +739,7 @@ var _ = Describe("RouteOptionsPlugin", func() { var _ = DescribeTable("mergeOptionsForRoute", func(route *gwv1.HTTPRoute, dst, src *v1.RouteOptions, expectedOptions *v1.RouteOptions, expectedResult glooutils.OptionsMergeResult) { mergedOptions, result := mergeOptionsForRoute(context.TODO(), route, dst, src) - Expect(proto.Equal(mergedOptions, expectedOptions)).To(BeTrue()) + Expect(cmp.Diff(mergedOptions, expectedOptions, protocmp.Transform())).To(BeEmpty()) Expect(result).To(Equal(expectedResult)) }, Entry("prefer dst options by default", @@ -890,6 +892,40 @@ var _ = DescribeTable("mergeOptionsForRoute", }, glooutils.OptionsMergedFull, ), + Entry("override and augment dst options with annotation: specific fields", + &gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{policyOverrideAnnotation: "faults,timeout"}, + }, + }, + &v1.RouteOptions{ + Faults: &faultinjection.RouteFaults{ + Abort: &faultinjection.RouteAbort{ + HttpStatus: 500, + }, + }, + Timeout: durationpb.New(5 * time.Second), + }, + &v1.RouteOptions{ + PrefixRewrite: &wrapperspb.StringValue{Value: "/src"}, + Faults: &faultinjection.RouteFaults{ + Abort: &faultinjection.RouteAbort{ + HttpStatus: 100, + }, + }, + Timeout: durationpb.New(10 * time.Second), + }, + &v1.RouteOptions{ + Faults: &faultinjection.RouteFaults{ + Abort: &faultinjection.RouteAbort{ + HttpStatus: 100, + }, + }, + PrefixRewrite: &wrapperspb.StringValue{Value: "/src"}, + Timeout: durationpb.New(10 * time.Second), + }, + glooutils.OptionsMergedFull, + ), ) func routeOption() *solokubev1.RouteOption { diff --git a/projects/gateway2/translator/testutils/inputs/delegation/route_options_multi_level_inheritance_override_partial.yaml b/projects/gateway2/translator/testutils/inputs/delegation/route_options_multi_level_inheritance_override_partial.yaml index badfc3480a2..8070a110334 100644 --- a/projects/gateway2/translator/testutils/inputs/delegation/route_options_multi_level_inheritance_override_partial.yaml +++ b/projects/gateway2/translator/testutils/inputs/delegation/route_options_multi_level_inheritance_override_partial.yaml @@ -112,6 +112,12 @@ spec: - path: type: PathPrefix value: /a/1 + filters: + - type: URLRewrite + urlRewrite: + path: + replacePrefixMatch: /rewrite/path + type: ReplacePrefixMatch backendRefs: - name: svc-a port: 8080 diff --git a/projects/gateway2/translator/testutils/outputs/delegation/route_options_multi_level_inheritance_override_partial.yaml b/projects/gateway2/translator/testutils/outputs/delegation/route_options_multi_level_inheritance_override_partial.yaml index ff5dd730ac6..e68e2599148 100644 --- a/projects/gateway2/translator/testutils/outputs/delegation/route_options_multi_level_inheritance_override_partial.yaml +++ b/projects/gateway2/translator/testutils/outputs/delegation/route_options_multi_level_inheritance_override_partial.yaml @@ -37,6 +37,7 @@ listeners: headerManipulation: requestHeadersToRemove: - override + prefixRewrite: /rewrite/path name: httproute-route-a-a-0-0 routeAction: single: diff --git a/projects/gloo/pkg/utils/merge.go b/projects/gloo/pkg/utils/merge.go index 6cbedab218d..132ff400e03 100644 --- a/projects/gloo/pkg/utils/merge.go +++ b/projects/gloo/pkg/utils/merge.go @@ -147,7 +147,10 @@ func MergeRouteOptionsWithOverrides(dst, src *v1.RouteOptions, allowedOverrides var dstFieldsOverwritten int for i := range dstValue.NumField() { dstField, srcField := dstValue.Field(i), srcValue.Field(i) - if overwriteByDefault && !(allowedOverrides.Has(wildcardField) || + // Allow overrides if the field in dst is unset as merging from src into dst by default + // allows src to augment dst by setting fields unset in dst, hence the override check only + // applies when the field in dst is set: !isEmptyValue(dstField). + if !isEmptyValue(dstField) && overwriteByDefault && !(allowedOverrides.Has(wildcardField) || allowedOverrides.Has(strings.ToLower(dstValue.Type().Field(i).Name))) { continue } From 5d09f575a13efb594f045311459980249a1b2328 Mon Sep 17 00:00:00 2001 From: Kevin Shelaga Date: Wed, 8 Jan 2025 06:24:03 -0500 Subject: [PATCH 06/10] comments fix --- pkg/utils/kubeutils/service.go | 2 +- test/kubernetes/testutils/assertions/service.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/utils/kubeutils/service.go b/pkg/utils/kubeutils/service.go index fc901757416..7367ec09d98 100644 --- a/pkg/utils/kubeutils/service.go +++ b/pkg/utils/kubeutils/service.go @@ -9,7 +9,7 @@ import ( "k8s.io/client-go/kubernetes" ) -// GetPodsForService gets all pods backing a deployment +// GetService gets the service from the provided name/namespace func GetService( ctx context.Context, kubeClient *kubernetes.Clientset, diff --git a/test/kubernetes/testutils/assertions/service.go b/test/kubernetes/testutils/assertions/service.go index 25ecf5e2cc5..85ee85ed3c8 100644 --- a/test/kubernetes/testutils/assertions/service.go +++ b/test/kubernetes/testutils/assertions/service.go @@ -12,7 +12,6 @@ import ( func (p *Provider) ExternalTrafficPolicy(ctx context.Context, service corev1.Service, externalTrafficPolicyMatcher types.GomegaMatcher) { p.Gomega.Eventually(func(innerG Gomega) { - // We intentionally rely only on Pods that have marked themselves as ready as a way of defining more explicit assertions service, err := kubeutils.GetService(ctx, p.clusterContext.Clientset, service.Name, service.Namespace) innerG.Expect(err).NotTo(HaveOccurred(), "can get service") innerG.Expect(service.Spec.ExternalTrafficPolicy).To(externalTrafficPolicyMatcher, "externalTrafficPolicy to match") From f9f7e513cd3f349da874c674a34c9034a32c7fc9 Mon Sep 17 00:00:00 2001 From: Kevin Shelaga <65769106+kevin-shelaga@users.noreply.github.com> Date: Wed, 8 Jan 2025 12:21:00 -0500 Subject: [PATCH 07/10] Update service.go Co-authored-by: Jenny Shu <28537278+jenshu@users.noreply.github.com> --- test/kubernetes/testutils/assertions/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/kubernetes/testutils/assertions/service.go b/test/kubernetes/testutils/assertions/service.go index 85ee85ed3c8..ef97cc0725e 100644 --- a/test/kubernetes/testutils/assertions/service.go +++ b/test/kubernetes/testutils/assertions/service.go @@ -10,7 +10,7 @@ import ( corev1 "k8s.io/api/core/v1" ) -func (p *Provider) ExternalTrafficPolicy(ctx context.Context, service corev1.Service, externalTrafficPolicyMatcher types.GomegaMatcher) { +func (p *Provider) EventuallyExternalTrafficPolicy(ctx context.Context, service corev1.Service, externalTrafficPolicyMatcher types.GomegaMatcher) { p.Gomega.Eventually(func(innerG Gomega) { service, err := kubeutils.GetService(ctx, p.clusterContext.Clientset, service.Name, service.Namespace) innerG.Expect(err).NotTo(HaveOccurred(), "can get service") From 3a5a4edf401b3c12fe758f5f24f96b8f08490f46 Mon Sep 17 00:00:00 2001 From: Shashank Ram <21697719+shashankram@users.noreply.github.com> Date: Wed, 8 Jan 2025 12:02:50 -0800 Subject: [PATCH 08/10] [backport] gateway2: use safer merging to avoid assuming zero values as being unset (#10559) Signed-off-by: Shashank Ram --- changelog/v1.18.4/merge-check.yaml | 17 +++++++ .../routeoptions/route_options_plugin_test.go | 2 +- .../gloo/pkg/translator/ssl_configuration.go | 2 +- projects/gloo/pkg/utils/merge.go | 48 ++++++++++++------- 4 files changed, 51 insertions(+), 18 deletions(-) create mode 100644 changelog/v1.18.4/merge-check.yaml diff --git a/changelog/v1.18.4/merge-check.yaml b/changelog/v1.18.4/merge-check.yaml new file mode 100644 index 00000000000..6172a013307 --- /dev/null +++ b/changelog/v1.18.4/merge-check.yaml @@ -0,0 +1,17 @@ +changelog: + - type: NON_USER_FACING + resolvesIssue: false + description: | + gateway2: use safer merging to avoid assuming zero values as being unset + + The legacy Edge code uses ShallowMerge() which can undesirably overwrite + zero values mistaking them for unset values. RouteOptions merging in + GatewayV2 uses the same API, but this can result in undesirable effects + if the merging considers zero valued fields as being unset. To avoid + this, the options merging used by GatewayV2 relies on a safer merge + that only allows merging of values that can be set to Nil (pointers, + slices, maps, etc.) which works since all user-facing fields on the + RouteOptions are nil-able. Functionally, this is the same as before + due to all fields being nil-able, but is a bit clearer to readers. + Moreover, trying to merge a non-nil field will panic which can catch + potential misuse of the API. diff --git a/projects/gateway2/translator/plugins/routeoptions/route_options_plugin_test.go b/projects/gateway2/translator/plugins/routeoptions/route_options_plugin_test.go index 85172b5ccf2..14269cb65e0 100644 --- a/projects/gateway2/translator/plugins/routeoptions/route_options_plugin_test.go +++ b/projects/gateway2/translator/plugins/routeoptions/route_options_plugin_test.go @@ -890,7 +890,7 @@ var _ = DescribeTable("mergeOptionsForRoute", PrefixRewrite: &wrapperspb.StringValue{Value: "/dst"}, Timeout: durationpb.New(10 * time.Second), }, - glooutils.OptionsMergedFull, + glooutils.OptionsMergedPartial, // PrefixRewrite is not overwritten ), Entry("override and augment dst options with annotation: specific fields", &gwv1.HTTPRoute{ diff --git a/projects/gloo/pkg/translator/ssl_configuration.go b/projects/gloo/pkg/translator/ssl_configuration.go index 6cec185dbb9..30c04ff64fb 100644 --- a/projects/gloo/pkg/translator/ssl_configuration.go +++ b/projects/gloo/pkg/translator/ssl_configuration.go @@ -72,7 +72,7 @@ func MergeSslConfig(dst, src *ssl.SslConfig) *ssl.SslConfig { for i := range dstValue.NumField() { dstField, srcField := dstValue.Field(i), srcValue.Field(i) - utils.ShallowMerge(dstField, srcField, false) + utils.ShallowMerge(dstField, srcField) } return dst diff --git a/projects/gloo/pkg/utils/merge.go b/projects/gloo/pkg/utils/merge.go index 132ff400e03..d129529b12a 100644 --- a/projects/gloo/pkg/utils/merge.go +++ b/projects/gloo/pkg/utils/merge.go @@ -25,14 +25,28 @@ const ( OptionsMergedFull ) -// ShallowMerge sets dst to the value of src, if src is non-zero and dst is zero-valued or overwrite=true. +// ShallowMerge sets dst to the value of src, if src is non-zero and dst is zero-valued // It returns a boolean indicating whether src overwrote dst. -func ShallowMerge(dst, src reflect.Value, overwrite bool) bool { +func ShallowMerge(dst, src reflect.Value) bool { if !src.IsValid() { return false } - if dst.CanSet() && !isEmptyValue(src) && (overwrite || isEmptyValue(dst)) { + if dst.CanSet() && !isEmptyValue(src) && isEmptyValue(dst) { + dst.Set(src) + return true + } + + return false +} + +// maySetValue sets dst to the value of src if: +// - src is set (has a non-nil value) and +// - dst is nil or overwrite is true +// +// It returns a boolean indicating whether src overwrote dst. +func maySetValue(dst, src reflect.Value, overwrite bool) bool { + if src.CanSet() && !src.IsNil() && dst.CanSet() && (overwrite || dst.IsNil()) { dst.Set(src) return true } @@ -84,7 +98,7 @@ func ShallowMergeRouteOptions(dst, src *v1.RouteOptions) (*v1.RouteOptions, bool overwrote := false for i := range dstValue.NumField() { dstField, srcField := dstValue.Field(i), srcValue.Field(i) - if srcOverride := ShallowMerge(dstField, srcField, false); srcOverride { + if srcOverride := ShallowMerge(dstField, srcField); srcOverride { overwrote = true } } @@ -110,7 +124,7 @@ func ShallowMergeVirtualHostOptions(dst, src *v1.VirtualHostOptions) (*v1.Virtua overwrote := false for i := range dstValue.NumField() { dstField, srcField := dstValue.Field(i), srcValue.Field(i) - if srcOverride := ShallowMerge(dstField, srcField, false); srcOverride { + if srcOverride := ShallowMerge(dstField, srcField); srcOverride { overwrote = true } } @@ -147,23 +161,25 @@ func MergeRouteOptionsWithOverrides(dst, src *v1.RouteOptions, allowedOverrides var dstFieldsOverwritten int for i := range dstValue.NumField() { dstField, srcField := dstValue.Field(i), srcValue.Field(i) + + // NOTE: important to pre-compute this because dstFieldsOverwritten must be + // incremented based on the original value of dstField and not the overwritten value + dstIsSet := dstField.CanSet() && !dstField.IsNil() + if dstIsSet { + dstFieldsSet++ + } + // Allow overrides if the field in dst is unset as merging from src into dst by default // allows src to augment dst by setting fields unset in dst, hence the override check only - // applies when the field in dst is set: !isEmptyValue(dstField). - if !isEmptyValue(dstField) && overwriteByDefault && !(allowedOverrides.Has(wildcardField) || + // applies when the field in dst is set (dstIsSet=true). + if dstIsSet && overwriteByDefault && !(allowedOverrides.Has(wildcardField) || allowedOverrides.Has(strings.ToLower(dstValue.Type().Field(i).Name))) { continue } - // NOTE: important to pre-compute this for use in the conditional below - // because dstFieldsOverwritten needs to be incremented based on the original value of dstField - // and not the state of the field after the merge - dstOverridable := dstField.CanSet() && !isEmptyValue(dstField) - if dstOverridable { - dstFieldsSet++ - } - if srcOverride := ShallowMerge(dstField, srcField, overwriteByDefault); srcOverride { + + if srcOverride := maySetValue(dstField, srcField, overwriteByDefault); srcOverride { srcFieldsUsed++ - if dstOverridable { + if dstIsSet { dstFieldsOverwritten++ } } From d8439a8dda6d6392dc4f0bd6c6429276e6a741c6 Mon Sep 17 00:00:00 2001 From: Kevin Shelaga Date: Wed, 8 Jan 2025 15:55:46 -0500 Subject: [PATCH 09/10] comments fix --- test/kubernetes/testutils/assertions/service.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/kubernetes/testutils/assertions/service.go b/test/kubernetes/testutils/assertions/service.go index ef97cc0725e..d309872fe9a 100644 --- a/test/kubernetes/testutils/assertions/service.go +++ b/test/kubernetes/testutils/assertions/service.go @@ -17,8 +17,6 @@ func (p *Provider) EventuallyExternalTrafficPolicy(ctx context.Context, service innerG.Expect(service.Spec.ExternalTrafficPolicy).To(externalTrafficPolicyMatcher, "externalTrafficPolicy to match") }). WithContext(ctx). - // It may take some time for pods to initialize and pull images from remote registries. - // Therefore, we set a longer timeout, to account for latency that may exist. WithTimeout(time.Second * 30). WithPolling(time.Millisecond * 200). Should(Succeed()) From 1719fb92a101d38093b44ed5ed7a17dab8a953e9 Mon Sep 17 00:00:00 2001 From: Kevin Shelaga Date: Wed, 8 Jan 2025 16:14:01 -0500 Subject: [PATCH 10/10] fix assert call --- test/kubernetes/e2e/features/deployer/suite.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/kubernetes/e2e/features/deployer/suite.go b/test/kubernetes/e2e/features/deployer/suite.go index 3d9cdbe628d..57c79ea2388 100644 --- a/test/kubernetes/e2e/features/deployer/suite.go +++ b/test/kubernetes/e2e/features/deployer/suite.go @@ -193,7 +193,7 @@ func (s *testingSuite) TestProvisionResourcesUpdatedWithValidParameters() { // the GatewayParameters modification should cause the deployer to re-run and update the // service to have ExternalTrafficPolicy = Local - s.testInstallation.Assertions.ExternalTrafficPolicy(s.ctx, *proxyService, gomega.Equal(corev1.ServiceExternalTrafficPolicyLocal)) + s.testInstallation.Assertions.EventuallyExternalTrafficPolicy(s.ctx, *proxyService, gomega.Equal(corev1.ServiceExternalTrafficPolicyLocal)) }