From 71cdf75ba94da3fa6655a2c46292a64acffb6560 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Ma=C5=82ek?= Date: Thu, 14 Nov 2024 15:59:34 +0100 Subject: [PATCH] fix(dataplane): fix setting ExternalTrafficPolicy on ingress service --- controller/dataplane/owned_resources.go | 18 +- controller/dataplane/owned_resources_test.go | 188 +++++++++++++++---- controller/pkg/builder/builder.go | 7 + test/integration/test_dataplane.go | 153 +++++++++------ 4 files changed, 264 insertions(+), 102 deletions(-) diff --git a/controller/dataplane/owned_resources.go b/controller/dataplane/owned_resources.go index 17b201c83..1f2c6e6e8 100644 --- a/controller/dataplane/owned_resources.go +++ b/controller/dataplane/owned_resources.go @@ -382,6 +382,7 @@ func ensureIngressServiceForDataPlane( if count == 1 { var updated bool existingService := &services[0] + old := existingService.DeepCopy() updated, existingService.ObjectMeta = k8sutils.EnsureObjectMetaIsUpdated(existingService.ObjectMeta, generatedService.ObjectMeta, // enforce all the annotations provided through the dataplane API func(existingMeta metav1.ObjectMeta, generatedMeta metav1.ObjectMeta) (bool, metav1.ObjectMeta) { @@ -402,11 +403,19 @@ func ensureIngressServiceForDataPlane( existingService.Spec.Type = generatedService.Spec.Type updated = true } - if generatedService.Spec.ExternalTrafficPolicy != "" && - existingService.Spec.ExternalTrafficPolicy != generatedService.Spec.ExternalTrafficPolicy { + + const ( + defaultExternalTrafficPolicy = corev1.ServiceExternalTrafficPolicyCluster + ) + // Do not update when + // - the existing service has the default value for ExternalTrafficPolicy + // - and the generated service has the default value for ExternalTrafficPolicy or is empty. + if !(existingService.Spec.ExternalTrafficPolicy == defaultExternalTrafficPolicy && + (generatedService.Spec.ExternalTrafficPolicy == "" || generatedService.Spec.ExternalTrafficPolicy == defaultExternalTrafficPolicy)) { existingService.Spec.ExternalTrafficPolicy = generatedService.Spec.ExternalTrafficPolicy updated = true } + if !cmp.Equal(existingService.Spec.Selector, generatedService.Spec.Selector) { existingService.Spec.Selector = generatedService.Spec.Selector updated = true @@ -421,10 +430,11 @@ func ensureIngressServiceForDataPlane( } if updated { - if err := cl.Update(ctx, existingService); err != nil { + res, existingService, err := patch.ApplyPatchIfNotEmpty(ctx, cl, logger, existingService, old, updated) + if err != nil { return op.Noop, existingService, fmt.Errorf("failed updating DataPlane Service %s: %w", existingService.Name, err) } - return op.Updated, existingService, nil + return res, existingService, nil } return op.Noop, existingService, nil } diff --git a/controller/dataplane/owned_resources_test.go b/controller/dataplane/owned_resources_test.go index 7be1f517f..8fca676ef 100644 --- a/controller/dataplane/owned_resources_test.go +++ b/controller/dataplane/owned_resources_test.go @@ -36,10 +36,14 @@ func TestEnsureIngressServiceForDataPlane(t *testing.T) { }{ { name: "should create a new service if service does not exist", - dataplane: builder.NewDataPlaneBuilder().WithObjectMeta(metav1.ObjectMeta{ - Namespace: "default", - Name: "dp-1", - }).WithIngressServiceType(corev1.ServiceTypeLoadBalancer).Build(), + dataplane: builder. + NewDataPlaneBuilder(). + WithObjectMeta(metav1.ObjectMeta{ + Namespace: "default", + Name: "dp-1", + }). + WithIngressServiceType(corev1.ServiceTypeLoadBalancer). + Build(), existingServiceModifier: func(t *testing.T, ctx context.Context, c client.Client, svc *corev1.Service) { require.NoError(t, dataplane.OwnedObjectPreDeleteHook(ctx, c, svc)) require.NoError(t, c.Delete(ctx, svc)) @@ -50,21 +54,29 @@ func TestEnsureIngressServiceForDataPlane(t *testing.T) { }, { name: "should not update when a service exists", - dataplane: builder.NewDataPlaneBuilder().WithObjectMeta(metav1.ObjectMeta{ - Namespace: "default", - Name: "dp-1", - }).WithIngressServiceType(corev1.ServiceTypeLoadBalancer).Build(), + dataplane: builder. + NewDataPlaneBuilder(). + WithObjectMeta(metav1.ObjectMeta{ + Namespace: "default", + Name: "dp-1", + }). + WithIngressServiceType(corev1.ServiceTypeLoadBalancer). + Build(), expectedCreatedOrUpdated: op.Noop, expectedServiceType: corev1.ServiceTypeLoadBalancer, expectedServicePorts: k8sresources.DefaultDataPlaneIngressServicePorts, }, { name: "should add annotations to existing service", - dataplane: builder.NewDataPlaneBuilder().WithObjectMeta(metav1.ObjectMeta{ - Namespace: "default", - Name: "dp-1", - }).WithIngressServiceType(corev1.ServiceTypeLoadBalancer). - WithIngressServiceAnnotations(map[string]string{"foo": "bar"}).Build(), + dataplane: builder. + NewDataPlaneBuilder(). + WithObjectMeta(metav1.ObjectMeta{ + Namespace: "default", + Name: "dp-1", + }). + WithIngressServiceType(corev1.ServiceTypeLoadBalancer). + WithIngressServiceAnnotations(map[string]string{"foo": "bar"}). + Build(), existingServiceModifier: func(t *testing.T, ctx context.Context, c client.Client, svc *corev1.Service) { svc.Annotations = nil require.NoError(t, c.Update(ctx, svc)) @@ -89,11 +101,15 @@ func TestEnsureIngressServiceForDataPlane(t *testing.T) { } require.NoError(t, c.Update(ctx, svc)) }, - dataplane: builder.NewDataPlaneBuilder().WithObjectMeta(metav1.ObjectMeta{ - Namespace: "default", - Name: "dp-1", - }).WithIngressServiceType(corev1.ServiceTypeLoadBalancer). - WithIngressServiceAnnotations(map[string]string{"foo": "bar"}).Build(), + dataplane: builder. + NewDataPlaneBuilder(). + WithObjectMeta(metav1.ObjectMeta{ + Namespace: "default", + Name: "dp-1", + }). + WithIngressServiceType(corev1.ServiceTypeLoadBalancer). + WithIngressServiceAnnotations(map[string]string{"foo": "bar"}). + Build(), expectedCreatedOrUpdated: op.Updated, expectedServiceType: corev1.ServiceTypeLoadBalancer, expectedServicePorts: k8sresources.DefaultDataPlaneIngressServicePorts, @@ -108,10 +124,14 @@ func TestEnsureIngressServiceForDataPlane(t *testing.T) { { name: "should create service when service does not contain additional labels", additionalLabels: map[string]string{"foo": "bar"}, - dataplane: builder.NewDataPlaneBuilder().WithObjectMeta(metav1.ObjectMeta{ - Namespace: "default", - Name: "dp-1", - }).WithIngressServiceType(corev1.ServiceTypeLoadBalancer).Build(), + dataplane: builder. + NewDataPlaneBuilder(). + WithObjectMeta(metav1.ObjectMeta{ + Namespace: "default", + Name: "dp-1", + }). + WithIngressServiceType(corev1.ServiceTypeLoadBalancer). + Build(), existingServiceModifier: func(t *testing.T, ctx context.Context, c client.Client, svc *corev1.Service) { if svc.Labels != nil { delete(svc.Labels, "foo") @@ -125,16 +145,21 @@ func TestEnsureIngressServiceForDataPlane(t *testing.T) { }, { name: "should update ports", - dataplane: builder.NewDataPlaneBuilder().WithObjectMeta(metav1.ObjectMeta{ - Namespace: "default", - Name: "dp-1", - }).WithIngressServiceType(corev1.ServiceTypeLoadBalancer).WithIngressServicePorts([]operatorv1beta1.DataPlaneServicePort{ - { - Name: "http", - Port: 8080, - TargetPort: intstr.FromInt(8000), - }, - }).Build(), + dataplane: builder. + NewDataPlaneBuilder(). + WithObjectMeta(metav1.ObjectMeta{ + Namespace: "default", + Name: "dp-1", + }). + WithIngressServiceType(corev1.ServiceTypeLoadBalancer). + WithIngressServicePorts([]operatorv1beta1.DataPlaneServicePort{ + { + Name: "http", + Port: 8080, + TargetPort: intstr.FromInt(8000), + }, + }). + Build(), expectedCreatedOrUpdated: op.Updated, expectedServiceType: corev1.ServiceTypeLoadBalancer, expectedServicePorts: []corev1.ServicePort{ @@ -146,6 +171,99 @@ func TestEnsureIngressServiceForDataPlane(t *testing.T) { }, }, }, + { + name: "should not need to update the service (LB) when it already has the cluster external traffic policy", + dataplane: builder. + NewDataPlaneBuilder(). + WithObjectMeta(metav1.ObjectMeta{ + Namespace: "default", + Name: "dp-1", + }). + WithIngressServiceType(corev1.ServiceTypeLoadBalancer). + Build(), + existingServiceModifier: func(t *testing.T, ctx context.Context, c client.Client, svc *corev1.Service) { + svc.Spec.ExternalTrafficPolicy = corev1.ServiceExternalTrafficPolicyTypeCluster + require.NoError(t, c.Update(ctx, svc)) + }, + expectedCreatedOrUpdated: op.Noop, + expectedServiceType: corev1.ServiceTypeLoadBalancer, + expectedServicePorts: k8sresources.DefaultDataPlaneIngressServicePorts, + }, + { + name: "should not need to update the service (LB) when it already has the cluster external traffic policy and dp spec has the same", + dataplane: builder. + NewDataPlaneBuilder(). + WithObjectMeta(metav1.ObjectMeta{ + Namespace: "default", + Name: "dp-1", + }). + WithIngressServiceType(corev1.ServiceTypeLoadBalancer). + WithIngressServiceExternalTrafficPolicy(corev1.ServiceExternalTrafficPolicyCluster). + Build(), + existingServiceModifier: func(t *testing.T, ctx context.Context, c client.Client, svc *corev1.Service) { + svc.Spec.ExternalTrafficPolicy = corev1.ServiceExternalTrafficPolicyCluster + require.NoError(t, c.Update(ctx, svc)) + }, + expectedCreatedOrUpdated: op.Noop, + expectedServiceType: corev1.ServiceTypeLoadBalancer, + expectedServicePorts: k8sresources.DefaultDataPlaneIngressServicePorts, + }, + { + name: "should update the service (LB) when it has the cluster external traffic policy and dp spec has local", + dataplane: builder. + NewDataPlaneBuilder(). + WithObjectMeta(metav1.ObjectMeta{ + Namespace: "default", + Name: "dp-1", + }). + WithIngressServiceType(corev1.ServiceTypeLoadBalancer). + WithIngressServiceExternalTrafficPolicy(corev1.ServiceExternalTrafficPolicyLocal). + Build(), + existingServiceModifier: func(t *testing.T, ctx context.Context, c client.Client, svc *corev1.Service) { + svc.Spec.ExternalTrafficPolicy = corev1.ServiceExternalTrafficPolicyCluster + require.NoError(t, c.Update(ctx, svc)) + }, + expectedCreatedOrUpdated: op.Updated, + expectedServiceType: corev1.ServiceTypeLoadBalancer, + expectedServicePorts: k8sresources.DefaultDataPlaneIngressServicePorts, + }, + { + name: "should update the service (LB) when it has the local external traffic policy and dp spec not specified it", + dataplane: builder. + NewDataPlaneBuilder(). + WithObjectMeta(metav1.ObjectMeta{ + Namespace: "default", + Name: "dp-1", + }). + WithIngressServiceType(corev1.ServiceTypeLoadBalancer). + Build(), + existingServiceModifier: func(t *testing.T, ctx context.Context, c client.Client, svc *corev1.Service) { + svc.Spec.ExternalTrafficPolicy = corev1.ServiceExternalTrafficPolicyLocal + require.NoError(t, c.Update(ctx, svc)) + }, + expectedCreatedOrUpdated: op.Updated, + expectedServiceType: corev1.ServiceTypeLoadBalancer, + expectedServicePorts: k8sresources.DefaultDataPlaneIngressServicePorts, + }, + { + name: "should not need to update the service (LB) when it has the local external traffic policy and dp spec has also local", + dataplane: builder. + NewDataPlaneBuilder(). + WithObjectMeta(metav1.ObjectMeta{ + Namespace: "default", + Name: "dp-1", + }). + WithIngressServiceType(corev1.ServiceTypeLoadBalancer). + WithIngressServiceExternalTrafficPolicy(corev1.ServiceExternalTrafficPolicyLocal). + Build(), + existingServiceModifier: func(t *testing.T, ctx context.Context, c client.Client, svc *corev1.Service) { + svc.Spec.ExternalTrafficPolicy = corev1.ServiceExternalTrafficPolicyLocal + require.NoError(t, c.Update(ctx, svc)) + }, + expectedCreatedOrUpdated: op.Noop, + expectedServiceType: corev1.ServiceTypeLoadBalancer, + expectedServicePorts: k8sresources.DefaultDataPlaneIngressServicePorts, + }, } for _, tc := range testCases { @@ -159,14 +277,12 @@ func TestEnsureIngressServiceForDataPlane(t *testing.T) { existingSvc, err := k8sresources.GenerateNewIngressServiceForDataPlane(tc.dataplane) require.NoError(t, err) k8sutils.SetOwnerForObject(existingSvc, tc.dataplane) - err = fakeClient.Create(ctx, existingSvc) - require.NoError(t, err) + require.NoError(t, fakeClient.Create(ctx, existingSvc)) if tc.existingServiceModifier != nil { tc.existingServiceModifier(t, ctx, fakeClient, existingSvc) } // create dataplane resource. - err = fakeClient.Create(ctx, tc.dataplane) - require.NoError(t, err, "should create dataplane successfully") + require.NoError(t, fakeClient.Create(ctx, tc.dataplane), "should create dataplane successfully") res, svc, err := ensureIngressServiceForDataPlane( ctx, logr.Discard(), diff --git a/controller/pkg/builder/builder.go b/controller/pkg/builder/builder.go index de59031c2..ea6367b47 100644 --- a/controller/pkg/builder/builder.go +++ b/controller/pkg/builder/builder.go @@ -57,6 +57,13 @@ func (b *testDataPlaneBuilder) WithIngressServiceType(typ corev1.ServiceType) *t return b } +// WithIngressServiceExternalTrafficPolicy sets the ExternalTrafficPolicy of the Ingress service. +func (b *testDataPlaneBuilder) WithIngressServiceExternalTrafficPolicy(typ corev1.ServiceExternalTrafficPolicyType) *testDataPlaneBuilder { + b.initIngressServiceOptions() + b.dataplane.Spec.DataPlaneOptions.Network.Services.Ingress.ExternalTrafficPolicy = typ + return b +} + // WithIngressServicePorts sets the Ports of the Ingress service. func (b *testDataPlaneBuilder) WithIngressServicePorts(ports []operatorv1beta1.DataPlaneServicePort) *testDataPlaneBuilder { b.initIngressServiceOptions() diff --git a/test/integration/test_dataplane.go b/test/integration/test_dataplane.go index a330eddfa..01b66944b 100644 --- a/test/integration/test_dataplane.go +++ b/test/integration/test_dataplane.go @@ -25,6 +25,11 @@ import ( "github.com/kong/gateway-operator/test/helpers" ) +const ( + waitTime = time.Minute + tickTime = 250 * time.Millisecond +) + func TestDataPlaneEssentials(t *testing.T) { t.Parallel() namespace, cleaner := helpers.SetupTestEnv(t, GetCtx(), GetEnv()) @@ -88,12 +93,12 @@ func TestDataPlaneEssentials(t *testing.T) { cleaner.Add(dataplane) t.Log("verifying dataplane gets marked provisioned") - require.Eventually(t, testutils.DataPlaneIsReady(t, GetCtx(), dataplaneName, GetClients().OperatorClient), time.Minute, time.Second) + require.Eventually(t, testutils.DataPlaneIsReady(t, GetCtx(), dataplaneName, GetClients().OperatorClient), waitTime, tickTime) t.Log("verifying deployments managed by the dataplane") require.Eventually(t, testutils.DataPlaneHasActiveDeployment(t, GetCtx(), dataplaneName, &appsv1.Deployment{}, client.MatchingLabels{ consts.GatewayOperatorManagedByLabel: consts.DataPlaneManagedLabelValue, - }, clients), time.Minute, time.Second) + }, clients), waitTime, tickTime) t.Logf("verifying that pod labels were set per the provided spec") require.Eventually(t, func() bool { @@ -142,7 +147,7 @@ func TestDataPlaneEssentials(t *testing.T) { require.Eventually(t, testutils.DataPlaneHasActiveService(t, GetCtx(), dataplaneName, &dataplaneIngressService, clients, client.MatchingLabels{ consts.GatewayOperatorManagedByLabel: consts.DataPlaneManagedLabelValue, consts.DataPlaneServiceTypeLabel: string(consts.DataPlaneIngressServiceLabelValue), - }), time.Minute, time.Second) + }), waitTime, tickTime) t.Log("verifying annotations on the proxy service managed by the dataplane") require.Equal(t, dataplaneIngressService.Annotations["foo"], "bar") @@ -156,9 +161,9 @@ func TestDataPlaneEssentials(t *testing.T) { return true } return false - }, time.Minute, time.Second) + }, waitTime, tickTime) - require.Eventually(t, Expect404WithNoRouteFunc(t, GetCtx(), "http://"+dataplaneIP), time.Minute, time.Second) + require.Eventually(t, Expect404WithNoRouteFunc(t, GetCtx(), "http://"+dataplaneIP), waitTime, tickTime) t.Log("deleting the dataplane deployment") dataplaneDeployments := testutils.MustListDataPlaneDeployments(t, GetCtx(), dataplane, clients, client.MatchingLabels{ @@ -170,7 +175,7 @@ func TestDataPlaneEssentials(t *testing.T) { t.Log("verifying deployments managed by the dataplane after deletion") require.Eventually(t, testutils.DataPlaneHasActiveDeployment(t, GetCtx(), dataplaneName, &appsv1.Deployment{}, client.MatchingLabels{ consts.GatewayOperatorManagedByLabel: consts.DataPlaneManagedLabelValue, - }, clients), time.Minute, time.Second) + }, clients), waitTime, tickTime) t.Log("deleting the dataplane service") require.NoError(t, GetClients().MgrClient.Delete(GetCtx(), &dataplaneIngressService)) @@ -179,7 +184,7 @@ func TestDataPlaneEssentials(t *testing.T) { require.Eventually(t, testutils.DataPlaneHasActiveService(t, GetCtx(), dataplaneName, &dataplaneIngressService, clients, client.MatchingLabels{ consts.GatewayOperatorManagedByLabel: consts.DataPlaneManagedLabelValue, consts.DataPlaneServiceTypeLabel: string(consts.DataPlaneIngressServiceLabelValue), - }), time.Minute, time.Second) + }), waitTime, tickTime) t.Log("verifying dataplane services receive IP addresses after deletion") require.Eventually(t, func() bool { @@ -190,19 +195,19 @@ func TestDataPlaneEssentials(t *testing.T) { return true } return false - }, time.Minute, time.Second) + }, waitTime, tickTime) - require.Eventually(t, Expect404WithNoRouteFunc(t, GetCtx(), "http://"+dataplaneIP), time.Minute, time.Second) + require.Eventually(t, Expect404WithNoRouteFunc(t, GetCtx(), "http://"+dataplaneIP), waitTime, tickTime) t.Log("verifying dataplane status is properly filled with backing service name and its addresses") - require.Eventually(t, testutils.DataPlaneHasServiceAndAddressesInStatus(t, GetCtx(), dataplaneName, clients), time.Minute, time.Second) + require.Eventually(t, testutils.DataPlaneHasServiceAndAddressesInStatus(t, GetCtx(), dataplaneName, clients), waitTime, tickTime) t.Log("updating dataplane spec with proxy service type of ClusterIP") require.Eventually(t, testutils.DataPlaneUpdateEventually(t, GetCtx(), dataplaneName, clients, func(dp *operatorv1beta1.DataPlane) { dp.Spec.Network.Services.Ingress.Type = corev1.ServiceTypeClusterIP }), - time.Minute, time.Second) + waitTime, tickTime) t.Log("checking if dataplane proxy service type changes to ClusterIP") require.Eventually(t, func() bool { @@ -218,7 +223,7 @@ func TestDataPlaneEssentials(t *testing.T) { } return true - }, time.Minute, time.Second) + }, waitTime, tickTime) } func TestDataPlaneUpdate(t *testing.T) { @@ -527,6 +532,10 @@ func TestDataPlaneHorizontalScaling(t *testing.T) { { Name: consts.DataPlaneProxyContainerName, Image: helpers.GetDefaultDataPlaneImage(), + ReadinessProbe: &corev1.Probe{ + InitialDelaySeconds: 1, + PeriodSeconds: 1, + }, }, }, }, @@ -544,24 +553,24 @@ func TestDataPlaneHorizontalScaling(t *testing.T) { cleaner.Add(dataplane) t.Log("verifying dataplane gets marked provisioned") - require.Eventually(t, testutils.DataPlaneIsReady(t, GetCtx(), dataplaneName, GetClients().OperatorClient), time.Minute, time.Second) + require.Eventually(t, testutils.DataPlaneIsReady(t, GetCtx(), dataplaneName, GetClients().OperatorClient), waitTime, tickTime) t.Log("verifying deployments managed by the dataplane") deployment := &appsv1.Deployment{} require.Eventually(t, testutils.DataPlaneHasActiveDeployment(t, GetCtx(), dataplaneName, deployment, client.MatchingLabels{ consts.GatewayOperatorManagedByLabel: consts.DataPlaneManagedLabelValue, - }, clients), time.Minute, time.Second) + }, clients), waitTime, tickTime) t.Log("verifying that dataplane has indeed 2 ready replicas") - require.Eventually(t, testutils.DataPlaneHasNReadyPods(t, GetCtx(), dataplaneName, clients, 2), time.Minute, time.Second) + require.Eventually(t, testutils.DataPlaneHasNReadyPods(t, GetCtx(), dataplaneName, clients, 2), waitTime, tickTime) t.Log("changing replicas in dataplane spec to 1 should scale down the deployment back to 1") require.Eventually(t, testutils.DataPlaneUpdateEventually(t, GetCtx(), dataplaneName, clients, func(dp *operatorv1beta1.DataPlane) { dp.Spec.Deployment.Replicas = lo.ToPtr(int32(1)) }), - time.Minute, time.Second) + waitTime, tickTime) t.Log("verifying that dataplane has indeed 1 ready replica after scaling down") - require.Eventually(t, testutils.DataPlaneHasNReadyPods(t, GetCtx(), dataplaneName, clients, 1), time.Minute, time.Second) + require.Eventually(t, testutils.DataPlaneHasNReadyPods(t, GetCtx(), dataplaneName, clients, 1), waitTime, tickTime) t.Log("changing from replicas to using autoscaling should create an HPA targeting the dataplane deployment") require.Eventually(t, @@ -585,11 +594,11 @@ func TestDataPlaneHorizontalScaling(t *testing.T) { } dp.Spec.Deployment.Replicas = nil }), - time.Minute, time.Second) + waitTime, tickTime) { var hpa autoscalingv2.HorizontalPodAutoscaler - require.Eventually(t, testutils.DataPlaneHasHPA(t, GetCtx(), dataplane, &hpa, clients), time.Minute, time.Second) + require.Eventually(t, testutils.DataPlaneHasHPA(t, GetCtx(), dataplane, &hpa, clients), waitTime, tickTime) require.NotNil(t, hpa) assert.Equal(t, int32(3), hpa.Spec.MaxReplicas) require.Len(t, hpa.Spec.Metrics, 1) @@ -732,12 +741,12 @@ func TestDataPlaneVolumeMounts(t *testing.T) { cleaner.Add(dataplane) t.Log("verifying that the dataplane gets marked as Ready") - require.Eventually(t, testutils.DataPlaneIsReady(t, GetCtx(), dataplaneName, GetClients().OperatorClient), time.Minute, time.Second) + require.Eventually(t, testutils.DataPlaneIsReady(t, GetCtx(), dataplaneName, GetClients().OperatorClient), waitTime, tickTime) t.Log("verifying deployments managed by the dataplane") require.Eventually(t, testutils.DataPlaneHasActiveDeployment(t, GetCtx(), dataplaneName, &appsv1.Deployment{}, client.MatchingLabels{ consts.GatewayOperatorManagedByLabel: consts.DataPlaneManagedLabelValue, - }, clients), time.Minute, time.Second) + }, clients), waitTime, tickTime) t.Log("verifying dataplane deployment volume mounts") deployments := testutils.MustListDataPlaneDeployments(t, GetCtx(), dataplane, clients, client.MatchingLabels{ @@ -818,7 +827,7 @@ func TestDataPlaneVolumeMounts(t *testing.T) { return false } return true - }, time.Minute, time.Second) + }, waitTime, tickTime) } func TestDataPlanePodDisruptionBudget(t *testing.T) { @@ -853,6 +862,10 @@ func TestDataPlanePodDisruptionBudget(t *testing.T) { { Name: consts.DataPlaneProxyContainerName, Image: helpers.GetDefaultDataPlaneImage(), + ReadinessProbe: &corev1.Probe{ + InitialDelaySeconds: 1, + PeriodSeconds: 1, + }, }, }, }, @@ -870,20 +883,20 @@ func TestDataPlanePodDisruptionBudget(t *testing.T) { cleaner.Add(dataplane) t.Log("verifying DataPlane gets marked provisioned") - require.Eventually(t, testutils.DataPlaneIsReady(t, GetCtx(), dataplaneName, GetClients().OperatorClient), time.Minute, time.Second) + require.Eventually(t, testutils.DataPlaneIsReady(t, GetCtx(), dataplaneName, GetClients().OperatorClient), waitTime, tickTime) t.Log("verifying deployments managed by the DataPlane") deployment := &appsv1.Deployment{} require.Eventually(t, testutils.DataPlaneHasActiveDeployment(t, GetCtx(), dataplaneName, deployment, client.MatchingLabels{ consts.GatewayOperatorManagedByLabel: consts.DataPlaneManagedLabelValue, - }, clients), time.Minute, time.Second) + }, clients), waitTime, tickTime) t.Log("verifying that DataPlane has indeed 2 ready replicas") - require.Eventually(t, testutils.DataPlaneHasNReadyPods(t, GetCtx(), dataplaneName, clients, 2), time.Minute, time.Second) + require.Eventually(t, testutils.DataPlaneHasNReadyPods(t, GetCtx(), dataplaneName, clients, 2), waitTime, tickTime) t.Log("verifying that the PodDisruptionBudget is created") pdb := policyv1.PodDisruptionBudget{} - require.Eventually(t, testutils.DataPlaneHasPodDisruptionBudget(t, GetCtx(), dataplane, &pdb, clients, testutils.AnyPodDisruptionBudget()), time.Minute, time.Second) + require.Eventually(t, testutils.DataPlaneHasPodDisruptionBudget(t, GetCtx(), dataplane, &pdb, clients, testutils.AnyPodDisruptionBudget()), waitTime, tickTime) t.Log("verifying the PodDisruptionBudget status is as expected") assert.EqualValues(t, 2, pdb.Status.ExpectedPods) @@ -893,26 +906,26 @@ func TestDataPlanePodDisruptionBudget(t *testing.T) { t.Log("changing the PodDisruptionBudget spec in DataPlane") require.Eventually(t, testutils.DataPlaneUpdateEventually(t, GetCtx(), dataplaneName, clients, func(dp *operatorv1beta1.DataPlane) { dp.Spec.Resources.PodDisruptionBudget.Spec.MinAvailable = lo.ToPtr(intstr.FromInt32(2)) - }), time.Minute, time.Second) + }), waitTime, tickTime) t.Log("verifying the PodDisruptionBudget status is updated accordingly") require.Eventually(t, testutils.DataPlaneHasPodDisruptionBudget(t, GetCtx(), dataplane, &pdb, clients, func(pdb policyv1.PodDisruptionBudget) bool { return pdb.Status.ExpectedPods == int32(2) && pdb.Status.DesiredHealthy == int32(2) && pdb.Status.DisruptionsAllowed == int32(0) - }), time.Minute, time.Second) + }), waitTime, tickTime) t.Log("removing the PodDisruptionBudget spec in DataPlane") require.Eventually(t, testutils.DataPlaneUpdateEventually(t, GetCtx(), dataplaneName, clients, func(dp *operatorv1beta1.DataPlane) { dp.Spec.Resources.PodDisruptionBudget = nil - }), time.Minute, time.Second) + }), waitTime, tickTime) t.Log("verifying the PodDisruptionBudget is deleted") require.EventuallyWithT(t, func(t *assert.CollectT) { _, err := GetClients().K8sClient.PolicyV1().PodDisruptionBudgets(namespace.Name).Get(GetCtx(), pdb.Name, metav1.GetOptions{}) assert.Error(t, err) assert.True(t, k8serrors.IsNotFound(err)) - }, time.Minute, time.Second) + }, waitTime, tickTime) } func TestDataPlaneServiceExternalTrafficPolicy(t *testing.T) { @@ -939,16 +952,47 @@ func TestDataPlaneServiceExternalTrafficPolicy(t *testing.T) { { Name: consts.DataPlaneProxyContainerName, Image: helpers.GetDefaultDataPlaneImage(), + ReadinessProbe: &corev1.Probe{ + InitialDelaySeconds: 1, + PeriodSeconds: 1, + }, }, }, }, }, }, }, + Network: operatorv1beta1.DataPlaneNetworkOptions{ + Services: &operatorv1beta1.DataPlaneServices{ + Ingress: &operatorv1beta1.DataPlaneServiceOptions{ + ServiceOptions: operatorv1beta1.ServiceOptions{ + ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyLocal, + }, + }, + }, + }, }, }, } + verifyEventuallyExternalTrafficPolicy := func( + t *testing.T, + dataplaneName types.NamespacedName, + expectedPolicy corev1.ServiceExternalTrafficPolicy, + ) { + t.Helper() + + require.Eventually(t, testutils.DataPlaneHasService(t, GetCtx(), dataplaneName, clients, + client.MatchingLabels{ + consts.GatewayOperatorManagedByLabel: consts.DataPlaneManagedLabelValue, + consts.DataPlaneServiceTypeLabel: string(consts.DataPlaneIngressServiceLabelValue), + }, + func(svc corev1.Service) bool { + return svc.Spec.ExternalTrafficPolicy == expectedPolicy + }, + ), waitTime, tickTime) + } + dataplaneClient := GetClients().OperatorClient.ApisV1beta1().DataPlanes(namespace.Name) dataplane, err := dataplaneClient.Create(GetCtx(), dataplane, metav1.CreateOptions{}) @@ -956,51 +1000,36 @@ func TestDataPlaneServiceExternalTrafficPolicy(t *testing.T) { cleaner.Add(dataplane) t.Log("verifying DataPlane gets marked provisioned") - require.Eventually(t, testutils.DataPlaneIsReady(t, GetCtx(), dataplaneName, GetClients().OperatorClient), time.Minute, time.Second) + require.Eventually(t, testutils.DataPlaneIsReady(t, GetCtx(), dataplaneName, GetClients().OperatorClient), waitTime, tickTime) t.Log("verifying deployments managed by the DataPlane") deployment := &appsv1.Deployment{} require.Eventually(t, testutils.DataPlaneHasActiveDeployment(t, GetCtx(), dataplaneName, deployment, client.MatchingLabels{ consts.GatewayOperatorManagedByLabel: consts.DataPlaneManagedLabelValue, - }, clients), time.Minute, time.Second) - - t.Log("changing in ExternalTrafficPolicy to Local") - require.Eventually(t, testutils.DataPlaneUpdateEventually(t, GetCtx(), dataplaneName, clients, func(dp *operatorv1beta1.DataPlane) { - dp.Spec.Network.Services = &operatorv1beta1.DataPlaneServices{ - Ingress: &operatorv1beta1.DataPlaneServiceOptions{ - ServiceOptions: operatorv1beta1.ServiceOptions{ - ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyLocal, - }, - }, - } - }), time.Minute, time.Second) + }, clients), waitTime, tickTime) t.Log("verifying the DataPlane Service ExternalTrafficPolicy is updated to Local") - require.Eventually(t, testutils.DataPlaneHasService(t, GetCtx(), dataplaneName, clients, - client.MatchingLabels{ - consts.GatewayOperatorManagedByLabel: consts.DataPlaneManagedLabelValue, - consts.DataPlaneServiceTypeLabel: string(consts.DataPlaneIngressServiceLabelValue), - }, - func(svc corev1.Service) bool { - return svc.Spec.ExternalTrafficPolicy == corev1.ServiceExternalTrafficPolicyLocal - }, - ), time.Minute, time.Second) + verifyEventuallyExternalTrafficPolicy(t, dataplaneName, corev1.ServiceExternalTrafficPolicyLocal) t.Log("setting DataPlane Service ExternalTrafficPolicy to Cluster") require.Eventually(t, testutils.DataPlaneUpdateEventually(t, GetCtx(), dataplaneName, clients, func(dp *operatorv1beta1.DataPlane) { dp.Spec.Network.Services.Ingress.ExternalTrafficPolicy = corev1.ServiceExternalTrafficPolicyCluster }, - ), time.Minute, time.Second) + ), waitTime, tickTime) t.Log("verifying the DataPlane Service ExternalTrafficPolicy is updated to Cluster") - require.Eventually(t, testutils.DataPlaneHasService(t, GetCtx(), dataplaneName, clients, - client.MatchingLabels{ - consts.GatewayOperatorManagedByLabel: consts.DataPlaneManagedLabelValue, - consts.DataPlaneServiceTypeLabel: string(consts.DataPlaneIngressServiceLabelValue), - }, - func(svc corev1.Service) bool { - return svc.Spec.ExternalTrafficPolicy == corev1.ServiceExternalTrafficPolicyTypeCluster - }, - ), time.Minute, time.Second) + verifyEventuallyExternalTrafficPolicy(t, dataplaneName, corev1.ServiceExternalTrafficPolicyCluster) + + t.Log("changing in ExternalTrafficPolicy to Local") + require.Eventually(t, testutils.DataPlaneUpdateEventually(t, GetCtx(), dataplaneName, clients, func(dp *operatorv1beta1.DataPlane) { + dp.Spec.Network.Services = &operatorv1beta1.DataPlaneServices{ + Ingress: &operatorv1beta1.DataPlaneServiceOptions{ + ServiceOptions: operatorv1beta1.ServiceOptions{ + ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyLocal, + }, + }, + } + }), waitTime, tickTime) + verifyEventuallyExternalTrafficPolicy(t, dataplaneName, corev1.ServiceExternalTrafficPolicyLocal) }