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

[BUG] make manager changes source code of project #51

Closed
allencloud opened this issue Jun 27, 2019 · 9 comments · Fixed by #54
Closed

[BUG] make manager changes source code of project #51

allencloud opened this issue Jun 27, 2019 · 9 comments · Fixed by #54
Assignees
Labels
kind/bug Something isn't working

Comments

@allencloud
Copy link
Member

What happened:
make manager changes source code of project:

What you expected to happen:
make manager does no change to source code

How to reproduce it (as minimally and precisely as possible):

kruise (master) $ make manager
go generate ./pkg/... ./cmd/...
go fmt ./pkg/... ./cmd/...
pkg/controller/statefulset/stateful_inplace_utils_test.go
go vet ./pkg/... ./cmd/...
go build -o bin/manager github.com/openkruise/kruise/cmd/manager
kruise (master) $ git status
On branch master
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

        modified:   pkg/controller/statefulset/stateful_inplace_utils_test.go

no changes added to commit (use "git add" and/or "git commit -a")
kruise (master) $ git diff pkg/controller/statefulset/stateful_inplace_utils_test.go
diff --git a/pkg/controller/statefulset/stateful_inplace_utils_test.go b/pkg/controller/statefulset/stateful_inplace_utils_test.go
index 84017390..f2b328c2 100644
--- a/pkg/controller/statefulset/stateful_inplace_utils_test.go
+++ b/pkg/controller/statefulset/stateful_inplace_utils_test.go
@@ -136,7 +136,7 @@ func TestShouldDoInPlaceUpdate(t *testing.T) {
                        },
                },
                {
-                       name:                  "podUpdatePolicy is not InPlaceIfPossible",
+                       name: "podUpdatePolicy is not InPlaceIfPossible",
                        rollingUpdateStrategy: &appsv1alpha1.RollingUpdateStatefulSetStrategy{},
                        updateRevision: &apps.ControllerRevision{
                                Data: runtime.RawExtension{Raw: []byte(`{"spec":{"template":{"$patch":"replace","spec":{"containers":[{"name":"c1","image":"foo2"}]}}}}`)},
kruise (master) $ 

Anything else we need to know?:
none

Environment:

  • Kruise version: master node
  • Kubernetes version (use kubectl version):
  • OS (e.g: cat /etc/os-release):
  • Kernel (e.g. uname -a):
  • Install tools:
  • Others:
@allencloud allencloud added the kind/bug Something isn't working label Jun 27, 2019
@jian-he
Copy link
Contributor

jian-he commented Jun 28, 2019

I tried to fix the format and run the "make manager" command, in fact, it will reset to current format.
is this because of some kind of format setting ?

@FillZpp
Copy link
Member

FillZpp commented Jun 28, 2019

I tried to run the command, nothing changed.

$ make manager
go generate ./pkg/... ./cmd/...
go fmt ./pkg/... ./cmd/...
go vet ./pkg/... ./cmd/...
go build -o bin/manager github.com/openkruise/kruise/cmd/manager
$ git status
On branch master
Your branch is up to date with 'origin/master'.

nothing to commit, working tree clean

@xiang90
Copy link
Contributor

xiang90 commented Jun 28, 2019

what is the go version on your machines? @allencloud @FillZpp

@FillZpp
Copy link
Member

FillZpp commented Jun 29, 2019

Go version on my machine go version go1.11.6 darwin/amd64.
Is your go version up to 1.12+ ? @allencloud

@allencloud
Copy link
Member Author

kruise (master) $ go version
go version go1.10.4 darwin/amd64
kruise (master) $

I am afraid it is only 1.10.4. Do we need a restriction that upgrade golang version to the 1.12+.
@xiang90 @FillZpp

@xiang90
Copy link
Contributor

xiang90 commented Jul 1, 2019

can we add a go version check in the make script? @jian-he @FillZpp @allencloud

@jian-he
Copy link
Contributor

jian-he commented Jul 1, 2019

we could add a warning in the script.
failing it would hurt user experience, IMO.
And this looks to be a common problem for all Golang projects

@FillZpp
Copy link
Member

FillZpp commented Jul 1, 2019

Common users will not use make, they should install kruise using helm install or kubectl apply.
I think it is ok to return a failure of go-version check to kruise developers.

@FillZpp
Copy link
Member

FillZpp commented Jul 1, 2019

@xiang90 @jian-he @allencloud Add a go version check #54

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants