From fab664e7a0950007f6e96727e6b2321eb4bdf92e Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 14 Oct 2021 14:36:59 -0700 Subject: [PATCH 01/17] Makefile: rm gofmt and golint, simplify gotest 1. Remove .gofmt and .golint targets -- they are to be replaced by golangci-lint a bit later. 2. Simplify .gotest target, add support for TESTFLAGS Signed-off-by: Kir Kolyshkin --- Makefile | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/Makefile b/Makefile index 87cfb451..70184857 100644 --- a/Makefile +++ b/Makefile @@ -61,20 +61,13 @@ $(VALIDATION_TESTS): %.t: %.go print-validation-tests: @echo $(VALIDATION_TESTS) -.PHONY: test .gofmt .govet .golint print-validation-tests +.PHONY: test .govet print-validation-tests PACKAGES = $(shell go list ./... | grep -v vendor) -test: .gofmt .govet .golint .gotest - -.gofmt: - OUT=$$(go fmt $(PACKAGES)); if test -n "$${OUT}"; then echo "$${OUT}" && exit 1; fi +test: .govet .gotest .govet: go vet -x $(PACKAGES) -.golint: - golint -set_exit_status $(PACKAGES) - -UTDIRS = ./filepath/... ./validate/... ./generate/... .gotest: - go test $(UTDIRS) + go test $(TESTFLAGS) ./... From 5432bc4b24f30217d9767b5dd813828667ae889e Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 14 Oct 2021 14:46:57 -0700 Subject: [PATCH 02/17] ci: replace travis with gha ci Signed-off-by: Kir Kolyshkin --- .github/workflows/test.yml | 32 ++++++++++++++++++++++++++++++++ .travis.yml | 19 ------------------- 2 files changed, 32 insertions(+), 19 deletions(-) create mode 100644 .github/workflows/test.yml delete mode 100644 .travis.yml diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 00000000..4a7f2d92 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,32 @@ +name: ci +on: + push: + tags: + - v* + branches: + - master + - release-* + pull_request: + +jobs: + test: + runs-on: ubuntu-20.04 + strategy: + fail-fast: false + matrix: + go-version: [1.16.x, 1.17.x] + rootless: ["rootless", ""] + race: ["-race", ""] + + steps: + - name: checkout + uses: actions/checkout@v2 + - name: install go ${{ matrix.go-version }} + uses: actions/setup-go@v2 + with: + stable: '!contains(${{ matrix.go-version }}, "beta") && !contains(${{ matrix.go-version }}, "rc")' + go-version: ${{ matrix.go-version }} + - name: build + run: make + - name: test + run: make TESTFLAGS="${{ matrix.race }}" test diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 1d5a62c2..00000000 --- a/.travis.yml +++ /dev/null @@ -1,19 +0,0 @@ -language: go -go: - - 1.10.x - - 1.11.x - -sudo: false - -before_install: - - mkdir --parents $GOPATH/src/golang.org/x - && git clone --depth=1 https://go.googlesource.com/lint $GOPATH/src/golang.org/x/lint - && go get golang.org/x/lint/golint - - go get github.com/vbatts/git-validation - -install: true - -script: - - git-validation -run DCO,short-subject -v -range ${TRAVIS_COMMIT_RANGE} - - make - - make test From 16dfbbd906afaac73eaaac8e6bf0bf16826eb701 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 14 Oct 2021 18:55:25 -0700 Subject: [PATCH 03/17] Makefile: add/use BUILD_FLAGS 1. Move the common go build flags to BUILD_FLAGS. Add an ability to specify more build flags via EXTRA_FLAGS. 2. Add STATIC_BUILD_FLAGS, use it for runtimetest static build. Commit 95617cb6335 added CGO_ENABLED=0 in order to produce a static build, which was the way things worked back in the day. Now we can use netgo, and we have to specify "-extldflags -static" in -ldflags in order to have a static build. 3. Enable to build binaries with -race from CI. Signed-off-by: Kir Kolyshkin --- .github/workflows/test.yml | 3 +-- Makefile | 8 +++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4a7f2d92..cd32d368 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -15,7 +15,6 @@ jobs: fail-fast: false matrix: go-version: [1.16.x, 1.17.x] - rootless: ["rootless", ""] race: ["-race", ""] steps: @@ -27,6 +26,6 @@ jobs: stable: '!contains(${{ matrix.go-version }}, "beta") && !contains(${{ matrix.go-version }}, "rc")' go-version: ${{ matrix.go-version }} - name: build - run: make + run: make EXTRA_FLAGS="${{ matrix.race }}" - name: test run: make TESTFLAGS="${{ matrix.race }}" test diff --git a/Makefile b/Makefile index 70184857..05615874 100644 --- a/Makefile +++ b/Makefile @@ -6,16 +6,18 @@ BUILDTAGS= RUNTIME ?= runc COMMIT=$(shell git rev-parse HEAD 2> /dev/null || true) VERSION := ${shell cat ./VERSION} +BUILD_FLAGS := -tags "$(BUILDTAGS)" -ldflags "-X main.gitCommit=$(COMMIT) -X main.version=$(VERSION)" $(EXTRA_FLAGS) +STATIC_BUILD_FLAGS := -tags "$(BUILDTAGS) netgo osusergo" -ldflags "-extldflags -static -X main.gitCommit=$(COMMIT) -X main.version=$(VERSION)" $(EXTRA_FLAGS) VALIDATION_TESTS ?= $(patsubst %.go,%.t,$(shell find ./validation/ -name *.go | grep -v util)) all: tool runtimetest validation-executables tool: - go build -tags "$(BUILDTAGS)" -ldflags "-X main.gitCommit=${COMMIT} -X main.version=${VERSION}" -o oci-runtime-tool ./cmd/oci-runtime-tool + go build $(BUILD_FLAGS) -o oci-runtime-tool ./cmd/oci-runtime-tool .PHONY: runtimetest runtimetest: - CGO_ENABLED=0 go build -installsuffix cgo -tags "$(BUILDTAGS)" -ldflags "-X main.gitCommit=${COMMIT} -X main.version=${VERSION}" -o runtimetest ./cmd/runtimetest + go build $(STATIC_BUILD_FLAGS) -o runtimetest ./cmd/runtimetest .PHONY: man man: @@ -56,7 +58,7 @@ validation-executables: $(VALIDATION_TESTS) .PRECIOUS: $(VALIDATION_TESTS) .PHONY: $(VALIDATION_TESTS) $(VALIDATION_TESTS): %.t: %.go - go build -tags "$(BUILDTAGS)" ${TESTFLAGS} -o $@ $< + go build $(BUILD_FLAGS) -o $@ $< print-validation-tests: @echo $(VALIDATION_TESTS) From 112c88ca082b66fb6232dc87bd9ca955f18d2b94 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 14 Oct 2021 19:19:42 -0700 Subject: [PATCH 04/17] Makefile: use fancy git commit ids Now, the commit ID will contain the closest tag, the number of commits since the tag, and the (abbreviated) git commit sha (see example below). This tag is still unique and can be used instead of bare sha for all git commands. Before: $ ./runtimetest -version runtimetest version 0.9.0, commit: ea2948e9efbfc634991aad90ebb6bf54995c9a37 After: $ ./runtimetest -version runtimetest version 0.9.0, commit: v0.9.0-28-ge91e387 This means that - the closest tag is v0.9.0 - there were 28 commits after the tag - the abbreviated sha is e91e387 Nice, isn't it? While at it, - allow to set commit id via environment (might be needed for distros); - remove unneeded "|| true" stance. Signed-off-by: Kir Kolyshkin --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 05615874..418d7bf5 100644 --- a/Makefile +++ b/Makefile @@ -4,7 +4,7 @@ TAP ?= tap BUILDTAGS= RUNTIME ?= runc -COMMIT=$(shell git rev-parse HEAD 2> /dev/null || true) +COMMIT ?= $(shell git describe --dirty --long --always --tags 2> /dev/null) VERSION := ${shell cat ./VERSION} BUILD_FLAGS := -tags "$(BUILDTAGS)" -ldflags "-X main.gitCommit=$(COMMIT) -X main.version=$(VERSION)" $(EXTRA_FLAGS) STATIC_BUILD_FLAGS := -tags "$(BUILDTAGS) netgo osusergo" -ldflags "-extldflags -static -X main.gitCommit=$(COMMIT) -X main.version=$(VERSION)" $(EXTRA_FLAGS) From e36f98fbeed86c1975dcdcab6eafa33e790c5832 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 14 Oct 2021 19:52:44 -0700 Subject: [PATCH 05/17] Fix deadcode linter warnings Fix the following warnings by removing the unused stuff. > generate/seccomp/consts.go:7:2: `kill` is unused (deadcode) > kill = "kill" > ^ > generate/seccomp/consts.go:8:2: `trap` is unused (deadcode) > trap = "trap" > ^ > generate/seccomp/consts.go:9:2: `trace` is unused (deadcode) > trace = "trace" > ^ > generate/seccomp/consts.go:10:2: `allow` is unused (deadcode) > allow = "allow" > ^ > generate/seccomp/consts.go:11:2: `errno` is unused (deadcode) > errno = "errno" > ^ > generate/seccomp/syscall_compare.go:95:6: `identicalExceptAction` is unused (deadcode) > func identicalExceptAction(config1, config2 *rspec.LinuxSyscall) bool { > ^ > generate/seccomp/syscall_compare.go:103:6: `identicalExceptArgs` is unused (deadcode) > func identicalExceptArgs(config1, config2 *rspec.LinuxSyscall) bool { > ^ All of the above was added by commit 60473d2e but never ever used. > generate/generate.go:1567:6: `strPtr` is unused (deadcode) > func strPtr(s string) *string { return &s } > ^ This was added by commit 80e63b3c, but later in afc8d3588cbf all its uses were removed. Signed-off-by: Kir Kolyshkin --- generate/generate.go | 3 --- generate/seccomp/consts.go | 5 ----- generate/seccomp/syscall_compare.go | 16 ---------------- 3 files changed, 24 deletions(-) diff --git a/generate/generate.go b/generate/generate.go index b8433ce8..55e8de0e 100644 --- a/generate/generate.go +++ b/generate/generate.go @@ -1563,9 +1563,6 @@ func (g *Generator) RemoveLinuxResourcesDevice(allow bool, devType string, major return } -// strPtr returns the pointer pointing to the string s. -func strPtr(s string) *string { return &s } - // SetSyscallAction adds rules for syscalls with the specified action func (g *Generator) SetSyscallAction(arguments seccomp.SyscallOpts) error { g.initConfigLinuxSeccomp() diff --git a/generate/seccomp/consts.go b/generate/seccomp/consts.go index eade5718..f28d8f58 100644 --- a/generate/seccomp/consts.go +++ b/generate/seccomp/consts.go @@ -4,9 +4,4 @@ const ( seccompOverwrite = "overwrite" seccompAppend = "append" nothing = "nothing" - kill = "kill" - trap = "trap" - trace = "trace" - allow = "allow" - errno = "errno" ) diff --git a/generate/seccomp/syscall_compare.go b/generate/seccomp/syscall_compare.go index dbf2aec1..5e84653a 100644 --- a/generate/seccomp/syscall_compare.go +++ b/generate/seccomp/syscall_compare.go @@ -92,22 +92,6 @@ func identical(config1, config2 *rspec.LinuxSyscall) bool { return reflect.DeepEqual(config1, config2) } -func identicalExceptAction(config1, config2 *rspec.LinuxSyscall) bool { - samename := sameName(config1, config2) - sameAction := sameAction(config1, config2) - sameArgs := sameArgs(config1, config2) - - return samename && !sameAction && sameArgs -} - -func identicalExceptArgs(config1, config2 *rspec.LinuxSyscall) bool { - samename := sameName(config1, config2) - sameAction := sameAction(config1, config2) - sameArgs := sameArgs(config1, config2) - - return samename && sameAction && !sameArgs -} - func sameName(config1, config2 *rspec.LinuxSyscall) bool { return reflect.DeepEqual(config1.Names, config2.Names) } From 1826c32e376364e78ae02e5fbedb96426b91b6f4 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 14 Oct 2021 20:02:05 -0700 Subject: [PATCH 06/17] Fix gosimple linter warnings These ones: generate/generate.go:1563:2: S1023: redundant `return` statement (gosimple) return ^ validation/util/linux_resources_devices.go:29:6: S1002: should omit comparison to bool constant, can be simplified to `device.Allow` (gosimple) if device.Allow == true { ^ cmd/runtimetest/main.go:985:94: S1039: unnecessary use of fmt.Sprintf (gosimple) rfcError, err := c.Ok(actual == expected, specerror.LinuxProcOomScoreAdjSet, spec.Version, fmt.Sprintf("has expected OOM score adjustment")) ^ validation/hostname/hostname.go:34:13: S1039: unnecessary use of fmt.Sprintf (gosimple) t.Skip(1, fmt.Sprintf("linux-specific namespace test")) ^ validation/linux_ns_itype/linux_ns_itype.go:122:13: S1039: unnecessary use of fmt.Sprintf (gosimple) t.Skip(1, fmt.Sprintf("linux-specific namespace test")) ^ Signed-off-by: Kir Kolyshkin --- cmd/runtimetest/main.go | 2 +- generate/generate.go | 1 - validation/hostname/hostname.go | 2 +- validation/linux_ns_itype/linux_ns_itype.go | 2 +- validation/linux_ns_nopath/linux_ns_nopath.go | 2 +- validation/util/linux_resources_devices.go | 2 +- 6 files changed, 5 insertions(+), 6 deletions(-) diff --git a/cmd/runtimetest/main.go b/cmd/runtimetest/main.go index a45e441b..6004614a 100644 --- a/cmd/runtimetest/main.go +++ b/cmd/runtimetest/main.go @@ -982,7 +982,7 @@ func (c *complianceTester) validateOOMScoreAdj(spec *rspec.Spec) error { if err != nil { return err } - rfcError, err := c.Ok(actual == expected, specerror.LinuxProcOomScoreAdjSet, spec.Version, fmt.Sprintf("has expected OOM score adjustment")) + rfcError, err := c.Ok(actual == expected, specerror.LinuxProcOomScoreAdjSet, spec.Version, "has expected OOM score adjustment") if err != nil { return err } diff --git a/generate/generate.go b/generate/generate.go index 55e8de0e..6484b02c 100644 --- a/generate/generate.go +++ b/generate/generate.go @@ -1560,7 +1560,6 @@ func (g *Generator) RemoveLinuxResourcesDevice(allow bool, devType string, major return } } - return } // SetSyscallAction adds rules for syscalls with the specified action diff --git a/validation/hostname/hostname.go b/validation/hostname/hostname.go index 543c3c71..02e01754 100644 --- a/validation/hostname/hostname.go +++ b/validation/hostname/hostname.go @@ -31,7 +31,7 @@ func main() { defer t.AutoPlan() if "linux" != runtime.GOOS { - t.Skip(1, fmt.Sprintf("linux-specific namespace test")) + t.Skip(1, "linux-specific namespace test") } hostnames := []string{ diff --git a/validation/linux_ns_itype/linux_ns_itype.go b/validation/linux_ns_itype/linux_ns_itype.go index 13b9adc6..a7399943 100644 --- a/validation/linux_ns_itype/linux_ns_itype.go +++ b/validation/linux_ns_itype/linux_ns_itype.go @@ -119,7 +119,7 @@ func main() { t.Header(0) if "linux" != runtime.GOOS { - t.Skip(1, fmt.Sprintf("linux-specific namespace test")) + t.Skip(1, "linux-specific namespace test") } err := testNamespaceInheritType(t) diff --git a/validation/linux_ns_nopath/linux_ns_nopath.go b/validation/linux_ns_nopath/linux_ns_nopath.go index ae6e8ec9..42fd16dc 100644 --- a/validation/linux_ns_nopath/linux_ns_nopath.go +++ b/validation/linux_ns_nopath/linux_ns_nopath.go @@ -120,7 +120,7 @@ func main() { t.Header(0) if "linux" != runtime.GOOS { - t.Skip(1, fmt.Sprintf("linux-specific namespace test")) + t.Skip(1, "linux-specific namespace test") } err := testNamespaceNoPath(t) diff --git a/validation/util/linux_resources_devices.go b/validation/util/linux_resources_devices.go index 43f8da8a..ed755168 100644 --- a/validation/util/linux_resources_devices.go +++ b/validation/util/linux_resources_devices.go @@ -26,7 +26,7 @@ func ValidateLinuxResourcesDevices(config *rspec.Spec, t *tap.T, state *rspec.St } for i, device := range config.Linux.Resources.Devices { - if device.Allow == true { + if device.Allow { found := false if lnd[i-1].Type == device.Type && *lnd[i-1].Major == *device.Major && *lnd[i-1].Minor == *device.Minor && lnd[i-1].Access == device.Access { found = true From d38bd63eb719c81c88ecf1591b199966fee99c3d Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 14 Oct 2021 20:09:46 -0700 Subject: [PATCH 07/17] Fix deprecation warnings from staticcheck linter Fix deprecation warnings related to commit 84a62c6a from 2016. In particular, these ones: validation/util/test.go:299:15: SA1019: g.Spec is deprecated: Replace with generator.Config. (staticcheck) if err := f(g.Spec(), t, &state); err != nil { ^ validation/config_updates_without_affect/config_updates_without_affect.go:71:2: SA1019: g.SetSpec is deprecated: Replace with: (staticcheck) g.SetSpec(spec) ^ validation/delete_resources/delete_resources.go:62:44: SA1019: g.Spec is deprecated: Replace with generator.Config. (staticcheck) if err := util.ValidateLinuxResourcesPids(g.Spec(), t, &state); err != nil { ^ validation/hooks/hooks.go:30:40: SA1019: g.Spec is deprecated: Replace with generator.Config. (staticcheck) output = filepath.Join(r.BundleDir, g.Spec().Root.Path, "output") ^ validation/killsig/killsig.go:37:38: SA1019: sigConfig.Spec is deprecated: Replace with generator.Config. (staticcheck) rootDir := filepath.Join(bundleDir, sigConfig.Spec().Root.Path) ^ validation/misc_props/misc_props.go:65:24: SA1019: annotationConfig.Spec is deprecated: Replace with generator.Config. (staticcheck) {extendedSpec{Spec: *annotationConfig.Spec()}, util.LifecycleActionCreate | util.LifecycleActionStart | util.LifecycleActionDelete, true, specerror.NewError(specerror.AnnotationsKeyIgnoreUnknown, fmt.Errorf("implementations that are reading/processing this configuration file MUST NOT generate an error if they encounter an unknown annotation key"), rspecs.Version)}, ^ validation/misc_props/misc_props.go:66:24: SA1019: basicConfig.Spec is deprecated: Replace with generator.Config. (staticcheck) {extendedSpec{Spec: *basicConfig.Spec(), Unknown: "unknown"}, util.LifecycleActionCreate | util.LifecycleActionStart | util.LifecycleActionDelete, true, specerror.NewError(specerror.ExtensibilityIgnoreUnknownProp, fmt.Errorf("runtimes that are reading or processing this configuration file MUST NOT generate an error if they encounter an unknown property"), rspecs.Version)}, ^ validation/misc_props/misc_props.go:67:24: SA1019: invalidConfig.Spec is deprecated: Replace with generator.Config. (staticcheck) {extendedSpec{Spec: *invalidConfig.Spec()}, util.LifecycleActionCreate | util.LifecycleActionStart | util.LifecycleActionDelete, false, specerror.NewError(specerror.ValidValues, fmt.Errorf("runtimes that are reading or processing this configuration file MUST generate an error when invalid or unsupported values are encountered"), rspecs.Version)}, ^ validation/prestart/prestart.go:31:40: SA1019: g.Spec is deprecated: Replace with generator.Config. (staticcheck) output = filepath.Join(r.BundleDir, g.Spec().Root.Path, "output") ^ validation/prestart/prestart.go:33:38: SA1019: g.Spec is deprecated: Replace with generator.Config. (staticcheck) Path: filepath.Join(r.BundleDir, g.Spec().Root.Path, "/bin/sh"), ^ validation/prestart_fail/prestart_fail.go:30:34: SA1019: g.Spec is deprecated: Replace with generator.Config. (staticcheck) Path: filepath.Join(bundleDir, g.Spec().Root.Path, "/bin/false"), ^ validation/poststop_fail/poststop_fail.go:31:37: SA1019: g.Spec is deprecated: Replace with generator.Config. (staticcheck) output := filepath.Join(bundleDir, g.Spec().Root.Path, "output") ^ validation/poststop_fail/poststop_fail.go:33:34: SA1019: g.Spec is deprecated: Replace with generator.Config. (staticcheck) Path: filepath.Join(bundleDir, g.Spec().Root.Path, "/bin/false"), ^ validation/poststop_fail/poststop_fail.go:38:34: SA1019: g.Spec is deprecated: Replace with generator.Config. (staticcheck) Path: filepath.Join(bundleDir, g.Spec().Root.Path, "/bin/sh"), ^ ...and so on (too many to list). Signed-off-by: Kir Kolyshkin --- .../config_updates_without_affect.go | 3 +-- validation/delete_resources/delete_resources.go | 2 +- validation/hooks/hooks.go | 4 ++-- validation/hooks_stdin/hooks_stdin.go | 8 ++++---- validation/killsig/killsig.go | 2 +- validation/misc_props/misc_props.go | 6 +++--- validation/poststart/poststart.go | 4 ++-- validation/poststart_fail/poststart_fail.go | 6 +++--- validation/poststop/poststop.go | 4 ++-- validation/poststop_fail/poststop_fail.go | 6 +++--- validation/prestart/prestart.go | 4 ++-- validation/prestart_fail/prestart_fail.go | 4 ++-- validation/start/start.go | 4 ++-- validation/util/test.go | 2 +- 14 files changed, 29 insertions(+), 30 deletions(-) diff --git a/validation/config_updates_without_affect/config_updates_without_affect.go b/validation/config_updates_without_affect/config_updates_without_affect.go index 77c02ed1..3c758049 100644 --- a/validation/config_updates_without_affect/config_updates_without_affect.go +++ b/validation/config_updates_without_affect/config_updates_without_affect.go @@ -65,10 +65,9 @@ func main() { util.Fatal(err) } - spec := &rspec.Spec{ + g.Config = &rspec.Spec{ Version: "1.0.0", } - g.SetSpec(spec) err = r.SetConfig(g) if err != nil { util.Fatal(err) diff --git a/validation/delete_resources/delete_resources.go b/validation/delete_resources/delete_resources.go index ff0e7d81..1dc0c6e3 100644 --- a/validation/delete_resources/delete_resources.go +++ b/validation/delete_resources/delete_resources.go @@ -59,7 +59,7 @@ func main() { if err != nil { util.Fatal(err) } - if err := util.ValidateLinuxResourcesPids(g.Spec(), t, &state); err != nil { + if err := util.ValidateLinuxResourcesPids(g.Config, t, &state); err != nil { util.Fatal(err) } diff --git a/validation/hooks/hooks.go b/validation/hooks/hooks.go index 3b72085e..c15afb60 100644 --- a/validation/hooks/hooks.go +++ b/validation/hooks/hooks.go @@ -27,8 +27,8 @@ func main() { if err != nil { util.Fatal(err) } - output = filepath.Join(r.BundleDir, g.Spec().Root.Path, "output") - shPath := filepath.Join(r.BundleDir, g.Spec().Root.Path, "/bin/sh") + output = filepath.Join(r.BundleDir, g.Config.Root.Path, "output") + shPath := filepath.Join(r.BundleDir, g.Config.Root.Path, "/bin/sh") err = g.AddPreStartHook(rspec.Hook{ Path: shPath, Args: []string{ diff --git a/validation/hooks_stdin/hooks_stdin.go b/validation/hooks_stdin/hooks_stdin.go index 77777980..b04a377f 100644 --- a/validation/hooks_stdin/hooks_stdin.go +++ b/validation/hooks_stdin/hooks_stdin.go @@ -94,25 +94,25 @@ func main() { if err != nil { util.Fatal(err) } - outputDir := filepath.Join(bundleDir, g.Spec().Root.Path) + outputDir := filepath.Join(bundleDir, g.Config.Root.Path) timeout := 1 g.AddAnnotation(annotationKey, annotationValue) g.AddPreStartHook(rspecs.Hook{ - Path: filepath.Join(bundleDir, g.Spec().Root.Path, "/bin/sh"), + Path: filepath.Join(bundleDir, g.Config.Root.Path, "/bin/sh"), Args: []string{ "sh", "-c", fmt.Sprintf("cat > %s", filepath.Join(outputDir, "prestart")), }, Timeout: &timeout, }) g.AddPostStartHook(rspecs.Hook{ - Path: filepath.Join(bundleDir, g.Spec().Root.Path, "/bin/sh"), + Path: filepath.Join(bundleDir, g.Config.Root.Path, "/bin/sh"), Args: []string{ "sh", "-c", fmt.Sprintf("cat > %s", filepath.Join(outputDir, "poststart")), }, Timeout: &timeout, }) g.AddPostStopHook(rspecs.Hook{ - Path: filepath.Join(bundleDir, g.Spec().Root.Path, "/bin/sh"), + Path: filepath.Join(bundleDir, g.Config.Root.Path, "/bin/sh"), Args: []string{ "sh", "-c", fmt.Sprintf("cat > %s", filepath.Join(outputDir, "poststop")), }, diff --git a/validation/killsig/killsig.go b/validation/killsig/killsig.go index d0cde75d..9d7c2619 100644 --- a/validation/killsig/killsig.go +++ b/validation/killsig/killsig.go @@ -34,7 +34,7 @@ func main() { os.RemoveAll(bundleDir) util.Fatal(err) } - rootDir := filepath.Join(bundleDir, sigConfig.Spec().Root.Path) + rootDir := filepath.Join(bundleDir, sigConfig.Config.Root.Path) for _, signal := range signals { sigConfig.SetProcessArgs([]string{"sh", "-c", fmt.Sprintf("trap 'touch /%s' %s; sleep 10 & wait $!", signal, signal)}) config := util.LifecycleConfig{ diff --git a/validation/misc_props/misc_props.go b/validation/misc_props/misc_props.go index 5a6402e2..c99b8a97 100644 --- a/validation/misc_props/misc_props.go +++ b/validation/misc_props/misc_props.go @@ -62,9 +62,9 @@ func main() { errExpected bool err error }{ - {extendedSpec{Spec: *annotationConfig.Spec()}, util.LifecycleActionCreate | util.LifecycleActionStart | util.LifecycleActionDelete, true, specerror.NewError(specerror.AnnotationsKeyIgnoreUnknown, fmt.Errorf("implementations that are reading/processing this configuration file MUST NOT generate an error if they encounter an unknown annotation key"), rspecs.Version)}, - {extendedSpec{Spec: *basicConfig.Spec(), Unknown: "unknown"}, util.LifecycleActionCreate | util.LifecycleActionStart | util.LifecycleActionDelete, true, specerror.NewError(specerror.ExtensibilityIgnoreUnknownProp, fmt.Errorf("runtimes that are reading or processing this configuration file MUST NOT generate an error if they encounter an unknown property"), rspecs.Version)}, - {extendedSpec{Spec: *invalidConfig.Spec()}, util.LifecycleActionCreate | util.LifecycleActionStart | util.LifecycleActionDelete, false, specerror.NewError(specerror.ValidValues, fmt.Errorf("runtimes that are reading or processing this configuration file MUST generate an error when invalid or unsupported values are encountered"), rspecs.Version)}, + {extendedSpec{Spec: *annotationConfig.Config}, util.LifecycleActionCreate | util.LifecycleActionStart | util.LifecycleActionDelete, true, specerror.NewError(specerror.AnnotationsKeyIgnoreUnknown, fmt.Errorf("implementations that are reading/processing this configuration file MUST NOT generate an error if they encounter an unknown annotation key"), rspecs.Version)}, + {extendedSpec{Spec: *basicConfig.Config, Unknown: "unknown"}, util.LifecycleActionCreate | util.LifecycleActionStart | util.LifecycleActionDelete, true, specerror.NewError(specerror.ExtensibilityIgnoreUnknownProp, fmt.Errorf("runtimes that are reading or processing this configuration file MUST NOT generate an error if they encounter an unknown property"), rspecs.Version)}, + {extendedSpec{Spec: *invalidConfig.Config}, util.LifecycleActionCreate | util.LifecycleActionStart | util.LifecycleActionDelete, false, specerror.NewError(specerror.ValidValues, fmt.Errorf("runtimes that are reading or processing this configuration file MUST generate an error when invalid or unsupported values are encountered"), rspecs.Version)}, } for _, c := range cases { diff --git a/validation/poststart/poststart.go b/validation/poststart/poststart.go index 837cb01c..7ca6b92a 100644 --- a/validation/poststart/poststart.go +++ b/validation/poststart/poststart.go @@ -29,9 +29,9 @@ func main() { if err != nil { util.Fatal(err) } - output = filepath.Join(r.BundleDir, g.Spec().Root.Path, "output") + output = filepath.Join(r.BundleDir, g.Config.Root.Path, "output") err = g.AddPostStartHook(rspec.Hook{ - Path: filepath.Join(r.BundleDir, g.Spec().Root.Path, "/bin/sh"), + Path: filepath.Join(r.BundleDir, g.Config.Root.Path, "/bin/sh"), Args: []string{ "sh", "-c", fmt.Sprintf("echo 'post-start called' >> %s", output), }, diff --git a/validation/poststart_fail/poststart_fail.go b/validation/poststart_fail/poststart_fail.go index 10b5deec..1057afc7 100644 --- a/validation/poststart_fail/poststart_fail.go +++ b/validation/poststart_fail/poststart_fail.go @@ -28,14 +28,14 @@ func main() { if err != nil { util.Fatal(err) } - output := filepath.Join(bundleDir, g.Spec().Root.Path, "output") + output := filepath.Join(bundleDir, g.Config.Root.Path, "output") poststart := rspec.Hook{ - Path: filepath.Join(bundleDir, g.Spec().Root.Path, "/bin/false"), + Path: filepath.Join(bundleDir, g.Config.Root.Path, "/bin/false"), Args: []string{"false"}, } g.AddPostStartHook(poststart) poststartOK := rspec.Hook{ - Path: filepath.Join(bundleDir, g.Spec().Root.Path, "/bin/sh"), + Path: filepath.Join(bundleDir, g.Config.Root.Path, "/bin/sh"), Args: []string{ "sh", "-c", fmt.Sprintf("echo 'post-start called' >> %s", output), }, diff --git a/validation/poststop/poststop.go b/validation/poststop/poststop.go index ccc15147..eec41108 100644 --- a/validation/poststop/poststop.go +++ b/validation/poststop/poststop.go @@ -30,9 +30,9 @@ func main() { if err != nil { util.Fatal(err) } - output = filepath.Join(r.BundleDir, g.Spec().Root.Path, "output") + output = filepath.Join(r.BundleDir, g.Config.Root.Path, "output") err = g.AddPostStopHook(rspec.Hook{ - Path: filepath.Join(r.BundleDir, g.Spec().Root.Path, "/bin/sh"), + Path: filepath.Join(r.BundleDir, g.Config.Root.Path, "/bin/sh"), Args: []string{ "sh", "-c", fmt.Sprintf("echo 'post-stop called' >> %s", output), }, diff --git a/validation/poststop_fail/poststop_fail.go b/validation/poststop_fail/poststop_fail.go index 63c7d445..808237ce 100644 --- a/validation/poststop_fail/poststop_fail.go +++ b/validation/poststop_fail/poststop_fail.go @@ -28,14 +28,14 @@ func main() { if err != nil { util.Fatal(err) } - output := filepath.Join(bundleDir, g.Spec().Root.Path, "output") + output := filepath.Join(bundleDir, g.Config.Root.Path, "output") poststop := rspec.Hook{ - Path: filepath.Join(bundleDir, g.Spec().Root.Path, "/bin/false"), + Path: filepath.Join(bundleDir, g.Config.Root.Path, "/bin/false"), Args: []string{"false"}, } g.AddPostStopHook(poststop) poststopOK := rspec.Hook{ - Path: filepath.Join(bundleDir, g.Spec().Root.Path, "/bin/sh"), + Path: filepath.Join(bundleDir, g.Config.Root.Path, "/bin/sh"), Args: []string{ "sh", "-c", fmt.Sprintf("echo 'post-stop called' >> %s", output), }, diff --git a/validation/prestart/prestart.go b/validation/prestart/prestart.go index 03989475..66bc98f7 100644 --- a/validation/prestart/prestart.go +++ b/validation/prestart/prestart.go @@ -28,9 +28,9 @@ func main() { if err != nil { util.Fatal(err) } - output = filepath.Join(r.BundleDir, g.Spec().Root.Path, "output") + output = filepath.Join(r.BundleDir, g.Config.Root.Path, "output") err = g.AddPreStartHook(rspec.Hook{ - Path: filepath.Join(r.BundleDir, g.Spec().Root.Path, "/bin/sh"), + Path: filepath.Join(r.BundleDir, g.Config.Root.Path, "/bin/sh"), Args: []string{ "sh", "-c", fmt.Sprintf("echo 'pre-start called' >> %s", output), }, diff --git a/validation/prestart_fail/prestart_fail.go b/validation/prestart_fail/prestart_fail.go index aaa8fc5b..cd069cd0 100644 --- a/validation/prestart_fail/prestart_fail.go +++ b/validation/prestart_fail/prestart_fail.go @@ -27,7 +27,7 @@ func main() { util.Fatal(err) } prestart := rspec.Hook{ - Path: filepath.Join(bundleDir, g.Spec().Root.Path, "/bin/false"), + Path: filepath.Join(bundleDir, g.Config.Root.Path, "/bin/false"), Args: []string{"false"}, } g.AddPreStartHook(prestart) @@ -45,7 +45,7 @@ func main() { } runErr := util.RuntimeLifecycleValidate(config) - _, outputErr := os.Stat(filepath.Join(bundleDir, g.Spec().Root.Path, "output")) + _, outputErr := os.Stat(filepath.Join(bundleDir, g.Config.Root.Path, "output")) // query the state r, _ := util.NewRuntime(util.RuntimeCommand, "") diff --git a/validation/start/start.go b/validation/start/start.go index 473f96a8..d4ffb1c3 100644 --- a/validation/start/start.go +++ b/validation/start/start.go @@ -40,7 +40,7 @@ func main() { if err != nil { util.Fatal(err) } - output := filepath.Join(bundleDir, g.Spec().Root.Path, "output") + output := filepath.Join(bundleDir, g.Config.Root.Path, "output") // start without id err = r.Start() @@ -98,7 +98,7 @@ func main() { return } - g.Spec().Process = nil + g.Config.Process = nil err = r.SetConfig(g) if err != nil { util.Fatal(err) diff --git a/validation/util/test.go b/validation/util/test.go index 6f964a36..3079c691 100644 --- a/validation/util/test.go +++ b/validation/util/test.go @@ -296,7 +296,7 @@ func RuntimeOutsideValidate(g *generate.Generator, t *tap.T, f AfterFunc) error if err != nil { return err } - if err := f(g.Spec(), t, &state); err != nil { + if err := f(g.Config, t, &state); err != nil { return err } } From 44e94961acb180248fb72b7d1d1b4c2ec4530fa1 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 14 Oct 2021 20:28:46 -0700 Subject: [PATCH 08/17] Fix "addr cannot be nil" staticcheck linter warnings This code was added by commits 9b1de8cf, fc0fc841, and f5e59a34. Since these new struct members are not pointers, no initialization is needed. Remove the initializers, call the parent initializers from the users. This fixes the following warnings: generate/config.go:191:5: SA4022: the address of a variable cannot be nil (staticcheck) if &g.Config.VM.Hypervisor == nil { ^ generate/config.go:198:5: SA4022: the address of a variable cannot be nil (staticcheck) if &g.Config.VM.Kernel == nil { ^ generate/config.go:205:5: SA4022: the address of a variable cannot be nil (staticcheck) if &g.Config.VM.Image == nil { ^ Signed-off-by: Kir Kolyshkin --- generate/config.go | 21 --------------------- generate/generate.go | 14 +++++++------- 2 files changed, 7 insertions(+), 28 deletions(-) diff --git a/generate/config.go b/generate/config.go index f68bdde3..9ae50fb8 100644 --- a/generate/config.go +++ b/generate/config.go @@ -185,24 +185,3 @@ func (g *Generator) initConfigVM() { g.Config.VM = &rspec.VM{} } } - -func (g *Generator) initConfigVMHypervisor() { - g.initConfigVM() - if &g.Config.VM.Hypervisor == nil { - g.Config.VM.Hypervisor = rspec.VMHypervisor{} - } -} - -func (g *Generator) initConfigVMKernel() { - g.initConfigVM() - if &g.Config.VM.Kernel == nil { - g.Config.VM.Kernel = rspec.VMKernel{} - } -} - -func (g *Generator) initConfigVMImage() { - g.initConfigVM() - if &g.Config.VM.Image == nil { - g.Config.VM.Image = rspec.VMImage{} - } -} diff --git a/generate/generate.go b/generate/generate.go index 6484b02c..cc8ed081 100644 --- a/generate/generate.go +++ b/generate/generate.go @@ -1687,14 +1687,14 @@ func (g *Generator) SetVMHypervisorPath(path string) error { if !strings.HasPrefix(path, "/") { return fmt.Errorf("hypervisorPath %v is not an absolute path", path) } - g.initConfigVMHypervisor() + g.initConfigVM() g.Config.VM.Hypervisor.Path = path return nil } // SetVMHypervisorParameters sets g.Config.VM.Hypervisor.Parameters func (g *Generator) SetVMHypervisorParameters(parameters []string) { - g.initConfigVMHypervisor() + g.initConfigVM() g.Config.VM.Hypervisor.Parameters = parameters } @@ -1703,14 +1703,14 @@ func (g *Generator) SetVMKernelPath(path string) error { if !strings.HasPrefix(path, "/") { return fmt.Errorf("kernelPath %v is not an absolute path", path) } - g.initConfigVMKernel() + g.initConfigVM() g.Config.VM.Kernel.Path = path return nil } // SetVMKernelParameters sets g.Config.VM.Kernel.Parameters func (g *Generator) SetVMKernelParameters(parameters []string) { - g.initConfigVMKernel() + g.initConfigVM() g.Config.VM.Kernel.Parameters = parameters } @@ -1719,7 +1719,7 @@ func (g *Generator) SetVMKernelInitRD(initrd string) error { if !strings.HasPrefix(initrd, "/") { return fmt.Errorf("kernelInitrd %v is not an absolute path", initrd) } - g.initConfigVMKernel() + g.initConfigVM() g.Config.VM.Kernel.InitRD = initrd return nil } @@ -1729,7 +1729,7 @@ func (g *Generator) SetVMImagePath(path string) error { if !strings.HasPrefix(path, "/") { return fmt.Errorf("imagePath %v is not an absolute path", path) } - g.initConfigVMImage() + g.initConfigVM() g.Config.VM.Image.Path = path return nil } @@ -1745,7 +1745,7 @@ func (g *Generator) SetVMImageFormat(format string) error { default: return fmt.Errorf("Commonly supported formats are: raw, qcow2, vdi, vmdk, vhd") } - g.initConfigVMImage() + g.initConfigVM() g.Config.VM.Image.Format = format return nil } From 6a9ad7c2cd02802eb825c160f476ff05ba6c0eca Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 15 Oct 2021 09:23:13 -0700 Subject: [PATCH 09/17] runtimtest: fix validatePosixMounts Commit 20a71e45 implemented a number of improvements, including a modification to validatePosixMounts, which results in the following linter warning: > cmd/runtimetest/main.go:1179:4: SA4006: this value of `rfcError` is never used (staticcheck) > rfcError, err = c.Ok(false, specerror.MountsInOrder, spec.Version, fmt.Sprintf("mounts[%d] (%s) found", i, configMount.Destination)) > ^ It seems that c.harness.YAML() call was mistakenly placed into the "else" code block. Move it out. Signed-off-by: Kir Kolyshkin --- cmd/runtimetest/main.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/cmd/runtimetest/main.go b/cmd/runtimetest/main.go index 6004614a..7dc1f2ec 100644 --- a/cmd/runtimetest/main.go +++ b/cmd/runtimetest/main.go @@ -1179,19 +1179,19 @@ func (c *complianceTester) validatePosixMounts(spec *rspec.Spec) error { rfcError, err = c.Ok(false, specerror.MountsInOrder, spec.Version, fmt.Sprintf("mounts[%d] (%s) found", i, configMount.Destination)) } else { rfcError, err = c.Ok(foundInOrder, specerror.MountsInOrder, spec.Version, fmt.Sprintf("mounts[%d] (%s) found in order", i, configMount.Destination)) - c.harness.YAML(map[string]interface{}{ - "level": rfcError.Level.String(), - "reference": rfcError.Reference, - "config": configMount, - "indexConfig": i, - "indexSystem": configSys[i], - "earlier": map[string]interface{}{ - "config": spec.Mounts[highestMatchedConfig], - "indexConfig": highestMatchedConfig, - "indexSystem": configSys[highestMatchedConfig], - }, - }) } + c.harness.YAML(map[string]interface{}{ + "level": rfcError.Level.String(), + "reference": rfcError.Reference, + "config": configMount, + "indexConfig": i, + "indexSystem": configSys[i], + "earlier": map[string]interface{}{ + "config": spec.Mounts[highestMatchedConfig], + "indexConfig": highestMatchedConfig, + "indexSystem": configSys[highestMatchedConfig], + }, + }) } return mountErrs From 6f4b5bab64a8220496f1c37764eb1dbce13c2b55 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 15 Oct 2021 09:28:00 -0700 Subject: [PATCH 10/17] validate: fix staticcheck linter warning Fix the following warning: > validate/validate.go:134:2: SA4006: this value of `err` is never used (staticcheck) > configRenamedToConfigSchemaVersion, err := semver.Parse("1.0.0-rc2") // config.json became config-schema.json in 1.0.0-rc2 > ^ As the error is unexpected here, ignore it. Signed-off-by: Kir Kolyshkin --- validate/validate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validate/validate.go b/validate/validate.go index 9c371052..a827f11f 100644 --- a/validate/validate.go +++ b/validate/validate.go @@ -131,7 +131,7 @@ func JSONSchemaURL(version string) (url string, err error) { if err != nil { return "", specerror.NewError(specerror.SpecVersionInSemVer, err, rspec.Version) } - configRenamedToConfigSchemaVersion, err := semver.Parse("1.0.0-rc2") // config.json became config-schema.json in 1.0.0-rc2 + configRenamedToConfigSchemaVersion, _ := semver.Parse("1.0.0-rc2") // config.json became config-schema.json in 1.0.0-rc2 if ver.Compare(configRenamedToConfigSchemaVersion) == -1 { return "", fmt.Errorf("unsupported configuration version (older than %s)", configRenamedToConfigSchemaVersion) } From 0ab61aed92a67a6e72b68fc0dd6322c95e4bb05b Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 15 Oct 2021 10:17:34 -0700 Subject: [PATCH 11/17] validation: fix/rename ReadStandardStreams Motivated by this linter warning: > validation/util/container.go:116:3: SA4006: this value of `err` is never used (staticcheck) > err = err2 > ^ Rather than fixing it, let's use bytes.Buffer (rather than files) for stdout and stderr, simplifying the whole thing. Signed-off-by: Kir Kolyshkin --- validation/util/container.go | 41 ++++++++++-------------------------- validation/util/test.go | 11 +--------- 2 files changed, 12 insertions(+), 40 deletions(-) diff --git a/validation/util/container.go b/validation/util/container.go index 1ba49844..da310f37 100644 --- a/validation/util/container.go +++ b/validation/util/container.go @@ -1,11 +1,10 @@ package util import ( + "bytes" "encoding/json" "errors" "fmt" - "io" - "io/ioutil" "os" "os/exec" "path/filepath" @@ -14,7 +13,6 @@ import ( rspecs "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/runtime-tools/generate" "github.com/opencontainers/runtime-tools/specerror" - "github.com/satori/go.uuid" ) // Runtime represents the basic requirement of a container runtime @@ -23,8 +21,8 @@ type Runtime struct { BundleDir string PidFile string ID string - stdout *os.File - stderr *os.File + stdout bytes.Buffer + stderr bytes.Buffer } // DefaultSignal represents the default signal sends to a container @@ -80,17 +78,10 @@ func (r *Runtime) Create() (err error) { args = append(args, r.ID) } cmd := exec.Command(r.RuntimeCommand, args...) - id := uuid.NewV4().String() - r.stdout, err = os.OpenFile(filepath.Join(r.bundleDir(), fmt.Sprintf("stdout-%s", id)), os.O_CREATE|os.O_EXCL|os.O_RDWR, 0600) - if err != nil { - return err - } - cmd.Stdout = r.stdout - r.stderr, err = os.OpenFile(filepath.Join(r.bundleDir(), fmt.Sprintf("stderr-%s", id)), os.O_CREATE|os.O_EXCL|os.O_RDWR, 0600) - if err != nil { - return err - } - cmd.Stderr = r.stderr + r.stdout.Reset() + cmd.Stdout = &r.stdout + r.stderr.Reset() + cmd.Stderr = &r.stderr err = cmd.Run() if err == nil { @@ -98,7 +89,7 @@ func (r *Runtime) Create() (err error) { } if e, ok := err.(*exec.ExitError); ok { - stdout, stderr, _ := r.ReadStandardStreams() + stdout, stderr := r.StandardStreams() if len(stderr) == 0 { stderr = stdout } @@ -108,19 +99,9 @@ func (r *Runtime) Create() (err error) { return err } -// ReadStandardStreams collects content from the stdout and stderr buffers. -func (r *Runtime) ReadStandardStreams() (stdout []byte, stderr []byte, err error) { - _, err = r.stdout.Seek(0, io.SeekStart) - stdout, err2 := ioutil.ReadAll(r.stdout) - if err == nil && err2 != nil { - err = err2 - } - _, err = r.stderr.Seek(0, io.SeekStart) - stderr, err2 = ioutil.ReadAll(r.stderr) - if err == nil && err2 != nil { - err = err2 - } - return stdout, stderr, err +// StandardStreams returns content from the stdout and stderr buffers. +func (r *Runtime) StandardStreams() (stdout, stderr []byte) { + return r.stdout.Bytes(), r.stderr.Bytes() } // Start a container diff --git a/validation/util/test.go b/validation/util/test.go index 3079c691..cae66818 100644 --- a/validation/util/test.go +++ b/validation/util/test.go @@ -228,16 +228,7 @@ func RuntimeInsideValidate(g *generate.Generator, t *tap.T, f PreFunc) (err erro return err } - stdout, stderr, err := r.ReadStandardStreams() - if err != nil { - if len(stderr) == 0 { - stderr = stdout - } - os.Stderr.WriteString("failed to read standard streams\n") - os.Stderr.Write(stderr) - return err - } - + stdout, stderr := r.StandardStreams() // Write stdout in the outter TAP if t != nil { diagnostic := map[string]string{ From fec9c3c247685181cc6481ca123ccba4454ccd0d Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 15 Oct 2021 10:48:03 -0700 Subject: [PATCH 12/17] validation: fix Clean It is only called from defer, so it does not make sense to return an error. So, log errors to stderr instead of returning them. Also, log errors from Kill and WaitingForStatus. Signed-off-by: Kir Kolyshkin --- validation/util/container.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/validation/util/container.go b/validation/util/container.go index da310f37..78f3ef5b 100644 --- a/validation/util/container.go +++ b/validation/util/container.go @@ -178,20 +178,25 @@ func (r *Runtime) Delete() (err error) { // directory is removed after the container is deleted successfully or, if // forceRemoveBundle is true, after the deletion attempt regardless of // whether it was successful or not. -func (r *Runtime) Clean(removeBundle bool, forceRemoveBundle bool) error { - r.Kill("KILL") - WaitingForStatus(*r, LifecycleStatusStopped, time.Second*10, time.Second/10) +func (r *Runtime) Clean(removeBundle bool, forceRemoveBundle bool) { + if err := r.Kill("KILL"); err != nil { + fmt.Fprintf(os.Stderr, "Clean: Kill: %v", err) + } + if err := WaitingForStatus(*r, LifecycleStatusStopped, time.Second*10, time.Second/10); err != nil { + fmt.Fprintf(os.Stderr, "Clean: %v", err) + } err := r.Delete() + if err != nil { + fmt.Fprintf(os.Stderr, "Clean: Delete: %v", err) + } if removeBundle && (err == nil || forceRemoveBundle) { - err2 := os.RemoveAll(r.bundleDir()) - if err2 != nil && err == nil { - err = err2 + err := os.RemoveAll(r.bundleDir()) + if err != nil { + fmt.Fprintf(os.Stderr, "Clean: %v", err) } } - - return err } func execWithStderrFallbackToStdout(cmd *exec.Cmd) (err error) { From 9505f163fd3587836d32773567fe7efbe2aafd45 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 15 Oct 2021 10:50:16 -0700 Subject: [PATCH 13/17] Explicitly ignore errors from YAML We do not expect any errors from YAML function, so explicitly ignore those to silence the errcheck linter. Signed-off-by: Kir Kolyshkin --- cmd/runtimetest/main.go | 52 +++++++++---------- validation/create/create.go | 6 +-- validation/hooks/hooks.go | 4 +- validation/linux_ns_itype/linux_ns_itype.go | 2 +- validation/linux_ns_nopath/linux_ns_nopath.go | 2 +- validation/linux_ns_path/linux_ns_path.go | 2 +- .../linux_ns_path_type/linux_ns_path_type.go | 2 +- validation/pidfile/pidfile.go | 2 +- validation/poststart/poststart.go | 2 +- validation/poststart_fail/poststart_fail.go | 2 +- validation/poststop/poststop.go | 2 +- validation/poststop_fail/poststop_fail.go | 2 +- validation/prestart/prestart.go | 2 +- validation/prestart_fail/prestart_fail.go | 2 +- validation/state/state.go | 4 +- validation/util/test.go | 6 +-- 16 files changed, 47 insertions(+), 47 deletions(-) diff --git a/cmd/runtimetest/main.go b/cmd/runtimetest/main.go index 7dc1f2ec..eaf26a19 100644 --- a/cmd/runtimetest/main.go +++ b/cmd/runtimetest/main.go @@ -159,14 +159,14 @@ func (c *complianceTester) validatePosixUser(spec *rspec.Spec) error { uid := uint32(os.Getuid()) c.harness.Ok(uid == spec.Process.User.UID, "has expected user ID") - c.harness.YAML(map[string]uint32{ + _ = c.harness.YAML(map[string]uint32{ "expected": spec.Process.User.UID, "actual": uid, }) gid := uint32(os.Getgid()) c.harness.Ok(gid == spec.Process.User.GID, "has expected group ID") - c.harness.YAML(map[string]uint32{ + _ = c.harness.YAML(map[string]uint32{ "expected": spec.Process.User.GID, "actual": gid, }) @@ -202,7 +202,7 @@ func (c *complianceTester) validateProcess(spec *rspec.Spec) error { return err } c.harness.Ok(cwd == spec.Process.Cwd, "has expected working directory") - c.harness.YAML(map[string]string{ + _ = c.harness.YAML(map[string]string{ "expected": spec.Process.Cwd, "actual": cwd, }) @@ -214,7 +214,7 @@ func (c *complianceTester) validateProcess(spec *rspec.Spec) error { expectedValue := parts[1] actualValue := os.Getenv(key) c.harness.Ok(expectedValue == actualValue, fmt.Sprintf("has expected environment variable %v", key)) - c.harness.YAML(map[string]string{ + _ = c.harness.YAML(map[string]string{ "variable": key, "expected": expectedValue, "actual": actualValue, @@ -237,13 +237,13 @@ func (c *complianceTester) validateLinuxProcess(spec *rspec.Spec) error { args := bytes.Split(bytes.Trim(cmdlineBytes, "\x00"), []byte("\x00")) c.harness.Ok(len(args) == len(spec.Process.Args), "has expected number of process arguments") - c.harness.YAML(map[string]interface{}{ + _ = c.harness.YAML(map[string]interface{}{ "expected": spec.Process.Args, "actual": args, }) for i, a := range args { c.harness.Ok(string(a) == spec.Process.Args[i], fmt.Sprintf("has expected process argument %d", i)) - c.harness.YAML(map[string]interface{}{ + _ = c.harness.YAML(map[string]interface{}{ "index": i, "expected": spec.Process.Args[i], "actual": string(a), @@ -337,7 +337,7 @@ func (c *complianceTester) validateHostname(spec *rspec.Spec) error { return err } c.harness.Ok(spec.Hostname == hostname, "has expected hostname") - c.harness.YAML(map[string]string{ + _ = c.harness.YAML(map[string]string{ "expected": spec.Hostname, "actual": hostname, }) @@ -365,7 +365,7 @@ func (c *complianceTester) validateRlimits(spec *rspec.Spec) error { if err != nil { return err } - c.harness.YAML(map[string]interface{}{ + _ = c.harness.YAML(map[string]interface{}{ "level": rfcError.Level.String(), "reference": rfcError.Reference, "type": r.Type, @@ -377,7 +377,7 @@ func (c *complianceTester) validateRlimits(spec *rspec.Spec) error { if err != nil { return err } - c.harness.YAML(map[string]interface{}{ + _ = c.harness.YAML(map[string]interface{}{ "level": rfcError.Level.String(), "reference": rfcError.Reference, "type": r.Type, @@ -402,7 +402,7 @@ func (c *complianceTester) validateSysctls(spec *rspec.Spec) error { } value := strings.TrimSpace(string(bytes.Trim(vBytes, "\x00"))) c.harness.Ok(value == v, fmt.Sprintf("has expected sysctl %v", k)) - c.harness.YAML(map[string]string{ + _ = c.harness.YAML(map[string]string{ "sysctl": k, "expected": v, "actual": value, @@ -522,7 +522,7 @@ func (c *complianceTester) validateRootFS(spec *rspec.Spec) error { if err != nil { return err } - c.harness.YAML(map[string]string{ + _ = c.harness.YAML(map[string]string{ "level": rfcError.Level.String(), "reference": rfcError.Reference, }) @@ -625,7 +625,7 @@ func (c *complianceTester) validateDefaultFS(spec *rspec.Spec) error { if err != nil { return err } - c.harness.YAML(map[string]string{ + _ = c.harness.YAML(map[string]string{ "level": rfcError.Level.String(), "reference": rfcError.Reference, "mount": fs, @@ -671,7 +671,7 @@ func (c *complianceTester) validateDevice(device *rspec.LinuxDevice, condition s if err != nil { return err } - c.harness.YAML(map[string]string{ + _ = c.harness.YAML(map[string]string{ "level": rfcError.Level.String(), "reference": rfcError.Reference, "path": device.Path, @@ -703,7 +703,7 @@ func (c *complianceTester) validateDevice(device *rspec.LinuxDevice, condition s if err != nil { return err } - c.harness.YAML(map[string]string{ + _ = c.harness.YAML(map[string]string{ "level": rfcError.Level.String(), "reference": rfcError.Reference, "path": device.Path, @@ -722,7 +722,7 @@ func (c *complianceTester) validateDevice(device *rspec.LinuxDevice, condition s if err != nil { return err } - c.harness.YAML(map[string]interface{}{ + _ = c.harness.YAML(map[string]interface{}{ "level": rfcError.Level.String(), "reference": rfcError.Reference, "path": device.Path, @@ -733,7 +733,7 @@ func (c *complianceTester) validateDevice(device *rspec.LinuxDevice, condition s if err != nil { return err } - c.harness.YAML(map[string]interface{}{ + _ = c.harness.YAML(map[string]interface{}{ "level": rfcError.Level.String(), "reference": rfcError.Reference, "path": device.Path, @@ -751,7 +751,7 @@ func (c *complianceTester) validateDevice(device *rspec.LinuxDevice, condition s if err != nil { return err } - c.harness.YAML(map[string]interface{}{ + _ = c.harness.YAML(map[string]interface{}{ "level": rfcError.Level.String(), "reference": rfcError.Reference, "path": device.Path, @@ -772,7 +772,7 @@ func (c *complianceTester) validateDevice(device *rspec.LinuxDevice, condition s if err != nil { return err } - c.harness.YAML(map[string]interface{}{ + _ = c.harness.YAML(map[string]interface{}{ "level": rfcError.Level.String(), "reference": rfcError.Reference, "path": device.Path, @@ -788,7 +788,7 @@ func (c *complianceTester) validateDevice(device *rspec.LinuxDevice, condition s if err != nil { return err } - c.harness.YAML(map[string]interface{}{ + _ = c.harness.YAML(map[string]interface{}{ "level": rfcError.Level.String(), "reference": rfcError.Reference, "path": device.Path, @@ -815,7 +815,7 @@ func (c *complianceTester) validateDefaultSymlinks(spec *rspec.Spec) error { if err != nil { return err } - c.harness.YAML(map[string]string{ + _ = c.harness.YAML(map[string]string{ "level": rfcError.Level.String(), "reference": rfcError.Reference, "path": symlink, @@ -833,7 +833,7 @@ func (c *complianceTester) validateDefaultSymlinks(spec *rspec.Spec) error { if err != nil { return err } - c.harness.YAML(map[string]interface{}{ + _ = c.harness.YAML(map[string]interface{}{ "level": rfcError.Level.String(), "reference": rfcError.Reference, "path": symlink, @@ -855,7 +855,7 @@ func (c *complianceTester) validateDefaultSymlinks(spec *rspec.Spec) error { if err != nil { return err } - c.harness.YAML(map[string]string{ + _ = c.harness.YAML(map[string]string{ "level": rfcError.Level.String(), "reference": rfcError.Reference, "path": symlink, @@ -986,7 +986,7 @@ func (c *complianceTester) validateOOMScoreAdj(spec *rspec.Spec) error { if err != nil { return err } - c.harness.YAML(map[string]interface{}{ + _ = c.harness.YAML(map[string]interface{}{ "level": rfcError.Level.String(), "reference": rfcError.Reference, "expected": expected, @@ -1047,7 +1047,7 @@ func (c *complianceTester) validateIDMappings(mappings []rspec.LinuxIDMapping, p return err } c.harness.Ok(len(idMaps) == len(mappings), fmt.Sprintf("%s has expected number of mappings", path)) - c.harness.YAML(map[string]interface{}{ + _ = c.harness.YAML(map[string]interface{}{ "expected": mappings, "actual": idMaps, }) @@ -1180,7 +1180,7 @@ func (c *complianceTester) validatePosixMounts(spec *rspec.Spec) error { } else { rfcError, err = c.Ok(foundInOrder, specerror.MountsInOrder, spec.Version, fmt.Sprintf("mounts[%d] (%s) found in order", i, configMount.Destination)) } - c.harness.YAML(map[string]interface{}{ + _ = c.harness.YAML(map[string]interface{}{ "level": rfcError.Level.String(), "reference": rfcError.Reference, "config": configMount, @@ -1231,7 +1231,7 @@ func (c *complianceTester) validateMountLabel(spec *rspec.Spec) error { return fmt.Errorf("Failed to get mountLabel of %v", mount.Destination) } c.harness.Ok(spec.Linux.MountLabel == fileLabel, "has expected mountlabel") - c.harness.YAML(map[string]string{ + _ = c.harness.YAML(map[string]string{ "expected": spec.Linux.MountLabel, "actual": fileLabel, }) diff --git a/validation/create/create.go b/validation/create/create.go index a06715f6..34bed311 100644 --- a/validation/create/create.go +++ b/validation/create/create.go @@ -66,13 +66,13 @@ func main() { } } } - t.YAML(diagnostic) + _ = t.YAML(diagnostic) if err == nil { state, err := r.State() t.Ok(err == nil && state.ID == c.id, "'state' MUST return the state of a container") if err == nil { - t.YAML(map[string]string{ + _ = t.YAML(map[string]string{ "container ID": c.id, "state ID": state.ID, }) @@ -85,7 +85,7 @@ func main() { diagnostic["stderr"] = string(e.Stderr) } } - t.YAML(diagnostic) + _ = t.YAML(diagnostic) } } } diff --git a/validation/hooks/hooks.go b/validation/hooks/hooks.go index c15afb60..5581fb5f 100644 --- a/validation/hooks/hooks.go +++ b/validation/hooks/hooks.go @@ -99,7 +99,7 @@ func main() { diagnostic := map[string]string{ "error": err.Error(), } - t.YAML(diagnostic) + _ = t.YAML(diagnostic) } else { diagnostic := map[string]string{ "error": err.Error(), @@ -109,7 +109,7 @@ func main() { diagnostic["stderr"] = string(e.Stderr) } } - t.YAML(diagnostic) + _ = t.YAML(diagnostic) } t.AutoPlan() diff --git a/validation/linux_ns_itype/linux_ns_itype.go b/validation/linux_ns_itype/linux_ns_itype.go index a7399943..1b08b458 100644 --- a/validation/linux_ns_itype/linux_ns_itype.go +++ b/validation/linux_ns_itype/linux_ns_itype.go @@ -22,7 +22,7 @@ func printDiag(t *tap.T, diagActual, diagExpected, diagNsType string, errNs erro "level": specErr.(*specerror.Error).Err.Level.String(), "reference": specErr.(*specerror.Error).Err.Reference, } - t.YAML(diagnostic) + _ = t.YAML(diagnostic) } func testNamespaceInheritType(t *tap.T) error { diff --git a/validation/linux_ns_nopath/linux_ns_nopath.go b/validation/linux_ns_nopath/linux_ns_nopath.go index 42fd16dc..7aea78d0 100644 --- a/validation/linux_ns_nopath/linux_ns_nopath.go +++ b/validation/linux_ns_nopath/linux_ns_nopath.go @@ -22,7 +22,7 @@ func printDiag(t *tap.T, diagActual, diagExpected, diagNsType string, errNs erro "level": specErr.(*specerror.Error).Err.Level.String(), "reference": specErr.(*specerror.Error).Err.Reference, } - t.YAML(diagnostic) + _ = t.YAML(diagnostic) } func testNamespaceNoPath(t *tap.T) error { diff --git a/validation/linux_ns_path/linux_ns_path.go b/validation/linux_ns_path/linux_ns_path.go index 65db1997..8744fb59 100644 --- a/validation/linux_ns_path/linux_ns_path.go +++ b/validation/linux_ns_path/linux_ns_path.go @@ -166,7 +166,7 @@ func main() { "level": rfcError.Level.String(), "reference": rfcError.Reference, } - t.YAML(diagnostic) + _ = t.YAML(diagnostic) } } diff --git a/validation/linux_ns_path_type/linux_ns_path_type.go b/validation/linux_ns_path_type/linux_ns_path_type.go index 7e903dc6..82846831 100644 --- a/validation/linux_ns_path_type/linux_ns_path_type.go +++ b/validation/linux_ns_path_type/linux_ns_path_type.go @@ -42,7 +42,7 @@ func checkNSPathMatchType(t *tap.T, ns, wrongNs string) error { "level": rfcError.Level.String(), "reference": rfcError.Reference, } - t.YAML(diagnostic) + _ = t.YAML(diagnostic) return fmt.Errorf("cannot validate path with wrong type") } diff --git a/validation/pidfile/pidfile.go b/validation/pidfile/pidfile.go index 8ec10df3..ed7e3ea9 100644 --- a/validation/pidfile/pidfile.go +++ b/validation/pidfile/pidfile.go @@ -76,7 +76,7 @@ func main() { diagnostic["stderr"] = string(e.Stderr) } } - t.YAML(diagnostic) + _ = t.YAML(diagnostic) } t.AutoPlan() diff --git a/validation/poststart/poststart.go b/validation/poststart/poststart.go index 7ca6b92a..62301c16 100644 --- a/validation/poststart/poststart.go +++ b/validation/poststart/poststart.go @@ -86,7 +86,7 @@ func main() { diagnostic["stderr"] = string(e.Stderr) } } - t.YAML(diagnostic) + _ = t.YAML(diagnostic) } t.AutoPlan() diff --git a/validation/poststart_fail/poststart_fail.go b/validation/poststart_fail/poststart_fail.go index 1057afc7..76487089 100644 --- a/validation/poststart_fail/poststart_fail.go +++ b/validation/poststart_fail/poststart_fail.go @@ -66,7 +66,7 @@ func main() { diagnostic := map[string]string{ "error": err.Error(), } - t.YAML(diagnostic) + _ = t.YAML(diagnostic) } t.AutoPlan() diff --git a/validation/poststop/poststop.go b/validation/poststop/poststop.go index eec41108..b1c232d3 100644 --- a/validation/poststop/poststop.go +++ b/validation/poststop/poststop.go @@ -103,7 +103,7 @@ func main() { diagnostic["stderr"] = string(e.Stderr) } } - t.YAML(diagnostic) + _ = t.YAML(diagnostic) } t.AutoPlan() diff --git a/validation/poststop_fail/poststop_fail.go b/validation/poststop_fail/poststop_fail.go index 808237ce..e7d5e851 100644 --- a/validation/poststop_fail/poststop_fail.go +++ b/validation/poststop_fail/poststop_fail.go @@ -66,7 +66,7 @@ func main() { diagnostic := map[string]string{ "error": err.Error(), } - t.YAML(diagnostic) + _ = t.YAML(diagnostic) } t.AutoPlan() diff --git a/validation/prestart/prestart.go b/validation/prestart/prestart.go index 66bc98f7..4261b8ab 100644 --- a/validation/prestart/prestart.go +++ b/validation/prestart/prestart.go @@ -85,7 +85,7 @@ func main() { diagnostic["stderr"] = string(e.Stderr) } } - t.YAML(diagnostic) + _ = t.YAML(diagnostic) } t.AutoPlan() diff --git a/validation/prestart_fail/prestart_fail.go b/validation/prestart_fail/prestart_fail.go index cd069cd0..3cf43e9f 100644 --- a/validation/prestart_fail/prestart_fail.go +++ b/validation/prestart_fail/prestart_fail.go @@ -64,7 +64,7 @@ func main() { diagnostic := map[string]string{ "error": err.Error(), } - t.YAML(diagnostic) + _ = t.YAML(diagnostic) } t.AutoPlan() diff --git a/validation/state/state.go b/validation/state/state.go index 986a52e0..170dd4fb 100644 --- a/validation/state/state.go +++ b/validation/state/state.go @@ -57,7 +57,7 @@ func main() { "reference": e.Err.Reference, "error": e.Err.Error(), } - t.YAML(diagnostic) + _ = t.YAML(diagnostic) continue } @@ -73,7 +73,7 @@ func main() { } } } - t.YAML(diagnostic) + _ = t.YAML(diagnostic) } t.AutoPlan() diff --git a/validation/util/test.go b/validation/util/test.go index cae66818..0f21fb69 100644 --- a/validation/util/test.go +++ b/validation/util/test.go @@ -100,7 +100,7 @@ func Skip(message string, diagnostic interface{}) { t.Header(1) t.Skip(1, message) if diagnostic != nil { - t.YAML(diagnostic) + _ = t.YAML(diagnostic) } } @@ -119,7 +119,7 @@ func SpecErrorOK(t *tap.T, expected bool, specErr error, detailedErr error) { } } } - t.YAML(diagnostic) + _ = t.YAML(diagnostic) } // PrepareBundle creates a test bundle in a temporary directory. @@ -238,7 +238,7 @@ func RuntimeInsideValidate(g *generate.Generator, t *tap.T, f PreFunc) (err erro if err != nil { diagnostic["error"] = fmt.Sprintf("%v", err) } - t.YAML(diagnostic) + _ = t.YAML(diagnostic) t.Ok(err == nil && !strings.Contains(string(stdout), "not ok"), g.Config.Annotations["TestName"]) } else { if runtimeInsideValidateCalled { From c0037c96771ad1be0861fa0a2dbd3e764af16042 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 15 Oct 2021 10:51:31 -0700 Subject: [PATCH 14/17] runtimetest: silence errlint on unix.Unmount Signed-off-by: Kir Kolyshkin --- cmd/runtimetest/main.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/runtimetest/main.go b/cmd/runtimetest/main.go index eaf26a19..d7df16a7 100644 --- a/cmd/runtimetest/main.go +++ b/cmd/runtimetest/main.go @@ -568,11 +568,11 @@ func (c *complianceTester) validateRootfsPropagation(spec *rspec.Spec) error { if err := unix.Mount("/", targetDir, "", unix.MS_BIND|unix.MS_REC, ""); err != nil { return err } - defer unix.Unmount(targetDir, unix.MNT_DETACH) + defer unix.Unmount(targetDir, unix.MNT_DETACH) //nolint:errcheck if err := unix.Mount(testDir, mountDir, "", unix.MS_BIND|unix.MS_REC, ""); err != nil { return err } - defer unix.Unmount(mountDir, unix.MNT_DETACH) + defer unix.Unmount(mountDir, unix.MNT_DETACH) //nolint:errcheck targetFile := filepath.Join(targetDir, filepath.Join(mountDir, filepath.Base(tmpfile.Name()))) var exposed bool _, err = os.Stat(targetFile) @@ -599,7 +599,7 @@ func (c *complianceTester) validateRootfsPropagation(spec *rspec.Spec) error { } else if err != nil { return err } - defer unix.Unmount(targetDir, unix.MNT_DETACH) + defer unix.Unmount(targetDir, unix.MNT_DETACH) //nolint:errcheck c.harness.Fail("root propagation is unbindable") return nil default: From a7cecde5aee438ff31453798db8c2eff9c7a51cd Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 15 Oct 2021 11:23:13 -0700 Subject: [PATCH 15/17] Add*Hook: do not return errors These functions never return errors, so let's simplify them and their callers. This fixes a bunch of errcheck linter warning, as some callers of these methods were not checking for errors. Signed-off-by: Kir Kolyshkin --- cmd/oci-runtime-tool/generate.go | 12 +++--------- generate/generate.go | 9 +++------ validation/hooks/hooks.go | 30 ++++++------------------------ validation/poststart/poststart.go | 5 +---- validation/poststop/poststop.go | 5 +---- validation/prestart/prestart.go | 5 +---- 6 files changed, 15 insertions(+), 51 deletions(-) diff --git a/cmd/oci-runtime-tool/generate.go b/cmd/oci-runtime-tool/generate.go index 950b6413..af250ab4 100644 --- a/cmd/oci-runtime-tool/generate.go +++ b/cmd/oci-runtime-tool/generate.go @@ -529,9 +529,7 @@ func setupSpec(g *generate.Generator, context *cli.Context) error { if err := json.Unmarshal([]byte(hook), &tmpHook); err != nil { return err } - if err := g.AddPostStartHook(tmpHook); err != nil { - return err - } + g.AddPostStartHook(tmpHook) } } @@ -546,9 +544,7 @@ func setupSpec(g *generate.Generator, context *cli.Context) error { if err := json.Unmarshal([]byte(hook), &tmpHook); err != nil { return err } - if err := g.AddPostStopHook(tmpHook); err != nil { - return err - } + g.AddPostStopHook(tmpHook) } } @@ -563,9 +559,7 @@ func setupSpec(g *generate.Generator, context *cli.Context) error { if err := json.Unmarshal([]byte(hook), &tmpHook); err != nil { return err } - if err := g.AddPreStartHook(tmpHook); err != nil { - return err - } + g.AddPreStartHook(tmpHook) } } diff --git a/generate/generate.go b/generate/generate.go index cc8ed081..8aba53f6 100644 --- a/generate/generate.go +++ b/generate/generate.go @@ -1025,10 +1025,9 @@ func (g *Generator) ClearPreStartHooks() { } // AddPreStartHook add a prestart hook into g.Config.Hooks.Prestart. -func (g *Generator) AddPreStartHook(preStartHook rspec.Hook) error { +func (g *Generator) AddPreStartHook(preStartHook rspec.Hook) { g.initConfigHooks() g.Config.Hooks.Prestart = append(g.Config.Hooks.Prestart, preStartHook) - return nil } // ClearPostStopHooks clear g.Config.Hooks.Poststop. @@ -1040,10 +1039,9 @@ func (g *Generator) ClearPostStopHooks() { } // AddPostStopHook adds a poststop hook into g.Config.Hooks.Poststop. -func (g *Generator) AddPostStopHook(postStopHook rspec.Hook) error { +func (g *Generator) AddPostStopHook(postStopHook rspec.Hook) { g.initConfigHooks() g.Config.Hooks.Poststop = append(g.Config.Hooks.Poststop, postStopHook) - return nil } // ClearPostStartHooks clear g.Config.Hooks.Poststart. @@ -1055,10 +1053,9 @@ func (g *Generator) ClearPostStartHooks() { } // AddPostStartHook adds a poststart hook into g.Config.Hooks.Poststart. -func (g *Generator) AddPostStartHook(postStartHook rspec.Hook) error { +func (g *Generator) AddPostStartHook(postStartHook rspec.Hook) { g.initConfigHooks() g.Config.Hooks.Poststart = append(g.Config.Hooks.Poststart, postStartHook) - return nil } // AddMount adds a mount into g.Config.Mounts. diff --git a/validation/hooks/hooks.go b/validation/hooks/hooks.go index 5581fb5f..4494e3ad 100644 --- a/validation/hooks/hooks.go +++ b/validation/hooks/hooks.go @@ -29,60 +29,42 @@ func main() { } output = filepath.Join(r.BundleDir, g.Config.Root.Path, "output") shPath := filepath.Join(r.BundleDir, g.Config.Root.Path, "/bin/sh") - err = g.AddPreStartHook(rspec.Hook{ + g.AddPreStartHook(rspec.Hook{ Path: shPath, Args: []string{ "sh", "-c", fmt.Sprintf("echo 'pre-start1 called' >> %s", output), }, }) - if err != nil { - return err - } - err = g.AddPreStartHook(rspec.Hook{ + g.AddPreStartHook(rspec.Hook{ Path: shPath, Args: []string{ "sh", "-c", fmt.Sprintf("echo 'pre-start2 called' >> %s", output), }, }) - if err != nil { - return err - } - err = g.AddPostStartHook(rspec.Hook{ + g.AddPostStartHook(rspec.Hook{ Path: shPath, Args: []string{ "sh", "-c", fmt.Sprintf("echo 'post-start1 called' >> %s", output), }, }) - if err != nil { - return err - } - err = g.AddPostStartHook(rspec.Hook{ + g.AddPostStartHook(rspec.Hook{ Path: shPath, Args: []string{ "sh", "-c", fmt.Sprintf("echo 'post-start2 called' >> %s", output), }, }) - if err != nil { - return err - } - err = g.AddPostStopHook(rspec.Hook{ + g.AddPostStopHook(rspec.Hook{ Path: shPath, Args: []string{ "sh", "-c", fmt.Sprintf("echo 'post-stop1 called' >> %s", output), }, }) - if err != nil { - return err - } - err = g.AddPostStopHook(rspec.Hook{ + g.AddPostStopHook(rspec.Hook{ Path: shPath, Args: []string{ "sh", "-c", fmt.Sprintf("echo 'post-stop2 called' >> %s", output), }, }) - if err != nil { - return err - } g.SetProcessArgs([]string{"true"}) return r.SetConfig(g) }, diff --git a/validation/poststart/poststart.go b/validation/poststart/poststart.go index 62301c16..08aa3e75 100644 --- a/validation/poststart/poststart.go +++ b/validation/poststart/poststart.go @@ -30,15 +30,12 @@ func main() { util.Fatal(err) } output = filepath.Join(r.BundleDir, g.Config.Root.Path, "output") - err = g.AddPostStartHook(rspec.Hook{ + g.AddPostStartHook(rspec.Hook{ Path: filepath.Join(r.BundleDir, g.Config.Root.Path, "/bin/sh"), Args: []string{ "sh", "-c", fmt.Sprintf("echo 'post-start called' >> %s", output), }, }) - if err != nil { - return err - } g.SetProcessArgs([]string{"sh", "-c", fmt.Sprintf("echo 'process called' >> %s", "/output")}) return r.SetConfig(g) }, diff --git a/validation/poststop/poststop.go b/validation/poststop/poststop.go index b1c232d3..7da2be18 100644 --- a/validation/poststop/poststop.go +++ b/validation/poststop/poststop.go @@ -31,15 +31,12 @@ func main() { util.Fatal(err) } output = filepath.Join(r.BundleDir, g.Config.Root.Path, "output") - err = g.AddPostStopHook(rspec.Hook{ + g.AddPostStopHook(rspec.Hook{ Path: filepath.Join(r.BundleDir, g.Config.Root.Path, "/bin/sh"), Args: []string{ "sh", "-c", fmt.Sprintf("echo 'post-stop called' >> %s", output), }, }) - if err != nil { - return err - } g.SetProcessArgs([]string{"sh", "-c", fmt.Sprintf("echo 'process called' >> %s", "/output")}) return r.SetConfig(g) }, diff --git a/validation/prestart/prestart.go b/validation/prestart/prestart.go index 4261b8ab..852ef4cb 100644 --- a/validation/prestart/prestart.go +++ b/validation/prestart/prestart.go @@ -29,15 +29,12 @@ func main() { util.Fatal(err) } output = filepath.Join(r.BundleDir, g.Config.Root.Path, "output") - err = g.AddPreStartHook(rspec.Hook{ + g.AddPreStartHook(rspec.Hook{ Path: filepath.Join(r.BundleDir, g.Config.Root.Path, "/bin/sh"), Args: []string{ "sh", "-c", fmt.Sprintf("echo 'pre-start called' >> %s", output), }, }) - if err != nil { - return err - } g.SetProcessArgs([]string{"sh", "-c", fmt.Sprintf("echo 'process called' >> %s", "/output")}) return r.SetConfig(g) }, From a22a8949b70d0a05e0eabe06932b959f18e9f225 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 15 Oct 2021 11:42:02 -0700 Subject: [PATCH 16/17] ci: add golangci-lint run Had to disable errcheck linter, as there are too many warnings yet to be fixed. Signed-off-by: Kir Kolyshkin --- .github/workflows/test.yml | 9 +++++++++ .golangci.yml | 5 +++++ 2 files changed, 14 insertions(+) create mode 100644 .golangci.yml diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index cd32d368..0c53a58a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -9,6 +9,15 @@ on: pull_request: jobs: + + lint: + runs-on: ubuntu-20.04 + steps: + - uses: actions/checkout@v2 + - uses: golangci/golangci-lint-action@v2 + with: + version: v1.42 + test: runs-on: ubuntu-20.04 strategy: diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..3ee5b4bc --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,5 @@ +# For documentation, see https://golangci-lint.run/usage/configuration/ + +linters: + disable: + - errcheck From 10c865d718d9d36e4295140c4e53c6b32ed8e961 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 15 Oct 2021 13:57:53 -0700 Subject: [PATCH 17/17] ci: re-add commit subject length validation To replace the one removed by commit 5432bc4b24f3021. Signed-off-by: Kir Kolyshkin --- .github/workflows/test.yml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0c53a58a..7fad2429 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -18,6 +18,24 @@ jobs: with: version: v1.42 + commit: + runs-on: ubuntu-20.04 + # Only check commits on pull requests. + if: github.event_name == 'pull_request' + steps: + - name: get pr commits + id: 'get-pr-commits' + uses: tim-actions/get-pr-commits@v1.1.0 + with: + token: ${{ secrets.GITHUB_TOKEN }} + + - name: check subject line length + uses: tim-actions/commit-message-checker-with-regex@v0.3.1 + with: + commits: ${{ steps.get-pr-commits.outputs.commits }} + pattern: '^.{0,72}(\n.*)*$' + error: 'Subject too long (max 72)' + test: runs-on: ubuntu-20.04 strategy: