Adding optional imagePullSecrets for proxy Image #1391
Conversation
|
Don't think the failing tests were due to my PR, i just used master and the tests don't actually consider this case, using a custom image with credentials so it's basically as if my code is not used at all on the test |
|
To give some context, this PR concerns (for our purposes resolves) the issue I filed the other day: #1383 (me and @hdimitriou work on the same effort). |
|
Please review this code again |
Sync with upstream 3 Nov 2019
|
While this PR mirrors an existing chart behavior for the hub pod that I've been part implementing, I now favor another approach: could you make this PR like https://github.com/jupyterhub/zero-to-jupyterhub-k8s/pull/1426/files instead? It feels like the most sustainable thing to support in the long run. |
|
I arranged the code according to zero-to-jupyterhub-k8s/issues/1467 giving the users the ability to leverage existing imagePullSecrets |
|
Ping @consideRatio |
|
Thank you for your work on this @hdimitrou! ❤️ 🌻 Could you make this PR not add another k8s Secret resource, but instead just reference a self-created k8s secret, as done in #1426? This chart is struggling with its own complexity, and adding another k8s Secret resource specifically for the proxy pod would add more than I think is justified. |
|
On my PR I support both secret reference & proxy-specific definition. If I understand well you are asking me to remove the proxy-specific definition and only let the user define a secret set externally to the chart. Isn't it worse to have inconsistency between hub that supports both methods and proxy, than the complexity of supporting a proxy-specific secret too? It would make more sense if we removed them all together, though I wonder if the effort is justified |
|
Ah... I thought we only had this for the singleuserimage currently. Deal let's add it. I'd like to make some adjustments, to reuse templates instead of duplicate them etc. Do I have rights to add commits to this PR? |
|
The proxy imagePullSecret wasn't referenced as it needed to be, and there was a past logic issue where you would only use one of secrets if multiple ones were available.
|
Arrrgh travis doesn't show up as it should. |
|
@hdimitriou can you help me review the code I submitted? |
|
Yes, I will review hopefully today |
|
Sorry but it's currently overwhelming for me to check this out and review, the logic has shifted a lot since I initially worked on it. |
|
Not really sure the failure is due to this PR, or due to master. Will trigger again in a couple of days hopefully |
commit to trigger rerun
|
Is there any update on this? I am trying to deploy jupyterhub in an enterprise environment which requires a private docker image repository. Seems like this (proxy) and the scheduler still don't support imagePullSecrets. Also, if there is any way that I could do it manually I would be open to that. |
|
@hdimitriou first of all thank you so much for your work and activity in this project. Considering @jfb74's question on workarounds to this, I found this blog post. That got me thinking. I think it is become quite complicated to duplicate the logic to create an image puller secret, and also to ask the users to specify separate image puller secrets multiple times over, for the hub pod, proxy pod, image puller pods, user-server-pods, autohttps pod, etc. Due to this, what would make sense to do? Well I think to make the imagePullSecrets a chart-global configuration instead. Then we configure it once, and all pods get that imagePullSecrets field added, rather than just the hub and/or singleuser pods. I think this is the only sensible solution. Since this has been open for quite some time, and has merges with master etc, and there is little code to reuse, I intent to open a new PR and add you (@hdimitriou) as a co-author of my commits to reflect your work towards this. Summary
Is this approach okay to you @hdimitriou ? |
|
I've now opened #1794 and jupyterhub/kubespawner#442 that together is to address this and is therefore closing this pr. |
I noticed that 'hub' image gives you the opportunity to overwrite and use a custom along with docker registry credentials, so I did the same for proxy too