Implement Go quickstarter #259

michaelsauter · 2019-07-05T11:42:16Z

Closes #255.

FYI @metmajer Would love to have your input even though I cannot assign you as reviewer atm ...

metmajer · 2019-07-05T13:11:23Z

Much appreciated, @michaelsauter, will do shortly!

metmajer · 2019-07-05T14:47:07Z

jenkins-slaves/golang/Dockerfile.rhel7

+    curl -LO https://storage.googleapis.com/golang/go$GO_VERSION.linux-amd64.tar.gz && \
+    tar -C /usr/local -xzf go$GO_VERSION.linux-amd64.tar.gz && \
+    rm -f *.tar.gz && \
+    cd && \


@michaelsauter What's the point of the cd here? Without a parameter, this would just leave you where you are.

cd without params brings you to you home directory. But yea, this is pretty pointless here. I will update to cd -, which brings you back to where you where before the last cd. I think that's good because then the RUN command does not change where you are ...

Actually from a maintenance point, I would need to figure out where you are (because that is somewhere in the slave-base). My tendency would be to be as explicit as possible here (putting in an absolute path).

@michaelsauter got it, thanks!

@rattermeyer I'd consider using WORKDIR good practice (and is explicit).

WORKDIR is already set. I will use cd -, which I still think is best.

metmajer · 2019-07-05T15:02:03Z

Nice work! Also, a great way for me to learn how you've created a new quickstarter from scratch in a single change set. Apart from my quick review above, I would have some suggestions on improving the developer experience for Go developers for your consideration:

Integrate golangci-lint for linting
Integrate Go Modules. This may not be the final answer to dependency resolution with Go, but it allows developers to write code that depends on other packages outside of the $GOPATH, and seems to be the "way to go" for the moment.
Define GOPKGS=$(go list ./... | grep -v /vendor) and use go test -v $GOPKGS instead of go test ./.... Otherwise, you would run tests in vendored Go packages.

If you're interested, feel free to have a look at segmentio/terraform-docs/Makefile for further information. Also, I could point you to the relevant pull requests that implemented these features over there, if helpful.

tbugfinder · 2019-07-05T16:16:52Z

boilerplates/be-golang/files/main.go

+		fmt.Fprintln(w, "Hello, world!")
+	})
+
+	http.ListenAndServe(":8080", nil)


Nice to see that a valid app and Dockerfile is being built which will deploy without issues compared to https://github.com/opendevstack/ods-project-quickstarters/blob/master/boilerplates/be-docker-plain/files/docker/Dockerfile .

@tbugfinder .. File a Bug - and provide a PR ;-) See you uptaking ODS?

michaelsauter

@metmajer I will look into the suggested changes. While I personally like Go modules, I left it out since the example does not have any dependencies ... and I think it would make the initial setup more complicated, but I'll re-check.

michaelsauter · 2019-07-08T07:04:41Z

jenkins-slaves/golang/Dockerfile.rhel7

+    curl -LO https://storage.googleapis.com/golang/go$GO_VERSION.linux-amd64.tar.gz && \
+    tar -C /usr/local -xzf go$GO_VERSION.linux-amd64.tar.gz && \
+    rm -f *.tar.gz && \
+    cd && \


cd without params brings you to you home directory. But yea, this is pretty pointless here. I will update to cd -, which brings you back to where you where before the last cd. I think that's good because then the RUN command does not change where you are ...

metmajer · 2019-07-08T08:24:43Z

@michaelsauter oh, it does have some deps. Here's a link to the go.mod file.

michaelsauter · 2019-07-08T09:21:38Z

@metmajer Oh yes, your project does have dependencies, but the quickstarter I am building here does not :)

metmajer · 2019-07-08T09:35:33Z

@michaelsauter Ah, ok :) Here's the idea: if you incorporate a go mod vendor into the flow, Go Modules would create files go.mod and go.sum in your repo's root and vendorize all libraries into the vendor subdirectory. Vendorized packages are then to be checked into the source code repo.

So, even if there are no dependencies yet, GO111MODULE=on go mod vendor (Go Modules need to be turned on), everything would be done automatically with the huge benefit, that you can develop Go code in any directory, and not within $GOPATH/src only. Thoughts?

metmajer · 2019-07-12T07:46:57Z

@michaelsauter and reviewers. I'd like to use this quickstarter already to migrate some application components into it. How do you plan to proceed?

michaelsauter · 2019-07-12T07:53:57Z

I'm looking into the feedback now, and suggest to proceed with merging once I made some changes - we can still fine-tune later.

metmajer · 2019-07-12T08:32:42Z

Very nice, thanks @michaelsauter!

michaelsauter · 2019-07-12T08:33:52Z

@metmajer @rattermeyer I've done the following changes now:

Go back to previous dir (9174b89)
Add golangci-lint and a stage in Jenkins using it (a609463)
Run tests not over vendored packages (a609463). This allows users opting for vendoring to use the built-in stage instead of having to write a custom one.
Add a note about Go modules (d65507c). I did not already commit a go.mod file because a) this would require building a Go image inside Rundeck and b) we don't know the full module path. Also, it is unclear to me if we should recommend using vendoring with modules or not - personally I feel that vendoring is at least initially not required.

My take would be that this is good enough to go in, and we can fine-tune until this ships with 1.2. Agreed?

metmajer · 2019-07-12T08:35:44Z

I fully agree, @michaelsauter.

michaelsauter · 2019-07-12T09:08:04Z

After testing everything, I noticed that go test $GOPKGS does not work if you're outside GOPATH and have no go.mod file. My resolution now is to create a go.mod file manually with a bogus domain. I think this is good enough for now. I'd say we can revisit this when we move to the new quickstarter architecture - I assume we will assemble the repos in Jenkins on the base slaves, so we will have Go installed and we know the BitBucket repo, so we should be able to create a more meaningful go.mod file then.

@metmajer Unless you disagree with this new approach, I will merge.

metmajer · 2019-07-12T15:57:19Z

@michaelsauter let me have a quick local test. I'll provide final feedback by Monday morning. Looking forward to using it already :) Cheers!

metmajer · 2019-07-13T11:15:05Z

@michaelsauter as I understand it, the problem that you see is because we're running go mod init in a subdirectory of the git repository. IMHO, we should use the longer go mod init [module] expression and supply the name of the repository as the module name. Ideally, it would be whatever the user enters in the provisioning app. If that's currently not possible, let's use be-golang for the time being (subject to be changed by the user). Here's an example:

$ pwd
/tmp/ods-project-quickstarters/boilerplates/be-golang/files

$ go mod init
go: cannot determine module path for source directory /tmp/ods-project-quickstarters/boilerplates/be-golang/files (outside GOPATH, no import comments)

$ go mod init this-is-a-test
go: creating new go.mod: module this-is-a-test

$ cat go.mod
module this-is-a-test

go 1.12

$ GOPKGS=$(go list ./... | grep -v /vendor)
$ echo $GOPKGS
this-is-a-test

$ go test -v $GOPKGS
?   	this-is-a-test	[no test files]

A longer thread on your issue around go mod init can be found here: golang/go#27951.

LGTM to me otherwise!

michaelsauter · 2019-07-15T06:49:47Z

@metmajer No, I think you misunderstood the issue. I wanted to run go mod init [module], but it is unclear what module should be since we do not have a domain at this point. Simply running go mod init does not work either, because we do not have any VCS information at the time we would need to run it. Plus, there isn't even a go binary to execute in the first place, unless I build a container with this ...

Therefore, I arrived at writing the go.mod file manually for now, which allows exactly what you are looking after. We will revisit once we change the quickstarter structure.

metmajer · 2019-07-15T06:51:06Z

@michaelsauter aah, now I better understand where you're coming from. Thanks!

Implement Go quickstarter

a1fb0c2

michaelsauter added the enhancement New feature or request label Jul 5, 2019

michaelsauter requested review from hugowschneider and clemensutschig July 5, 2019 11:42

michaelsauter self-assigned this Jul 5, 2019

metmajer reviewed Jul 5, 2019

View reviewed changes

tbugfinder reviewed Jul 5, 2019

View reviewed changes

michaelsauter commented Jul 8, 2019

View reviewed changes

Michael Sauter added 3 commits July 12, 2019 10:10

Go back to previous directory

9174b89

Add linter and test only non-vendored packages

a609463

Add note about using Go modules

d65507c

Michael Sauter added 2 commits July 12, 2019 10:47

Fix linter errors

2197229

Create go.mod file

b01c080

michaelsauter merged commit 3440332 into opendevstack:master Jul 15, 2019

michaelsauter deleted the feature/go-quickstarter branch July 15, 2019 06:50

Implement Go quickstarter #259

Implement Go quickstarter #259

Uh oh!

Conversation

michaelsauter commented Jul 5, 2019

Uh oh!

metmajer commented Jul 5, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

metmajer commented Jul 5, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michaelsauter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

metmajer commented Jul 8, 2019

Uh oh!

michaelsauter commented Jul 8, 2019

Uh oh!

metmajer commented Jul 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

metmajer commented Jul 12, 2019

Uh oh!

michaelsauter commented Jul 12, 2019

Uh oh!

metmajer commented Jul 12, 2019

Uh oh!

michaelsauter commented Jul 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

metmajer commented Jul 12, 2019

Uh oh!

michaelsauter commented Jul 12, 2019

Uh oh!

metmajer commented Jul 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

metmajer commented Jul 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michaelsauter commented Jul 15, 2019

Uh oh!

metmajer commented Jul 15, 2019

Uh oh!

Uh oh!

metmajer commented Jul 8, 2019 •

edited

Loading

michaelsauter commented Jul 12, 2019 •

edited

Loading

metmajer commented Jul 12, 2019 •

edited

Loading

metmajer commented Jul 13, 2019 •

edited

Loading