apache / airflow

Apache Airflow - A platform to programmatically author, schedule, and monitor workflows
https://airflow.apache.org/
Apache License 2.0
35.31k stars 13.79k forks source link

helm chart: duplicate key in safe-to-evict annotation for services like scheduler #40553

Open rzjfr opened 4 days ago

rzjfr commented 4 days ago

Official Helm Chart version

1.14.0 (latest released)

Apache Airflow version

2.9.2

Kubernetes Version

1.28.9

Helm Chart configuration

No response

Docker Image customizations

No response

What happened

If you set executor to be KubernetesExecutor or CeleryKubernetesExecutor all the non worker service would have two cluster-autoscaler.kubernetes.io/safe-to-evict key in their annotations, second one would be value of safeToEvict the one set in the worker and the first one would the the safeToEvict set for the service itself

What you think should happen instead

there should be no key duplication in annotations of triggerer, scheduler, dag-processor.

How to reproduce

helm template test chart --debug --namespace test --set executor=CeleryKubernetesExecutor

Anything else

for example:

# Source: airflow/templates/scheduler/scheduler-deployment.yaml
################################
## Airflow Scheduler Deployment/StatefulSet
#################################
# Are we using a local executor?
# Is persistence enabled on the _workers_?
# This is important because in $local mode, the scheduler assumes the role of the worker
# If we're using a StatefulSet
# We can skip DAGs mounts on scheduler if dagProcessor is enabled, except with $local mode
# If we're using elasticsearch logging
apiVersion: apps/v1
kind: Deployment
metadata:
  name: test-scheduler
  labels:
    tier: airflow
    component: scheduler
    release: test
    chart: "airflow-1.15.0-dev"
    heritage: Helm
    executor: CeleryKubernetesExecutor
spec:
  replicas: 1
  selector:
    matchLabels:
      tier: airflow
      component: scheduler
      release: test
  template:
    metadata:
      labels:
        tier: airflow
        component: scheduler
        release: test
      annotations:
        checksum/metadata-secret: 9b7cb2216ff792b68a33619535f2ab6fa5b139a430718535c9ec2306e5b50c53
        checksum/result-backend-secret: 98a68f230007cfa8f5d3792e1aff843a76b0686409e4a46ab2f092f6865a1b71
        checksum/pgbouncer-config-secret: 1dae2adc757473469686d37449d076b0c82404f61413b58ae68b3c5e99527688
        checksum/airflow-config: b421cbba34abf16405a84826cc8b2258cfcde9f09ed7581a352921ac52b10a54
        checksum/extra-configmaps: e862ea47e13e634cf17d476323784fa27dac20015550c230953b526182f5cac8
        checksum/extra-secrets: e9582fdd622296c976cbc10a5ba7d6702c28a24fe80795ea5b84ba443a56c827
        cluster-autoscaler.kubernetes.io/safe-to-evict: "true"
        cluster-autoscaler.kubernetes.io/safe-to-evict: "false"

Are you willing to submit PR?

Code of Conduct

rzjfr commented 4 days ago

Hi @potiuk it would be great if it can be assigned to me, I already submitted a PR

potiuk commented 4 days ago

Sure. Assigned. FYI -> you do not need to create issue for such changes/fixes. Just PR is enough.