Add hack/update-goimports.sh

[johnSchnake]

What type of PR is this?
/kind cleanup

What this PR does / why we need it:
When doing large refactors, if you move functionality from
one package to another, it may be necessary to update the
imports of a large number of files. Rather than require
this to be a manual process, this script automates it.

In this initial commit, there are also a number of imports
updated as aliases. This is something goimports does as a
result of the real path differing from the import path
(due to symlinks in the repo).

Which issue(s) this PR fixes:
In support of:
ref #76206
ref #76728

Special notes for your reviewer:
This specifically came up when I was trying to work on #76728 which required updates to the imports of hundreds of files.

In this PR I simply added the update-goimports.sh file (modeled on update-gofmt.sh) and then ran it once.

Does this PR introduce a user-facing change?:

NONE

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: johnSchnake
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: lavalamp

If they are not already assigned, you can assign the PR to them by writing /assign @lavalamp in a comment when ready.

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:

• cluster/gce/gci/OWNERS
• cmd/OWNERS
• hack/OWNERS
• pkg/OWNERS
• plugin/pkg/admission/OWNERS
• plugin/pkg/auth/OWNERS
• staging/OWNERS
• test/OWNERS

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

[johnSchnake]

Obviously a large diff but again, the only thing I did was add the script and run goimports just like update-gofmt.sh.

To sanity check the changes I also ran:

git diff upstream/master | grep "^[+-]"| grep -v \+\+\+ | grep -v \\-\\-\\-| sort | uniq

Obviously, you can look at each but this was helpful to me. There are actually very few changes and they fall into a few categories:

• rearranging/moving imports based on goimports rules
• aliasing some imports
• the script I added

[johnSchnake]

So there is some gauth issue preventing some things from running (says the service is down)

but besides that there is some issue for me to resolve/consider. I wouldn't have expected bazel to need updated after this but doing so I sfound a fair number of changes like this:

-        "//pkg/apis/core/v1:go_default_library",
         "//pkg/security/podsecuritypolicy/util:go_default_library",
         "//staging/src/k8s.io/api/policy/v1beta1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
+        "//vendor/github.com/kubernetes/kubernetes/pkg/apis/core/v1:go_default_library",
     ],
 )

I guess it is picking up my local structure and using that instead of how it should be working.

[johnSchnake]

Confirmed that in some cases it did changes like this:

+	v1 "github.com/kubernetes/kubernetes/pkg/apis/authorization/v1"
...
	"k8s.io/kubernetes/pkg/apis/authorization/v1"

It added the local import path instead of the k8s.io one.

[johnSchnake]

Got the 'not something we can merge' error; retesting.

I think the fix for the vendor issue was to ensure go modules were getting used.

/retest

[johnSchnake]

not something we can merge...

/retest

[johnSchnake]

So using go modules fixed one set of problems but now I am running into an issue where goimports is redefining certain imports in addition to the one that is already there (ie adding the alias without removing the unalised version)

Could fix up a number of files manually but the whole point of the script is that that shouldn't be necessary.

[k8s-ci-robot]

@johnSchnake: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-bazel-test	0f1a3d4	link	/test pull-kubernetes-bazel-test
pull-kubernetes-typecheck	0f1a3d4	link	/test pull-kubernetes-typecheck
pull-kubernetes-integration	0f1a3d4	link	/test pull-kubernetes-integration
pull-kubernetes-verify	0f1a3d4	link	/test pull-kubernetes-verify
pull-kubernetes-e2e-gce-100-performance	0f1a3d4	link	/test pull-kubernetes-e2e-gce-100-performance

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

[neolit123]

the script and diff in the second commit LGTM (as much as github can show it).

So using go modules fixed one set of problems but now I am running into an issue where goimports is redefining certain imports in addition to the one that is already there (ie adding the alias without removing the unalised version)

sounds like a goimports bug.

[timothysc]

So we've never been strict about this behavior.

@ixdy @fejta - thoughts?

[johnSchnake]

I'm effectively suggesting adding this because I needed it and it seemed like a useful util if other people do refactor with a similarly large impact.

I effectively had so many files impacted that it made sense to script it, but then I hit all the other aliases and it made the diff even larger so I split it out into this PR.

If we'd rather not put something like this explicitly in the code base I understand and will just try and leverage it to accomplish my goal without running goimports against all these other files.

[caesarxuchao]

/remove-sig api-machinery

[k8s-ci-robot]

@johnSchnake: PR needs rebase.

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.

[draveness]

Confirmed that in some cases it did changes like this:

+	v1 "github.com/kubernetes/kubernetes/pkg/apis/authorization/v1"
...
	"k8s.io/kubernetes/pkg/apis/authorization/v1"

It added the local import path instead of the k8s.io one.

Did you place the kubernetes directory in go/src/github.com/kubernetes? If you did, maybe you could solve it by moving it into go/src/k8s.io/ instead.

I ran into the same problem with import error after running the script....

$  cat staging/src/k8s.io/client-go/kubernetes/typed/core/v1/fake/fake_namespace_expansion.go
// ...
package fake

import (
	"k8s.io/api/core/v1"
	v1 "k8s.io/api/core/v1"
	core "k8s.io/client-go/testing"
)

[timothysc]

/assign @timothysc @ixdy

[johnSchnake]

Going to close this since it has too much surface area to be feasible at this time, let alone the bugs that I seemed to run into.

I will probably try smaller endeavors to just run goimports on swathes of the codebase which are of interest. Anyone is welcome to follow suit.