fluxcd / flagger

Progressive delivery Kubernetes operator (Canary, A/B Testing and Blue/Green deployments)
https://docs.flagger.app
Apache License 2.0
4.89k stars 730 forks source link

Migrating from Deployment to Canary causes downtime #1444

Closed miguelvalerio closed 1 year ago

miguelvalerio commented 1 year ago

Describe the bug

Currently, we have the following (simplified) setup:

apiVersion: traefik.containo.us/v1alpha1
kind: TraefikService
metadata:
  name: test-app
  namespace: test
spec:
  weighted:
    services:
      - name: test-app
        namespace: test
        port: 5000
        weight: 100

---
apiVersion: v1
kind: Service
metadata:
  labels:
    app.kubernetes.io/name: test-app
  name: test-app
  namespace: test
spec:
  ports:
    - name: http
      port: 5000
      protocol: TCP
      targetPort: 5000
  selector:
    app.kubernetes.io/name: test-app
---
apiVersion: apps/v1
kind: Deployment
metadata:
  labels:
    app.kubernetes.io/name: test-app
  name: test-app
  namespace: test
spec:
  progressDeadlineSeconds: 100
  replicas: 2
  revisionHistoryLimit: 10
  selector:
    matchLabels:
      app.kubernetes.io/name: test-app
  strategy:
    rollingUpdate:
      maxSurge: 1
      maxUnavailable: 0
    type: RollingUpdate
  template:
    metadata:
      labels:
        app.kubernetes.io/name: test-app
    spec:
      containers:
          image: >-
            some-image
          name: test-app
          ports:
            - containerPort: 5000
              name: http
              protocol: TCP
         .......

During the initialization that is performed when migrating from a standard Deployment to a Canary, there is a slight downtime when we apply the following canary:

apiVersion: flagger.app/v1beta1
kind: Canary
metadata:
  name:  test-app
spec:
  provider: traefik
  targetRef:
    apiVersion: apps/v1
    kind: Deployment
    name: test-app
  progressDeadlineSeconds: 60
  service:
    port: 5000
    targetPort: 5000
  analysis:
    interval: 1m
    threshold: 10
    maxWeight: 50
    stepWeight: 20

This happens because final part of the initialization (from my understanding) is done in this order:

  1. Scaling down the replicas for the test-app Deployment to 0
  2. Update the test-app Service's selectorLabels to reference the primary pods
  3. Update the TraefikService to point to the test-app-primary Service

Due to this, during steps 1 and 2, there is a slight time window in which there will be a couple of 502 errors returned, since the test-app Service has no pods to reference.

To Reproduce

Apply the configurations above, and have some tool such as https://github.com/tsenart/vegeta doing requests to the ingress, and see the 502 errors being returned.

Expected behavior

No 502s returned when migrating to Canary.

Additional context

Updatting the Service before scaling down the Deployment (swapping points 1 and 2) seems to be a good option to fix this issue. I wouldn't mind submitting a PR with this fix, but I'd like to make sure that this would be a correct approach.

aryan9600 commented 1 year ago

hello @miguelvalerio, thanks for opening this issue and volunteering to fix it! please take it on, your recommended fix should work. looking forward to your PR :)