Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

livenessProbe is not added to gatewayparameters #10384

Open
jmunozro opened this issue Nov 27, 2024 · 2 comments
Open

livenessProbe is not added to gatewayparameters #10384

jmunozro opened this issue Nov 27, 2024 · 2 comments
Labels
Type: Bug Something isn't working

Comments

@jmunozro
Copy link
Contributor

jmunozro commented Nov 27, 2024

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

  1. Having probes OOTB is something that was asked in the issue

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

  1. 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.

  2. 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

@jmunozro jmunozro added the Type: Bug Something isn't working label Nov 27, 2024
@davidjumani
Copy link
Contributor

@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
Copy link
Contributor Author

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.

bleggett pushed a commit to bleggett/gloo that referenced this issue Dec 2, 2024
When only Kube GW proxies are present, we still rely on the edge translator_syncer for extension syncing.
The edge translator will mark Upstreams & UpstreamGroups as Accepted
then perform xds translation where status may be changed to e.g. Rejected if there is an error.

However, in the case where there are no edge proxies, 
translation doesn't actually occur, so any actual errors on the Upstream are never encountered,
thus the status is never set to Rejected.
We end up in a scenario where the Kube GW syncer (correctly) reports Rejected status
while the Edge syncer reports Accepted and they will fight each other indefinitely.

This changes the edge translator_syncer to no longer mark Upstream[Group]s as Accepted unless it will also perform translation.

track obj status in krt collections
    
the status reporter compares the desired status with the
existing status in the solo-kit object to determine if it
should actually UPDATE the resource.

the current proxy_syncer will do a once per second status sync
and relies on this status comparison to be functional to prevent
endless object UPDATEs.

this commit fixes the solo-kit objects (really wrappers) in the
krt collections to contain the status so an accurate comparison
can take place.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants