WIP: Check if storage driver is overlay2 before preload

[afbjorklund]

For #7626

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afbjorklund

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [afbjorklund]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[codecov-io]

Codecov Report

Merging #7628 into master will decrease coverage by 0.04%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #7628      +/-   ##
==========================================
- Coverage   36.49%   36.44%   -0.05%     
==========================================
  Files         147      147              
  Lines        9139     9150      +11     
==========================================
  Hits         3335     3335              
- Misses       5416     5427      +11     
  Partials      388      388              

Impacted Files	Coverage Δ
pkg/minikube/download/preload.go	0.00% <0.00%> (ø)

[medyagh]

lets use the oci.DaemonInfo for this (add the storage driver to it)
(we can later make that a cached call so we dont have to keep calling it)

also lets try to download the image for that driver
if that driver doesnt exist then dont preload
(we have the storage driver in the filename) so that shoudl make it easy.

so in the future if we mmake preload images for that storage driver we can suppor it
#7626 (comment)

[medyagh]

here is the link for DaemonInfo

I believe u could update the struct with the new format, so the Storage Driver be unmarshalized into it

[afbjorklund]

I just copied the code from the generator

https://github.com/kubernetes/minikube/blob/master/hack/preload-images/preload_images.go#L82

[afbjorklund]

The current code is just to stop minikube from killing docker (the current behavior)

Adding a working cache for non-overlayfs2 requires more work, and might not be doable
(for instance with some drivers - like dm, the actual content was kept on another partition)

So it was not part of this PR. Could use the so called "oci" DaemonInfo, though.

[medyagh]

The current code is just to stop minikube from killing docker (the current behavior)

Adding a working cache for non-overlayfs2 requires more work, and might not be doable
(for instance with some drivers - like dm, the actual content was kept on another partition)

So it was not part of this PR. Could use the so called "oci" DaemonInfo, though.

I agree adding new storage should not be part of this PR,
what I meant was
we should check for the existance of the URL (which includes the storage driver in the filename) if the URL (with correct storage driver) doesnt exist then we will skip preload.

[afbjorklund]

Need to make sure it is using the correct docker daemon, when determining storage driver...

In this case, the none driver was using aufs but the docker driver was using overlay2

[medyagh]

Need to make sure it is using the correct docker daemon, when determining storage driver...

In this case, the none driver was using aufs but the docker driver was using overlay2

do you mean pointing to the Docker on the user's system and not inside minikube ? if thats what you mean, we have a func "PointToHostDockerDaemon()" that we run during init

do you mean the Docker inside Docker should use the same storage driver as the Host ? (if yes why ? is it there evidence it doesnt work ? )

[afbjorklund]

do you mean the Docker inside Docker should use the same storage driver as the Host ? (if yes why ? is it there evidence it doesnt work ? )

It is fine if we hardcode the internal storage driver, but the external one could be anything...

[tstromberg]

What do we need to move this PR forward?

[afbjorklund]

What do we need to move this PR forward?

I got confused half-way through the implementation...

I thought that we could continue with hardcoding the internal storage driver (to overlay2) and let the user choose whatever external storage driver they want... But it turns out that if you pick an odd outer storage, that could limit your internal options (to something basic like vfs). So we probably need to re-visit this, and have it silently fallback to not use any preloads in such cases ?

[tstromberg]

Closing until we're sure what to do with this. Please feel free to /reopen once clarity can be achieved.