diff --git a/pkg/controller/common/service_control.go b/pkg/controller/common/service_control.go index 22a12675ef..70422e14f2 100644 --- a/pkg/controller/common/service_control.go +++ b/pkg/controller/common/service_control.go @@ -6,7 +6,6 @@ package common import ( "context" - "net" "reflect" "go.elastic.co/apm" @@ -90,20 +89,17 @@ func applyServerSideValues(expected, reconciled *corev1.Service) { if expected.Spec.Type == "" { expected.Spec.Type = reconciled.Spec.Type } - // ClusterIP might not exist in the expected service, + // ClusterIPs might not exist in the expected service, // but might have been set after creation by k8s on the actual resource. // In such case, we want to use these values for comparison. // But only if we are not changing the type of service and the api server has assigned an IP - if expected.Spec.Type == reconciled.Spec.Type && expected.Spec.ClusterIP == "" && net.ParseIP(reconciled.Spec.ClusterIP) != nil { - expected.Spec.ClusterIP = reconciled.Spec.ClusterIP - } - - // ClusterIPs also might not exist in the expected service, - // but might have been set after creation by k8s on the actual resource. - // In such case, we want to use these values for comparison. - // But only if we are not changing the type of service and the api server has assigned IPs - if expected.Spec.Type == reconciled.Spec.Type && len(expected.Spec.ClusterIPs) == 0 && validClusterIPs(reconciled.Spec.ClusterIPs) { - expected.Spec.ClusterIPs = reconciled.Spec.ClusterIPs + if expected.Spec.Type == reconciled.Spec.Type { + if expected.Spec.ClusterIP == "" { + expected.Spec.ClusterIP = reconciled.Spec.ClusterIP + } + if len(expected.Spec.ClusterIPs) == 0 { + expected.Spec.ClusterIPs = reconciled.Spec.ClusterIPs + } } // SessionAffinity may be defaulted by the api server @@ -140,15 +136,11 @@ func applyServerSideValues(expected, reconciled *corev1.Service) { if expected.Spec.IPFamilyPolicy == nil { expected.Spec.IPFamilyPolicy = reconciled.Spec.IPFamilyPolicy } -} -func validClusterIPs(clusterIPs []string) bool { - for _, ip := range clusterIPs { - if net.ParseIP(ip) == nil { - return false - } + // InternalTrafficPolicy may be defaulted by the api server starting K8S v1.22 + if expected.Spec.InternalTrafficPolicy == nil { + expected.Spec.InternalTrafficPolicy = reconciled.Spec.InternalTrafficPolicy } - return true } // hasNodePort returns for a given service type, if the service ports have a NodePort or not. diff --git a/pkg/controller/common/service_control_test.go b/pkg/controller/common/service_control_test.go index 39dafc8a70..2cccb02316 100644 --- a/pkg/controller/common/service_control_test.go +++ b/pkg/controller/common/service_control_test.go @@ -257,6 +257,9 @@ func Test_needsDelete(t *testing.T) { } func Test_applyServerSideValues(t *testing.T) { + ptr := func(policyType corev1.ServiceInternalTrafficPolicyType) *corev1.ServiceInternalTrafficPolicyType { + return &policyType + } type args struct { expected corev1.Service reconciled corev1.Service @@ -285,7 +288,7 @@ func Test_applyServerSideValues(t *testing.T) { }}, }, { - name: "Reconciled ClusterIP[s] is not used if the reconciled ClusterIP[s] are not valid IPs", + name: "Reconciled ClusterIP[s] is also used if the reconciled ClusterIP[s] are not valid IPs", args: args{ expected: corev1.Service{Spec: corev1.ServiceSpec{}}, reconciled: corev1.Service{Spec: corev1.ServiceSpec{ @@ -297,6 +300,8 @@ func Test_applyServerSideValues(t *testing.T) { }, want: corev1.Service{Spec: corev1.ServiceSpec{ Type: corev1.ServiceTypeClusterIP, + ClusterIP: "None", + ClusterIPs: []string{"None"}, SessionAffinity: corev1.ServiceAffinityClientIP, }}, }, @@ -515,6 +520,32 @@ func Test_applyServerSideValues(t *testing.T) { IPFamilies: []corev1.IPFamily{corev1.IPv6Protocol}, }}, }, + { + name: "Reconciled InternalTrafficPolicy is used if the expected one is empty", + args: args{ + expected: corev1.Service{Spec: corev1.ServiceSpec{}}, + reconciled: corev1.Service{Spec: corev1.ServiceSpec{ + InternalTrafficPolicy: ptr(corev1.ServiceInternalTrafficPolicyCluster), + }}, + }, + want: corev1.Service{Spec: corev1.ServiceSpec{ + InternalTrafficPolicy: ptr(corev1.ServiceInternalTrafficPolicyCluster), + }}, + }, + { + name: "Expected InternalTrafficPolicy is used if not empty", + args: args{ + expected: corev1.Service{Spec: corev1.ServiceSpec{ + InternalTrafficPolicy: ptr(corev1.ServiceInternalTrafficPolicyLocal), + }}, + reconciled: corev1.Service{Spec: corev1.ServiceSpec{ + InternalTrafficPolicy: ptr(corev1.ServiceInternalTrafficPolicyCluster), + }}, + }, + want: corev1.Service{Spec: corev1.ServiceSpec{ + InternalTrafficPolicy: ptr(corev1.ServiceInternalTrafficPolicyLocal), + }}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {