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

Helm chart: add gateway.backend.imagePullSecrets #606

Merged
merged 3 commits into from
Oct 9, 2022

Conversation

maxime1907
Copy link
Contributor

@maxime1907 maxime1907 commented Sep 19, 2022

Proposal

Currently, dask-gateway retrieves images that are hosted on public registries.
This behavior is fine but if we want to use private registries, it is only possible through worker_extra_pod_config and scheduler_extra_pod_config.
I have then tought of a way to implement it properly without introducing any breaking change.

Implementation

  • Add a KubeClusterConfig configuration field called image_pull_secrets that lets dask scheduler and worker pods to pull images from private registries.
  • Change the helm chart to add support for the configuration field called image_pull_secrets thanks to the following field:
    • imagePullSecrets: Image pull secrets to access private registries

Behind the scene

From the user and dask operator POV, it doesn't have any impact on the current behavior of dask-gateway.


@maxime1907 maxime1907 force-pushed the imagepullsecrets branch 2 times, most recently from cdd54ec to 535bc92 Compare September 19, 2022 13:19
@maxime1907
Copy link
Contributor Author

cc @consideRatio

@consideRatio
Copy link
Collaborator

Thanks @maxime1907! Note though that you have opened a PR, and there is already another PR about this opened.

As #583 currently doesn't have the key part of the PR, making the controller Python logic create the scheduler and worker pods based on the configuration. Let's go for getting this PR merged.

Ping @chadsr and @szachovy from #583.

@consideRatio consideRatio changed the title feat: gateway: backend: support imagePullSecrets Helm chart: add gateway.backend.imagePullSecrets Oct 7, 2022
@consideRatio
Copy link
Collaborator

Also, I see that there is descriptions in the schema file about tag, pullPolicy, or pullSecrets, could you delete the pullSecrets part as that is not correct and semi-related to this PR that implements the imagePullSecrets field?

Copy link
Collaborator

@consideRatio consideRatio left a comment

Choose a reason for hiding this comment

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

With the suggested changes, this LGTM.

@consideRatio
Copy link
Collaborator

@maxime1907 I applied the change suggestions, if this looks good to you, let's go for a merge!

@maxime1907
Copy link
Contributor Author

@maxime1907 I applied the change suggestions, if this looks good to you, let's go for a merge!

Thank you a lot!!
Yes you can merge like this 🙂

@consideRatio consideRatio merged commit 8397cc6 into dask:main Oct 9, 2022
@consideRatio
Copy link
Collaborator

Thank you @maxime1907, @chadsr, and @szachovy for your contributions towards this!!! ❤️ 🎉 🌻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Helm Chart: gateway.backend Missing "imagePullSecrets"
2 participants