`kubernetes cleanup-pods` CLI command only available when using KubernetesExecutor

[jedcunningham]

Apache Airflow version

main (development)

If "Other Airflow 2 version" selected, which one?

No response

What happened?

Currently, the k8s cleanup command is directly tied to the KubernetesExecutor, but the command is helpful for and documented to be used for KubernetesPodOperator users as well.

What you think should happen instead?

The command should work regardless of executor choice, if you have the kubernetes provider installed.

How to reproduce

With LocalExecutor:

$ AIRFLOW__CORE__EXECUTOR=LocalExecutor airflow kubernetes cleanup-pods --help
Usage: airflow [-h] GROUP_OR_COMMAND ...
...
airflow command error: argument GROUP_OR_COMMAND: invalid choice: 'kubernetes' (choose from 'assets', 'backfill', 'cheat-sheet', 'config', 'connections', 'd
ag-processor', 'dags', 'db', 'fab-db', 'fastapi-api', 'info', 'jobs', 'kerberos', 'plugins', 'pools', 'providers', 'roles', 'rotate-fernet-key', 'scheduler'
, 'standalone', 'sync-perm', 'tasks', 'triggerer', 'users', 'variables', 'version', 'webserver'), see help above.

With KubernetesExecutor:

$ AIRFLOW__CORE__EXECUTOR=KubernetesExecutor airflow kubernetes cleanup-pods --help
Usage: airflow kubernetes cleanup-pods [-h] [--min-pending-minutes MIN_PENDING_MINUTES] [--namespace NAMESPACE] [-v]

Clean up Kubernetes pods (created by KubernetesExecutor/KubernetesPodOperator) in evicted/failed/succeeded/pending states

Options:
  -h, --help            show this help message and exit
  --min-pending-minutes MIN_PENDING_MINUTES
                        Pending pods created before the time interval are to be cleaned up, measured in minutes. Default value is 30(m). The minimum value is 5(m).
  --namespace NAMESPACE
                        Kubernetes Namespace. Default value is ` namespace` in configuration.
  -v, --verbose         Make logging output more verbose

Operating System

macos

Versions of Apache Airflow Providers

No response

Deployment

Virtualenv installation

Deployment details

No response

Anything else?

This likely means that providers generally need to be able to add cli commands, not just things in providers (e.g. executors).

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

[jason810496]

Hi @jedcunningham I can work on this issue, could you assign to me ? Thanks !

[jason810496]

Hi @jedcunningham,
I think we could introduce a CliDefinitionLoader class, following the same approach as ExecutorLoader.

Currently, only AuthManager.get_cli_commands() and Executor.get_cli_commands() are called and extended in cli_parser:

airflow/airflow/cli/cli_parser.py

Lines 39 to 87 in 4c031cb

	from airflow.api_fastapi.app import get_auth_manager_cls
	from airflow.cli.cli_config import (
	DAG_CLI_DICT,
	ActionCommand,
	DefaultHelpParser,
	GroupCommand,
	core_commands,
	)
	from airflow.cli.utils import CliConflictError
	from airflow.exceptions import AirflowException
	from airflow.executors.executor_loader import ExecutorLoader
	from airflow.utils.helpers import partition
	if TYPE_CHECKING:
	from airflow.cli.cli_config import (
	Arg,
	CLICommand,
	)
	airflow_commands = core_commands.copy() # make a copy to prevent bad interactions in tests
	log = logging.getLogger(__name__)
	for executor_name in ExecutorLoader.get_executor_names():
	try:
	executor, _ = ExecutorLoader.import_executor_cls(executor_name)
	airflow_commands.extend(executor.get_cli_commands())
	except Exception:
	log.exception("Failed to load CLI commands from executor: %s", executor_name)
	log.error(
	"Ensure all dependencies are met and try again. If using a Celery based executor install "
	"a 3.3.0+ version of the Celery provider. If using a Kubernetes executor, install a "
	"7.4.0+ version of the CNCF provider"
	)
	# Do not re-raise the exception since we want the CLI to still function for
	# other commands.
	try:
	auth_mgr = get_auth_manager_cls()
	airflow_commands.extend(auth_mgr.get_cli_commands())
	except Exception as e:
	log.warning("cannot load CLI commands from auth manager: %s", e)
	log.warning("Authentication manager is not configured and webserver will not be able to start.")
	# do not re-raise for the same reason as above
	if len(sys.argv) > 1 and sys.argv[1] == "webserver":
	log.exception(e)
	sys.exit(1)

By introducing CliDefinitionLoader, we can extend cli_parser to support provider module-level CLI commands. Here is a rough POC of CliDefinitionLoader: https://github.com/apache/airflow/compare/main...jason810496:airflow:refactor/add-cli-definition-loader-poc?expand=1

Additionally, as a next step, we could decouple the get_cli_commands method from both Executor and AuthManager. This would address the root cause of the current CLI slowdown and potentially improve CLI performance by 3–4x, as shown in the benchmark: #46789.

Would love to hear your thoughts on this approach!

[jedcunningham]

Paging @potiuk - you probably have thoughts or opinions on this.

High level, sounds good. Tradeoffs moving it out of the executor and authmanager interface though. Having it there is nice in some ways, authors don't need to do conditionals on their own.

[potiuk]

Paging @potiuk - you probably have thoughts or opinions on this.

High level, sounds good. Tradeoffs moving it out of the executor and authmanager interface though. Having it there is nice in some ways, authors don't need to do conditionals on their own.

There is also a problem with performance - potentially. Currenlty the list of commands available is built partially dynamically basd on configured executor - and that means that ProvidersManager and PluginsManager and discovery of avaialble providers, Hooks, Connections etc. does not have to be done when CLI methods are parsed.

And that also means that for example airflow --help - and any other command - might take much longer - even 100-200 ms is a lot for those. Not sure how to address it easily though.

[jason810496]

There is also a problem with performance - potentially. Currenlty the list of commands available is built partially dynamically basd on configured executor - and that means that ProvidersManager and PluginsManager and discovery of avaialble providers, Hooks, Connections etc. does not have to be done when CLI methods are parsed.

And that also means that for example airflow --help - and any other command - might take much longer - even 100-200 ms is a lot for those. Not sure how to address it easily though.

Thanks for highlighting the potential performance issue! I will run benchmarks after adding the CliDefinitionLoader to measure both the overhead of adding provider-module-level CLI definitions and the speedup gained by splitting out CLI definitions.