livenessProbe is not added to gatewayparameters

jmunozro commented 3 days ago

Gloo Edge Product

Enterprise

Gloo Edge Version

1.18.0-rc2

Kubernetes Version

1.28

Describe the bug

gracefulShutdown and probes are working, but livenessProbeEnabled is not having any effect in the gatewayparameters.

It is working as expected, in a quick test i didn't observe any downtime during upgrade (PE team will have to do a proper test, but it looks good so far).

Expected Behavior

Having probes OOTB is something that was asked in the issue

That can't hardly be considered a breaking change, but an improvement.

In addition, I didn't find any way to activate livenessProbe without explicitly providing a custom one, which I can do with gracefulshutdown and probes.
The names probes meaning readiness and livenessProbe for liveness are also not intuitive

Steps to reproduce the bug

helm template gloo glooe/gloo-ee --devel --namespace gloo-system --version=1.18.0-rc2 \
  --create-namespace --dry-run --output-dir ./dist  -f - <<EOF
licenseKey: fff
license_key: fff
gloo:
  kubeGateway:
    enabled: true
    gatewayParameters:
      glooGateway:
        podTemplate:
          gracefulShutdown:
            enabled: true
          probes: true
          livenessProbeEnabled: true
EOF

cat ./dist/gloo-ee/charts/gloo/templates/43-gatewayparameters.yaml               
---
# Source: gloo-ee/charts/gloo/templates/43-gatewayparameters.yaml
kind: GatewayParameters
apiVersion: gateway.gloo.solo.io/v1alpha1
metadata:
  labels:
    gloo: kube-gateway
  name: gloo-gateway
  namespace: gloo-system
spec:
  kube:
    deployment:
      replicas: 1
    service:
      type: LoadBalancer
    floatingUserId: false
    envoyContainer:
      image:

        registry: quay.io/solo-io
        repository: gloo-ee-envoy-wrapper
        tag: 1.18.0-rc2
        digest: sha256:f882eead4550761a9626cd1d96d9f7e6e40a1694a28e0ecd8ebae1825ef09680
        pullPolicy: IfNotPresent
      securityContext:
        allowPrivilegeEscalation: false
        capabilities:
          add:
          - NET_BIND_SERVICE
          drop:
          - ALL
        readOnlyRootFilesystem: true
        runAsNonRoot: true
        runAsUser: 10101
    podTemplate:
      extraLabels:
        gloo: kube-gateway
      gracefulShutdown:
        enabled: true
      readinessProbe:
        httpGet:
          scheme: HTTP
          port: 8082
          path: /envoy-hc
        initialDelaySeconds: 5
        periodSeconds: 5
        failureThreshold: 2
    sdsContainer:
      image:

        registry: quay.io/solo-io
        repository: sds
        tag: 1.18.0-rc2
        pullPolicy: IfNotPresent
      bootstrap:
        logLevel: info
    istio:
      istioProxyContainer:
        image:

          registry: docker.io/istio
          repository: proxyv2
          tag: 1.22.0
          pullPolicy: IfNotPresent
        logLevel: warning
        istioDiscoveryAddress: istiod.istio-system.svc:15012
        istioMetaMeshId: cluster.local
        istioMetaClusterId: Kubernetes
    stats:
      enableStatsRoute: true
      enabled: true
      routePrefixRewrite: /stats/prometheus
      statsRoutePrefixRewrite: /stats
    aiExtension:
      enabled: false
      image:

        registry: quay.io/solo-io
        repository: gloo-ai-extension
        tag: 1.18.0-rc2
        pullPolicy: IfNotPresent

Additional Environment Detail

No response

Additional Context

No response

davidjumani commented 3 days ago

@jmunozro The naming of these values is to mimic the existing values on edge classic. Per the discussion here we decided not to have a default liveness probe but rather have the user specify their own via customLivenessProbe

jmunozro commented 3 days ago

In edge you have default liveness probe, we are not forcing customers to specify a custom one. I think we are creating a worse UX if we decide to 'respect' some edge classic standards, even if they are clearly not ideal, and change others.

k8sgateway / k8sgateway