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

Image load: Allow loading local images from tar or cache #10807

Merged
merged 14 commits into from
Mar 25, 2021

Conversation

afbjorklund
Copy link
Collaborator

Now can load both from daemon/remote and from tarball:

minikube image load image
minikube image load image.tar

To be on the safe side, one should use qualified names:

minikube image load docker.io/image
minikube image load ./image.tar

Also allows for loading more than one image at a time...

Closes #10744

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 13, 2021
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 13, 2021
Previously only allowed loading from daemon or remote
via the cache, and not directly from local tarball.
They need to all be cached or all be local, though.
If one appears to be local, then assume that all are.
@medyagh
Copy link
Member

medyagh commented Mar 23, 2021

@afbjorklund please rebase, so we could include in this release for end of march

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 23, 2021
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 23, 2021
@medyagh
Copy link
Member

medyagh commented Mar 23, 2021

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Mar 23, 2021
@medyagh medyagh changed the title Allow loading local images as well as from cache Image load: Allow loading local images from tar or cache Mar 23, 2021
@medyagh
Copy link
Member

medyagh commented Mar 23, 2021

@afbjorklund this pr does not change the behavior of Image Load to not load to all profiles, right ?

Please clarify,
Currently we have a —all flag for cache add Is cache being addded to all profiles only when —all is set ?

(It would be great if our integration test would have covered it(

@kubernetes kubernetes deleted a comment from minikube-pr-bot Mar 23, 2021
@kubernetes kubernetes deleted a comment from minikube-pr-bot Mar 23, 2021
@minikube-pr-bot
Copy link

kvm2 Driver
Times for minikube: 67.6s 64.5s 63.2s
Average time for minikube: 65.1s

Times for Minikube (PR 10807): 65.7s 66.6s 63.8s
Average time for Minikube (PR 10807): 65.4s

Averages Time Per Log

+--------------------------------------------+----------+---------------------+
|                    LOG                     | MINIKUBE | MINIKUBE (PR 10807) |
+--------------------------------------------+----------+---------------------+
| * minikube v1.18.1 on Debian               | 0.1s     | 0.1s                |
| 9.11 (kvm/amd64)                           |          |                     |
| * Using the kvm2 driver based              | 0.0s     | 0.0s                |
| on user configuration                      |          |                     |
| * Starting control plane node              | 0.0s     | 0.0s                |
| minikube in cluster minikube               |          |                     |
| * Creating kvm2 VM (CPUs=2,                | 40.2s    | 39.7s               |
| Memory=3700MB, Disk=20000MB)               |          |                     |
| ...                                        |          |                     |
| * Preparing Kubernetes v1.20.2             | 2.1s     | 8.6s                |
| on Docker 20.10.3 ...                      |          |                     |
|   - Generating certificates                | 3.9s     | 3.2s                |
| and keys ...                               |          |                     |
|   - Booting up control plane               | 15.6s    | 10.6s               |
| ...                                        |          |                     |
|   - Configuring RBAC rules ...             | 1.3s     | 1.1s                |
| * Verifying Kubernetes                     | 0.1s     | 0.1s                |
| components...                              |          |                     |
|   - Using image                            | 1.2s     | 1.4s                |
| gcr.io/k8s-minikube/storage-provisioner:v4 |          |                     |
| * Enabled addons:                          | 0.6s     | 0.3s                |
| storage-provisioner,                       |          |                     |
| default-storageclass                       |          |                     |
| * Done! kubectl is now                     | 0.0s     | 0.0s                |
| configured to use "minikube"               |          |                     |
| cluster and "default"                      |          |                     |
| namespace by default                       |          |                     |
+--------------------------------------------+----------+---------------------+

docker Driver
Times for minikube: 27.2s 28.5s 34.3s
Average time for minikube: 30.0s

Times for Minikube (PR 10807): 28.1s 28.1s 36.6s
Average time for Minikube (PR 10807): 30.9s

Averages Time Per Log

+--------------------------------------------+----------+---------------------+
|                    LOG                     | MINIKUBE | MINIKUBE (PR 10807) |
+--------------------------------------------+----------+---------------------+
| * minikube v1.18.1 on Debian               | 0.2s     | 0.2s                |
| 9.11 (kvm/amd64)                           |          |                     |
| * Using the docker driver                  | 0.1s     | 0.1s                |
| based on user configuration                |          |                     |
| * Starting control plane node              | 0.1s     | 0.1s                |
| minikube in cluster minikube               |          |                     |
| * Creating docker container                | 11.7s    | 12.2s               |
| (CPUs=2, Memory=3700MB) ...                |          |                     |
| * Preparing Kubernetes v1.20.2             | 16.5s    | 16.1s               |
| on Docker 20.10.3 ...                      |          |                     |
| * Verifying Kubernetes                     | 0.1s     | 0.1s                |
| components...                              |          |                     |
|   - Using image                            | 1.3s     | 2.1s                |
| gcr.io/k8s-minikube/storage-provisioner:v4 |          |                     |
| * Enabled addons:                          | 0.1s     | 0.1s                |
| storage-provisioner,                       |          |                     |
| default-storageclass                       |          |                     |
| * Done! kubectl is now                     | 0.0s     | 0.0s                |
| configured to use "minikube"               |          |                     |
| cluster and "default"                      |          |                     |
| namespace by default                       |          |                     |
+--------------------------------------------+----------+---------------------+

@afbjorklund
Copy link
Collaborator Author

afbjorklund commented Mar 23, 2021

this pr does not change the behavior of Image Load to not load to all profiles, right ?

It is hardcoded to only load to the current profile. []*config.Profile{profile})

                profile, err := config.LoadProfile(viper.GetString(config.ProfileName))

There are no changes to the "cache" commands, only renamed some internal functions.

Added a "Cache" to the functions that wrap the cache dir, and Load does the load directly.

@minikube-pr-bot
Copy link

kvm2 Driver
Times for minikube: 67.7s 66.5s 64.3s
Average time for minikube: 66.2s

Times for Minikube (PR 10807): 64.6s 66.1s 65.6s
Average time for Minikube (PR 10807): 65.4s

Averages Time Per Log

+--------------------------------------------+----------+---------------------+
|                    LOG                     | MINIKUBE | MINIKUBE (PR 10807) |
+--------------------------------------------+----------+---------------------+
| * minikube v1.18.1 on Debian               | 0.1s     | 0.0s                |
| 9.11 (kvm/amd64)                           |          |                     |
| * Using the kvm2 driver based              | 0.0s     | 0.0s                |
| on user configuration                      |          |                     |
| * Starting control plane node              | 0.0s     | 0.0s                |
| minikube in cluster minikube               |          |                     |
| * Creating kvm2 VM (CPUs=2,                | 40.4s    | 39.8s               |
| Memory=3700MB, Disk=20000MB)               |          |                     |
| ...                                        |          |                     |
| * Preparing Kubernetes v1.20.2             | 9.2s     | 9.2s                |
| on Docker 20.10.3 ...                      |          |                     |
|   - Generating certificates                | 2.9s     | 2.7s                |
| and keys ...                               |          |                     |
|   - Booting up control plane               | 10.1s    | 10.3s               |
| ...                                        |          |                     |
|   - Configuring RBAC rules ...             | 1.2s     | 1.3s                |
| * Verifying Kubernetes                     | 0.1s     | 0.1s                |
| components...                              |          |                     |
|   - Using image                            | 1.4s     | 1.5s                |
| gcr.io/k8s-minikube/storage-provisioner:v4 |          |                     |
| * Enabled addons:                          | 0.9s     | 0.2s                |
| storage-provisioner,                       |          |                     |
| default-storageclass                       |          |                     |
| * Done! kubectl is now                     | 0.0s     | 0.0s                |
| configured to use "minikube"               |          |                     |
| cluster and "default"                      |          |                     |
| namespace by default                       |          |                     |
+--------------------------------------------+----------+---------------------+

docker Driver
Times for minikube: 29.5s 36.4s 29.9s
Average time for minikube: 32.0s

Times for Minikube (PR 10807): 30.7s 27.8s 29.8s
Average time for Minikube (PR 10807): 29.4s

Averages Time Per Log

+--------------------------------------------+----------+---------------------+
|                    LOG                     | MINIKUBE | MINIKUBE (PR 10807) |
+--------------------------------------------+----------+---------------------+
| * minikube v1.18.1 on Debian               | 0.2s     | 0.5s                |
| 9.11 (kvm/amd64)                           |          |                     |
| * Using the docker driver                  | 0.1s     | 0.1s                |
| based on user configuration                |          |                     |
| * Starting control plane node              | 0.1s     | 0.1s                |
| minikube in cluster minikube               |          |                     |
| * Creating docker container                | 14.2s    | 12.0s               |
| (CPUs=2, Memory=3700MB) ...                |          |                     |
| * Preparing Kubernetes v1.20.2             | 15.9s    | 15.5s               |
| on Docker 20.10.3 ...                      |          |                     |
| * Verifying Kubernetes                     | 0.1s     | 0.1s                |
| components...                              |          |                     |
|   - Using image                            | 1.3s     | 1.1s                |
| gcr.io/k8s-minikube/storage-provisioner:v4 |          |                     |
| * Enabled addons:                          | 0.1s     | 0.1s                |
| storage-provisioner,                       |          |                     |
| default-storageclass                       |          |                     |
| * Done! kubectl is now                     | 0.0s     | 0.0s                |
| configured to use "minikube"               |          |                     |
| cluster and "default"                      |          |                     |
| namespace by default                       |          |                     |
+--------------------------------------------+----------+---------------------+

@medyagh
Copy link
Member

medyagh commented Mar 23, 2021

the cache related tests are failing
https://storage.googleapis.com/minikube-builds/logs/10807/e35057a/Docker_Linux.html#fail_TestFunctional%2fserial%2fCacheCmd%2fcache%2fadd_remote


TestFunctional/serial/CacheCmd/cache/add_remote | 1.4
-- | --
52 | TestFunctional/serial/CacheCmd/cache/add_local | 1.85
53 | TestFunctional/serial/CacheCmd/cache/delete_k8s.gcr.io/pause:3.3 | 0.09
55 | TestFunctional/serial/CacheCmd/cache/verify_cache_inside_node | 0.37
56 | TestFunctional/serial/CacheCmd/cache/cache_reload | 1.44
57 | TestFunctional/serial/CacheCmd/cache/delete


@medyagh medyagh self-requested a review March 23, 2021 22:49
Copy link
Member

@medyagh medyagh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@afbjorklund
Copy link
Collaborator Author

afbjorklund commented Mar 24, 2021

plz check the cache tests failures

it didn't fall back to remote correctly

It was supposed to try the local daemon first, and if that fails then also try the remote registry.

But make it possible to select which to try, and error out if not selecting any of the methods....

@afbjorklund afbjorklund requested a review from medyagh March 24, 2021 12:50
@minikube-pr-bot
Copy link

kvm2 Driver
Times for minikube: 65.3s 65.9s 68.1s
Average time for minikube: 66.4s

Times for Minikube (PR 10807): 66.5s 65.5s 64.5s
Average time for Minikube (PR 10807): 65.5s

Averages Time Per Log

+--------------------------------------------+----------+---------------------+
|                    LOG                     | MINIKUBE | MINIKUBE (PR 10807) |
+--------------------------------------------+----------+---------------------+
| * minikube v1.18.1 on Debian               | 0.0s     | 0.0s                |
| 9.11 (kvm/amd64)                           |          |                     |
| * Using the kvm2 driver based              | 0.0s     | 0.0s                |
| on user configuration                      |          |                     |
| * Starting control plane node              | 0.0s     | 0.0s                |
| minikube in cluster minikube               |          |                     |
| * Creating kvm2 VM (CPUs=2,                | 40.4s    | 39.8s               |
| Memory=3700MB, Disk=20000MB)               |          |                     |
| ...                                        |          |                     |
| * Preparing Kubernetes v1.20.2             | 8.9s     |                     |
| on Docker 20.10.4 ...                      |          |                     |
|   - Generating certificates                | 3.3s     | 3.1s                |
| and keys ...                               |          |                     |
|   - Booting up control plane               | 10.4s    | 10.1s               |
| ...                                        |          |                     |
|   - Configuring RBAC rules ...             | 1.1s     | 1.1s                |
| * Verifying Kubernetes                     | 0.1s     | 0.1s                |
| components...                              |          |                     |
|   - Using image                            | 1.3s     | 1.3s                |
| gcr.io/k8s-minikube/storage-provisioner:v4 |          |                     |
| * Enabled addons:                          | 0.9s     |                     |
| default-storageclass,                      |          |                     |
| storage-provisioner                        |          |                     |
| * Done! kubectl is now                     | 0.0s     | 0.0s                |
| configured to use "minikube"               |          |                     |
| cluster and "default"                      |          |                     |
| namespace by default                       |          |                     |
+--------------------------------------------+----------+---------------------+

docker Driver
Times for minikube: 28.9s 27.2s 26.3s
Average time for minikube: 27.4s

Times for Minikube (PR 10807): 31.3s 27.7s 28.3s
Average time for Minikube (PR 10807): 29.1s

Averages Time Per Log

+--------------------------------------------+----------+---------------------+
|                    LOG                     | MINIKUBE | MINIKUBE (PR 10807) |
+--------------------------------------------+----------+---------------------+
| * minikube v1.18.1 on Debian               | 0.2s     | 0.2s                |
| 9.11 (kvm/amd64)                           |          |                     |
| * Using the docker driver                  | 0.1s     | 0.1s                |
| based on user configuration                |          |                     |
| * Starting control plane node              | 0.1s     | 0.1s                |
| minikube in cluster minikube               |          |                     |
| * Creating docker container                | 10.4s    | 10.9s               |
| (CPUs=2, Memory=3700MB) ...                |          |                     |
| * Preparing Kubernetes v1.20.2             | 15.4s    | 16.4s               |
| on Docker 20.10.3 ...                      |          |                     |
| * Verifying Kubernetes                     | 0.1s     | 0.1s                |
| components...                              |          |                     |
|   - Using image                            | 1.1s     | 1.3s                |
| gcr.io/k8s-minikube/storage-provisioner:v4 |          |                     |
| * Enabled addons:                          | 0.1s     | 0.1s                |
| storage-provisioner,                       |          |                     |
| default-storageclass                       |          |                     |
| * Done! kubectl is now                     | 0.0s     | 0.0s                |
| configured to use "minikube"               |          |                     |
| cluster and "default"                      |          |                     |
| namespace by default                       |          |                     |
+--------------------------------------------+----------+---------------------+

@afbjorklund
Copy link
Collaborator Author

@medyagh : There is also some bogus message about "cache" being replaced by "image load" ?

! "minikube cache" will be deprecated in upcoming versions, please switch to "minikube image load"

Even though image doesn't have any "memory".

So you have to remember to load them again

@minikube-pr-bot
Copy link

kvm2 Driver
Times for minikube: 68.2s 65.7s 65.7s
Average time for minikube: 66.5s

Times for Minikube (PR 10807): 64.8s 66.2s 65.7s
Average time for Minikube (PR 10807): 65.6s

Averages Time Per Log

+--------------------------------------------+----------+---------------------+
|                    LOG                     | MINIKUBE | MINIKUBE (PR 10807) |
+--------------------------------------------+----------+---------------------+
| * minikube v1.18.1 on Debian               | 0.1s     | 0.0s                |
| 9.11 (kvm/amd64)                           |          |                     |
| * Using the kvm2 driver based              | 0.0s     | 0.0s                |
| on user configuration                      |          |                     |
| * Starting control plane node              | 0.0s     | 0.0s                |
| minikube in cluster minikube               |          |                     |
| * Creating kvm2 VM (CPUs=2,                | 41.2s    | 40.7s               |
| Memory=3700MB, Disk=20000MB)               |          |                     |
| ...                                        |          |                     |
| * Preparing Kubernetes v1.20.2             | 8.9s     | 15.9s               |
| on Docker 20.10.4 ...                      |          |                     |
|   - Generating certificates                | 2.6s     | 1.2s                |
| and keys ...                               |          |                     |
|   - Booting up control plane               | 10.3s    | 4.9s                |
| ...                                        |          |                     |
|   - Configuring RBAC rules ...             | 1.2s     | 0.8s                |
| * Verifying Kubernetes                     | 0.1s     | 0.1s                |
| components...                              |          |                     |
|   - Using image                            | 1.4s     | 1.3s                |
| gcr.io/k8s-minikube/storage-provisioner:v4 |          |                     |
| * Enabled addons:                          | 1.0s     | 0.6s                |
| storage-provisioner,                       |          |                     |
| default-storageclass                       |          |                     |
| * Done! kubectl is now                     | 0.0s     | 0.0s                |
| configured to use "minikube"               |          |                     |
| cluster and "default"                      |          |                     |
| namespace by default                       |          |                     |
+--------------------------------------------+----------+---------------------+

docker Driver
Times for minikube: 28.2s 33.2s 31.5s
Average time for minikube: 31.0s

Times for Minikube (PR 10807): 29.2s 31.4s 34.3s
Average time for Minikube (PR 10807): 31.6s

Averages Time Per Log

+--------------------------------------------+----------+---------------------+
|                    LOG                     | MINIKUBE | MINIKUBE (PR 10807) |
+--------------------------------------------+----------+---------------------+
| * minikube v1.18.1 on Debian               | 0.3s     | 0.2s                |
| 9.11 (kvm/amd64)                           |          |                     |
| * Using the docker driver                  | 0.1s     | 0.1s                |
| based on user configuration                |          |                     |
| * Starting control plane node              | 0.1s     | 0.1s                |
| minikube in cluster minikube               |          |                     |
| * Creating docker container                | 12.2s    | 13.8s               |
| (CPUs=2, Memory=3700MB) ...                |          |                     |
| * Preparing Kubernetes v1.20.2             | 16.9s    | 15.9s               |
| on Docker 20.10.3 ...                      |          |                     |
| * Verifying Kubernetes                     | 0.1s     | 0.1s                |
| components...                              |          |                     |
|   - Using image                            | 1.2s     | 1.4s                |
| gcr.io/k8s-minikube/storage-provisioner:v4 |          |                     |
| * Enabled addons:                          | 0.1s     | 0.1s                |
| storage-provisioner,                       |          |                     |
| default-storageclass                       |          |                     |
| * Done! kubectl is now                     | 0.0s     | 0.0s                |
| configured to use "minikube"               |          |                     |
| cluster and "default"                      |          |                     |
| namespace by default                       |          |                     |
+--------------------------------------------+----------+---------------------+

@medyagh
Copy link
Member

medyagh commented Mar 25, 2021

@medyagh : There is also some bogus message about "cache" being replaced by "image load" ?

! "minikube cache" will be deprecated in upcoming versions, please switch to "minikube image load"

Even though image doesn't have any "memory".

So you have to remember to load them again

yeah saw that, thats not very good, we gotta remove that

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afbjorklund, medyagh

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,medyagh]

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

@medyagh medyagh merged commit 77c6de3 into kubernetes:master Mar 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. pr_verified size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The new load command still uses the cache directory
4 participants