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

Offline: always transfer image if lookup fails, always download drivers #6111

Merged
merged 7 commits into from
Dec 19, 2019

Conversation

tstromberg
Copy link
Contributor

@tstromberg tstromberg commented Dec 17, 2019

This fixes two offline bugs. One found by user issue, and one found by adding an integration test for it.

  • If reference lookups fail due to lack of external connectivity or lack of a locker Docker daemon, transfer the images anyways. The integration test simulates this by providing a bad proxy and docker environment.

  • If --download-only is specified, we were not downloading drivers, so the subsequently airgapped run would fail.

Fixes #6103 #6116

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 17, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tstromberg

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 17, 2019
@codecov-io
Copy link

codecov-io commented Dec 17, 2019

Codecov Report

Merging #6111 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #6111      +/-   ##
=========================================
- Coverage    36.7%   36.7%   -0.01%     
=========================================
  Files         113     113              
  Lines        8355    8356       +1     
=========================================
  Hits         3067    3067              
- Misses       4887    4888       +1     
  Partials      401     401
Impacted Files Coverage Δ
pkg/minikube/machine/cache_images.go 4.34% <0%> (-0.04%) ⬇️
cmd/minikube/cmd/start.go 22.7% <0%> (+0.03%) ⬆️

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 17, 2019
@tstromberg tstromberg changed the title WIP: Always transfer images when local reference lookup fails Fix offline mode: always transfer image if lookup fails Dec 17, 2019
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 17, 2019
@tstromberg
Copy link
Contributor Author

/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 Dec 17, 2019
@minikube-bot
Copy link
Collaborator

All Times minikube: [ 127.055862 115.152244 125.869987]
All Times Minikube (PR 6111): [ 126.210117 125.469309 125.048183]

Average minikube: 122.692698
Average Minikube (PR 6111): 125.575870

Averages Time Per Log

+----------------------+-----------+--------------------+
|         LOG          | MINIKUBE  | MINIKUBE (PR 6111) |
+----------------------+-----------+--------------------+
| minikube v           |  0.281601 |           0.284042 |
| Creating kvm2        | 47.672722 |          47.207873 |
| Preparing Kubernetes | 48.597293 |          51.779932 |
| Pulling images       |  2.848315 |           3.417374 |
| Launching Kubernetes | 20.185430 |          19.937815 |
| Waiting for cluster  |  3.057201 |           2.897314 |
+----------------------+-----------+--------------------+

test/integration/offline_test.go Outdated Show resolved Hide resolved
@tstromberg tstromberg changed the title Fix offline mode: always transfer image if lookup fails Offline fixes: always transfer image if lookup fails, always download drivers Dec 18, 2019
@tstromberg tstromberg changed the title Offline fixes: always transfer image if lookup fails, always download drivers Offline: always transfer image if lookup fails, always download drivers Dec 18, 2019
@minikube-bot
Copy link
Collaborator

All Times minikube: [ 121.223707 123.956422 119.502534]
All Times Minikube (PR 6111): [ 109.120621 122.108481 120.232857]

Average minikube: 121.560888
Average Minikube (PR 6111): 117.153986

Averages Time Per Log

+----------------------+-----------+--------------------+
|         LOG          | MINIKUBE  | MINIKUBE (PR 6111) |
+----------------------+-----------+--------------------+
| minikube v           |  0.253499 |           0.243622 |
| Creating kvm2        | 45.905812 |          45.297773 |
| Preparing Kubernetes | 49.692484 |          47.528081 |
| Pulling images       |  3.203871 |           2.624211 |
| Launching Kubernetes | 19.899237 |          18.855639 |
| Waiting for cluster  |  2.559773 |           2.560995 |
+----------------------+-----------+--------------------+

Copy link

@priyawadhwa priyawadhwa left a comment

Choose a reason for hiding this comment

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

lgtm

@minikube-bot
Copy link
Collaborator

All Times minikube: [ 124.102523 119.469092 124.420490]
All Times Minikube (PR 6111): [ 123.091943 122.003560 124.030090]

Average minikube: 122.664035
Average Minikube (PR 6111): 123.041865

Averages Time Per Log

+----------------------+-----------+--------------------+
|         LOG          | MINIKUBE  | MINIKUBE (PR 6111) |
+----------------------+-----------+--------------------+
| minikube v           |  0.250627 |           0.245423 |
| Creating kvm2        | 46.270367 |          45.986086 |
| Preparing Kubernetes | 49.780346 |          50.386461 |
| Pulling images       |  2.967455 |           2.901823 |
| Launching Kubernetes | 20.632335 |          20.396361 |
| Waiting for cluster  |  2.715480 |           3.081527 |
+----------------------+-----------+--------------------+

@tstromberg tstromberg merged commit 4e0454d into kubernetes:master Dec 19, 2019
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. 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.

offline support broken: LoadImages makes HTTPS queries
7 participants