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

Support ...image.pull[Policy|Secrets] consistently and hook into JupyterHub's chart wide configuration for imagePullSecrets #1444

Merged
merged 2 commits into from
Nov 2, 2022

Conversation

emrahkaya
Copy link
Contributor

@emrahkaya emrahkaya commented Jan 18, 2022

Fixing the unused image.pullSecrets value in the binderhub Deployment.

Update by Erik

Thanks to jupyterhub/zero-to-jupyterhub-k8s#2546, we can hook into z2jh's chart wide configuration of imagePullSecrets. With this PR, we do that for all our pods where we specify an image.

I also added support for image.pullPolicy which wasn't supported in practice even though the schema supported it.

@emrahkaya
Copy link
Contributor Author

Our tests here doesn't cover this case, but I've tested this my system, using a private registry.

In my tests, I have put this step in the test.yml:

      - name: Create an image pull secret
        run: |
          kubectl create secret docker-registry acr-secret \
            --namespace default \
            --docker-server="${{ secrets.DOCKER_REGISTRY }}" \
            --docker-username="${{ secrets.DOCKER_USERNAME }}" \
            --docker-password="${{ secrets.DOCKER_PASSWORD }}"

and modified testing/k8s-binder-k8s-hub/binderhub-chart-config.yaml file, adding

image:
  pullSecrets:
    - acr-secret

there.

Such a test might not apply here, but I'm putting this information for anyone interested using/testing it (or a proof for my tests :) ).

@consideRatio consideRatio changed the title Use image.pullSecrets in binderhub Deployment Support specifying binder pod's imagePullSecrets via image.pullSecrets or jupyterhub.imagePull[Secret|Secrets] Oct 19, 2022
emrahkaya and others added 2 commits October 20, 2022 01:20
Fixing the unused image.pullSecrets value in the binderhub Deployment.
With this we allow this chart's pods to rely on jupyterhub's chart wide
configuration options `jupyterhub.imagePullSecret` and
`jupyterhub.imagePullSecrets`.

With this, we also would support `...image.pullSecrets` for specific
pods in this helm chart, as well as `...image.pullPolicy`.
@consideRatio consideRatio changed the title Support specifying binder pod's imagePullSecrets via image.pullSecrets or jupyterhub.imagePull[Secret|Secrets] Support ...image.pull[Policy|Secrets] consistently and hook into JupyterHub's chart wide configuration for imagePullSecrets Oct 19, 2022
@consideRatio consideRatio requested a review from manics October 19, 2022 23:46
@minrk minrk merged commit 5099ba7 into jupyterhub:main Nov 2, 2022
@welcome
Copy link

welcome bot commented Nov 2, 2022

Congrats on your first merged pull request in this project! 🎉
congrats
Thank you for contributing, we are very proud of you! ❤️

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

Successfully merging this pull request may close these issues.

3 participants