Image-cleaner: mount docker dir instead of socket #2030
Conversation
| # cull images until it drops below 60% | ||
| imageGCThresholdHigh: 80 | ||
| imageGCThresholdLow: 60 | ||
| # cull images on the host docker as well as dind |
There was a problem hiding this comment.
I don't think it's ever done both
| - name: storage-{{ $builderName }} | ||
| hostPath: | ||
| path: {{ eq $builderName "dind" | ternary $builder.hostLibDir $builder.hostStorageDir }} | ||
| type: DirectoryOrCreate |
There was a problem hiding this comment.
The image-cleaner should never create Docker directories, that should be handled by Docker.
BinderHub/K8s has a long standing bug where sometimes the docker socket is created as a host directory.
manics
force-pushed
the
docker-socket-dir
branch
from
2a5fefc to
9f57d20
Compare
| type: boolean | ||
| description: | | ||
| DEPRECATED: use imageCleaner.enabled if the cleaner shall not used. | ||
| dockerSocket: |
There was a problem hiding this comment.
If we want to help people make a transition, this entry could remain allowed, and then if its ovserved in NOTES.txt, we would use the fail template command to error with a message.
We are already erroring if its specified, so this would just be a help to transition
There was a problem hiding this comment.
Type would be allowed to be null as well then, and then we would look for it to be set.
I recall z2jh have some deprecation and breaking change message template ready if you want to go for it
There was a problem hiding this comment.
Good point, done!
Shouldn't the hasKey check mean we don't need to make it nullable?`
|
🎉 nice! |
jupyterhub/binderhub#2030 Merge pull request #2030 from manics/docker-socket-dir
2 participants
BinderHub/K8s has a long standing bug where sometimes the docker socket is created as a host directory.
Closes #1230
This is a breaking change since
imageCleaner.host.dockerSocketis replaced byimageCleaner.host.dockerSocket{Dir,Name}.However given that dind has been the recommended way to run BinderHub for a quite a while I don't think it's worth the complexity of adding the Helm template logic to handle the old case.