You'd think since golangci-lint embeds misspell we could use that,
but it fails to run if it finds no Go source files, which is the
case in our website/ directory that we want to check.
gometalinter has been deprecated, with golangci-lint as its spiritual
and recommended successor. Here we switch to using it with an equivalent
configuration, albeit with newer versions of some linters.
To maintain compatibility with existing settings, we have a couple of
things disabled here, specifically:
- tests
We have a lot of unused code in our tests that choke deadcode.
We should attempt to clean these up soon so that we can lint our
testcode.
- govet.check-shadowing = false
This breaks on redefining `err` which we do all over the nomad
codebase.
This is an attempt to ease dependency management for external driver
plugins, by avoiding requiring them to compile ugorji/go generated
files. Plugin developers reported some pain with the brittleness of
ugorji/go dependency in particular, specially when using go mod, the
default go mod manager in golang 1.13.
Context
--------
Nomad uses msgpack to persist and serialize internal structs, using
ugorji/go library. As an optimization, we use ugorji/go code generation
to speedup process and aovid the relection-based slow path.
We commit these generated files in repository when we cut and tag the
release to ease reproducability and debugging old releases. Thus,
downstream projects that depend on release tag, indirectly depends on
ugorji/go generated code.
Sadly, the generated code is brittle and specific to the version of
ugorji/go being used. When go mod picks another version of ugorji/go
then nomad (go mod by default uses release according to semver),
downstream projects face compilation errors.
Interestingly, downstream projects don't commonly serialize nomad
internal structs. Drivers and device plugins use grpc instead of
msgpack for the most part. In the few cases where they use msgpag (e.g.
decoding task config), they do without codegen path as they run on
driver specific structs not the nomad internal structs. Also, the
ugorji/go serialization through reflection is generally backward
compatible (mod some ugorji/go regression bugs that get introduced every
now and then :( ).
Proposal
---------
The proposal here is to keep committing ugorji/go codec generated files
for releases but to use a go tag for them.
All nomad development through the makefile, including releasing, CI and
dev flow, has the tag enabled.
Downstream plugin projects, by default, will skip these files and life
proceed as normal for them.
The downside is that nomad developers who use generated code but avoid
using make must start passing additional go tag argument. Though this
is not a blessed configuration.
We currently use an container image for `test-devices` job only; while
all other jobs use machine executor.
This allows us to switch golang and protoc verions easily without
manually managing Docker images (which requires building them manually
on a dev machines, etc). All that while, we install dependencies on
every build in all other jobs..
`test-devices` now is one of the fastest jobs and isn't a constraint or
a bottleneck, so increasing its overhead by few seconds doesn't hurt the
overall developer iteration.
If we split tests effectively later, we can revisit.
Allow honoring `GO_TAGS` environment variable if set. Currently, users
must set variable as a makefile argument e.g. `make GO_TAGS=ui dev`, and
this allows us to use env-var syntax (e.g. `GO_TAGS=ui make dev`) and
make it convenient to set GO_TAGS globally.
This commit provides an initial migration of general testing CI
infrastructure to CircleCI.
It uses CircleCI 2.1 paramereterised jobs to provide two base
configurations: a vm based `test-machine`, and docker based
`test-container`.
Jobs that require root, docker, or other similar features require the
machine based jobs, but others should be ran using the `test-container` package
as they are both cheaper and faster to run.
Our build scripts pass `$(GO_TAGS)` to `-tags` go compile flags, except
for `make dev`, where `$(NOMAD_UI_TAG)` is used. This change ensures
`make dev` is inline with the rest of makefile targets.
I use the flag primarily to enable the nomad ui using the committed
compiled assets without regenerating them, as I find using stale ui
satisfactory most of the time.
`make check` runs very intensive linters that slow and seem to behave
differently on different machines.
Linting is still a part of our CI and we shouldn't be cutting a release
when CI isn't green anyway.
Noticed that the protobuf files are out of sync with ones generated by 1.2.0 protoc go plugin.
The cause for these files seem to be related to release processes, e.g. [0.9.0-beta1 preperation](ecec3d38de (diff-da4da188ee496377d456025c2eab4e87)), and [0.9.0-beta3 preperation](b849d84f2f).
This restores the changes to that of the pinned protoc version and fails build if protobuf files are out of sync. Sample failing Travis job is that of the first commit change: https://travis-ci.org/hashicorp/nomad/jobs/506285085
`gotestsum` has user friendlier output that emits final summary, also it can emit junit xml file for
automated analysis instead of current format that should significantly
ease automated analysis of CI.
workaround a regression in 1.11.3
> We are aware of a functionality regression in "go get" when executed in GOPATH mode on an import path pattern containing "..." (e.g., "go get github.com/golang/pkg/..."), when downloading packages not already present in the GOPATH workspace. This is issue golang.org/issue/29241. It will be resolved in the next minor patch releases, Go 1.11.4 and Go 1.10.7, which we plan to release soon. We apologize for any disruption.
Lowering the runtime here to pre 7ca535aa90748caff1522468cc0c4ab672a74abb expectations.
The longest package at the time `client/driver` shrunk significantly,
and now the longest packages take less than 5 minutes.
We do have some long running timed out projects due to a stuck shutdown,
but in completed jobs (though they failed), the longest packages took
less than 5 minutes. The longest running packages in
https://travis-ci.org/hashicorp/nomad/jobs/464640776 were:
```
FAIL github.com/hashicorp/nomad/nomad 268.089s
ok github.com/hashicorp/nomad/drivers/docker 203.903s coverage: 68.8% of statements
ok github.com/hashicorp/nomad/drivers/rkt 132.104s coverage: 65.0% of statements
ok github.com/hashicorp/nomad/api 123.193s coverage: 62.9% of statements
ok github.com/hashicorp/nomad/command/agent 74.657s coverage: 72.3% of statements
ok github.com/hashicorp/nomad/command 63.592s coverage: 42.7% of statements
```
nomad/client take very long and exceed 15m sometimes:
In https://travis-ci.org/hashicorp/nomad/jobs/452990197 :
```
panic: test timed out after 15m0s
goroutine 4739 [running]:
testing.(*M).startAlarm.func1()
/home/travis/.gimme/versions/go1.11.2.linux.amd64/src/testing/testing.go:1296 +0xfd
....
goroutine 4665 [select]:
github.com/hashicorp/nomad/vendor/google.golang.org/grpc.newClientStream.func5(0xc0003dd500, 0xc000420120, 0x2b3f86295588, 0xc000496810)
/home/travis/gopath/src/github.com/hashicorp/nomad/vendor/google.golang.org/grpc/stream.go:287 +0xd7
created by github.com/hashicorp/nomad/vendor/google.golang.org/grpc.newClientStream
/home/travis/gopath/src/github.com/hashicorp/nomad/vendor/google.golang.org/grpc/stream.go:286 +0x842
FAIL github.com/hashicorp/nomad/client/driver 900.036s
```
Michael Schurter