Pod 823/cache #1245
Pod 823/cache #1245
Conversation
bkneis
force-pushed
the
POD-823/cache
branch
from
dea14c3
to
1d91603
Compare
|
|
||
| return nil | ||
| // reload docker daemon | ||
| return exec.CommandContext(ctx, "pkill", "-HUP", "dockerd").Run() |
There was a problem hiding this comment.
is this the best way to reload the docker daemon? I was surprised docker did not have a command for this and I didn't think I could assume systemd was being used to manage docker
There was a problem hiding this comment.
this looks scary 😬
There was a problem hiding this comment.
can't think of a better way to do it though, unless we'd start this build container with already mounted config? is that even possible?
There was a problem hiding this comment.
I agree, maybe I could try to detect how dockerd is managed (systemd,systemctl etc) using uname and if not available use this as a last resort? Yes that is possible, I thought of the mounted config but this would be provider specific and need to be implemented by each machine provider, gcloud, aws, digital ocean etc. :/ At least with HUP it's a reload and not restart but it's a bit "under the hood"
|
LGTM! |
|
@bkneis approved with comments, feel free to ignore if not applicable |
|
@pascalbreuninger fab thanks! Just finishing testing with a remote k8s cluster on GKE then it should be ready to merge. Do I need to update any file to reference the new version of the kubernetes driver? i.e. loft-sh/devpod-provider-kubernetes#55 |
|
@bkneis nope, that's done by releasing a new version of the kubernetes provider over in the other repo👍 |
|
@pascalbreuninger just finished testing with GKE 🥳 It was my auto pilot cluster giving me issues and killing workspaces with a return code 137 (OOM). Using the new GKE cluster workspaces are spinning up no problem and using the cache. In the end I didn't need to make any changes to the kubernetes driver, only dockerless so this PR is good to go IMO |
This PR contains the changes to facilitate the use of remote caching via a registry to speed up build times. It supports docker and kubernetes (via kaniko) and uses the context options REGISTRY_CACHE as the registry url. Caching options are not exposed currently in order to ensure the expected functionality.
The PR was tested using the following workflow:
Without cache
ghcr.io/devcontainers/features/github-cli:1as a feature to devcontainer.json 2m45secho "test" > examples/build/app/test) 2m52sWith cache
ghcr.io/devcontainers/features/github-cli:1as a feature to devcontainer.json 1m25secho "test" > examples/build/app/test) 13sAs you can see step 1 is almost the same, since no cache has been made yet. Step 2 however is interesting, we saved around 50% of the build time with the cache but 1m16s was used to upload the cache back to the registry. This shows that it is important for us to either omit the
--cache-toparameter forup(but notbuild), or suppress pushing of the cache manifest until the end of the command in the background once the workspace / IDE has already launched.Also note that I am using my local docker / kind cluster and a remote registry, when the registry is closer to the devcontainer I would expect even greater savings in build time due to shorter download times.
Lastly we have step 3 that simulates a common problematic workflow, which is a user updating the build context then
uping a workspace. Here we see significant savings where only building the last layer is needed, instead of the entire image due to a cache miss.EDIT: I have now implemented a boolean
ExportCachein toggle the--cache-toparameter, we now only upload the cache when runningbuild, notup. This providesupeven faster start times of workspaces as we don't wait until the cache is uploaded.Also note for machine providers I needed to enable the containerd snapshotter flag in the docker daemon, this was done during
initWorkspace. For non machine providers like local docker we expect the user to enable this themselves. When trying to use the remote cache without this a WARNING will be printed from docker.NOTE: We need to merge loft-sh/dockerless#27 first and update the dockerless release tag in single.go
Lastly, I have updated the
CalculatePrebuildHashfunction to traverse the parsed Dockerfile and extract any file paths that could affect the build context. I then filter for these files as an "include" list before adding the path to the hashed contents. This should cause fewer cache misses and allow developers to reuse similar images.