Vamsi-klu commented on code in PR #61935:
URL: https://github.com/apache/airflow/pull/61935#discussion_r2810491346
##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/hooks/kubernetes.py:
##########
@@ -57,6 +58,13 @@
LOADING_KUBE_CONFIG_FILE_RESOURCE = "Loading Kubernetes configuration file
kube_config from {}..."
+# AWS CLI cache directory that can trigger a race condition (FileExistsError)
+# when multiple processes attempt to create it concurrently during exec-based
+# authentication (e.g. ``aws eks get-token``). Pre-creating it avoids the
race.
+# See: https://github.com/apache/airflow/issues/60943
+# https://github.com/boto/botocore/commit/f1c1bc90
+_AWS_CLI_CACHE_DIR = os.path.join(os.path.expanduser("~"), ".aws", "cli",
"cache")
Review Comment:
Thanks for the review @jscheffl, and fair point. I see your comment on
#61936 as well and I agree the K8s provider should stay cloud agnostic.
The reason this ended up here is that the race condition hits any user whose
kubeconfig uses aws eks get-token as an exec plugin, even if they never touch
EksHook or the AWS provider directly. A lot of users just configure a plain
Kubernetes connection with a kubeconfig that happens to have AWS exec auth, so
putting the fix only in the AWS provider would leave those users unprotected.
That said, I can see a few ways to address your concern and wanted to get
your preference before moving forward.
Option 1: Move entirely into the AWS provider
Put the fix in EksHook so the K8s package stays completely clean. The trade
off is that plain KubernetesHook users with EKS kubeconfigs won't benefit
unless they switch hooks or upgrade botocore themselves.
Option 2: Generic pre connect hook mechanism
Add a small entry point based extension in KubernetesHook.get_conn() that
any provider can register callbacks with. The K8s provider would have zero
cloud specific code, it just calls whatever pre connect hooks are registered.
The AWS provider then registers the cache directory pre creation through that
entry point. This protects all users regardless of which hook they use and is
extensible for other providers too.
Option 3: Keep here but remove all AWS naming
Strip out the AWS branding from constants and function names, frame it as
generic exec plugin cache directory pre creation. The directory path is what it
is, but the code itself would read as cloud neutral.
Would any of these work for you, or do you have a different preference?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
