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

CSI Hostpath Driver & VolumeSnapshots addons #8461

Merged

Conversation

11janci
Copy link
Contributor

@11janci 11janci commented Jun 12, 2020

Fixes #8343
Fixes #8815

Adds the csi-hostpath-driver and volumesnapshots addons to minikube. Splitting it into two separate addons gives the users the flexibility to configure the CSI driver of their choice (should they prefer a different driver than CSI Hostpath), without having to set up volume snapshots by themselves.

The change is split into three logical commits - it might be easier to review it as such:

  1. volumesnapshots addon
  2. csi-hostpath-driver addon
  3. documentation and integration test

@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. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 12, 2020
@k8s-ci-robot
Copy link
Contributor

Hi @11janci. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jun 12, 2020
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 11janci
To complete the pull request process, please assign josedonizetti
You can assign the PR to them by writing /assign @josedonizetti 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:

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

@codecov-commenter
Copy link

Codecov Report

Merging #8461 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #8461   +/-   ##
=======================================
  Coverage   34.05%   34.05%           
=======================================
  Files         153      153           
  Lines        9840     9840           
=======================================
  Hits         3351     3351           
  Misses       6086     6086           
  Partials      403      403           

@11janci 11janci changed the title [WIP] Add support for VolumeSnapshots Add support for VolumeSnapshots Jun 17, 2020
@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 Jun 17, 2020
@11janci
Copy link
Contributor Author

11janci commented Jun 17, 2020

/assign @josedonizetti

@11janci 11janci force-pushed the jjanik-enable-volumesnapshots branch 2 times, most recently from d2c8b8f to 6d175ba Compare July 4, 2020 14:12
@11janci
Copy link
Contributor Author

11janci commented Jul 4, 2020

/assign @medyagh

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 8, 2020
@11janci 11janci force-pushed the jjanik-enable-volumesnapshots branch from 6d175ba to 19b8123 Compare July 11, 2020 09:29
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 11, 2020
@11janci 11janci force-pushed the jjanik-enable-volumesnapshots branch from 19b8123 to fbb2341 Compare July 14, 2020 09:32
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.

Hey @11janci thank you for your contribution! There are two things I'd like to add to this PR:

  1. Documentation around what this addon does and how to use it
  2. An integration test to validate the addon works

You can write the documentation as volume-snapshot.md here: https://github.com/kubernetes/minikube/tree/master/site/content/en/docs/tutorials

For the integration test --

The test would:

  1. Start a minikube cluster, and enable the addon by adding the --addons=<addon name> flag (Sample code here)
  2. Makes sure the installation was successful (as in, some expected pod came up as Running) (sample code here)
  3. Try to snapshot a volume and make sure it worked as expected

You can add TestVolumeSnapshotAddon to our addons_test.go file.

A lot of the code from TestAddons can be copied into TestVolumeSnapshotAddon (I linked some key lines above).

Docs for quickly running an integration test locally can be found here. Please let me know if you have any questions!

@@ -415,10 +415,42 @@ var Addons = map[string]*Addon{
MustBinAsset(
"deploy/addons/ambassador/ambassadorinstallation.yaml",
vmpath.GuestAddonsDir,
"ambassadorinstallation.yaml.yaml",
"ambassadorinstallation.yaml",

Choose a reason for hiding this comment

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

nice catch!

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 22, 2020
@11janci 11janci force-pushed the jjanik-enable-volumesnapshots branch from fbb2341 to 3d1f666 Compare July 23, 2020 17:39
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 23, 2020
@11janci 11janci changed the title Add support for VolumeSnapshots [WIP] Add support for VolumeSnapshots Jul 29, 2020
@k8s-ci-robot 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 29, 2020
@11janci 11janci force-pushed the jjanik-enable-volumesnapshots branch from 3d1f666 to 7e4bcc7 Compare August 6, 2020 15:28
@11janci 11janci force-pushed the jjanik-enable-volumesnapshots branch 2 times, most recently from 810d62c to abceb71 Compare August 12, 2020 18:20
@11janci
Copy link
Contributor Author

11janci commented Aug 12, 2020

@priyawadhwa thank you for your comments, the PR is now ready for review. Pls see the updated description

@11janci 11janci force-pushed the jjanik-enable-volumesnapshots branch from abceb71 to aa80902 Compare August 31, 2020 18:45
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.

Thanks for adding the integration test! I've left a few small nits to fix. I'll kick off the integration tests and let's see how they do.

test/integration/addons_test.go Outdated Show resolved Hide resolved
site/content/en/docs/tutorials/volume_snapshots_and_csi.md Outdated Show resolved Hide resolved
site/content/en/docs/tutorials/volume_snapshots_and_csi.md Outdated Show resolved Hide resolved
test/integration/helpers_test.go Outdated Show resolved Hide resolved
test/integration/helpers_test.go Outdated Show resolved Hide resolved
pkg/minikube/assets/addons.go Outdated Show resolved Hide resolved
@priyawadhwa
Copy link

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 2, 2020
@11janci
Copy link
Contributor Author

11janci commented Sep 2, 2020

@priyawadhwa thanks for all the comments! I will fix the small issues right away.

Why did you choose to enable this addon by default? Typically, I prefer to keep the number of default addons to a minimum so that a standard cluster doesn't have extra CPU overhead, but if a lot of users will be needing this addon then it could make sense.

Basically the only reason was that it is enabled by default in standard k8s as well, so I thought I'll mirror that. I'll be happy to disable it if you'd prefer that.

And lastly, I see bunch of tests failed but they should be all unrelated. The new test passed.

@11janci 11janci force-pushed the jjanik-enable-volumesnapshots branch from aa80902 to 804b8aa Compare September 2, 2020 12:04
@priyawadhwa
Copy link

Hey @11janci yah, let's disable by default then!

Also, if both of those addons need to be enabled for them to work, I'd add a warning in as a validation. Something like:

<this addon> requires <that addon> to be enabled as well, please run:
    minikube addons enable <addon name>

@minikube-pr-bot
Copy link

kvm2 Driver
Times for minikube: 86.6s 84.0s 85.2s
Average time for minikube: 85.3s

Times for Minikube (PR 8461): 88.4s 89.1s 85.8s
Average time for Minikube (PR 8461): 87.8s

Averages Time Per Log

+--------------------------------+----------+--------------------+
|              LOG               | MINIKUBE | MINIKUBE (PR 8461) |
+--------------------------------+----------+--------------------+
| * minikube v1.12.3 on Debian   | 0.1s     | 0.1s               |
|                           9.11 |          |                    |
| * 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,    | 39.6s    | 40.6s              |
| Memory=3700MB, Disk=20000MB)   |          |                    |
| ...                            |          |                    |
| * Preparing Kubernetes v1.19.0 | 43.4s    | 44.0s              |
| on Docker 19.03.12 ...         |          |                    |
| * Verifying Kubernetes         | 1.8s     | 2.9s               |
| components...                  |          |                    |
| * Enabled addons:              | 0.4s     | 0.0s               |
| default-storageclass,          |          |                    |
| storage-provisioner            |          |                    |
| * Done! kubectl is now         | 0.1s     | 0.1s               |
| configured to use "minikube"   |          |                    |
|                                | 0.0s     | 0.0s               |
+--------------------------------+----------+--------------------+

docker Driver
Times for minikube: 44.2s 44.7s 55.0s
Average time for minikube: 48.0s

Times for Minikube (PR 8461): 55.4s 54.8s 44.9s
Average time for Minikube (PR 8461): 51.7s

Averages Time Per Log

+--------------------------------+----------+--------------------+
|              LOG               | MINIKUBE | MINIKUBE (PR 8461) |
+--------------------------------+----------+--------------------+
| * minikube v1.12.3 on Debian   | 0.1s     | 0.1s               |
|                           9.11 |          |                    |
| * Using the docker driver      | 0.0s     | 0.0s               |
| based on user configuration    |          |                    |
|                                | 34.3s    | 0.1s               |
|                                |          |                    |
| * Starting control plane node  |          |                    |
| minikube in cluster minikube   |          |                    |
| * Creating docker container    |          |                    |
| (CPUs=2, Memory=3700MB) ...    |          |                    |
| * Preparing Kubernetes v1.19.0 |          |                    |
| on Docker 19.03.8 ...          |          |                    |
| * Verifying Kubernetes         |          |                    |
| components...                  |          |                    |
| * Enabled addons:              |          |                    |
| default-storageclass,          |          |                    |
| storage-provisioner            |          |                    |
| * Done! kubectl is now         |          |                    |
| configured to use "minikube"   |          |                    |
|                                |          |                    |
+--------------------------------+----------+--------------------+

@11janci 11janci force-pushed the jjanik-enable-volumesnapshots branch 2 times, most recently from fa19319 to bad98aa Compare September 4, 2020 15:07
@TravisBuddy
Copy link

Travis tests have failed

Hey @11janci,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

1st Build

View build log

make test
which go-bindata || GO111MODULE=off GOBIN="/home/travis/gopath/bin" go get github.com/jteeuwen/go-bindata/...
PATH="/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.14.6.linux.amd64/bin:/home/travis/bin:/home/travis/bin:/home/travis/.local/bin:/usr/local/lib/jvm/openjdk11/bin:/opt/pyenv/shims:/home/travis/.phpenv/shims:/home/travis/perl5/perlbrew/bin:/home/travis/.nvm/versions/node/v8.12.0/bin:/home/travis/.rvm/gems/ruby-2.5.3/bin:/home/travis/.rvm/gems/ruby-2.5.3@global/bin:/home/travis/.rvm/rubies/ruby-2.5.3/bin:/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.11.1.linux.amd64/bin:/usr/local/maven-3.6.3/bin:/usr/local/cmake-3.12.4/bin:/usr/local/clang-7.0.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/travis/.rvm/bin:/home/travis/.phpenv/bin:/opt/pyenv/bin:/home/travis/.yarn/bin:/home/travis/gopath/bin" go-bindata -nomemcopy -o pkg/minikube/assets/assets.go -pkg assets deploy/addons/...
gofmt -s -w pkg/minikube/assets/assets.go
which go-bindata || GO111MODULE=off GOBIN="/home/travis/gopath/bin" go get github.com/jteeuwen/go-bindata/...
/home/travis/gopath/bin/go-bindata
PATH="/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.14.6.linux.amd64/bin:/home/travis/bin:/home/travis/bin:/home/travis/.local/bin:/usr/local/lib/jvm/openjdk11/bin:/opt/pyenv/shims:/home/travis/.phpenv/shims:/home/travis/perl5/perlbrew/bin:/home/travis/.nvm/versions/node/v8.12.0/bin:/home/travis/.rvm/gems/ruby-2.5.3/bin:/home/travis/.rvm/gems/ruby-2.5.3@global/bin:/home/travis/.rvm/rubies/ruby-2.5.3/bin:/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.11.1.linux.amd64/bin:/usr/local/maven-3.6.3/bin:/usr/local/cmake-3.12.4/bin:/usr/local/clang-7.0.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/travis/.rvm/bin:/home/travis/.phpenv/bin:/opt/pyenv/bin:/home/travis/.yarn/bin:/home/travis/gopath/bin" go-bindata -nomemcopy -o pkg/minikube/translate/translations.go -pkg translate translations/...
gofmt -s -w pkg/minikube/translate/translations.go
MINIKUBE_LDFLAGS="-X k8s.io/minikube/pkg/version.version=v1.13.0 -X k8s.io/minikube/pkg/version.isoVersion=v1.13.0 -X k8s.io/minikube/pkg/version.isoPath=minikube/iso -X k8s.io/minikube/pkg/version.gitCommitID="aa29b8fe3fa236b0c204a3c5db45cd21d16b4cbe" -X k8s.io/minikube/pkg/version.storageProvisionerVersion=v3" ./test.sh
= make lint =============================================================
golangci/golangci-lint info checking GitHub for tag 'v1.30.0'
golangci/golangci-lint info found version: 1.30.0 for v1.30.0/linux/amd64
golangci/golangci-lint info installed out/linters/golangci-lint
pkg/addons/config.go:180: File is not `goimports`-ed (goimports)
		name:      "csi-hostpath-driver",
		set:       SetBool,
pkg/addons/validations.go:20: File is not `goimports`-ed (goimports)
	"fmt"
Makefile:408: recipe for target 'lint-ci' failed
make[1]: *** [lint-ci] Error 1
= go mod ================================================================
unused golang.org/x/exp
ok
= boilerplate ===========================================================
ok
Makefile:292: recipe for target 'test' failed
make: *** [test] Error 4
TravisBuddy Request Identifier: 727408c0-eec0-11ea-9705-b7c481b3600a

@11janci 11janci force-pushed the jjanik-enable-volumesnapshots branch from bad98aa to b54e3b2 Compare September 4, 2020 15:11
@TravisBuddy
Copy link

Travis tests have failed

Hey @11janci,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

1st Build

View build log

make test
which go-bindata || GO111MODULE=off GOBIN="/home/travis/gopath/bin" go get github.com/jteeuwen/go-bindata/...
PATH="/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.14.6.linux.amd64/bin:/home/travis/bin:/home/travis/bin:/home/travis/.local/bin:/usr/local/lib/jvm/openjdk11/bin:/opt/pyenv/shims:/home/travis/.phpenv/shims:/home/travis/perl5/perlbrew/bin:/home/travis/.nvm/versions/node/v8.12.0/bin:/home/travis/.rvm/gems/ruby-2.5.3/bin:/home/travis/.rvm/gems/ruby-2.5.3@global/bin:/home/travis/.rvm/rubies/ruby-2.5.3/bin:/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.11.1.linux.amd64/bin:/usr/local/maven-3.6.3/bin:/usr/local/cmake-3.12.4/bin:/usr/local/clang-7.0.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/travis/.rvm/bin:/home/travis/.phpenv/bin:/opt/pyenv/bin:/home/travis/.yarn/bin:/home/travis/gopath/bin" go-bindata -nomemcopy -o pkg/minikube/assets/assets.go -pkg assets deploy/addons/...
gofmt -s -w pkg/minikube/assets/assets.go
which go-bindata || GO111MODULE=off GOBIN="/home/travis/gopath/bin" go get github.com/jteeuwen/go-bindata/...
/home/travis/gopath/bin/go-bindata
PATH="/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.14.6.linux.amd64/bin:/home/travis/bin:/home/travis/bin:/home/travis/.local/bin:/usr/local/lib/jvm/openjdk11/bin:/opt/pyenv/shims:/home/travis/.phpenv/shims:/home/travis/perl5/perlbrew/bin:/home/travis/.nvm/versions/node/v8.12.0/bin:/home/travis/.rvm/gems/ruby-2.5.3/bin:/home/travis/.rvm/gems/ruby-2.5.3@global/bin:/home/travis/.rvm/rubies/ruby-2.5.3/bin:/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.11.1.linux.amd64/bin:/usr/local/maven-3.6.3/bin:/usr/local/cmake-3.12.4/bin:/usr/local/clang-7.0.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/travis/.rvm/bin:/home/travis/.phpenv/bin:/opt/pyenv/bin:/home/travis/.yarn/bin:/home/travis/gopath/bin" go-bindata -nomemcopy -o pkg/minikube/translate/translations.go -pkg translate translations/...
gofmt -s -w pkg/minikube/translate/translations.go
MINIKUBE_LDFLAGS="-X k8s.io/minikube/pkg/version.version=v1.13.0 -X k8s.io/minikube/pkg/version.isoVersion=v1.13.0 -X k8s.io/minikube/pkg/version.isoPath=minikube/iso -X k8s.io/minikube/pkg/version.gitCommitID="bfb91a19d40544e31fa3ff9ed8ef86b82093e352" -X k8s.io/minikube/pkg/version.storageProvisionerVersion=v3" ./test.sh
= make lint =============================================================
golangci/golangci-lint info checking GitHub for tag 'v1.30.0'
golangci/golangci-lint info found version: 1.30.0 for v1.30.0/linux/amd64
golangci/golangci-lint info installed out/linters/golangci-lint
pkg/addons/config.go:180: File is not `goimports`-ed (goimports)
		name:      "csi-hostpath-driver",
		set:       SetBool,
pkg/addons/validations.go:20: File is not `goimports`-ed (goimports)
	"fmt"
Makefile:408: recipe for target 'lint-ci' failed
make[1]: *** [lint-ci] Error 1
= go mod ================================================================
unused golang.org/x/exp
ok
= boilerplate ===========================================================
ok
Makefile:292: recipe for target 'test' failed
make: *** [test] Error 4
TravisBuddy Request Identifier: 7128cea0-eec1-11ea-9705-b7c481b3600a

@11janci 11janci force-pushed the jjanik-enable-volumesnapshots branch from b54e3b2 to 78d135e Compare September 4, 2020 15:18
@TravisBuddy
Copy link

Travis tests have failed

Hey @11janci,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

1st Build

View build log

make test
which go-bindata || GO111MODULE=off GOBIN="/home/travis/gopath/bin" go get github.com/jteeuwen/go-bindata/...
PATH="/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.14.6.linux.amd64/bin:/home/travis/bin:/home/travis/bin:/home/travis/.local/bin:/usr/local/lib/jvm/openjdk11/bin:/opt/pyenv/shims:/home/travis/.phpenv/shims:/home/travis/perl5/perlbrew/bin:/home/travis/.nvm/versions/node/v8.12.0/bin:/home/travis/.rvm/gems/ruby-2.5.3/bin:/home/travis/.rvm/gems/ruby-2.5.3@global/bin:/home/travis/.rvm/rubies/ruby-2.5.3/bin:/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.11.1.linux.amd64/bin:/usr/local/maven-3.6.3/bin:/usr/local/cmake-3.12.4/bin:/usr/local/clang-7.0.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/travis/.rvm/bin:/home/travis/.phpenv/bin:/opt/pyenv/bin:/home/travis/.yarn/bin:/home/travis/gopath/bin" go-bindata -nomemcopy -o pkg/minikube/assets/assets.go -pkg assets deploy/addons/...
gofmt -s -w pkg/minikube/assets/assets.go
which go-bindata || GO111MODULE=off GOBIN="/home/travis/gopath/bin" go get github.com/jteeuwen/go-bindata/...
/home/travis/gopath/bin/go-bindata
PATH="/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.14.6.linux.amd64/bin:/home/travis/bin:/home/travis/bin:/home/travis/.local/bin:/usr/local/lib/jvm/openjdk11/bin:/opt/pyenv/shims:/home/travis/.phpenv/shims:/home/travis/perl5/perlbrew/bin:/home/travis/.nvm/versions/node/v8.12.0/bin:/home/travis/.rvm/gems/ruby-2.5.3/bin:/home/travis/.rvm/gems/ruby-2.5.3@global/bin:/home/travis/.rvm/rubies/ruby-2.5.3/bin:/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.11.1.linux.amd64/bin:/usr/local/maven-3.6.3/bin:/usr/local/cmake-3.12.4/bin:/usr/local/clang-7.0.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/travis/.rvm/bin:/home/travis/.phpenv/bin:/opt/pyenv/bin:/home/travis/.yarn/bin:/home/travis/gopath/bin" go-bindata -nomemcopy -o pkg/minikube/translate/translations.go -pkg translate translations/...
gofmt -s -w pkg/minikube/translate/translations.go
MINIKUBE_LDFLAGS="-X k8s.io/minikube/pkg/version.version=v1.13.0 -X k8s.io/minikube/pkg/version.isoVersion=v1.13.0 -X k8s.io/minikube/pkg/version.isoPath=minikube/iso -X k8s.io/minikube/pkg/version.gitCommitID="0cedadd0f7d441695a74a695175e6619be2fe2a3" -X k8s.io/minikube/pkg/version.storageProvisionerVersion=v3" ./test.sh
= make lint =============================================================
golangci/golangci-lint info checking GitHub for tag 'v1.30.0'
golangci/golangci-lint info found version: 1.30.0 for v1.30.0/linux/amd64
golangci/golangci-lint info installed out/linters/golangci-lint
pkg/addons/config.go:180: File is not `goimports`-ed (goimports)
		name:      "csi-hostpath-driver",
		set:       SetBool,
pkg/addons/validations.go:20: File is not `goimports`-ed (goimports)
	"fmt"
Makefile:408: recipe for target 'lint-ci' failed
make[1]: *** [lint-ci] Error 1
= go mod ================================================================
unused golang.org/x/exp
ok
= boilerplate ===========================================================
ok
Makefile:292: recipe for target 'test' failed
make: *** [test] Error 4
TravisBuddy Request Identifier: 3a464010-eec2-11ea-9705-b7c481b3600a

@TravisBuddy
Copy link

Travis tests have failed

Hey @11janci,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

1st Build

View build log

make test
which go-bindata || GO111MODULE=off GOBIN="/home/travis/gopath/bin" go get github.com/jteeuwen/go-bindata/...
PATH="/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.14.6.linux.amd64/bin:/home/travis/bin:/home/travis/bin:/home/travis/.local/bin:/usr/local/lib/jvm/openjdk11/bin:/opt/pyenv/shims:/home/travis/.phpenv/shims:/home/travis/perl5/perlbrew/bin:/home/travis/.nvm/versions/node/v8.12.0/bin:/home/travis/.rvm/gems/ruby-2.5.3/bin:/home/travis/.rvm/gems/ruby-2.5.3@global/bin:/home/travis/.rvm/rubies/ruby-2.5.3/bin:/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.11.1.linux.amd64/bin:/usr/local/maven-3.6.3/bin:/usr/local/cmake-3.12.4/bin:/usr/local/clang-7.0.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/travis/.rvm/bin:/home/travis/.phpenv/bin:/opt/pyenv/bin:/home/travis/.yarn/bin:/home/travis/gopath/bin" go-bindata -nomemcopy -o pkg/minikube/assets/assets.go -pkg assets deploy/addons/...
gofmt -s -w pkg/minikube/assets/assets.go
which go-bindata || GO111MODULE=off GOBIN="/home/travis/gopath/bin" go get github.com/jteeuwen/go-bindata/...
/home/travis/gopath/bin/go-bindata
PATH="/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.14.6.linux.amd64/bin:/home/travis/bin:/home/travis/bin:/home/travis/.local/bin:/usr/local/lib/jvm/openjdk11/bin:/opt/pyenv/shims:/home/travis/.phpenv/shims:/home/travis/perl5/perlbrew/bin:/home/travis/.nvm/versions/node/v8.12.0/bin:/home/travis/.rvm/gems/ruby-2.5.3/bin:/home/travis/.rvm/gems/ruby-2.5.3@global/bin:/home/travis/.rvm/rubies/ruby-2.5.3/bin:/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.11.1.linux.amd64/bin:/usr/local/maven-3.6.3/bin:/usr/local/cmake-3.12.4/bin:/usr/local/clang-7.0.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/travis/.rvm/bin:/home/travis/.phpenv/bin:/opt/pyenv/bin:/home/travis/.yarn/bin:/home/travis/gopath/bin" go-bindata -nomemcopy -o pkg/minikube/translate/translations.go -pkg translate translations/...
gofmt -s -w pkg/minikube/translate/translations.go
MINIKUBE_LDFLAGS="-X k8s.io/minikube/pkg/version.version=v1.13.0 -X k8s.io/minikube/pkg/version.isoVersion=v1.13.0 -X k8s.io/minikube/pkg/version.isoPath=minikube/iso -X k8s.io/minikube/pkg/version.gitCommitID="1d46ede70720eb78b166e3b363ce368a61560cf2" -X k8s.io/minikube/pkg/version.storageProvisionerVersion=v3" ./test.sh
= make lint =============================================================
golangci/golangci-lint info checking GitHub for tag 'v1.30.0'
golangci/golangci-lint info found version: 1.30.0 for v1.30.0/linux/amd64
golangci/golangci-lint info installed out/linters/golangci-lint
pkg/addons/config.go:180: File is not `goimports`-ed (goimports)
		name:      "csi-hostpath-driver",
		set:       SetBool,
pkg/addons/validations.go:20: File is not `goimports`-ed (goimports)
	"fmt"
Makefile:408: recipe for target 'lint-ci' failed
make[1]: *** [lint-ci] Error 1
= go mod ================================================================
unused golang.org/x/exp
ok
= boilerplate ===========================================================
ok
Makefile:292: recipe for target 'test' failed
make: *** [test] Error 4
TravisBuddy Request Identifier: adc44a50-eec2-11ea-9705-b7c481b3600a

@11janci 11janci force-pushed the jjanik-enable-volumesnapshots branch from 78d135e to b696eb6 Compare September 4, 2020 16:54
@11janci
Copy link
Contributor Author

11janci commented Sep 5, 2020

Hey @11janci yah, let's disable by default then!

Also, if both of those addons need to be enabled for them to work, I'd add a warning in as a validation. Something like:

@priyawadhwa all done & ready for review 🙂

@kubernetes kubernetes deleted a comment from minikube-pr-bot Sep 8, 2020
@sharifelgamal
Copy link
Collaborator

Thanks for this contribution!

@sharifelgamal sharifelgamal merged commit 070cbb9 into kubernetes:master Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add CSI Hostpath driver as addon to minikube feature-gates: add support for VolumeSnapshotDataSource
10 participants