From 0f9dff49c8ab79a7dea478435a90bed2360837bc Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Tue, 23 Sep 2025 11:43:23 +0200 Subject: [PATCH] include external traffic policy comparison into service diffing --- pkg/cluster/cluster.go | 4 ++ pkg/cluster/cluster_test.go | 77 ++++++++++++++++++++++++++++++------- 2 files changed, 67 insertions(+), 14 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index f1d9ce164..b06386f39 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -845,6 +845,10 @@ func (c *Cluster) compareServices(old, new *v1.Service) (bool, string) { return false, "new service's selector does not match the current one" } + if old.Spec.ExternalTrafficPolicy != new.Spec.ExternalTrafficPolicy { + return false, "new service's ExternalTrafficPolicy does not match the current one" + } + return true, "" } diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index 09d9df972..25f61db98 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -1341,14 +1341,21 @@ func TestCompareEnv(t *testing.T) { } } -func newService(ann map[string]string, svcT v1.ServiceType, lbSr []string) *v1.Service { +func newService( + annotations map[string]string, + svcType v1.ServiceType, + sourceRanges []string, + selector map[string]string, + policy v1.ServiceExternalTrafficPolicyType) *v1.Service { svc := &v1.Service{ Spec: v1.ServiceSpec{ - Type: svcT, - LoadBalancerSourceRanges: lbSr, + Selector: selector, + Type: svcType, + LoadBalancerSourceRanges: sourceRanges, + ExternalTrafficPolicy: policy, }, } - svc.Annotations = ann + svc.Annotations = annotations return svc } @@ -1365,13 +1372,18 @@ func TestCompareServices(t *testing.T) { }, } + defaultPolicy := v1.ServiceExternalTrafficPolicyTypeCluster + serviceWithOwnerReference := newService( map[string]string{ constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, }, v1.ServiceTypeClusterIP, - []string{"128.141.0.0/16", "137.138.0.0/16"}) + []string{"128.141.0.0/16", "137.138.0.0/16"}, + nil, + defaultPolicy, + ) ownerRef := metav1.OwnerReference{ APIVersion: "acid.zalan.do/v1", @@ -1397,14 +1409,16 @@ func TestCompareServices(t *testing.T) { constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, }, v1.ServiceTypeClusterIP, - []string{"128.141.0.0/16", "137.138.0.0/16"}), + []string{"128.141.0.0/16", "137.138.0.0/16"}, + nil, defaultPolicy), new: newService( map[string]string{ constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, }, v1.ServiceTypeClusterIP, - []string{"128.141.0.0/16", "137.138.0.0/16"}), + []string{"128.141.0.0/16", "137.138.0.0/16"}, + nil, defaultPolicy), match: true, }, { @@ -1415,14 +1429,16 @@ func TestCompareServices(t *testing.T) { constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, }, v1.ServiceTypeClusterIP, - []string{"128.141.0.0/16", "137.138.0.0/16"}), + []string{"128.141.0.0/16", "137.138.0.0/16"}, + nil, defaultPolicy), new: newService( map[string]string{ constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, }, v1.ServiceTypeLoadBalancer, - []string{"128.141.0.0/16", "137.138.0.0/16"}), + []string{"128.141.0.0/16", "137.138.0.0/16"}, + nil, defaultPolicy), match: false, reason: `new service's type "LoadBalancer" does not match the current one "ClusterIP"`, }, @@ -1434,14 +1450,16 @@ func TestCompareServices(t *testing.T) { constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, }, v1.ServiceTypeLoadBalancer, - []string{"128.141.0.0/16", "137.138.0.0/16"}), + []string{"128.141.0.0/16", "137.138.0.0/16"}, + nil, defaultPolicy), new: newService( map[string]string{ constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, }, v1.ServiceTypeLoadBalancer, - []string{"185.249.56.0/22"}), + []string{"185.249.56.0/22"}, + nil, defaultPolicy), match: false, reason: `new service's LoadBalancerSourceRange does not match the current one`, }, @@ -1453,14 +1471,16 @@ func TestCompareServices(t *testing.T) { constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, }, v1.ServiceTypeLoadBalancer, - []string{"128.141.0.0/16", "137.138.0.0/16"}), + []string{"128.141.0.0/16", "137.138.0.0/16"}, + nil, defaultPolicy), new: newService( map[string]string{ constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, }, v1.ServiceTypeLoadBalancer, - []string{}), + []string{}, + nil, defaultPolicy), match: false, reason: `new service's LoadBalancerSourceRange does not match the current one`, }, @@ -1472,10 +1492,39 @@ func TestCompareServices(t *testing.T) { constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, }, v1.ServiceTypeClusterIP, - []string{"128.141.0.0/16", "137.138.0.0/16"}), + []string{"128.141.0.0/16", "137.138.0.0/16"}, + nil, defaultPolicy), new: serviceWithOwnerReference, match: false, }, + { + about: "new service has a label selector", + current: newService( + map[string]string{}, + v1.ServiceTypeClusterIP, + []string{}, + nil, defaultPolicy), + new: newService( + map[string]string{}, + v1.ServiceTypeClusterIP, + []string{}, + map[string]string{"cluster-name": "clstr", "spilo-role": "master"}, defaultPolicy), + match: false, + }, + { + about: "services differ on external traffic policy", + current: newService( + map[string]string{}, + v1.ServiceTypeClusterIP, + []string{}, + nil, defaultPolicy), + new: newService( + map[string]string{}, + v1.ServiceTypeClusterIP, + []string{}, + nil, v1.ServiceExternalTrafficPolicyTypeLocal), + match: false, + }, } for _, tt := range tests {