Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: set DataPlane's ExternalTrafficPolicy on updates and patches #750

Merged
merged 2 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@
[#710](https://github.com/Kong/gateway-operator/pull/710)
- Do not reconcile Gateways nor assign any finalizers when the referred GatewayClass is not supported.
[#711](https://github.com/Kong/gateway-operator/pull/711)
- Fixed setting `ExternalTrafficPolicy` on `DataPlane`'s ingress `Service during update and patch operations.
pmalek marked this conversation as resolved.
Show resolved Hide resolved
[#750](https://github.com/Kong/gateway-operator/pull/750)

### Changes

Expand Down
4 changes: 4 additions & 0 deletions controller/dataplane/owned_resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,10 @@ func ensureIngressServiceForDataPlane(
existingService.Spec.Type = generatedService.Spec.Type
updated = true
}
if existingService.Spec.ExternalTrafficPolicy != generatedService.Spec.ExternalTrafficPolicy {
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
Expand Down
4 changes: 4 additions & 0 deletions controller/konnect/ops/ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ func Create[

case *configurationv1alpha1.KongRoute:
err = createRoute(ctx, sdk.GetRoutesSDK(), ent)

// TODO: modify the create* operation wrappers to not set Programmed conditions and return
// a KonnectEntityCreatedButRelationsFailedError if the entity was created but its relations assignment failed.

case *configurationv1.KongConsumer:
err = createConsumer(ctx, sdk.GetConsumersSDK(), sdk.GetConsumerGroupsSDK(), cl, ent)
case *configurationv1beta1.KongConsumerGroup:
Expand Down
20 changes: 18 additions & 2 deletions pkg/utils/test/predicates.go
Original file line number Diff line number Diff line change
Expand Up @@ -431,10 +431,26 @@ func DataPlaneHasNReadyPods(t *testing.T, ctx context.Context, dataplaneName typ
// DataPlaneHasService is a helper function for tests that returns a function
// that can be used to check if a DataPlane has a service created.
// Should be used in conjunction with require.Eventually or assert.Eventually.
func DataPlaneHasService(t *testing.T, ctx context.Context, dataplaneName types.NamespacedName, clients K8sClients, matchingLabels client.MatchingLabels) func() bool {
func DataPlaneHasService(
t *testing.T,
ctx context.Context,
dataplaneName types.NamespacedName,
clients K8sClients,
matchingLabels client.MatchingLabels,
asserts ...func(corev1.Service) bool,
) func() bool {
return DataPlanePredicate(t, ctx, dataplaneName, func(dataplane *operatorv1beta1.DataPlane) bool {
services := MustListDataPlaneServices(t, ctx, dataplane, clients.MgrClient, matchingLabels)
return len(services) == 1
if len(services) != 1 {
return false
}
for _, a := range asserts {
if !a(services[0]) {
return false
}
}

return true
}, clients.OperatorClient)
}

Expand Down
90 changes: 90 additions & 0 deletions test/integration/test_dataplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -914,3 +914,93 @@ func TestDataPlanePodDisruptionBudget(t *testing.T) {
assert.True(t, k8serrors.IsNotFound(err))
}, time.Minute, time.Second)
}

func TestDataPlaneServiceExternalTrafficPolicy(t *testing.T) {
t.Parallel()
namespace, cleaner := helpers.SetupTestEnv(t, GetCtx(), GetEnv())

t.Log("deploying DataPlane resource with 2 replicas")
dataplaneName := types.NamespacedName{
Namespace: namespace.Name,
Name: uuid.NewString(),
}
dataplane := &operatorv1beta1.DataPlane{
ObjectMeta: metav1.ObjectMeta{
Namespace: dataplaneName.Namespace,
Name: dataplaneName.Name,
},
Spec: operatorv1beta1.DataPlaneSpec{
DataPlaneOptions: operatorv1beta1.DataPlaneOptions{
Deployment: operatorv1beta1.DataPlaneDeploymentOptions{
DeploymentOptions: operatorv1beta1.DeploymentOptions{
PodTemplateSpec: &corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Name: consts.DataPlaneProxyContainerName,
Image: helpers.GetDefaultDataPlaneImage(),
},
},
},
},
},
},
},
},
}

dataplaneClient := GetClients().OperatorClient.ApisV1beta1().DataPlanes(namespace.Name)

dataplane, err := dataplaneClient.Create(GetCtx(), dataplane, metav1.CreateOptions{})
require.NoError(t, err)
cleaner.Add(dataplane)

t.Log("verifying DataPlane gets marked provisioned")
require.Eventually(t, testutils.DataPlaneIsReady(t, GetCtx(), dataplaneName, GetClients().OperatorClient), time.Minute, time.Second)

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)

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)

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)

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)
}
1 change: 1 addition & 0 deletions test/integration/zz_generated_registered_testcases.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading