diff --git a/pkg/controller/common/service_control.go b/pkg/controller/common/service_control.go index 551552b239..3c49136d8b 100644 --- a/pkg/controller/common/service_control.go +++ b/pkg/controller/common/service_control.go @@ -88,6 +88,11 @@ func needsUpdate(expected *corev1.Service, reconciled *corev1.Service) bool { // applyServerSideValues applies any default that may have been set from the reconciled version. func applyServerSideValues(expected, reconciled *corev1.Service) { + // skip if the service type changes from something different to the default ClusterIP value + if reconciled.Spec.Type != corev1.ServiceTypeClusterIP && expected.Spec.Type != reconciled.Spec.Type { + return + } + // Type may be defaulted by the api server if expected.Spec.Type == "" { expected.Spec.Type = reconciled.Spec.Type diff --git a/pkg/controller/common/service_control_test.go b/pkg/controller/common/service_control_test.go index 6404017907..0807e0fe90 100644 --- a/pkg/controller/common/service_control_test.go +++ b/pkg/controller/common/service_control_test.go @@ -317,14 +317,12 @@ func Test_applyServerSideValues(t *testing.T) { args: args{ expected: corev1.Service{Spec: corev1.ServiceSpec{}}, reconciled: corev1.Service{Spec: corev1.ServiceSpec{ - Type: corev1.ServiceTypeClusterIP, ClusterIP: "1.2.3.4", ClusterIPs: []string{"1.2.3.4"}, SessionAffinity: corev1.ServiceAffinityClientIP, }}, }, want: corev1.Service{Spec: corev1.ServiceSpec{ - Type: corev1.ServiceTypeClusterIP, ClusterIP: "1.2.3.4", ClusterIPs: []string{"1.2.3.4"}, SessionAffinity: corev1.ServiceAffinityClientIP, @@ -335,14 +333,12 @@ func Test_applyServerSideValues(t *testing.T) { args: args{ expected: corev1.Service{Spec: corev1.ServiceSpec{}}, reconciled: corev1.Service{Spec: corev1.ServiceSpec{ - Type: corev1.ServiceTypeClusterIP, ClusterIP: "None", ClusterIPs: []string{"None"}, SessionAffinity: corev1.ServiceAffinityClientIP, }}, }, want: corev1.Service{Spec: corev1.ServiceSpec{ - Type: corev1.ServiceTypeClusterIP, ClusterIP: "None", ClusterIPs: []string{"None"}, SessionAffinity: corev1.ServiceAffinityClientIP, @@ -531,14 +527,12 @@ func Test_applyServerSideValues(t *testing.T) { args: args{ expected: corev1.Service{Spec: corev1.ServiceSpec{}}, reconciled: corev1.Service{Spec: corev1.ServiceSpec{ - Type: corev1.ServiceTypeClusterIP, ClusterIP: "1.2.3.4", SessionAffinity: corev1.ServiceAffinityClientIP, IPFamilies: []corev1.IPFamily{corev1.IPv6Protocol}, }}, }, want: corev1.Service{Spec: corev1.ServiceSpec{ - Type: corev1.ServiceTypeClusterIP, ClusterIP: "1.2.3.4", SessionAffinity: corev1.ServiceAffinityClientIP, IPFamilies: []corev1.IPFamily{corev1.IPv6Protocol}, @@ -551,13 +545,11 @@ func Test_applyServerSideValues(t *testing.T) { IPFamilies: []corev1.IPFamily{corev1.IPv6Protocol}, }}, reconciled: corev1.Service{Spec: corev1.ServiceSpec{ - Type: corev1.ServiceTypeClusterIP, ClusterIP: "1.2.3.4", SessionAffinity: corev1.ServiceAffinityClientIP, }}, }, want: corev1.Service{Spec: corev1.ServiceSpec{ - Type: corev1.ServiceTypeClusterIP, ClusterIP: "1.2.3.4", SessionAffinity: corev1.ServiceAffinityClientIP, IPFamilies: []corev1.IPFamily{corev1.IPv6Protocol}, @@ -602,7 +594,9 @@ func Test_applyServerSideValues(t *testing.T) { { name: "Reconciled LoadBalancerClass is used if the expected one is empty", args: args{ - expected: corev1.Service{Spec: corev1.ServiceSpec{}}, + expected: corev1.Service{Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + }}, reconciled: corev1.Service{Spec: corev1.ServiceSpec{ Type: corev1.ServiceTypeLoadBalancer, LoadBalancerClass: ptr.To("service.k8s.aws/nlb"), @@ -643,6 +637,37 @@ func Test_applyServerSideValues(t *testing.T) { Type: corev1.ServiceTypeClusterIP, }}, }, + { + name: "Do not apply server side values if Type changed to the default ClusterIP from another type", + args: args{ + expected: corev1.Service{Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + }}, + reconciled: corev1.Service{Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeNodePort, + ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyCluster, + }}, + }, + want: corev1.Service{Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + }}, + }, + { + name: "Apply server side values if Type changed from the default ClusterIP to another type", + args: args{ + expected: corev1.Service{Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeNodePort, + }}, + reconciled: corev1.Service{Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeNodePort, + ClusterIP: "1.2.3.4", + }}, + }, + want: corev1.Service{Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeNodePort, + ClusterIP: "1.2.3.4", + }}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/controller/elasticsearch/driver/driver.go b/pkg/controller/elasticsearch/driver/driver.go index cae7fae24c..7bb8bfd40f 100644 --- a/pkg/controller/elasticsearch/driver/driver.go +++ b/pkg/controller/elasticsearch/driver/driver.go @@ -13,6 +13,7 @@ import ( "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/client-go/tools/record" "k8s.io/utils/ptr" controller "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -140,6 +141,9 @@ func (d *defaultDriver) Reconcile(ctx context.Context) *reconciler.Results { externalService, err := common.ReconcileService(ctx, d.Client, services.NewExternalService(d.ES), &d.ES) if err != nil { + if k8serrors.IsAlreadyExists(err) { + return results.WithReconciliationState(defaultRequeue.WithReason(fmt.Sprintf("Pending %s service recreation", services.ExternalServiceName(d.ES.Name)))) + } return results.WithError(err) } diff --git a/pkg/controller/elasticsearch/services/services.go b/pkg/controller/elasticsearch/services/services.go index 71284a22bb..4e9ababd50 100644 --- a/pkg/controller/elasticsearch/services/services.go +++ b/pkg/controller/elasticsearch/services/services.go @@ -103,6 +103,10 @@ func NewExternalService(es esv1.Elasticsearch) *corev1.Service { svc.ObjectMeta.Namespace = es.Namespace svc.ObjectMeta.Name = ExternalServiceName(es.Name) + // defaults to ClusterIP if not set + if svc.Spec.Type == "" { + svc.Spec.Type = corev1.ServiceTypeClusterIP + } labels := label.NewLabels(nsn) ports := []corev1.ServicePort{ { diff --git a/pkg/controller/elasticsearch/services/services_test.go b/pkg/controller/elasticsearch/services/services_test.go index ccbde26fc2..a1501da4d9 100644 --- a/pkg/controller/elasticsearch/services/services_test.go +++ b/pkg/controller/elasticsearch/services/services_test.go @@ -68,6 +68,10 @@ func TestNewExternalService(t *testing.T) { httpConf commonv1.HTTPConfig wantSvc func() corev1.Service }{ + { + name: "default clusterIP service", + wantSvc: mkHTTPSService, + }, { name: "no TLS", httpConf: commonv1.HTTPConfig{ @@ -92,11 +96,7 @@ func TestNewExternalService(t *testing.T) { }, }, }, - wantSvc: func() corev1.Service { - svc := mkHTTPService() - svc.Spec.Ports[0].Name = "https" - return svc - }, + wantSvc: mkHTTPSService, }, { name: "user-provided certificate", @@ -107,11 +107,7 @@ func TestNewExternalService(t *testing.T) { }, }, }, - wantSvc: func() corev1.Service { - svc := mkHTTPService() - svc.Spec.Ports[0].Name = "https" - return svc - }, + wantSvc: mkHTTPSService, }, } @@ -171,6 +167,7 @@ func mkHTTPService() corev1.Service { }, }, Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, Ports: []corev1.ServicePort{ { Name: "http", @@ -186,6 +183,12 @@ func mkHTTPService() corev1.Service { } } +func mkHTTPSService() corev1.Service { + svc := mkHTTPService() + svc.Spec.Ports[0].Name = "https" + return svc +} + func mkTransportService() corev1.Service { return corev1.Service{ ObjectMeta: metav1.ObjectMeta{