build / release makeover

[krancour]

By request from @jeremyrickard

Highlights:

• Tracks vendored code (it's a modest 36 MB or so)
• All dev tasks (tests, lint checks, etc) occur within a consistent, containerized environment
• Multi-stage image builds
• DinD image builds (don't require the host's docker socket to be mounted into containers)

cc @vdice

[bacongobbler]

Isn't the purpose of a lockfile to pin dependencies at particular versions which can rebuild the vendor tree? I'm not following why the vendor directory needs to be checked into source control.

[glyn]

If I run make build-all-bins without go in the path, the command fails with:

bash -c "LDFLAGS=\"-w -s -X github.com/deislabs/duffle/pkg/version.Version=c6a43ae\" scripts/build.sh"
building linux-amd64
scripts/build.sh: line 11: go: command not found
make: *** [build-all-bins] Error 127

So it seems that go is still needed.

[krancour]

You may have caught a mistake here. Will look.

[glyn]

I love not having to make bootstrap any more. The vendor/ directory will also insulate us from various network issues.

I'd like a quick/memorable way to make bin/duffle on macOS or linux.

It seems that Go is still needed, which is fine by me.

I'm not sure that building the docker image should be part of build, but that's fine so long as there are alternatives which can avoid it.

[krancour]

I'd like a quick/memorable way to make bin/duffle on macOS or linux.

See doc changes. There is a way, but perhaps not adequately quick or memorable. I would be happy to add a few OS-specific convenience targets.

[silvin-lubecki]

LGTM for the build and vendoring part (not enough confident on brigade.js to review it properly...)

About versioning the vendoring, I don't have strong opinion about that, even if I prefer having everything versioned (easier to review/backtrack changes while bumping dependencies, safer if any dependency closes it's github account), but both options are fine with me.

[krancour]

Isn't the purpose of a lockfile to pin dependencies at particular versions which can rebuild the vendor tree? I'm not following why the vendor directory needs to be checked into source control.

This is a classic debate and from what I have observed, dep users have mostly been erring on the side of tracking vendored code for some time now. You are absolutely correct that the .lock for enables a dependency tree to be rebuilt... as long as no one has deleted any library from github...

Beyond that, when vendor code isn't tracked, there's a need for some additional dependency resolution step (e.g. make bootstrap) to be run. For good measure, that should be run every time you check out a given branch, else you're potentially building with the wrong dependencies. So tracking vendored code has the advantage of being a little more foolproof in terms of the moment anyone clones the repo or checks out a branch, they already have the exact desired dependencies and are ready to build or test with no further adieu.

[glyn]

I'd like a quick/memorable way to make bin/duffle on macOS or linux.

See doc changes. There is a way, but perhaps not adequately quick or memorable. I would be happy to add a few OS-specific convenience targets.

That would be really helpful. I'd like something like make duffle or make darwin to produce bin/duffle (without any os/arch naming).

[krancour]

Yep... there a mistake on this line. The prefix here should just be DOCKER_CMD. GO_DOCKER_CMD is some copy/pasta and is always undefined and @glyn, this is why you were still needing Go on your path. I will fix this as soon as I get to a proper keyboard.

[glyn]

Yep... there a mistake on this line. The prefix here should just be DOCKER_CMD. GO_DOCKER_CMD is some copy/pasta and is always undefined and @glyn, this is why you were still needing Go on your path. I will fix this as soon as I get to a proper keyboard.

Thanks. But actually I like being able to do a "local" (i.e. non-docker) build using my Go tooling, so please can we preserve that, at least when just building bin/duffle for the current OS?

[krancour]

You can opt out of it with SKIP_DOCKER=true.

[krancour]

That would be really helpful. I'd like something like make duffle or make darwin to produce bin/duffle (without any os/arch naming).

@glyn I added a build-% target. You can build for your OS of choice with make build-<OS>. It assumes amd64 as the architecture.

without any os/arch naming

That makes me uneasy. The same build script that produces binaries on-demand for you also produces deployable artifacts in our pipeline. I think it's best if those artifacts are named unambiguously. This being said...

Elsewhere we discussed make install going away and I suggested creating a symlink instead. If you do so, there's no inconvenience to binaries having OS and architecture embedded in their name.

Does that seem ok?

[glyn]

That would be really helpful. I'd like something like make duffle or make darwin to produce bin/duffle (without any os/arch naming).

@glyn I added a build-% target. You can build for your OS of choice with make build-<OS>. It assumes amd64 as the architecture.

without any os/arch naming

That makes me uneasy. The same build script that produces binaries on-demand for you also produces deployable artifacts in our pipeline. I think it's best if those artifacts are named unambiguously. This being said...

Elsewhere we discussed make install going away and I suggested creating a symlink instead. If you do so, there's no inconvenience to binaries having OS and architecture embedded in their name.

Does that seem ok?

Yes. I can use a symlink duffle -> duffle-darwin-amd64.

[glyn]

I re-tested and SKIP_DOCKER=true make build-darwin works as expected as does make build-darwin. I wonder if you would mind documenting this in the developer's guide, for posterity?

[glyn]

Looks great now. Ideally, I'd like make duffle-darwin et al. described in the developer's guide, for posterity.

[krancour]

Looks great now. Ideally, I'd like make duffle-darwin et al. described in the developer's guide, for posterity.

Yes. I will add that.

[vdice]

I'll leave the rest of the CI changes up to the interested parties who'll more actively interact w/ them, but otherwise all looks good to me. One minor comment on the brigade.js update...

[radu-matei]

Just tested on Windows:

PS C:\projects\go\src\github.com\deislabs\duffle> $env:SKIP_DOCKER
true
PS C:\projects\go\src\github.com\deislabs\duffle> make -f .\Makefile build
bash -c "LDFLAGS=\"-w -s -X github.com/deislabs/duffle/pkg/version.Version=de5062b\" scripts/build.sh"
building Windows_NT-amd64
cmd/go: unsupported GOOS/GOARCH pair Windows_NT/amd64
make: *** [build-all-bins] Error 2

Of course, this assumes WSL is configured on the machine; or requires the build to use Docker, none of which should be requirements for a simple go build.

[krancour]

@radu-matei lemme look at that. This is really odd. I'm not sure how Windows_NT got set as.the OS here. Do you have an existing env var by that name?

[krancour]

Ok. Apparently the OS environment variable (which I am using) is pre-defined as Windows_NT on Windows (but not in WSL). So, if not executing within a container, in a place where I expected OS to either be a valid OS for Go cross-compiling or be undefined, it is neither of those things. I can fix this.

[krancour]

@radu-matei the latest issue you reported should be fixed by the latest commit.

[jeremyrickard]

LGTM

[technosophos]

This currently has 4 LGTMs by core maintainers, so I am merging.