fix: go module requirements for semantic versioning

[alex1989hu]

What type of PR is this?
/kind bug

What this PR does / why we need it:
Fix go module requirements for semantic versioning

Which issue(s) this PR fixes:
Fixes #239

Special notes for your reviewer:
As per reviewer's request, added the compatibility with dep (Go dependency management tool). TODO: fix hack/update-generated-code.sh
Does this PR introduce a user-facing change?:

Update package path to v2. Vendoring with dep depends on https://github.com/golang/dep/pull/1963 or the workaround described in v2/README.md.

[k8s-ci-robot]

Welcome @alex1989hu!

It looks like this is your first PR to kubernetes-csi/external-snapshotter 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-csi/external-snapshotter has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @alex1989hu. Thanks for your PR.

I'm waiting for a kubernetes-csi 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.

[alex1989hu]

/assign @xing-yang

[pohly]

/ok-to-test

[xing-yang]

This needs a release note as it is a breaking change.
Also we still need the "dep" compatibility symlinks as external-provisioner depends on external-snapshotter.

[alex1989hu]

This needs a release note as it is a breaking change.
Also we still need the "dep" compatibility symlinks as external-provisioner depends on external-snapshotter.

external-provisioner uses Go Modules and locked to v1.2 bba3584, please check it https://github.com/kubernetes-csi/external-provisioner/blob/master/go.mod#L10

Did I miss something during examining of external-provisioner/go.mod?

Furthermore, if we create symlink then we break Windows users.

[xing-yang]

Yeah, external-provisioner is probably ok for now, but it will break others who are using the existing path.

@pohly can you explain why csi-test needs to keep the symlink?

[pohly]

Xing Yang <[email protected]> writes:

Yeah, external-provisioner is probably ok for now, but it will break others who are using the existing path.

Anyone trying to import "v2.0.0" already got broken when that version was tagged. This PR just fixes that.

@pohly can you explain why csi-test needs to keep the symlink?

The rationale was that csi-test might get imported by a larger variety of different CSI drivers of which some perhaps still are doing vendoring with dep. For example, PMEM-CSI... however, since then we have also switched to "go mod" in PMEM-CSI. For external-snapshotter, the number of downstream projects is probably smaller and well-known (i.e. just external-provisioner).

[xing-yang]

Chatted with @msau42 . She also suggested to keep symlink for backward compatibility now as there are folks who use external-snapshotter repo.

[alex1989hu]

Chatted with @msau42 . She also suggested to keep symlink for backward compatibility now as there are folks who use external-snapshotter repo.

I have just added two additional commit with the requested modification

[xing-yang]

This one should be modified too:
https://github.com/kubernetes-csi/external-snapshotter/blob/master/hack/update-generated-code.sh#L30

[alex1989hu]

This one should be modified too:
https://github.com/kubernetes-csi/external-snapshotter/blob/master/hack/update-generated-code.sh#L30

Done 👍

[xing-yang]

This one should be modified too:
https://github.com/kubernetes-csi/external-snapshotter/blob/master/hack/update-generated-code.sh#L30

Done

On a second thought, since everything under v2 is thru a symlink, I think the files should be generated directly under pkg as before. Can you change this back?

I looked at the rest of the changes. They look good to me. Once you change this one back, I'll merge it. Thanks.

[alex1989hu]

This one should be modified too:
https://github.com/kubernetes-csi/external-snapshotter/blob/master/hack/update-generated-code.sh#L30

Done

On a second thought, since everything under v2 is thru a symlink, I think the files should be generated directly under pkg as before. Can you change this back?

I looked at the rest of the changes. They look good to me. Once you change this one back, I'll merge it. Thanks.

Honestly, currently I am not familiar with code-generator, but I made some quick tests.

I made the following scenarios:

Scenario 1

Drop commit fix: add v2 path for code-generator 87ae56e80d

Result: ❌

# bash -x hack/update-generated-code.sh
+ set -o errexit
+ set -o nounset
+ set -o pipefail
++ unset CDPATH
+++ dirname hack/update-generated-code.sh
++ cd hack/..
++ pwd
+ SCRIPT_ROOT=/external-snapshotter
++ cd /external-snapshotter
++ ls -d -1 ./vendor/k8s.io/code-generator
+ CODEGEN_PKG=./vendor/k8s.io/code-generator
+ bash ./vendor/k8s.io/code-generator/generate-groups.sh deepcopy,client,informer,lister github.com/kubernetes-csi/external-snapshotter/pkg/client github.com/kubernetes-csi/external-snapshotter/pkg/apis volumesnapshot:v1beta1 --go-header-file /external-snapshotter/hack/boilerplate.go.txt
Generating deepcopy funcs
F0120 14:31:30.799150    9534 main.go:82] Error: Failed making a parser: unable to add directory "github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1": unable to import "github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1": go/build: importGo github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1: exit status 1
go: finding github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1 latest
go: finding github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot latest
go: finding github.com/kubernetes-csi/external-snapshotter/pkg/apis latest
go: finding github.com/kubernetes-csi/external-snapshotter/pkg latest
go: finding github.com/kubernetes-csi/external-snapshotter v1.2.2
go: downloading github.com/kubernetes-csi/external-snapshotter v1.2.2
go: extracting github.com/kubernetes-csi/external-snapshotter v1.2.2
can't load package: package github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1: unknown import path "github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1": cannot find module providing package github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1

Scenario 2

Drop commits: feat: add Go dep compatibility e950e58028 + feat: add Go dep v2 README 5d9cbbcf6d, so there is no v2 physical path.
Result: ✅

Conclusion

Those are package path, not physical location of files.

[xing-yang]

Running update-generated-code.sh should be able re-generate everything under https://github.com/kubernetes-csi/external-snapshotter/tree/master/pkg/client. If I delete everything under pkg/client, it won't be re-generated any more. If we let everything to be generated under v2, then they are no longer symlinks.

[alex1989hu]

Okay, I will drop the last commit and force push the branch

[xing-yang]

So even if the last commit is dropped, I still have problem re-generate files under client. I have not found out the best solution yet. I wonder if v2/* should be the physical path while pkg and cmd should be symlinks.

[alex1989hu]

I dropped the last commit locally and I think code-generator's message is also connected to the original issue what this PR addressed.

can't load package: github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1

You add volumesnapshot/v1beta1 in 2.0.0 but 1.2.2 has volumesnapshot/v1alpha1. That is the main issue why I opened the PR.

# pwd
/go/src/github.com/kubernetes-csi/external-snapshotter
# bash -x hack/update-generated-code.sh
+ set -o errexit
+ set -o nounset
+ set -o pipefail
++ unset CDPATH
+++ dirname hack/update-generated-code.sh
++ cd hack/..
++ pwd
+ SCRIPT_ROOT=/go/src/github.com/kubernetes-csi/external-snapshotter
++ cd /go/src/github.com/kubernetes-csi/external-snapshotter
++ ls -d -1 ./vendor/k8s.io/code-generator
+ CODEGEN_PKG=./vendor/k8s.io/code-generator
+ bash ./vendor/k8s.io/code-generator/generate-groups.sh deepcopy,client,informer,lister github.com/kubernetes-csi/external-snapshotter/pkg/client github.com/kubernetes-csi/external-snapshotter/pkg/apis volumesnapshot:v1beta1 --go-header-file /go/src/github.com/kubernetes-csi/external-snapshotter/hack/boilerplate.go.txt
Generating deepcopy funcs
F0120 17:44:24.285299    5755 main.go:82] Error: Failed making a parser: unable to add directory "github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1": unable to import "github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1": go/build: importGo github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1: exit status 1
go: finding github.com/kubernetes-csi/external-snapshotter v1.2.2
go: downloading github.com/kubernetes-csi/external-snapshotter v1.2.2
go: extracting github.com/kubernetes-csi/external-snapshotter v1.2.2
can't load package: package github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1: module github.com/kubernetes-csi/external-snapshotter@latest found (v1.2.2), but does not contain package github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1

It is weird that go.sum has a new entry once code-generator fails:

diff --git a/go.sum b/go.sum
index c6aa7aff..d023d559 100644
--- a/go.sum
+++ b/go.sum
@@ -129,6 +129,7 @@ github.com/kubernetes-csi/csi-lib-utils v0.7.0 h1:t1cS7HTD7z5D7h9iAdjWuHtMxJPb9s
 github.com/kubernetes-csi/csi-lib-utils v0.7.0/go.mod h1:bze+2G9+cmoHxN6+WyG1qT4MDxgZJMLGwc7V4acPNm0=
 github.com/kubernetes-csi/csi-test v2.0.0+incompatible h1:ia04uVFUM/J9n/v3LEMn3rEG6FmKV5BH9QLw7H68h44=
 github.com/kubernetes-csi/csi-test v2.0.0+incompatible/go.mod h1:YxJ4UiuPWIhMBkxUKY5c267DyA0uDZ/MtAimhx/2TA0=
+github.com/kubernetes-csi/external-snapshotter v1.2.2 h1:OPXoJydNqkWjhLwJ20dSqOhkmUYcpm+CCO0pYm+C8Q8=
 github.com/mailru/easyjson v0.0.0-20160728113105-d5b7844b561a/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=
 github.com/mailru/easyjson v0.0.0-20190614124828-94de47d64c63/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=
 github.com/mailru/easyjson v0.0.0-20190626092158-b2ccc519800e/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=

It looks like Go Modules "ecosystem" wants to have those package with no success.

I think once we merge this in and tag the repository we will able to re-generate again.

My proposal is to drop the last commit as you suggested, tag the repository what @pohly said. Thoughts?

[xing-yang]

It looks like csi-test has been releasing directly from master without cutting a release-xxx branch.

External-snapshotter has release-2.0 branch. So once this change is merged, the normal procedure is to backport it to release-2.0 before cutting the 2.0.1 release.

[alex1989hu]

Rebased and dropped commit fix: add v2 path for code-generator

[xing-yang]

Can you reword the release notes following the release notes in kubernetes-csi/csi-test#232?
Also add a TODO in update-generated-code.sh that it needs to be fixed following this PR.

[xing-yang]

Can you also squash your commits into one?

[alex1989hu]

Squashed commits into one and added TODO section both into commit message and PR description.

[xing-yang]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alex1989hu, xing-yang

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 [xing-yang]

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