From 3a5176140609a2e463760689b486a5331ab0ded4 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Fri, 6 Sep 2019 08:43:26 -0400 Subject: [PATCH] dev: avoid codecgen code in downstream projects 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. --- GNUmakefile | 8 +++++--- client/structs/structs.go | 2 +- nomad/structs/generate.sh | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/GNUmakefile b/GNUmakefile index 325e5d197..4d08da015 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -6,7 +6,7 @@ GIT_COMMIT := $(shell git rev-parse HEAD) GIT_DIRTY := $(if $(shell git status --porcelain),+CHANGES) GO_LDFLAGS := "-X github.com/hashicorp/nomad/version.GitCommit=$(GIT_COMMIT)$(GIT_DIRTY)" -GO_TAGS ?= +GO_TAGS ?= codec_generated GO_TEST_CMD = $(if $(shell which gotestsum),gotestsum --,go test) @@ -251,11 +251,11 @@ dev: vendorfmt changelogfmt ## Build for the current development platform @cp $(PROJECT_ROOT)/$(DEV_TARGET) $(GOPATH)/bin .PHONY: prerelease -prerelease: GO_TAGS=ui release +prerelease: GO_TAGS=ui codegen_generated release prerelease: generate-all ember-dist static-assets ## Generate all the static assets for a Nomad release .PHONY: release -release: GO_TAGS=ui release +release: GO_TAGS=ui codegen_generated release release: clean $(foreach t,$(ALL_TARGETS),pkg/$(t).zip) ## Build all release packages which can be built on this platform. @echo "==> Results:" @tree --dirsfirst $(PROJECT_ROOT)/pkg @@ -282,6 +282,7 @@ test-nomad: dev ## Run Nomad test suites $(if $(ENABLE_RACE),-race) $(if $(VERBOSE),-v) \ -cover \ -timeout=15m \ + -tags "$(GO_TAGS)" \ $(GOTEST_PKGS) $(if $(VERBOSE), >test.log ; echo $$? > exit-code) @if [ $(VERBOSE) ] ; then \ bash -C "$(PROJECT_ROOT)/scripts/test_check.sh" ; \ @@ -294,6 +295,7 @@ e2e-test: dev ## Run the Nomad e2e test suite $(if $(ENABLE_RACE),-race) $(if $(VERBOSE),-v) \ -cover \ -timeout=900s \ + -tags "$(GO_TAGS)" \ github.com/hashicorp/nomad/e2e/vault/ \ -integration diff --git a/client/structs/structs.go b/client/structs/structs.go index 169cf824c..eff8ceaf3 100644 --- a/client/structs/structs.go +++ b/client/structs/structs.go @@ -1,6 +1,6 @@ package structs -//go:generate codecgen -d 102 -o structs.generated.go structs.go +//go:generate codecgen -d 102 -t codec_generated -o structs.generated.go structs.go import ( "errors" diff --git a/nomad/structs/generate.sh b/nomad/structs/generate.sh index 04141c34a..c0409e707 100755 --- a/nomad/structs/generate.sh +++ b/nomad/structs/generate.sh @@ -2,4 +2,4 @@ set -e FILES="$(ls ./*.go | grep -v -e _test.go -e .generated.go | tr '\n' ' ')" -codecgen -d 100 -o structs.generated.go ${FILES} +codecgen -d 100 -t codec_generated -o structs.generated.go ${FILES}