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

[kubernetes] Use KUBECONFIG if set #1716

Merged
merged 3 commits into from
Feb 5, 2024

Conversation

leogargu
Copy link
Contributor

@leogargu leogargu commented Feb 5, 2024

Closes #1715

Not sure where to document this (it'd be useful for folks setting up their CI pipelines) - any ideas?

elif os.getenv("KUBERNETES_SERVICE_HOST"):
# We are inside a pod, authenticate via ServiceAccount assigned to us
config.load_incluster_config()
else:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this might be a backwards incompatible change and will break many currently well functioning flows. do we expect KUBECONFIG env var to be always available?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be made backward compatible if the original call to config.load_kube_config in the else case is retained instead of throwing the KubernetesException.

…t, default to loading kubeconfig to keep current behaviour
@leogargu leogargu requested a review from savingoyal February 5, 2024 18:58
Copy link
Collaborator

@savingoyal savingoyal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a comment in the code base reflecting why this change is needed - it will help in preserving the semantics going forward

@leogargu leogargu requested a review from savingoyal February 5, 2024 19:25
@savingoyal savingoyal merged commit 7240235 into Netflix:master Feb 5, 2024
32 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Service token file does not exist" error when deploying flow to Argo from CI
4 participants