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

Keda does not work properly when using gitSync #44798

Open
1 of 2 tasks
yovio-rca opened this issue Dec 9, 2024 · 3 comments · May be fixed by #44963
Open
1 of 2 tasks

Keda does not work properly when using gitSync #44798

yovio-rca opened this issue Dec 9, 2024 · 3 comments · May be fixed by #44963
Labels
area:helm-chart Airflow Helm Chart good first issue kind:bug This is a clearly a bug

Comments

@yovio-rca
Copy link

Official Helm Chart version

1.15.0 (latest released)

Apache Airflow version

2.10.3

Kubernetes Version

1.31.2

Helm Chart configuration

I have the below section of workers form my values.yaml:

workers:
replicas: 1
keda:
enabled: true
minReplicaCount: 1 # each worker can has 16 celery concurencies
maxReplicaCount: 20
resources:
requests:
cpu: 100m
memory: 1750Mi

Docker Image customizations

No response

What happened

I try to use keda to autoscale my airflow workers which use CeleryKubernetesExecutor.

I followed instruction in https://airflow.apache.org/docs/helm-chart/stable/keda.html to install Keda and change my airflow values.yaml accordingly.

After applying the changes, I observed lots of warning from keda-operator with error message:
"error parsing postgreSQL metadata: error parsing postgresql metadata: no host given"

It seems to me that Keda ScaledObject doesnt work properly.

I checked that:

  1. ScaledObject created on correct namespace, same as my airflow namespace
  2. It has correct trigger type: postgresql (I use postgres for airflow metadata)
  3. it has connectionFromEnv: AIRFLOW_CONN_AIRFLOW_DB

After further investigation, I found that ScaledObject scaleTargetRef has other property called "envSourceContainerName"
as explain in https://keda.sh/docs/2.16/reference/scaledobject-spec/#scaletargetref

The purpose of that property is to tell Keda name of container on which AIRFLOW_CONN_AIRFLOW_DB defined.

If I modify airflow-worker ScaledObject by adding envSourceContainerName: worker into scaleTargetRef, it start working properly.

When I check my airflow-worker StatefulSet, I can see that it has 3 containers: git-sync, worker-log-groomer, and worker.

I'm aware that on the chart template workers/worker-deployment.yaml, worker container is defined as 1st container, but for some reason when I apply the template, worker container is 3rd container in the StatefulSet. I tried deleting airflow-worker StatefulSet and do another helm install with same result.

In my opinion, we cant rely on the position of worker container within the StatefulSet nor Deployment, but we should specify worker container name in ScaledObject scaleTargetRef

What you think should happen instead

Keda autoscaller should work regardless of position of worker container in StatefulSet or Deployment.

How to reproduce

  1. Deploy aiflow with git-sync enable
  2. Deploy Keda into Kubernetes cluster
  3. Configure worker to use keda
  4. Observe that ScaledObj has no warning

Anything else

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@yovio-rca yovio-rca 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 Dec 9, 2024
Copy link

boring-cyborg bot commented Dec 9, 2024

Thanks for opening your first issue here! Be sure to follow the issue template! If you are willing to raise PR to address this issue please do so, no need to wait for approval.

@yovio-rca
Copy link
Author

Reopen as it was mistakenly close

@yovio-rca yovio-rca reopened this Dec 9, 2024
@yovio-rca yovio-rca changed the title Keda does not work properly whwn using gitSync Keda does not work properly when using gitSync Dec 9, 2024
@potiuk potiuk added good first issue and removed needs-triage label for new issues that we didn't triage yet labels Dec 9, 2024
@jx2lee
Copy link
Contributor

jx2lee commented Dec 14, 2024

HI, Can I take this?

@jx2lee jx2lee linked a pull request Dec 16, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:helm-chart Airflow Helm Chart good first issue kind:bug This is a clearly a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants