haproxytech / helm-charts

Helm chart for HAProxy Kubernetes Ingress Controller
Apache License 2.0
155 stars 121 forks source link

[BUG] Controller service spec invalid due to multiple protocols #230

Closed mecampbellsoup closed 8 months ago

mecampbellsoup commented 8 months ago

A quic port was added to the controller service template:

https://github.com/haproxytech/helm-charts/commit/af6247950174c3794780dabb4b766a8d7ef23805

However, the service is no longer valid since there are 2 protocols which seems to be unsupported as I'm getting this error:

Starting deploy...
Helm release cloud-app not installed. Installing...
Error: INSTALLATION FAILED: 1 error occurred:
        * Service "cloud-app-kubernetes-ingress" is invalid: spec.ports: Invalid value: []core.ServicePort{core.ServicePort{Name:"http", Protocol:"TCP", AppProtocol:(*string)(0xc010afdd70), Port:80, TargetPort:intstr.IntOrString{Type:1, IntVal:0, StrVal:"http"}, NodePort:0}, core.ServicePort{Name:"https", Protocol:"TCP", AppProtocol:(*string)(0xc010afdd80), Port:443, TargetPort:intstr.IntOrString{Type:1, IntVal:0, StrVal:"https"}, NodePort:0}, core.ServicePort{Name:"quic", Protocol:"UDP", AppProtocol:(*string)(0xc010afdd90), Port:443, TargetPort:intstr.IntOrString{Type:1, IntVal:0, StrVal:"quic"}, NodePort:0}, core.ServicePort{Name:"prometheus", Protocol:"TCP", AppProtocol:(*string)(nil), Port:6060, TargetPort:intstr.IntOrString{Type:1, IntVal:0, StrVal:"prometheus"}, NodePort:0}}: may not contain more than 1 protocol when type is 'LoadBalancer'

This is the template being installed:

apiVersion: v1
kind: Service
metadata:
  annotations: null
  labels:
    app.kubernetes.io/instance: cloud-app
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: kubernetes-ingress
    app.kubernetes.io/version: 1.11.1
    helm.sh/chart: kubernetes-ingress-1.38.3
  name: cloud-app-kubernetes-ingress
  namespace: cloud
spec:
  externalIPs: null
  ports:
    - appProtocol: http
      name: http
      port: 80
      protocol: TCP
      targetPort: http
    - appProtocol: https
      name: https
      port: 443
      protocol: TCP
      targetPort: https
    - appProtocol: https
      name: quic
      port: 443
      protocol: UDP
      targetPort: quic
    - name: prometheus
      port: 6060
      protocol: TCP
      targetPort: prometheus
  selector:
    app.kubernetes.io/instance: cloud-app
    app.kubernetes.io/name: kubernetes-ingress
  type: LoadBalancer

For now I can set enablePorts.quic = false, but at some point I probably would need to be able to enable that port so I wanted to inform you of this error.

I believe later versions of k8s would not see this error. We are running 1.20.15 IIRC.

Related to https://github.com/kubernetes/kubernetes/issues/23880.

dkorunic commented 8 months ago

This is tehnically more like a bug in K8s which doesn't permit something rather reasonable, but from the looks of it it should work for K8s implementing MixedProtocolLBService, meaning K8s v1.24+. We can gate this for v1.24+, certainly, thanks for reporting this.

dkorunic commented 8 months ago

Fix incoming in https://github.com/haproxytech/helm-charts/commit/cad184c970f233d653b30120c160858cf64a1609. We have discussed this matter and alternatives for K8s < 1.24 are all rather inelegant, so we have opted for requiring K8s 1.24 (which has MixedProtocolLBService feature gate enabled by default) to be able to use QUIC and bind to both udp and tcp protocol for the same port. Even K8s 1.24 is rather "old" in terms of release and its active support has ended 7 months ago, so this seems like a reasonable choice going forward.