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.

Sign up for GitHub

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

Jump to bottom

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

Open
2 tasks done
rzjfr opened this issue Jul 2, 2024 · 2 comments · May be fixed by #40554
Open
2 tasks done

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

rzjfr opened this issue Jul 2, 2024 · 2 comments · May be fixed by #40554
Assignees
@rzjfr
Labels
area:helm-chart Airflow Helm Chart good first issue kind:bug This is a clearly a bug

Comments

@rzjfr
Copy link
Contributor

rzjfr commented Jul 2, 2024 •
edited
Loading

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?

  • Yes I am willing to submit a PR!

Code of Conduct
@rzjfr rzjfr added area:helm-chart Airflow Helm Chart kind:bug This is a clearly a bug needs-triage label for new issues that we didn't triage yet labels Jul 2, 2024
@rzjfr rzjfr linked a pull request Jul 2, 2024 that will close this issue
Open
@potiuk potiuk added good first issue and removed needs-triage label for new issues that we didn't triage yet labels Jul 2, 2024
@rzjfr
Copy link
Contributor Author

rzjfr commented Jul 2, 2024

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

@potiuk
Copy link
Member

potiuk commented Jul 2, 2024

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

@eladkal eladkal mentioned this issue Jul 4, 2024
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rzjfr rzjfr

Labels
area:helm-chart Airflow Helm Chart good first issue kind:bug This is a clearly a bug
Projects
None yet
Milestone
No milestone
2 participants
@potiuk @rzjfr