scheduler: livenessProbe redundant exec leads to false-positive result

[Wilderone]

Official Helm Chart version

1.4.0 (latest released)

Apache Airflow version

2.2.4 (latest released)

Kubernetes Version

v1.21.5-eks-bc4871b

Helm Chart configuration

No response

Docker Image customisations

No response

What happened

Current livenessProbe in scheduler returns 0 exit code even if execution failed, this leads to container won't restart if it necessary.
I see two ways to fix this:

1. Simply remove - exec from livenessProbe
2. Move livenessProbe to values for customisation by client

What you expected to happen

Restart container if livenessProve failed

How to reproduce

Fastest way - change livenessProbe to:

- sh
- c
- exec
- cat /123

You will see no errors in pod describe

Anything else

No response

Are you willing to submit PR?

• Yes I am willing to submit a PR!

Code of Conduct

• I agree to follow this project's Code of Conduct

[boring-cyborg]

Thanks for opening your first issue here! Be sure to follow the issue template!

[potiuk]

Hey @ephraimbuddy @jedcunningham - I think that one might be important enough to cancel 1.5.0 of chart and make a release candidate 2. I just checked it and I think this is is indeed wrong what we have.

The current liveness probe is:

            exec:
              command:
                - sh
                - -c
                - exec
                - |
                  CONNECTION_CHECK_MAX_COUNT=0 /entrypoint python -Wignore -c "
                  import os
                  os.environ['AIRFLOW__CORE__LOGGING_LEVEL'] = 'ERROR'
                  os.environ['AIRFLOW__LOGGING__LOGGING_LEVEL'] = 'ERROR'

                  from airflow.jobs.scheduler_job import SchedulerJob
                  from airflow.utils.db import create_session
                  from airflow.utils.net import get_hostname
                  import sys

                  with create_session() as session:
                      job = session.query(SchedulerJob).filter_by(hostname=get_hostname()).order_by(
                          SchedulerJob.latest_heartbeat.desc()).limit(1).first()

                  sys.exit(0 if job.is_alive() else 1)
                  "

But it should be IMHO:

            exec:
              command:
                - sh
                - -c
                - | 
                  CONNECTION_CHECK_MAX_COUNT=0 /entrypoint python -Wignore -c "
                  import os
                  os.environ['AIRFLOW__CORE__LOGGING_LEVEL'] = 'ERROR'
                  os.environ['AIRFLOW__LOGGING__LOGGING_LEVEL'] = 'ERROR'

                  from airflow.jobs.scheduler_job import SchedulerJob
                  from airflow.utils.db import create_session
                  from airflow.utils.net import get_hostname
                  import sys

                  with create_session() as session:
                      job = session.query(SchedulerJob).filter_by(hostname=get_hostname()).order_by(
                          SchedulerJob.latest_heartbeat.desc()).limit(1).first()

                  sys.exit(0 if job.is_alive() else 1)
                  "

[ephraimbuddy]

Hey @ephraimbuddy @jedcunningham - I think that one might be important enough to cancel 1.5.0 of chart and make a release candidate 2. I just checked it and I think this is is indeed wrong what we have.

Yeah. I support having an RC 2.

[jedcunningham]

This is fixed by #22041.