Service xxx is invalid: spec.loadBalancerClass: Invalid value: "null"

[alekc]

Bug Report

What did you do?
Given the logstash config

apiVersion: logstash.k8s.elastic.co/v1alpha1
kind: Logstash
metadata:
  name: ${stack_name}
  namespace: ${namespace}
spec:
  count: 1
  elasticsearchRefs:
    - name: ${stack_name}
      clusterName: ${stack_name}
  version: ${version}
  pipelines:
    - pipeline.id: gelf
      config.string: |
        input {
          gelf {
            port => 12201
            add_field => { "input_source" => "gelf" }
          }
        }
        output {
          elasticsearch {
            hosts => [ "$${MAIN_ES_HOSTS}" ]
            user => "$${MAIN_ES_USER}"
            password => "$${MAIN_ES_PASSWORD}"
            ssl_certificate_authorities => "$${MAIN_ES_SSL_CERTIFICATE_AUTHORITY}"
            index => "gelf-%%{+YYYY.MM.dd}"
          }
        }
  services:
    - name: beats
      service:
        spec:
          type: NodePort
          ports:
            - port: 5044
              name: "filebeat"
              protocol: TCP
              targetPort: 5044
    - name: udp
      service:
        spec:
          type: LoadBalancer
          ports:
            - port: 12201
              name: "filebeat"
              protocol: UDP
              targetPort: 12201

The reconciliation was failing with

{"log.level":"info","@timestamp":"2024-04-05T11:15:26.411Z","log.logger":"logstash-controller","message":"Updating resource","service.version":"2.12.1+a56215f7","service.type":"eck","ecs.version":"1.4.0","iteration":"20","namespace":"logging","logstash_name":"main","kind":"Service","namespace":"logging","name":"main-ls-udp"}
{"log.level":"info","@timestamp":"2024-04-05T11:15:26.437Z","log.logger":"logstash-es","message":"Ending reconciliation run","service.version":"2.12.1+a56215f7","service.type":"eck","ecs.version":"1.4.0","iteration":"2","namespace":"logging","logstash_name":"main","took":0.099525177}
{"log.level":"info","@timestamp":"2024-04-05T11:15:26.437Z","log.logger":"logstash-es","message":"Starting reconciliation run","service.version":"2.12.1+a56215f7","service.type":"eck","ecs.version":"1.4.0","iteration":"3","namespace":"logging","logstash_name":"main"}
{"log.level":"info","@timestamp":"2024-04-05T11:15:26.437Z","log.logger":"logstash-controller","message":"Ending reconciliation run","service.version":"2.12.1+a56215f7","service.type":"eck","ecs.version":"1.4.0","iteration":"20","namespace":"logging","logstash_name":"main","took":0.026232342}
{"log.level":"error","@timestamp":"2024-04-05T11:15:26.448Z","log.logger":"manager.eck-operator","message":"Reconciler error","service.version":"2.12.1+a56215f7","service.type":"eck","ecs.version":"1.4.0","controller":"logstash-controller","object":{"name":"main","namespace":"logging"},"namespace":"logging","name":"main","reconcileID":"fd0c79d7-32a7-4846-bada-3e8afd619e62","error":"Service \"main-ls-udp\" is invalid: spec.loadBalancerClass: Invalid value: \"null\": may not change once set","errorCauses":[{"error":"Service \"main-ls-udp\" is invalid: spec.loadBalancerClass: Invalid value: \"null\": may not change once set"}],"error.stack_trace":"sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler\n\t/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/internal/controller/controller.go:329\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\t/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/internal/controller/controller.go:266\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2\n\t/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/internal/controller/controller.go:227"}
{"log.level":"info","@timestamp":"2024-04-05T11:15:26.523Z","log.logger":"logstash-es","message":"Ending reconciliation run","service.version":"2.12.1+a56215f7","service.type":"eck","ecs.version":"1.4.0","iteration":"3","namespace":"logging","logstash_name":"main","took":0.086298936}

preventing any further reconciliation (i.e. pipelines changes was not reflected in the secrets). Since the error was silent (pod not crashing etc), it took a while to get noticed.

In order to fix the issue I had to explicitely define the loadBalancerClass field

  services:
    - name: udp
      service:
        spec:
          type: LoadBalancer
          loadBalancerClass: service.k8s.aws/nlb
          ports:
            - port: 12201
              name: "filebeat"
              protocol: UDP
              targetPort: 12201

AFAIK, this is not a required field, so the lack of it should not prevent the creation of the service.

What did you see instead? Under which circumstances?

Environment

• ECK version:

  2.12.1

• Kubernetes information:
  AWS

Server Version: version.Info{Major:"1", Minor:"27+", GitVersion:"v1.27.10-eks-508b6b3", GitCommit:"e99f7c75641f738090d483d988dc4a70001e01cf", GitTreeState:"clean", BuildDate:"2024-01-29T20:59:05Z", GoVersion:"go1.20.13", Compiler:"gc", Platform:"linux/amd64"}

[pebrc]

I think

cloud-on-k8s/pkg/controller/common/service_control.go

Lines 85 to 142 in f6ec623

	func applyServerSideValues(expected, reconciled *corev1.Service) {
	// Type may be defaulted by the api server
	if expected.Spec.Type == "" {
	expected.Spec.Type = reconciled.Spec.Type
	}
	// 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 {
	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
	if expected.Spec.SessionAffinity == "" {
	expected.Spec.SessionAffinity = reconciled.Spec.SessionAffinity
	}
	// same for the target port and node port
	if len(expected.Spec.Ports) == len(reconciled.Spec.Ports) {
	for i := range expected.Spec.Ports {
	if expected.Spec.Ports[i].TargetPort.IntValue() == 0 {
	expected.Spec.Ports[i].TargetPort = reconciled.Spec.Ports[i].TargetPort
	}
	// check if NodePort makes sense for this service type
	if hasNodePort(expected.Spec.Type) && expected.Spec.Ports[i].NodePort == 0 {
	expected.Spec.Ports[i].NodePort = reconciled.Spec.Ports[i].NodePort
	}
	}
	}
	if expected.Spec.HealthCheckNodePort == 0 {
	expected.Spec.HealthCheckNodePort = reconciled.Spec.HealthCheckNodePort
	}
	expected.Annotations = maps.MergePreservingExistingKeys(expected.Annotations, reconciled.Annotations)
	expected.Labels = maps.MergePreservingExistingKeys(expected.Labels, reconciled.Labels)
	// IPFamily is immutable and cannot be modified so we should retain the existing value from the server if there's no explicit override.
	if expected.Spec.IPFamilies == nil {
	expected.Spec.IPFamilies = reconciled.Spec.IPFamilies
	}
	// IPFamilyPolicy is immutable and cannot be modified so we should retain the existing value from the server if there's no explicit override.
	if expected.Spec.IPFamilyPolicy == nil {
	expected.Spec.IPFamilyPolicy = reconciled.Spec.IPFamilyPolicy
	}
	// InternalTrafficPolicy may be defaulted by the api server starting K8S v1.22
	if expected.Spec.InternalTrafficPolicy == nil {
	expected.Spec.InternalTrafficPolicy = reconciled.Spec.InternalTrafficPolicy
	}
	}

needs to be updated to support loadBalancerClass