Rebuild gvisor image for integration tests #4717

priyawadhwa · 2019-07-08T22:55:28Z

We should rebuild the gvisor image for integration tests, so that if
changes are made to the gvisor image they are tested. I added an
environment variable that, when set, will change the expected gvisor
image repo.

should unblock #4494

We should rebuild the gvisor image for integration tests, so that if changes are made to the gvisor image they are tested. I added an environment variable that, when set, will change the expected gvisor image repo.

medyagh · 2019-07-08T23:03:43Z

@minikube-bot OK to test

medyagh · 2019-07-08T23:04:35Z

pkg/minikube/constants/constants.go

@@ -430,6 +430,9 @@ const (
 	GvisorContainerdShimURL = "https://github.com/google/gvisor-containerd-shim/releases/download/v0.0.1-rc.0/gvisor-containerd-shim-v0.0.1-rc.0.linux-amd64"
 	// GvisorURL is the url to download gvisor
 	GvisorURL = "https://storage.googleapis.com/gvisor/releases/nightly/2018-12-07/runsc"
+
+	// MinikubeGvisorAddonImageRepo is the environment variable set by integration tests to change the gvisor image repo.
+	MinikubeGvisorAddonImageRepo = "MINIKUBE_GVISOR_ADDON_IMAGE_REPO"


do we need to add set this evn to hack/jenkins/ bash files?

I don't think so since I set the environment variable programatically & only for the gvisor tests.

ianlewis · 2019-07-08T23:32:38Z

pkg/minikube/assets/addons.go

 	}{
 		Arch:            a,
 		ExoticArch:      ea,
 		ImageRepository: cfg.ImageRepository,
+		GvisorImageRepo: os.Getenv(constants.MinikubeGvisorAddonImageRepo),


Would this not require normal addon users to set this environment variable? Seems like that would require an update to the docs. I'd prefer if it defaulted to cfg.ImageRepository

Since the default value is gcr.io/k8s-skaffold, users should never have to set the environment variable, since it should be empty. This way, they should always be using the latest version of the gvisor image.

I opted out of using cfg.ImageRepository because that repo would be applied to other images minikube pulls as well (storage provisioner, etc), which wouldn't exist.

This way, if the env variable is set (which should only happen in integration tests), then only in that case will a non-default gvisor image will be used.

I was thinking some folks would want to override the image repo globally. My thought was to use GvisorImageRepo if it is set, but use ImageRepository if it's not set (i.e. go back to current behavior)

ianlewis · 2019-07-09T00:37:06Z

pkg/minikube/assets/addons.go

 	}{
 		Arch:            a,
 		ExoticArch:      ea,
 		ImageRepository: cfg.ImageRepository,
+		GvisorImageRepo: os.Getenv(constants.MinikubeGvisorAddonImageRepo),


I was thinking some folks would want to override the image repo globally. My thought was to use GvisorImageRepo if it is set, but use ImageRepository if it's not set (i.e. go back to current behavior)

…to make sure that image is used

…rom common.sh

ianlewis · 2019-07-10T09:37:14Z

pkg/gvisor/enable.go

@@ -44,6 +44,7 @@ const (
 //   3. copies necessary containerd config files
 //   4. restarts containerd
 func Enable() error {
+	return fmt.Errorf("local image used correctly")


I assume this was for debugging? I would have thought that Go would complain about unreachable code when compiling.

Yup, I want to make sure minikube actually pulls the locally built gvisor image, so tests should fail with this error

Currently it fails with compile error so you may want to remove the code after this line if you are intentionally returning an error.

Where are you seeing the compile error? If I run make out/gvisor-addon locally the binary does compile, and running the docker image from make gvisor-addon-image gives me:

minikube$ docker run gcr.io/k8s-minikube/gvisor-addon:latest 2019/07/10 23:20:28 local image used correctly

Hmm, I'm seeing the error in Travis. Go's compiler complains if code is unreachable.
https://travis-ci.org/kubernetes/minikube/builds/557049237#L541

My bad. It was go vet the provides the error. In any case, the code afterwards isn't used.

hack/jenkins/common.sh

k8s-ci-robot · 2019-07-10T20:01:26Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: priyawadhwa
To complete the pull request process, please assign ra489
You can assign the PR to them by writing /assign @ra489 in a comment when ready.

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

Needs approval from an approver in each of these files:

OWNERS

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

which can be picked up during integration testing. I opted to do it this way because the locally built gvisor image wasn't being picked up by minikube, because the docker daemon wasn't configured, since minikube isn't up and running yet. Even if the docker daemon was configured to point to minikube, we wouldn't be able to build the gvisor-image from the test itself.

…mage

priyawadhwa · 2019-08-05T22:37:32Z

hey @ianlewis thanks for the ping! For a quick update: I've found a couple bugs while trying to fix this -- right now this is blocked on #4985, which is needed to make sure we are running the local version of the gvisor image to correctly test changes

… which doesn't load the image into the runtime

…er image

…gvisor-image

ianlewis · 2019-08-09T01:25:23Z

@priyawadhwa Thanks for the update! Sounds like we open a can of worms!

…mage

medyagh · 2019-08-16T01:11:27Z

PR looks good, only one test fails, but it might be a flake fail.

--- FAIL: TestContainerd (0.00s)
    --- FAIL: TestContainerd/GvisorRestart (1244.80s)
        containerd_test.go:59: waiting for gvisor controller to come up

I am gonna restart the KVM test
for furture reference the error is :

 22:41:54 | ! I0815 22:41:54.934127   24312 ssh_runner.go:101] SSH: sudo podman load -i /tmp/pause_3.1
 22:41:55 | ! W0815 22:41:55.145638   22225 exit.go:99] Wait failed: waiting for k8s-app=kube-proxy: timed out waiting for the condition
 22:41:55 | ! * 
 22:41:55 | ! X Wait failed: waiting for k8s-app=kube-proxy: timed out waiting for the condition
 22:41:55 | ! * 
 22:41:55 | ! * Sorry that minikube crashed. If this was unexpected, we would love to hear from you:
 22:41:55 | !   - https://github.com/kubernetes/minikube/issues/new/choose
 22:41:55 | > * Waiting for: apiserver proxy

medyagh · 2019-08-16T05:02:57Z

@priyawadhwa the PR looks good to me, are you okay with merging this PR ?

priyawadhwa · 2019-08-16T17:52:21Z

@medyagh yah I think this should be ok!

medyagh · 2019-08-16T19:25:02Z

awesome thank you for fixing this !

Rebuild gvisor image for integration tests

a8f1ed7

We should rebuild the gvisor image for integration tests, so that if changes are made to the gvisor image they are tested. I added an environment variable that, when set, will change the expected gvisor image repo.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 8, 2019

k8s-ci-robot requested review from balopat and RA489 July 8, 2019 22:55

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 8, 2019

priyawadhwa mentioned this pull request Jul 8, 2019

Update gvisor runsc version #4494

Merged

medyagh reviewed Jul 8, 2019

View reviewed changes

ianlewis suggested changes Jul 8, 2019

View reviewed changes

fix cmd directory

5688e17

ianlewis suggested changes Jul 9, 2019

View reviewed changes

build gvisor image locally in integration tests; add temporary error …

d2e5a4b

…to make sure that image is used

priyawadhwa force-pushed the gvisor-image branch from 0cff305 to aa9b344 Compare July 9, 2019 23:07

Upload gvisor-addon to integration tests bucket; build gvisor image f…

1a49eec

…rom common.sh

priyawadhwa force-pushed the gvisor-image branch from aa9b344 to 1a49eec Compare July 10, 2019 00:08

ianlewis suggested changes Jul 10, 2019

View reviewed changes

fix destination in common.sh

6086259

priyawadhwa changed the title ~~Rebuild gvisor image for integration tests~~ [wip] Rebuild gvisor image for integration tests Jul 10, 2019

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 10, 2019

Priya Wadhwa added 2 commits July 10, 2019 14:13

Point docker daemon to minikube and rebuild docker image

3ffe2af

priyawadhwa force-pushed the gvisor-image branch from 01ba8aa to 3ffe2af Compare July 10, 2019 23:14

Priya Wadhwa added 2 commits July 11, 2019 11:58

Attempt to copy gvisor tar and load with ctr

d68e054

Merge branch 'master' of github.com:kubernetes/minikube into gvisor-i…

1ec4012

…mage

priyawadhwa force-pushed the gvisor-image branch from 500cdf4 to 23d2739 Compare July 11, 2019 20:50

Priya Wadhwa added 2 commits August 5, 2019 13:53

rebased, fixed merge conflicts

24a2748

Updated gvisor test

cdbb2dd

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 5, 2019

priyawadhwa mentioned this pull request Aug 5, 2019

Pods can't find cached images in minikube with containerd #4985

Closed

Run "sudo ctr -n=k8s.io images import" instead of "ctr images import"…

ee4cbbb

… which doesn't load the image into the runtime

priyawadhwa force-pushed the gvisor-image branch from 46c1163 to ee4cbbb Compare August 8, 2019 18:13

No longer need to build out/gvisor-addon since it's built in the dock…

d5e4cc1

…er image

priyawadhwa force-pushed the gvisor-image branch 2 times, most recently from 11b51cc to 6cd9b58 Compare August 8, 2019 19:49

Remove error in gvisor image since it works now

94638de

priyawadhwa force-pushed the gvisor-image branch from 6cd9b58 to 94638de Compare August 8, 2019 19:52

Priya Wadhwa added 2 commits August 8, 2019 13:10

Merge branch 'master' of https://github.com/kubernetes/minikube into …

bd17652

…gvisor-image

gvisor-addon depends on generated files

a294bdb

Priya Wadhwa added 3 commits August 12, 2019 16:18

Update go.mod to work with daemon pkg in go-containerregistry

f708cfa

Merge branch 'master' of github.com:kubernetes/minikube into gvisor-i…

198d639

…mage

updated cache_image

c611cf8

priyawadhwa force-pushed the gvisor-image branch from d8da8a7 to c611cf8 Compare August 15, 2019 21:32

priyawadhwa changed the title ~~[wip] Rebuild gvisor image for integration tests~~ Rebuild gvisor image for integration tests Aug 16, 2019

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 Aug 16, 2019

medyagh merged commit 021ca86 into kubernetes:master Aug 16, 2019

priyawadhwa deleted the gvisor-image branch August 19, 2019 17:50

medyagh mentioned this pull request Aug 20, 2019

skip gvisor test setup on mac #5136

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rebuild gvisor image for integration tests #4717

Rebuild gvisor image for integration tests #4717

priyawadhwa commented Jul 8, 2019 •

edited

Loading

medyagh commented Jul 8, 2019

medyagh Jul 8, 2019

priyawadhwa Jul 9, 2019

ianlewis Jul 8, 2019

priyawadhwa Jul 9, 2019 •

edited

Loading

ianlewis Jul 9, 2019

ianlewis Jul 9, 2019

ianlewis Jul 10, 2019

priyawadhwa Jul 10, 2019

ianlewis Jul 10, 2019

priyawadhwa Jul 10, 2019

ianlewis Jul 11, 2019

ianlewis Jul 11, 2019

k8s-ci-robot commented Jul 10, 2019

priyawadhwa commented Aug 5, 2019

ianlewis commented Aug 9, 2019

medyagh commented Aug 16, 2019

medyagh commented Aug 16, 2019 •

edited

Loading

priyawadhwa commented Aug 16, 2019

medyagh commented Aug 16, 2019

Rebuild gvisor image for integration tests #4717

Rebuild gvisor image for integration tests #4717

Conversation

priyawadhwa commented Jul 8, 2019 • edited Loading

medyagh commented Jul 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

priyawadhwa Jul 9, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jul 10, 2019

priyawadhwa commented Aug 5, 2019

ianlewis commented Aug 9, 2019

medyagh commented Aug 16, 2019

medyagh commented Aug 16, 2019 • edited Loading

priyawadhwa commented Aug 16, 2019

medyagh commented Aug 16, 2019

priyawadhwa commented Jul 8, 2019 •

edited

Loading

priyawadhwa Jul 9, 2019 •

edited

Loading

medyagh commented Aug 16, 2019 •

edited

Loading