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

🎉 Add variable to customize airbyte-pod-sweeper delete pod action #17985

Conversation

caiohasouza
Copy link
Contributor

What

Solve issue #17576.

How

Add two new variables to make delete action time customizable.

Recommended reading order

  • charts/airbyte-pod-sweeper/values.yaml
  • charts/airbyte-pod-sweeper/templates/configmap.yaml

🚨 User Impact 🚨

No, the currently values are maintened.

@CLAassistant
Copy link

CLAassistant commented Oct 14, 2022

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added area/platform issues related to the platform kubernetes labels Oct 14, 2022
@marcosmarxm marcosmarxm changed the title Add variable to customize airbyte-pod-sweeper delete pod action 🎉 Add variable to customize airbyte-pod-sweeper delete pod action Oct 14, 2022
@marcosmarxm
Copy link
Member

@caiohasouza amazing contribution this solves a huge pain point for some users! Thanks.
I requested to @xpuska513 and @git-phu to help here validating if there is anything else to do.

@caiohasouza
Copy link
Contributor Author

Hi @marcosmarxm

Ok!

@marcosmarxm
Copy link
Member

@caiohasouza @git-phu will look review this monday!

@marcosmarxm
Copy link
Member

@git-phu please don't forget to review this today, thanks!

Copy link
Contributor

@xpuska513 xpuska513 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@git-phu git-phu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@caiohasouza awesome, thanks for submitting this PR!

Looks great!
Btw the way, have you tested this locally? Usually for code contributions we'll ask contributors to add test cases and provide the output from running the test suite, but for this particular kind of change just a screenshot of this change working in your setup should suffice. Thanks!

@caiohasouza
Copy link
Contributor Author

Hi @git-phu ,

I got it, no problem. Yes, i tested locally, i think that a "helm diff" output is enough, right? If yes:

helm3 diff upgrade --install airbyte --namespace production -f helm-values-production.yaml chart/airbyte/ --version 0.40.17
production, airbyte-pod-sweeper, Deployment (apps) has changed:
  # Source: airbyte/charts/pod-sweeper/templates/deployment.yaml
  apiVersion: apps/v1
  kind: Deployment
  metadata:
    name: airbyte-pod-sweeper
    labels:
      helm.sh/chart: pod-sweeper-0.40.17
      app.kubernetes.io/name: airbyte
      app.kubernetes.io/instance: airbyte
      app.kubernetes.io/version: "0.39.37-alpha"
      app.kubernetes.io/managed-by: Helm
    annotations:
-     checksum/config: 60cbfe17501b4574a2f6ec926c5e9bcb4509e42a5f6eff2344955ae970c6b63a
+     checksum/config: 6a2472fcb930fdf3c5ae836b5e12d57fa0c1f3ab6f8f1cc1cdb98a30899b3dd1
  spec:
    replicas: 1
    selector:
      matchLabels:
        app.kubernetes.io/name: airbyte
        app.kubernetes.io/instance: airbyte
    template:
      metadata:
        labels:
          app.kubernetes.io/name: airbyte
          app.kubernetes.io/instance: airbyte
      spec:
        serviceAccountName: airbyte
        affinity:
          nodeAffinity:
            requiredDuringSchedulingIgnoredDuringExecution:
              nodeSelectorTerms:
              - matchExpressions:
                - key: environment
                  operator: In
                  values:
                  - airbyte-production
        containers:
        - name: airbyte-pod-sweeper
          image: bitnami/kubectl:latest
          imagePullPolicy: "IfNotPresent"
          env:
          - name: KUBE_NAMESPACE
            valueFrom:
              fieldRef:
                fieldPath: metadata.namespace
          volumeMounts:
          - mountPath: /script/sweep-pod.sh
            subPath: sweep-pod.sh
            name: sweep-pod-script
          - mountPath: /.kube
            name: kube-config
          command: ["/bin/bash", "-c", /script/sweep-pod.sh]
          resources:
            limits:
              cpu: 200m
              memory: 128Mi
            requests:
              cpu: 40m
              memory: 128Mi
          livenessProbe:
            exec:
              command:
              - /bin/sh
              - -ec
              - grep -aq sweep-pod.sh /proc/1/cmdline
            initialDelaySeconds: 5
            periodSeconds: 30
            timeoutSeconds: 1
            successThreshold: 1
            failureThreshold: 3
          readinessProbe:
            exec:
              command:
              - /bin/sh
              - -ec
              - grep -aq sweep-pod.sh /proc/1/cmdline
            initialDelaySeconds: 5
            periodSeconds: 30
            timeoutSeconds: 1
            successThreshold: 1
            failureThreshold: 3
        volumes:
        - name: kube-config
          emptyDir: {}
        - name: sweep-pod-script
          configMap:
            name: airbyte-sweep-pod-script
            defaultMode: 0755
production, airbyte-sweep-pod-script, ConfigMap (v1) has changed:
  # Source: airbyte/charts/pod-sweeper/templates/configmap.yaml
  apiVersion: v1
  kind: ConfigMap
  metadata:
    name: airbyte-sweep-pod-script
    labels:
      helm.sh/chart: pod-sweeper-0.40.17
      app.kubernetes.io/name: airbyte
      app.kubernetes.io/instance: airbyte
      app.kubernetes.io/version: "0.39.37-alpha"
      app.kubernetes.io/managed-by: Helm
  
  data:
    sweep-pod.sh: |
      #!/bin/bash
-     
+ 
      get_worker_pods () {
          kubectl -n ${KUBE_NAMESPACE} -L airbyte -l airbyte=worker-pod \
            --field-selector status.phase!=Running get pods \
            -o=jsonpath='{range .items[*]} {.metadata.name} {.status.phase} {.status.conditions[0].lastTransitionTime} {.status.startTime}{"\n"}{end}'
      }
-     
+ 
      delete_worker_pod() {
          printf "From status '%s' since '%s', " $2 $3
          echo "$1" | grep -v "STATUS" | awk '{print $1}' | xargs --no-run-if-empty kubectl -n ${KUBE_NAMESPACE} delete pod
      }
-     
+ 
      while :
      do
          # Shorter time window for completed pods
-         SUCCESS_DATE_STR=`date -d 'now - 2 hours' --utc -Ins`
+         SUCCESS_DATE_STR=`date -d 'now - 2 minutes' --utc -Ins`
          SUCCESS_DATE=`date -d $SUCCESS_DATE_STR +%s`
          # Longer time window for pods in error (to debug)
-         NON_SUCCESS_DATE_STR=`date -d 'now - 24 hours' --utc -Ins`
+         NON_SUCCESS_DATE_STR=`date -d 'now - 1440 minutes' --utc -Ins`
          NON_SUCCESS_DATE=`date -d $NON_SUCCESS_DATE_STR +%s`
          (
              IFS=$'\n'
              for POD in `get_worker_pods`; do
                  IFS=' '
                  POD_NAME=`echo $POD | cut -d " " -f 1`
                  POD_STATUS=`echo $POD | cut -d " " -f 2`
                  POD_DATE_STR=`echo $POD | cut -d " " -f 3`
                  POD_START_DATE_STR=`echo $POD | cut -d " " -f 4`
                  POD_DATE=`date -d ${POD_DATE_STR:-$POD_START_DATE_STR} '+%s'`
                  if [ "$POD_STATUS" = "Succeeded" ]; then
                  if [ "$POD_DATE" -lt "$SUCCESS_DATE" ]; then
                      delete_worker_pod "$POD_NAME" "$POD_STATUS" "$POD_DATE_STR"
                  fi
                  else
                  if [ "$POD_DATE" -lt "$NON_SUCCESS_DATE" ]; then
                      delete_worker_pod "$POD_NAME" "$POD_STATUS" "$POD_DATE_STR"
                  fi
                  fi
              done
          )
          sleep 60
      done

If you need something else please ask me.

Regards

@git-phu
Copy link
Contributor

git-phu commented Oct 19, 2022

@caiohasouza cool thanks!

would you also be able to post a bit of log output from the sweeper pod? Just to verify it still behaves as expected. Thanks!

@caiohasouza
Copy link
Contributor Author

Hi @git-phu

Sure:

kubectl logs -f -n production airbyte-pod-sweeper-8f6cf9487-dgl9f 
From status 'Succeeded' since '2022-10-19T15:52:31Z', pod "source-mysql-check-7f551878-e141-45e3-84ff-3c511563c198-0-iwrxq" deleted

Regards

Copy link
Contributor

@git-phu git-phu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perfect, really appreciate the contribution @caiohasouza!

@caiohasouza
Copy link
Contributor Author

Hi @git-phu

Perfect, thank you!

Regards

@git-phu git-phu merged commit 79c1f3c into airbytehq:master Oct 19, 2022
jhammarstedt pushed a commit to jhammarstedt/airbyte that referenced this pull request Oct 31, 2022
…rbytehq#17985)

* Add variable to customize airbyte-pod-sweeper delete pod

* Add variable to customize airbyte-pod-sweeper delete pod

* Add variable to customize airbyte-pod-sweeper delete pod

* Add variable to customize airbyte-pod-sweeper delete pod
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/platform issues related to the platform community kubernetes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants