Add build instructions

[errordeveloper]

For a Go developer building the project is not a major question, but we need instructions that would be helpful for someone who's a biginner with Go.

This is what I think those instructions should be:

mkdir eksctl
cd eksctl
export GOPATH="$(pwd)"
go get -d github.com/weaveworks/eksctl
cd src/github.com/weaveworks/eksctl
make build

Also, we don't have much in terms of docs around testing. Contributor docs would be great! Even if we are to say that one can run make to get help, and that there unit tests and integrations tests and point them at the code.

[dcherman]

This would definitely be helpful. Speaking as someone that is literally brand new to go (and also developing on windows at the moment), it took a bit of finagling to be able to build this project. I ended up resorting to using the Dockerfile in the build directory, building that, then mounting my src dir as a docker volume and building in that environment.

[errordeveloper]

I believe you can just run 'make eksctl-image' to get dockerized build going, but there is a chance it might not work exactly as expected on Windows.

On Sat, 19 Jan 2019, 9:57 pm dherman <notifications@github.com wrote:

This would definitely be helpful. Speaking as someone that is literally brand new to go (and also developing on windows at the moment), it took a bit of finagling to be able to build this project. I ended up resorting to using the Dockerfile in the build directory, building that, then mounting my src dir as a docker volume and building in that environment.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/weaveworks/eksctl/issues/449#issuecomment-455818646, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPWSxam_obHfur8MMj4VCh2OiwZLEJBks5vE5TkgaJpZM4aHhto .

[dcherman]

@errordeveloper So I just ran the build again since I couldn't remember all of the steps that I had to take to get things running. You're correct that we should be able to use that image, but when I build from master (freshly pulled just now), building with make eksctl-image without modifying anything else yields the following:

https://gist.github.com/dcherman/a9bba39c763a2fa7c686be28300b7a20

I didn't really triage why it was deleting some of the .md files, or why any diff existed in the Gopkg.lock. Since it should be building in the identical environment that you are (because Docker), I shouldn't be getting any different behavior there.

In order to submit the PR that I did the other day, I actually ended up removing the git diff part of the tests in order to get the build passing (didn't commit it of course).

I think that if you want cross-platform instructions, providing a Dockerfile (as you have) is likely the easiest way since on Windows, you cannot build in cmd even with Make installed, you have to use a bash like git bash or something.

C:\Users\dherman\Documents\src\eksctl>make build

'CGO_ENABLED' is not recognized as an internal or external command,
operable program or batch file.
make: *** [build] Error 1

[errordeveloper]

On Mon, 21 Jan 2019, 7:38 pm dherman <notifications@github.com wrote:

I think that if you want cross-platform instructions, providing a Dockerfile (as you have) is likely the easiest way since on Windows, you cannot build in cmd even with Make installed, you have to use a bash like git bash or something.

C:\Users\dherman\Documents\src\eksctl>make build

'CGO_ENABLED' is not recognized as an internal or external command, operable program or batch file. make: *** [build] Error 1

Ok, so it doesn't like the 'VAR=x ' syntax. That is a little surprising to me, I worry that it would be hard to maintain Windows builds without a CI of some sort.

[errordeveloper]

@dcherman could you please confirm if you are using Docker for Windows or something else?

[dcherman]

Docker for Windows

[errordeveloper]

@dcherman the deletion of README.md and other files is a mystery to me. However Gopkg.lock changes are not unexpected, it usually occurs when different version of dep are used, I just pushed a fix to master - 0781e1ed.

[dcherman]

@errordeveloper Yea, it's weird. I'm gonna try to set aside some time and figure out exactly what step is causing those files to be deleted

[dcherman]

So the deleted files are simpler than we thought. One of your lines in the test rule is the following:

git diff --exit-code pkg/nodebootstrap/assets.go > /dev/null || (git --no-pager diff; exit 1)

So if there's a diff in that file, it'll hit the OR condition and print all diffs to stdout. In your .dockerignore, we're ignoring *.md, so the files were never copied to the Docker workspace to begin with. As far as git was concerned, they were deleted, but the original problem was that I had a diff in assets.go.

That file is generated by go-bindata per a comment at the top. The diff is all in permissions, and I think that's related to my usage of Docker for Windows. Related: https://github.com/docker/for-win/issues/2042

[errordeveloper]

@dcherman thanks for spotting this, I've pushed fd5c37a95946a42b337b5a1328aa3dc712cee4ef just now to fix the issue with the diff test.

[errordeveloper]

So with go-bindata issue, one way we could "fix" it is by forcing file mode to be constant, but that will break the functionality that we have for executable files.

https://github.com/weaveworks/eksctl/blob/fd5c37a95946a42b337b5a1328aa3dc712cee4ef/pkg/nodebootstrap/userdata.go#L50-L55

Per docker/for-win#2042, one can use volumes to solve this, but that would mean that we change how the build works, and I'm not entirely sure if volumes can be used with docker build.

An alternative approach to try would be involve creating a tarball of files that we want and pass those to docker build on standard input (instead of letting it read files from disk), but that may yield exactly the same results. I have seen this being used in LinuxKit project, and wonder if it preserves unix permissions...

I think the only other option is to disable git diff test in the containerized build, but that will break CI, where we actually need to test if someone forgotten to check in any generated code.

Perhaps we could augment the test in a way that basically ignores mode diff in assets.go, or even any diff in that file, but only in case if Windows is used. What do you think?

One other option would be to refactor pkg/nodebootstrap/userdata.go, so that it doesn't rely on permissions defined by the asset, and instead has more explicit approach, e.g. with executable scripts in a different subdirectory. But that means we have to add more runtime logic to fix a buildtime issue.

[dcherman]

Sorry for the delay here, I've been debugging why I'm getting a difference in linting as well, but I'll address that separately.

For the go-bindata issue, what if we add this to the generate step? There's already precedent for changing file permissions there, and this paves over the differences here.

@chmod -x ./pkg/nodebootstrap/assets/*

By doing that, we don't need to add any runtime complexity, and we're not adding branches depending on what OS you're building on.

[errordeveloper]

@chmod -x ./pkg/nodebootstrap/assets/*

We cannot do this, we actually depend on permissions. Files with executable permissions get those permissions on the node also.

By doing that, we don't need to add any runtime complexity, and we're not adding branches depending on what OS you're building on.

No, because that means we need to add a method for handling executable vs non-executable files somehow differently.

[errordeveloper]

@dholbach let's just add logic in makefile that doesn't run git diff on Windows, that's the easiest. Any test failures will be caught in CI basically, so it's not a problem. The only complication is that make doesn't know it's Windows, it's inside a container... so we will need to use --build-arg.

[dcherman]

@errordeveloper A fresh git clone on a linux box shows that none of the files in the assets folder are executable, and that's confirmed by the file that's checked in since the expected permissions are 420 (octal 0644) which is rwr-r-, so my suggestion doesn't actually change any of the expected file permissions.

I'll defer to you on whether or not you think it's a safe change to make, but it has no functional effect on the output as it is today

[errordeveloper]

@dcherman yes, that is the case, but functionality of the nodebootsrap package allows for us to add scripts with executable permissions, if we need to do so.

https://github.com/weaveworks/eksctl/blob/fd5c37a95946a42b337b5a1328aa3dc712cee4ef/pkg/nodebootstrap/userdata.go#L50-L55

Removing this functionality is another question, but if you ask today, I'd say we should keep it.

[errordeveloper]

@dcherman please take a look at https://github.com/weaveworks/eksctl/pull/486, it'd be great if you could test it on Windows.

[errordeveloper]

So now that we have diff tests fixed up, I think we still haven't nailed windows development workflow. Having make eksctl-image work is a good thing, but that still only gives you a Docker image that you still need to run separately and provide your AWS credentials by mounting ~/.aws etc. It's not very convenient as such, and it doesn't make much sense to provide a wrapper around the image. I think one thing that make eksctl-image helps you with is to test if changes you have made locally compile before you open a PR. So I think we really need to figure out how to build Windows binaries directly. We do have a method to cross-compile them on release, i.e. make release, but that has a side-effect of uploading binaries to Github (won't work if you don't have the permissions of course)...

@dcherman have you equipped yourself with the Go toolchain already? Perhaps you could try using go build directly and report back?

[jacobtomlinson]

I've followed these instructions and I can make build with no problems but other things like make generate-ami doesn't work.

$ make generate-ami
static_resolver_ami_generate.go:9:2: cannot find package "github.com/aws/aws-sdk-go/aws" in any of:
    /opt/boxen/homebrew/Cellar/go/1.11.5/libexec/src/github.com/aws/aws-sdk-go/aws (from $GOROOT)
    /Users/jacob/Projects/go/eksctl/src/github.com/aws/aws-sdk-go/aws (from $GOPATH)
static_resolver_ami_generate.go:10:2: cannot find package "github.com/aws/aws-sdk-go/aws/credentials/stscreds" in any of:
    /opt/boxen/homebrew/Cellar/go/1.11.5/libexec/src/github.com/aws/aws-sdk-go/aws/credentials/stscreds (from $GOROOT)
    /Users/jacob/Projects/go/eksctl/src/github.com/aws/aws-sdk-go/aws/credentials/stscreds (from $GOPATH)
static_resolver_ami_generate.go:11:2: cannot find package "github.com/aws/aws-sdk-go/aws/session" in any of:
    /opt/boxen/homebrew/Cellar/go/1.11.5/libexec/src/github.com/aws/aws-sdk-go/aws/session (from $GOROOT)
    /Users/jacob/Projects/go/eksctl/src/github.com/aws/aws-sdk-go/aws/session (from $GOPATH)
static_resolver_ami_generate.go:12:2: cannot find package "github.com/aws/aws-sdk-go/service/ec2" in any of:
    /opt/boxen/homebrew/Cellar/go/1.11.5/libexec/src/github.com/aws/aws-sdk-go/service/ec2 (from $GOROOT)
    /Users/jacob/Projects/go/eksctl/src/github.com/aws/aws-sdk-go/service/ec2 (from $GOPATH)
static_resolver_ami_generate.go:17:2: cannot find package "github.com/dave/jennifer/jen" in any of:
    /opt/boxen/homebrew/Cellar/go/1.11.5/libexec/src/github.com/dave/jennifer/jen (from $GOROOT)
    /Users/jacob/Projects/go/eksctl/src/github.com/dave/jennifer/jen (from $GOPATH)
pkg/ami/static_resolver.go:8: running "go": exit status 1
make: *** [generate-ami] Error 1

[bensussman]

The build instructions here did not work for me:

~/code $ mkdir eksctl
~/code $ cd eksctl/
~/code/eksctl $ export GOPATH="$(pwd)"
~/code/eksctl $ go get -d github.com/weaveworks/eksctl
package github.com/weaveworks/eksctl: no Go files in /Users/ben/code/eksctl/src/github.com/weaveworks/eksctl
~/code/eksctl $ cd src/github.com/weaveworks/eksctl/
~/code/eksctl/src/github.com/weaveworks/eksctl $ make build
# github.com/weaveworks/eksctl/pkg/printers
pkg/printers/json.go:62:14: undefined: strings.ReplaceAll
pkg/printers/table.go:64:14: undefined: strings.ReplaceAll
pkg/printers/yaml.go:61:14: undefined: strings.ReplaceAll
make: *** [build] Error 2

Any advice?

Note, i'm on go 1.11.4 darwin/amd64 on OSX 10.14.3

$ go version
go version go1.11.4 darwin/amd64

[dcherman]

@bensussman ReplaceAll was added in 1.12 - https://golang.org/doc/go1.12#strings

Run brew upgrade go and it should work

@errordeveloper Should we document the minimum version of Go that's supported for building?

[errordeveloper]

Yes, we should document the dependency on 1.12.