From 99221d0ef17d8d921608ed71339d440ec95c48e2 Mon Sep 17 00:00:00 2001 From: Krisztian Litkey Date: Tue, 15 Sep 2026 21:52:35 +0300 Subject: [PATCH 1/6] e2e: boot the node back to a NUMA kernel whatever happens. The two test30-numa-disabled tests boot the node with a kernel which has NUMA support compiled out, and boot it back at the end by calling enable-numa as their last statement. A failure anywhere in between never gets there: error exits the test, so the node keeps running the kernel without NUMA. If we are running multiple tests, every test after the failure sees a machine with a single node... and will fail. Restore the kernel from an EXIT trap instead, the way the tests which have state to put back already do it, and let enable-numa return without doing anything if NUMA is enabled, so that calling it when there is nothing to restore is free. Looking into a failure of these tests is easier on a node which still has NUMA disabled, so take keep_numa_disabled=1 to leave it that way: keep_numa_disabled=1 ./run_tests.sh \ policies.test-suite/balloons/n4c16/test30-numa-disabled Assisted-by: Claude Opus 5 (1M context) Signed-off-by: Krisztian Litkey --- test/e2e/lib/test.bash | 16 +++++++++++++++- .../n4c16/test30-numa-disabled/code.var.sh | 6 ++++-- .../n4c16/test30-numa-disabled/code.var.sh | 5 ++++- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/test/e2e/lib/test.bash b/test/e2e/lib/test.bash index a164f0293..5fba05e4b 100644 --- a/test/e2e/lib/test.bash +++ b/test/e2e/lib/test.bash @@ -349,7 +349,21 @@ enable-numa() { # script API # # Boot the node back with a kernel which has NUMA support, and make sure # that the container runtime and POLICY, $POLICY by default, are running - # afterwards. + # afterwards. Do nothing if NUMA is already enabled, so that this can be + # called to restore the node whether it needs restoring or not. + # + # Set keep_numa_disabled to 1, true or yes to leave the node as it is, for + # looking into a failure on a node which has no NUMA. Note that this leaves + # the VM that way for every test which runs on it afterwards. + vm-command '[ -d /sys/devices/system/node ]' && return 0 + + case "${keep_numa_disabled:-no}" in + 1|true|yes) + echo "keep_numa_disabled=$keep_numa_disabled, leaving the node without NUMA..." + return 0 + ;; + esac + vm-kernel-pkgs-uninstall vm-post-reboot-runtime-check "${1:-$POLICY}" } diff --git a/test/e2e/policies.test-suite/balloons/n4c16/test30-numa-disabled/code.var.sh b/test/e2e/policies.test-suite/balloons/n4c16/test30-numa-disabled/code.var.sh index 5a77df410..59e503f97 100644 --- a/test/e2e/policies.test-suite/balloons/n4c16/test30-numa-disabled/code.var.sh +++ b/test/e2e/policies.test-suite/balloons/n4c16/test30-numa-disabled/code.var.sh @@ -1,5 +1,9 @@ disable-numa +# Boot the node back with a NUMA capable kernel however this test ends. Every +# test which runs on this VM afterwards depends on it. +trap enable-numa EXIT + relaunch-policy balloons "$TEST_DIR/balloons-numa-disabled.cfg" POD_ANNOTATION=( @@ -19,5 +23,3 @@ verify "cpus['pod1c0'].isdisjoint({'cpu06', 'cpu07'})" \ "len(cpus['pod1c0']) == 5" \ "len(cpus['pod1c1']) == 5" \ "disjoint_sets(cpus['pod1c0'], cpus['pod1c1'])" - -enable-numa diff --git a/test/e2e/policies.test-suite/topology-aware/n4c16/test30-numa-disabled/code.var.sh b/test/e2e/policies.test-suite/topology-aware/n4c16/test30-numa-disabled/code.var.sh index ab8292dc6..417b055c5 100644 --- a/test/e2e/policies.test-suite/topology-aware/n4c16/test30-numa-disabled/code.var.sh +++ b/test/e2e/policies.test-suite/topology-aware/n4c16/test30-numa-disabled/code.var.sh @@ -1,5 +1,9 @@ disable-numa +# Boot the node back with a NUMA capable kernel however this test ends. Every +# test which runs on this VM afterwards depends on it. +trap enable-numa EXIT + helm-terminate helm_config=$(instantiate helm-config.yaml) helm-launch topology-aware @@ -16,4 +20,3 @@ verify \ delete-pods --all helm-terminate -enable-numa From 90aa7b9877ad8e947be5d552a7db120d0a0eff17 Mon Sep 17 00:00:00 2001 From: Krisztian Litkey Date: Tue, 15 Sep 2026 14:06:42 +0300 Subject: [PATCH 2/6] e2e: resolve a single pod when waiting for one. Print a single pod in vm-wait-pod-regexp, and never one which is terminating. Otherwise port forwarding got two names while one pod was terminating and another starting, and kubectl refused: error: TYPE/NAME and list of ports are required for port-forward Assisted-by: Claude Opus 5 (1M context) Signed-off-by: Krisztian Litkey --- test/e2e/lib/vm.bash | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/e2e/lib/vm.bash b/test/e2e/lib/vm.bash index c911eb20a..598d138ff 100644 --- a/test/e2e/lib/vm.bash +++ b/test/e2e/lib/vm.bash @@ -1081,7 +1081,9 @@ vm-wait-pod-regexp() { # Rudimentary wait as "kubectl wait" will timeout immediately if pod is not yet there. vm-run-until --timeout "$wait_t" "kubectl get pods $namespace_args | grep -q $pod_regexp" || error "timeout while waiting $pod_regexp" - local POD="$(vm-command-q "kubectl get pods $namespace_args | awk '/${pod_regexp}/ { print \$1 }'")" + # Print a single pod and never a terminating one. + local POD="$(vm-command-q "kubectl get pods $namespace_args | \ + awk '/${pod_regexp}/ && \$3 != \"Terminating\" { print \$1; exit }'")" if [ -z "$POD" ]; then command-error "Pod $pod_regexp not found" fi From b8d62fc40dd4df68add5d78e647e26edcfab8491 Mon Sep 17 00:00:00 2001 From: Krisztian Litkey Date: Tue, 15 Sep 2026 12:28:30 +0300 Subject: [PATCH 3/6] e2e: stop using a top-level ansible fact. Use ansible_facts to disable swap during provisioning. Signed-off-by: Krisztian Litkey --- test/e2e/playbook/provision.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/playbook/provision.yaml b/test/e2e/playbook/provision.yaml index be961c0bf..1dcd93b52 100644 --- a/test/e2e/playbook/provision.yaml +++ b/test/e2e/playbook/provision.yaml @@ -59,7 +59,7 @@ with_items: - for swp in `systemctl --type swap | awk '/\.swap/ { print $1 }'`; do systemctl stop "$swp"; systemctl mask "$swp"; done - swapoff --all - when: ansible_swaptotal_mb > 0 + when: ansible_facts['swaptotal_mb'] > 0 - name: Add Kubernetes APT repository to sources.list.d ansible.builtin.copy: From f1e80b7782f7e5690b6694588005b229a574c30a Mon Sep 17 00:00:00 2001 From: Krisztian Litkey Date: Tue, 15 Sep 2026 12:20:15 +0300 Subject: [PATCH 4/6] instrumentation: serve coverage data over HTTP. Add a coverage package which serves the coverage data of a plugin built with go build -cover over the instrumentation HTTP server: the meta-data, a snapshot of the counters, and the ID of the binary tying the two together. The ID is the one the go runtime names the files it writes to $GOCOVERDIR with, dug out of the meta-data header, so that data served over HTTP and data dumped at exit merge as they are. This is how we get the coverage of a plugin which is still running, or which never gets to exit gracefully. Serve it from the resource manager, so from the nri-resource-policy-* plugins, and only with the test APIs enabled. runtime/coverage works in a binary built with -cover and never in a test binary, so the tests build a helper to check that what we serve is what go tool covdata expects. Assisted-by: Claude Opus 5 (1M context) Signed-off-by: Krisztian Litkey --- pkg/instrumentation/coverage/coverage.go | 205 +++++++++++++ pkg/instrumentation/coverage/coverage_test.go | 271 ++++++++++++++++++ .../coverage/internal/coverage-server/main.go | 51 ++++ pkg/resmgr/resource-manager.go | 13 + 4 files changed, 540 insertions(+) create mode 100644 pkg/instrumentation/coverage/coverage.go create mode 100644 pkg/instrumentation/coverage/coverage_test.go create mode 100644 pkg/instrumentation/coverage/internal/coverage-server/main.go diff --git a/pkg/instrumentation/coverage/coverage.go b/pkg/instrumentation/coverage/coverage.go new file mode 100644 index 000000000..8535077f3 --- /dev/null +++ b/pkg/instrumentation/coverage/coverage.go @@ -0,0 +1,205 @@ +// Copyright The NRI Plugins Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package coverage serves the coverage data of a plugin built with coverage +// instrumentation, using make COVER=1, over the instrumentation HTTP server. +// It lets tests which run a plugin as a whole, in a cluster, collect coverage +// data from it the way go test -cover does for unit tests. Our e2e tests use +// this to take a snapshot of the counters per test case. +// +// Save the served data to files named covmeta. and covcounters..N.M, +// with from the ID endpoint, and go tool covdata can digest the result. +// A plugin built without instrumentation serves an error for all endpoints. +package coverage + +import ( + "bytes" + "encoding/binary" + "encoding/hex" + "fmt" + "io" + "net/http" + "runtime/coverage" + "sync" + + xhttp "github.com/containers/nri-plugins/pkg/http" + logger "github.com/containers/nri-plugins/pkg/log" +) + +const ( + // IDPath serves the ID of the instrumented binary. + IDPath = "/coverage/id" + // MetaPath serves the coverage meta-data of the binary. + MetaPath = "/coverage/meta" + // CountersPath serves a snapshot of the coverage counters. + CountersPath = "/coverage/counters" + // ClearPath resets the coverage counters. + ClearPath = "/coverage/clear" +) + +// Layout of the leading fields of the coverage meta-data, as emitted by +// runtime/coverage and declared by internal/coverage.MetaFileHeader. All we +// need from it is the hash which identifies the instrumented binary, and +// which the go coverage tooling expects to find in the names of the files +// the data is stored in. Check the magic and version to fail loudly instead +// of serving a bogus ID, in case the layout ever changes. +const ( + metaMagicLen = 4 + metaVersionLen = 4 + metaHashOffset = 24 + metaHashLen = 16 + metaHeaderLen = metaHashOffset + metaHashLen +) + +var ( + metaMagic = [metaMagicLen]byte{0x00, 0x63, 0x76, 0x6d} + metaVersion = uint32(1) +) + +var ( + log = logger.NewLogger("coverage") + + idOnce sync.Once + id string + idErr error +) + +// Setup prepares the given HTTP request multiplexer for serving coverage data. +func Setup(mux *xhttp.ServeMux) { + if id, err := binaryID(); err != nil { + log.Warnf("coverage data is not available: %v", err) + } else { + log.Infof("serving coverage data of instrumented binary %s", id) + } + + mux.HandleFunc(IDPath, serveID) + mux.HandleFunc(MetaPath, serveMeta) + mux.HandleFunc(CountersPath, serveCounters) + mux.HandleFunc(ClearPath, clearCounters) +} + +// serveID serves the ID of the instrumented binary. +func serveID(w http.ResponseWriter, req *http.Request) { + id, err := binaryID() + if err != nil { + serveError(w, err) + return + } + + w.Header().Set("Content-Type", "text/plain") + writeResponse(w, []byte(id)) +} + +// serveMeta serves the coverage meta-data of the binary. +func serveMeta(w http.ResponseWriter, req *http.Request) { + serveData(w, coverage.WriteMeta) +} + +// serveCounters serves a snapshot of the coverage counters. +func serveCounters(w http.ResponseWriter, req *http.Request) { + serveData(w, coverage.WriteCounters) +} + +// clearCounters resets the coverage counters. This allows a test to collect +// the coverage of a single test case, instead of everything since startup. +func clearCounters(w http.ResponseWriter, req *http.Request) { + if err := coverage.ClearCounters(); err != nil { + serveError(w, err) + return + } + + log.Infof("coverage counters cleared") + + w.Header().Set("Content-Type", "text/plain") + writeResponse(w, []byte("ok")) +} + +// serveData serves the data produced by the given coverage dump function. +// The data is buffered so that a failed dump is reported as an error, instead +// of as a partial and silently unusable response. +func serveData(w http.ResponseWriter, dump func(io.Writer) error) { + buf := &bytes.Buffer{} + + if err := dump(buf); err != nil { + serveError(w, err) + return + } + + w.Header().Set("Content-Type", "application/octet-stream") + writeResponse(w, buf.Bytes()) +} + +// serveError fails a request with the given error. +func serveError(w http.ResponseWriter, err error) { + log.Errorf("failed to serve coverage data: %v", err) + http.Error(w, err.Error(), http.StatusInternalServerError) +} + +// writeResponse writes a successful response. +func writeResponse(w http.ResponseWriter, data []byte) { + if _, err := w.Write(data); err != nil { + log.Errorf("failed to write response: %v", err) + } +} + +// binaryID returns the ID of this instrumented binary, digging it out of the +// header of the coverage meta-data. It fails if the binary was built without +// coverage instrumentation. +func binaryID() (string, error) { + idOnce.Do(func() { + hdr := &metaHeader{} + if err := coverage.WriteMeta(hdr); err != nil { + idErr = fmt.Errorf("failed to read coverage meta-data: %w", err) + return + } + id, idErr = hdr.binaryID() + }) + + return id, idErr +} + +// metaHeader collects the header of the coverage meta-data, discarding the +// rest of it. +type metaHeader struct { + data []byte +} + +// Write collects data until we have the full header. +func (m *metaHeader) Write(b []byte) (int, error) { + if need := metaHeaderLen - len(m.data); need > 0 { + m.data = append(m.data, b[:min(need, len(b))]...) + } + return len(b), nil +} + +// binaryID extracts the binary ID from the collected header. +func (m *metaHeader) binaryID() (string, error) { + if len(m.data) < metaHeaderLen { + return "", fmt.Errorf("coverage meta-data short by %d bytes", + metaHeaderLen-len(m.data)) + } + + if magic := [metaMagicLen]byte(m.data[:metaMagicLen]); magic != metaMagic { + return "", fmt.Errorf("unexpected coverage meta-data magic %#x, expected %#x", + magic, metaMagic) + } + + version := binary.LittleEndian.Uint32(m.data[metaMagicLen : metaMagicLen+metaVersionLen]) + if version != metaVersion { + return "", fmt.Errorf("unexpected coverage meta-data version %d, expected %d", + version, metaVersion) + } + + return hex.EncodeToString(m.data[metaHashOffset:metaHeaderLen]), nil +} diff --git a/pkg/instrumentation/coverage/coverage_test.go b/pkg/instrumentation/coverage/coverage_test.go new file mode 100644 index 000000000..441533196 --- /dev/null +++ b/pkg/instrumentation/coverage/coverage_test.go @@ -0,0 +1,271 @@ +// Copyright The NRI Plugins Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package coverage + +import ( + "bufio" + "encoding/binary" + "io" + "net/http" + "os" + "os/exec" + "path/filepath" + "regexp" + "strings" + "syscall" + "testing" + "time" + + "github.com/stretchr/testify/require" + + xhttp "github.com/containers/nri-plugins/pkg/http" +) + +var hexID = regexp.MustCompile(`^[0-9a-f]{32}$`) + +// TestUninstrumentedBinaryFailsCleanly checks that we fail requests with an +// error, instead of serving unusable data, when there is no coverage data to +// serve. Note that this is always the case for a test binary: runtime/coverage +// only works in binaries built with go build -cover, so not even go test +// -cover makes the data available here. TestInstrumentedBinary covers the +// other case, using a helper binary. +func TestUninstrumentedBinaryFailsCleanly(t *testing.T) { + url := serve(t) + + for _, path := range []string{IDPath, MetaPath, CountersPath, ClearPath} { + status, body := get(t, url+path) + require.Equal(t, http.StatusInternalServerError, status, "status of %s", path) + require.NotEmpty(t, body, "error message of %s", path) + } +} + +func TestBinaryIDFromMetaHeader(t *testing.T) { + hash := []byte{ + 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, + 0xfe, 0xdc, 0xba, 0x98, 0x76, 0x54, 0x32, 0x10, + } + + header := func(magic []byte, version uint32, hash []byte) []byte { + data := append([]byte{}, magic...) + data = binary.LittleEndian.AppendUint32(data, version) + data = append(data, make([]byte, metaHashOffset-len(data))...) + return append(data, hash...) + } + + for _, tc := range []struct { + name string + data []byte + id string + }{ + { + name: "valid header", + data: header(metaMagic[:], metaVersion, hash), + id: "0123456789abcdeffedcba9876543210", + }, + { + name: "truncated header", + data: header(metaMagic[:], metaVersion, hash)[:metaHeaderLen-1], + }, + { + name: "wrong magic", + data: header([]byte{0x00, 0x00, 0x00, 0x00}, metaVersion, hash), + }, + { + name: "unknown version", + data: header(metaMagic[:], metaVersion+1, hash), + }, + } { + t.Run(tc.name, func(t *testing.T) { + m := &metaHeader{} + + // feed the data in small chunks, the way a real dump arrives + for chunk := tc.data; len(chunk) > 0; { + n := min(7, len(chunk)) + cnt, err := m.Write(chunk[:n]) + require.NoError(t, err, "write to header collector") + require.Equal(t, n, cnt, "bytes accepted by header collector") + chunk = chunk[n:] + } + + id, err := m.binaryID() + if tc.id == "" { + require.Error(t, err, "binary ID of %s", tc.name) + return + } + require.NoError(t, err, "binary ID of %s", tc.name) + require.Equal(t, tc.id, id, "binary ID") + }) + } +} + +// TestInstrumentedBinary checks that an instrumented binary serves coverage +// data which the go tools can digest, and that it serves the same binary ID +// which the runtime uses for the data it writes to $GOCOVERDIR. Our callers +// depend on both: the ID is what ties the counter data we serve to the +// meta-data it belongs to, and using the same one keeps the data collected +// over HTTP mergeable with the data dumped at exit. +func TestInstrumentedBinary(t *testing.T) { + if testing.Short() { + t.Skip("skipping test which builds a helper binary in short mode") + } + + gocmd, err := exec.LookPath("go") + if err != nil { + t.Skip("no go toolchain available for building the helper binary") + } + + dir := t.TempDir() + helper := filepath.Join(dir, "coverage-server") + covdir := filepath.Join(dir, "gocoverdir") + require.NoError(t, os.Mkdir(covdir, 0o755), "create GOCOVERDIR") + + build := exec.Command(gocmd, "build", "-cover", "-covermode=atomic", + "-o", helper, "./internal/coverage-server") + out, err := build.CombinedOutput() + require.NoError(t, err, "build helper binary: %s", out) + + url, stop := start(t, helper, covdir) + + status, body := get(t, url+IDPath) + require.Equal(t, http.StatusOK, status, "status of %s", IDPath) + id := string(body) + require.Regexp(t, hexID, id, "served binary ID") + + status, meta := get(t, url+MetaPath) + require.Equal(t, http.StatusOK, status, "status of %s", MetaPath) + require.Greater(t, len(meta), metaHeaderLen, "length of served meta-data") + require.Equal(t, metaMagic[:], meta[:metaMagicLen], "magic of served meta-data") + + status, counters := get(t, url+CountersPath) + require.Equal(t, http.StatusOK, status, "status of %s", CountersPath) + require.NotEmpty(t, counters, "served counters") + + status, body = get(t, url+ClearPath) + require.Equal(t, http.StatusOK, status, "status of %s", ClearPath) + require.Equal(t, "ok", string(body), "response of %s", ClearPath) + + status, cleared := get(t, url+CountersPath) + require.Equal(t, http.StatusOK, status, "status of %s after clearing", CountersPath) + require.NotEmpty(t, cleared, "served counters after clearing") + + // Check that the data we served is usable as is, once saved under the + // names go tool covdata expects. + saved := filepath.Join(dir, "saved") + require.NoError(t, os.Mkdir(saved, 0o755), "create directory for served data") + require.NoError(t, os.WriteFile(filepath.Join(saved, "covmeta."+id), meta, 0o644), + "save served meta-data") + require.NoError(t, os.WriteFile(filepath.Join(saved, "covcounters."+id+".1.1"), counters, 0o644), + "save served counters") + + profile := filepath.Join(dir, "profile.txt") + out, err = exec.Command(gocmd, "tool", "covdata", "textfmt", + "-i="+saved, "-o="+profile).CombinedOutput() + require.NoError(t, err, "convert served data to a text profile: %s", out) + + text, err := os.ReadFile(profile) + require.NoError(t, err, "read converted profile") + require.Contains(t, string(text), "instrumentation/coverage/coverage.go", + "packages in the converted profile") + + // Terminate the helper gracefully to get its data written to GOCOVERDIR, + // then check that it uses the same ID we served. + stop(t) + + metaFiles, err := filepath.Glob(filepath.Join(covdir, "covmeta.*")) + require.NoError(t, err, "look for meta-data in GOCOVERDIR") + require.Len(t, metaFiles, 1, "meta-data files in GOCOVERDIR") + require.Equal(t, "covmeta."+id, filepath.Base(metaFiles[0]), + "name of the meta-data file written to GOCOVERDIR") +} + +// serve starts a server with our endpoints registered and returns its URL. +func serve(t *testing.T) string { + t.Helper() + + srv := xhttp.NewServer() + Setup(srv.GetMux()) + require.NoError(t, srv.Start("127.0.0.1:0"), "start test server") + t.Cleanup(srv.Stop) + + return "http://" + srv.GetAddress() +} + +// start runs the given helper binary with GOCOVERDIR set to the given +// directory. It returns the URL the helper serves and a function which +// terminates it gracefully. +func start(t *testing.T, helper, covdir string) (string, func(*testing.T)) { + t.Helper() + + cmd := exec.Command(helper) + cmd.Env = append(os.Environ(), "GOCOVERDIR="+covdir) + cmd.Stderr = os.Stderr + + stdout, err := cmd.StdoutPipe() + require.NoError(t, err, "pipe stdout of helper binary") + require.NoError(t, cmd.Start(), "start helper binary") + + stopped := false + stop := func(t *testing.T) { + t.Helper() + if stopped { + return + } + stopped = true + require.NoError(t, cmd.Process.Signal(syscall.SIGTERM), "terminate helper binary") + require.NoError(t, cmd.Wait(), "wait for helper binary to exit") + } + t.Cleanup(func() { + if !stopped { + _ = cmd.Process.Kill() + _ = cmd.Wait() + } + }) + + addr := make(chan string, 1) + go func() { + if scanner := bufio.NewScanner(stdout); scanner.Scan() { + addr <- strings.TrimSpace(scanner.Text()) + } + close(addr) + }() + + select { + case a, ok := <-addr: + require.True(t, ok, "read address from helper binary") + return "http://" + a, stop + case <-time.After(10 * time.Second): + require.FailNow(t, "timeout waiting for the helper binary to serve") + } + + return "", stop +} + +// get performs a GET request and returns the status code and the body. +func get(t *testing.T, url string) (int, []byte) { + t.Helper() + + rpl, err := http.Get(url) //nolint:noctx + require.NoError(t, err, "GET %s", url) + defer func() { + if err := rpl.Body.Close(); err != nil { + t.Logf("failed to close HTTP reply body: %v", err) + } + }() + + body, err := io.ReadAll(rpl.Body) + require.NoError(t, err, "read body of %s", url) + + return rpl.StatusCode, body +} diff --git a/pkg/instrumentation/coverage/internal/coverage-server/main.go b/pkg/instrumentation/coverage/internal/coverage-server/main.go new file mode 100644 index 000000000..b78cac465 --- /dev/null +++ b/pkg/instrumentation/coverage/internal/coverage-server/main.go @@ -0,0 +1,51 @@ +// Copyright The NRI Plugins Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// This is a helper for the tests of the coverage package, and handy for +// poking at the endpoints by hand. runtime/coverage, which the package is +// built around, only works in binaries built with go build -cover, never in +// a test binary. So the tests build and run this program to check that we +// serve coverage data which the go tools can digest. +// +// Print the address of the server, then serve until terminated. Terminate +// with SIGTERM to get the coverage data written to $GOCOVERDIR, too. +package main + +import ( + "fmt" + "os" + "os/signal" + "syscall" + + xhttp "github.com/containers/nri-plugins/pkg/http" + "github.com/containers/nri-plugins/pkg/instrumentation/coverage" +) + +func main() { + srv := xhttp.NewServer() + coverage.Setup(srv.GetMux()) + + if err := srv.Start("127.0.0.1:0"); err != nil { + fmt.Fprintf(os.Stderr, "failed to start server: %v\n", err) + os.Exit(1) + } + + fmt.Println(srv.GetAddress()) + + sigs := make(chan os.Signal, 1) + signal.Notify(sigs, syscall.SIGTERM, syscall.SIGINT) + <-sigs + + srv.Stop() +} diff --git a/pkg/resmgr/resource-manager.go b/pkg/resmgr/resource-manager.go index 596ca6dea..bad1eb516 100644 --- a/pkg/resmgr/resource-manager.go +++ b/pkg/resmgr/resource-manager.go @@ -23,6 +23,7 @@ import ( "github.com/containers/nri-plugins/pkg/agent" "github.com/containers/nri-plugins/pkg/healthz" "github.com/containers/nri-plugins/pkg/instrumentation" + "github.com/containers/nri-plugins/pkg/instrumentation/coverage" logger "github.com/containers/nri-plugins/pkg/log" "github.com/containers/nri-plugins/pkg/pidfile" "github.com/containers/nri-plugins/pkg/resmgr/cache" @@ -30,6 +31,7 @@ import ( "github.com/containers/nri-plugins/pkg/resmgr/policy" "github.com/containers/nri-plugins/pkg/sysfs" "github.com/containers/nri-plugins/pkg/topology" + "github.com/containers/nri-plugins/pkg/utils" "sigs.k8s.io/yaml" cfgapi "github.com/containers/nri-plugins/pkg/apis/config/v1alpha1" @@ -113,6 +115,7 @@ func NewResourceManager(backend policy.Backend, agt *agent.Agent) (ResourceManag } m.setupHealthCheck() + m.setupCoverage() return m, nil } @@ -269,6 +272,16 @@ func (m *resmgr) setupHealthCheck() { healthz.Setup(mux) } +// setupCoverage prepares the resource manager for serving coverage data. +func (m *resmgr) setupCoverage() { + if !utils.TestAPIsEnabled() { + return + } + + mux := instrumentation.HTTPServer().GetMux() + coverage.Setup(mux) +} + // setupControllers sets up the resource controllers. func (m *resmgr) setupControllers() error { var err error From 6b602f4b91a519a30e4a71f2a2eb7defd92bbdf7 Mon Sep 17 00:00:00 2001 From: Krisztian Litkey Date: Tue, 15 Sep 2026 12:20:37 +0300 Subject: [PATCH 5/6] build: add COVER=1 for instrumented plugin builds. COVER=1 builds the resource-manager-based plugins with coverage instrumentation. Instrument our own packages only, and with atomic counters: the plugins are concurrent, and clearing the counters at runtime does not work without them. Assisted-by: Claude Opus 5 (1M context) Signed-off-by: Krisztian Litkey --- Makefile | 27 ++++++++++++++++++++++++--- cmd/plugins/balloons/Dockerfile | 2 ++ cmd/plugins/template/Dockerfile | 2 ++ cmd/plugins/topology-aware/Dockerfile | 2 ++ 4 files changed, 30 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 1ccdefbcf..064f0b6cc 100644 --- a/Makefile +++ b/Makefile @@ -124,6 +124,24 @@ ifeq ($(DEBUG),1) endif endif +# Packages to instrument for coverage with COVER=1. Only our own code: the +# main module is instrumented by default, pkg/topology needs to be listed as +# it is a module of its own. +COVER_PKGS ?= github.com/containers/nri-plugins/...,github.com/containers/nri-plugins/pkg/topology/... + +# COVER=1 builds the resource-manager-based plugins, in other words the +# nri-resource-policy-* ones, with coverage instrumentation. Such a plugin +# collects coverage data while it runs, which can be dumped by pointing +# GOCOVERDIR at a writable directory and/or by asking for it over the +# instrumentation HTTP server. See test/e2e/README.md for how the e2e tests +# use this. Note that atomic counters are mandatory here: the plugins are +# concurrent and runtime/coverage.ClearCounters() refuses to run without them. +ifeq ($(COVER),1) + COVER_FLAGS := -cover -covermode=atomic -coverpkg=$(COVER_PKGS) + COVER_TYPE := "instrumented " + DOCKER_BUILD_COVER := --build-arg COVER=1 +endif + # Documentation-related variables SPHINXOPTS ?= -W SPHINXBUILD = sphinx-build @@ -161,7 +179,7 @@ verify: verify-godeps verify-fmt verify-generate verify-build verify-docs build-plugins: $(foreach bin,$(PLUGINS),$(BIN_PATH)/$(bin)) build-plugins-static: - $(MAKE) STATIC=1 DEBUG=$(DEBUG) NORACE=$(NORACE) build-plugins + $(MAKE) STATIC=1 DEBUG=$(DEBUG) NORACE=$(NORACE) COVER=$(COVER) build-plugins build-binaries: $(foreach bin,$(BINARIES),$(BIN_PATH)/$(bin)) @@ -205,10 +223,10 @@ clean-cache: # $(BIN_PATH)/nri-resource-policy-%: .static.%.$(STATIC) - $(Q)echo "Building $(STATIC_TYPE)$@ (version $(BUILD_VERSION), build $(BUILD_BUILDID))..."; \ + $(Q)echo "Building $(COVER_TYPE)$(STATIC_TYPE)$@ (version $(BUILD_VERSION), build $(BUILD_BUILDID))..."; \ src="./cmd/plugins/$(patsubst nri-resource-policy-%,%,$(notdir $@))"; \ mkdir -p $(BIN_PATH); \ - cd "$$src" && $(GO_BUILD) $(BUILD_TAGS) $(LDFLAGS) $(GCFLAGS) -o $@ + cd "$$src" && $(GO_BUILD) $(BUILD_TAGS) $(LDFLAGS) $(GCFLAGS) $(COVER_FLAGS) -o $@ $(BIN_PATH)/nri-%: .static.%.$(STATIC) $(Q)echo "Building $(STATIC_TYPE)$@ (version $(BUILD_VERSION), build $(BUILD_BUILDID))..."; \ @@ -247,9 +265,11 @@ image.nri-resource-policy-% \ image.nri-% \ image.%: $(Q)mkdir -p $(IMAGE_PATH); \ + cover=""; \ case $@ in \ *.nri-resource-policy-*) \ dir=$(patsubst image.nri-resource-policy-%,cmd/plugins/%,$@); \ + cover="$(DOCKER_BUILD_COVER)"; \ ;; \ *.nri-*) \ dir=$(patsubst image.nri-%,cmd/plugins/%,$@); \ @@ -263,6 +283,7 @@ image.%: $(DOCKER_BUILD) . -f "$$dir/Dockerfile" \ --build-arg GO_VERSION=$(GO_VERSION) \ $(DOCKER_BUILD_DEBUG) \ + $$cover \ --build-arg IMAGE_VERSION=$(IMAGE_VERSION) \ --build-arg BUILD_VERSION=$(BUILD_VERSION) \ --build-arg BUILD_BUILDID=$(BUILD_BUILDID) \ diff --git a/cmd/plugins/balloons/Dockerfile b/cmd/plugins/balloons/Dockerfile index dc4fdd552..fd245f875 100644 --- a/cmd/plugins/balloons/Dockerfile +++ b/cmd/plugins/balloons/Dockerfile @@ -7,6 +7,7 @@ ARG BUILD_VERSION ARG BUILD_BUILDID ARG DEBUG=0 ARG NORACE=0 +ARG COVER=0 ARG SKIP_LICENSES=0 WORKDIR /go/builder @@ -47,6 +48,7 @@ RUN --mount=type=cache,target=/go/pkg/mod/ \ V=$DEBUG \ DEBUG=$DEBUG \ NORACE=$NORACE \ + COVER=$COVER \ BINARIES="" \ OTHER_IMAGE_TARGETS="" \ PLUGINS=nri-resource-policy-balloons \ diff --git a/cmd/plugins/template/Dockerfile b/cmd/plugins/template/Dockerfile index 8fcac511f..8b7717152 100644 --- a/cmd/plugins/template/Dockerfile +++ b/cmd/plugins/template/Dockerfile @@ -7,6 +7,7 @@ ARG BUILD_VERSION ARG BUILD_BUILDID ARG DEBUG=0 ARG NORACE=0 +ARG COVER=0 ARG SKIP_LICENSES=0 WORKDIR /go/builder @@ -39,6 +40,7 @@ RUN --mount=type=cache,target=/go/pkg/mod/ \ V=$DEBUG \ DEBUG=$DEBUG \ NORACE=$NORACE \ + COVER=$COVER \ BINARIES="" \ OTHER_IMAGE_TARGETS="" \ PLUGINS=nri-resource-policy-template \ diff --git a/cmd/plugins/topology-aware/Dockerfile b/cmd/plugins/topology-aware/Dockerfile index 15cc75462..0d5ff0c64 100644 --- a/cmd/plugins/topology-aware/Dockerfile +++ b/cmd/plugins/topology-aware/Dockerfile @@ -7,6 +7,7 @@ ARG BUILD_VERSION ARG BUILD_BUILDID ARG DEBUG=0 ARG NORACE=0 +ARG COVER=0 ARG SKIP_LICENSES=0 WORKDIR /go/builder @@ -39,6 +40,7 @@ RUN --mount=type=cache,target=/go/pkg/mod/ \ V=$DEBUG \ DEBUG=$DEBUG \ NORACE=$NORACE \ + COVER=$COVER \ BINARIES="" \ OTHER_IMAGE_TARGETS="" \ PLUGINS=nri-resource-policy-topology-aware \ From 61bba6fb7625d9e129789459a09e470eb39f116d Mon Sep 17 00:00:00 2001 From: Krisztian Litkey Date: Tue, 15 Sep 2026 12:20:56 +0300 Subject: [PATCH 6/6] e2e: collect and report the coverage of tests. Collect the coverage data of the plugins the tests exercise, and report the coverage of each plugin and the total at the end of a run. The tests clear the counters when they launch a plugin and ask it for a dump once done. make e2e-tests builds the plugins with COVER=1 so that a run has something to collect; collecting never fails a test. go tool covdata percent cannot report per plugin, and prints a package which has no statements without a percentage and without a line break, running the next package into it, so calculate the numbers from the profile instead, weighted by statements the way go tool cover does. Assisted-by: Claude Opus 5 (1M context) Signed-off-by: Krisztian Litkey --- Makefile | 5 +- test/e2e/README.md | 91 +++++++++++++++++++++++ test/e2e/lib/vm.bash | 141 +++++++++++++++++++++++++++++++++++ test/e2e/report-coverage.sh | 144 ++++++++++++++++++++++++++++++++++++ test/e2e/run.sh | 20 ++++- test/e2e/run_tests.sh | 27 ++++++- 6 files changed, 424 insertions(+), 4 deletions(-) create mode 100755 test/e2e/report-coverage.sh diff --git a/Makefile b/Makefile index 064f0b6cc..5e23e7048 100644 --- a/Makefile +++ b/Makefile @@ -440,7 +440,10 @@ ginkgo-subpkgs-tests: ginkgo-test-setup # TODO(klihub): coverage done; \ done -e2e-tests: build images +# The e2e tests collect coverage data from the plugins they exercise and report +# the total at the end of the run, so build the plugins with instrumentation. +e2e-tests: + $(Q)$(MAKE) COVER=1 build images mkdir -p $(shell realpath $(E2E_WORKDIR)) && \ cd $(shell realpath $(E2E_WORKDIR)) && \ $(E2E_RUN) $(realpath $(E2E_TESTS)) diff --git a/test/e2e/README.md b/test/e2e/README.md index 1f78180a8..ab720bead 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -161,6 +161,97 @@ Worth knowing: running any test. Raise `CLUSTER_READY_TIMEOUT` (300 seconds by default) if a large topology needs longer. +## Collecting coverage data + +The tests can collect the same kind of coverage data from the plugins they +exercise that `go test -cover` collects from unit tests. This only works for +the resource-manager-based plugins, in other words balloons, topology-aware +and template. + +Collection is always on, so there is nothing to enable: + +```shell +make e2e-tests +``` + +This builds the plugins with coverage instrumentation, runs the tests, and ends +the run with the coverage of all the tests it ran, on top of the usual test +summary: the coverage of the logic of each plugin, in other words of the code +under `cmd/plugins/PLUGIN`, and the total over everything instrumented. + +The only thing coverage data needs is instrumented plugins, which `make +e2e-tests` takes care of. When running the tests directly, build the images +with `COVER=1` to get anything to report on: + +```shell +make COVER=1 images +cd test/e2e +./run_tests.sh policies.test-suite +``` + +Without that the tests run just fine, they simply have nothing to collect. + +The data of each test case is stored in a `coverage` directory in the output +directory of that test case, so it also tells which test covers what. The +report is merged from all of those at the end of the run, and can be +regenerated, or generated for a subset of the tests, with: + +```shell +./report-coverage.sh [DIR] +``` + +DIR defaults to the current directory, and the report is written to +`DIR/coverage-report`: `coverprofile` in the usual text format for `go tool +cover`, and `coverage.html` for browsing. Run `go tool cover -func` on the +profile for the numbers per package and per function. Note that the report +covers all data found under DIR, so point it at a single policy or topology +directory for a report on those tests alone. + +Worth knowing: + +- An instrumented plugin dumps its data when it exits, and serves it on request + over its instrumentation HTTP server. The framework uses both: it asks for a + dump before terminating a plugin and again at the end of a test, and picks up + what plugins which already exited wrote to `$GOCOVERDIR`. This is why the data + survives a test which kills a plugin instead of terminating it, and a test + which launches a plugin several times. Asking twice costs nothing: the same + data merges as one, only the hit counts of a block add up. + +- A plugin starts with its counters at zero, so the data of a test covers what + that test's plugins did, the configuration it launched them with included. + What keeps one test out of another's data is the collected data being + discarded on the VM before each test. A plugin which a test leaves running is + only terminated by the next test, so its final dump is counted for the latter. + +- Data collected earlier is added to, not replaced, which is what makes a total + over several partial runs possible. A run which reruns a test replaces the + data of that test, but the data of the tests it does not run stays. Start the + run from scratch with `reset_coverage`, and the report covers this run alone: + + ```shell + reset_coverage=1 ./run_tests.sh policies.test-suite + ``` + + `1` enables it and `0` disables it, which is also the default; anything else + is an error, rather than a value which quietly leaves the earlier data in + place. It discards everything collected earlier once, before the first test, + so it never throws away the data of the tests of the ongoing run. + +- The endpoints, which the framework enables with + `--set plugin.test.enableAPIs=true`, are also there for poking at by hand: + + | endpoint | serves | + | --- | --- | + | `/coverage/id` | the ID of the instrumented binary, which the names of the data files are based on | + | `/coverage/meta` | the coverage meta-data, to be saved as `covmeta.` | + | `/coverage/counters` | a snapshot of the counters, to be saved as `covcounters..N.M` | + | `/coverage/clear` | resets the counters | + +- Only the plugins are instrumented, so the report covers the packages linked + into a plugin, and nothing else. Do not read a total percentage over it as + the coverage of the repository, and do not compare it to the unit test + coverage of `make test`, which measures a different set of packages. + ## Writing tests A test case is a `code.var.sh` file in a diff --git a/test/e2e/lib/vm.bash b/test/e2e/lib/vm.bash index 598d138ff..c43b23c1b 100644 --- a/test/e2e/lib/vm.bash +++ b/test/e2e/lib/vm.bash @@ -1179,6 +1179,147 @@ vm-stop-log-collection() { vm-command "fuser --kill $log_file 2>/dev/null || :" } +# Collecting coverage data from the plugins. The tests always collect it, and +# get something to collect when the plugins were built with instrumentation, +# using make COVER=1, which is what make e2e-tests does. Everything here is +# best-effort: a plugin without instrumentation simply has nothing to give, and +# no failure to collect data ever fails a test. +# +# The plugin stores its data in a directory under the data directory it mounts +# from the host anyway, so the data is readily available on the VM, without +# having to reach into the container. It writes the data there when it exits, +# and serves it over the instrumentation HTTP server on request, which is how +# we get the data of a plugin which is still running, or which never gets to +# exit gracefully. +vm_coverage_dir=/var/lib/nri-resource-policy/coverage +vm_coverage_url=http://localhost:8891/coverage +vm_coverage_curl="curl --silent --show-error --fail --noproxy localhost" + +# Where report-coverage.sh puts the report, relative to the output directory. +# Deliberately not named coverage, unlike the per test data directories, to +# keep the report and the data it is generated from apart. +vm_coverage_report_dir=coverage-report + +# Print the GOCOVERDIR the plugins are launched with. +vm-coverage-gocoverdir() { + echo "$vm_coverage_dir" +} + +# Return success if all coverage data collected earlier should be discarded +# before running any tests. Set reset_coverage to +# 1: start from scratch, so that the report covers this run only +# 0: add to the data collected earlier (the default) +# +# Without this, the report of a run also covers tests which that run did not +# rerun, which is what allows collecting a total over several partial runs. +# +# Refuse anything else rather than taking it for a no: a value we do not +# recognize would leave the data of earlier runs in place, and the report of the +# run would cover more than the run without saying so. +vm-coverage-reset-requested() { + case "${reset_coverage:-0}" in + 1) + return 0 + ;; + 0) + return 1 + ;; + esac + + error "invalid reset_coverage=\"$reset_coverage\", expected 1 or 0" +} + +# Discard all coverage data collected under the given directory, and the report +# generated from it, if reset_coverage asks for it. Call this once per run, +# before running any tests, never per test: a test which runs overwrites its own +# data anyway, and wiping everything between tests would throw away the data of +# the tests which already ran. +vm-coverage-discard-collected() { + local dir="$1" + + vm-coverage-reset-requested || return 0 + + case "$dir" in + ""|/) + echo "WARNING: refusing to discard coverage data under \"$dir\"" + return 0 + ;; + esac + [ -d "$dir" ] || return 0 + + echo "Discarding all coverage data collected earlier under $dir..." + + # Delete our data files where we put them: both the name of a file and the + # coverage directory holding it have to match. Either on its own is too + # little, as the directory to discard under can be anywhere, the source tree + # included, where a directory named coverage is just as likely to be a + # package of ours, and a covmeta file outside one is not ours to remove. + find "$dir" -type f \( -path '*/coverage/covmeta.*' \ + -o -path '*/coverage/covcounters.*' \) -delete + rm -rf "$dir/$vm_coverage_report_dir" +} + +# Discard the coverage data an earlier test left on the VM, so that what we +# collect is the data of this test alone. Call this once per test, before it +# starts. +vm-coverage-reset() { + vm-command-q "rm -rf $vm_coverage_dir && mkdir -p $vm_coverage_dir" || + echo "WARNING: failed to reset coverage directory $vm_coverage_dir" +} + +# Prepare for collecting the coverage data of a plugin about to be launched. +# The plugin needs the directory to exist, it does not create it. Note that a +# test can launch and terminate several plugins, all of which dump their data +# here when they exit, so this must not discard what is already there. +vm-coverage-prepare() { + vm-command-q "mkdir -p $vm_coverage_dir" || + echo "WARNING: failed to create coverage directory $vm_coverage_dir" +} + +# Dump the coverage data of the running plugin on the VM. Save it under the +# names go tool covdata expects: the ID the plugin serves is what ties the +# counters to the meta-data they belong to. +vm-coverage-snapshot() { + local id + + id=$(vm-command-q "$vm_coverage_curl $vm_coverage_url/id") + if [ -z "$id" ]; then + # An uninstrumented build, one with the test APIs off, or a plugin the + # test already terminated. Whatever such a plugin wrote to $GOCOVERDIR + # on its way out is still collected. + echo "no coverage data to dump from the plugin" + return 0 + fi + + vm-command-q "cd $vm_coverage_dir && \ + $vm_coverage_curl -o covmeta.$id $vm_coverage_url/meta && \ + $vm_coverage_curl -o covcounters.$id.1.\$(date +%s%N) $vm_coverage_url/counters" \ + >/dev/null || + echo "WARNING: failed to dump coverage data of the plugin" +} + +# Copy the coverage data collected on the VM to the given directory on the +# host. report-coverage.sh merges the data of all tests into a single report. +vm-coverage-collect() { + local dir="$1" + + if ! vm-command-q "ls $vm_coverage_dir/covmeta.* >/dev/null 2>&1"; then + echo "no coverage data collected on the VM" + return 0 + fi + + # Start from scratch, so that we don't mix in the data of an earlier run of + # this test, potentially with a different build of the plugin. + rm -rf "$dir" + if ! mkdir -p "$dir"; then + echo "WARNING: failed to create coverage directory $dir" + return 0 + fi + + host-command "$SCP $VM_HOSTNAME:$vm_coverage_dir/'cov*' \"$dir/\"" || + echo "WARNING: copying coverage data from the VM failed" +} + vm-seconds-now() { vm-command-q "date +%s" } diff --git a/test/e2e/report-coverage.sh b/test/e2e/report-coverage.sh new file mode 100755 index 000000000..f332bea26 --- /dev/null +++ b/test/e2e/report-coverage.sh @@ -0,0 +1,144 @@ +#!/bin/bash +# +# Report the total coverage of the e2e tests. The tests collect the coverage +# data of the plugins they exercise into a coverage directory per test case. +# This script merges the data of all of them into a single report. +# +# Usage: report-coverage.sh [DIR] +# +# Look for the data under DIR, the current working directory by default, and +# write the report to DIR/coverage-report. Print a per-package summary and the +# total. +# +# The plugins must have been built with coverage instrumentation for there to +# be any data to report on. make e2e-tests does that, but a manual "make +# images" does not, unless given COVER=1. + +set -o pipefail + +GO_CMD="${GO_CMD:-go}" + +usage() { + echo "Usage: report-coverage.sh [DIR]" + echo "Merge and report the coverage data the e2e tests collected under DIR." +} + +# Usage: summarize plugins|total PROFILE +# +# Print the coverage of the logic of each plugin in PROFILE, in other words of +# the code under cmd/plugins/PLUGIN, or the total over everything instrumented. +# Weight both by statements, the way go tool cover calculates its percentages. +# +# Note that go tool covdata percent cannot report either of these: it only ever +# reports per package, and it prints a package which has no statements at all, +# such as one declaring nothing but types, without a percentage and without a +# line break, running the line of the next package into it. +summarize() { + local what="$1" profile="$2" + + awk -v what="$what" ' + function report(label, hits, stmts) { + printf " %-28s %5.1f%% (%d/%d statements)\n", + label, 100 * hits / stmts, hits, stmts + } + + NR > 1 { + split($1, path, ":") + hit = ($3 > 0) ? $2 : 0 + + total_stmts += $2 + total_hits += hit + + # Attribute cmd/plugins/PLUGIN/... to the logic of PLUGIN. + cnt = split(path[1], part, "/") + for (i = 1; i + 2 <= cnt; i++) { + if (part[i] == "cmd" && part[i + 1] == "plugins") { + plugin = part[i + 2] + stmts[plugin] += $2 + hits[plugin] += hit + break + } + } + } + + END { + if (what == "total") { + if (total_stmts > 0) + report("all instrumented packages", total_hits, total_stmts) + else + print " nothing instrumented to report on" + exit + } + + for (plugin in stmts) + if (stmts[plugin] > 0) + report("cmd/plugins/" plugin, hits[plugin], stmts[plugin]) + } + ' "$profile" +} + +case "$1" in + -h|--help|help) + usage + exit 0 + ;; +esac + +dir="${1:-$(pwd)}" +if [ ! -d "$dir" ]; then + echo "report-coverage.sh: no such directory: $dir" >&2 + exit 1 +fi +dir=$(realpath "$dir") + +outdir="$dir/coverage-report" +merged="$outdir/merged" +profile="$outdir/coverprofile" +html="$outdir/coverage.html" + +# Collect the directories which hold the data of a test case, skipping our own +# output. Both the name of a file and the coverage directory holding it have to +# match, the same way as when the data is discarded: either on its own is too +# little, as the directory to report on can be anywhere, the source tree +# included, where a directory named coverage is just as likely to be a package +# of ours, and a covmeta file outside one is not from a test of ours. +data_dirs=$(find "$dir" -type f -path '*/coverage/covmeta.*' \ + -not -path "$outdir/*" -printf '%h\n' | sort -u | paste -sd, -) + +if [ -z "$data_dirs" ]; then + echo "No coverage data found under $dir." + echo "Were the plugins built with coverage instrumentation (make COVER=1)?" + exit 0 +fi + +if ! command -v "$GO_CMD" >/dev/null; then + echo "No $GO_CMD available, cannot report on the collected coverage data." >&2 + exit 1 +fi + +rm -rf "$outdir" +mkdir -p "$merged" || exit 1 + +echo "" +echo "Merging the coverage data of the tests..." +if ! "$GO_CMD" tool covdata merge -i="$data_dirs" -o="$merged"; then + echo "Failed to merge the collected coverage data." >&2 + exit 1 +fi + +if ! "$GO_CMD" tool covdata textfmt -i="$merged" -o="$profile"; then + echo "Failed to convert the merged coverage data." >&2 + exit 1 +fi + +"$GO_CMD" tool cover -html="$profile" -o="$html" || + echo "WARNING: failed to generate $html" + +echo "" +echo "Coverage of the e2e tests:" +summarize plugins "$profile" | LC_ALL=C sort +summarize total "$profile" + +echo "" +echo " per package and function: $GO_CMD tool cover -func=$profile" +echo " browsable report: $html" diff --git a/test/e2e/run.sh b/test/e2e/run.sh index d40392609..dbb7a15cc 100755 --- a/test/e2e/run.sh +++ b/test/e2e/run.sh @@ -422,7 +422,8 @@ helm-set-args() { # script API --set image.pullPolicy=Never \ --set resources.cpu=50m \ --set resources.memory=256Mi \ - --set plugin.test.enableAPIs=true" + --set plugin.test.enableAPIs=true \ + --set extraEnv.GOCOVERDIR=$(vm-coverage-gocoverdir)" } helm-launch() { # script API @@ -469,6 +470,8 @@ helm-launch() { # script API host-command "$SCP \"$helm_config\" $VM_HOSTNAME:" || command-error "copying \"$helm_config\" to VM failed" + vm-coverage-prepare + vm-command "helm install $rollback -n kube-system $helm_name ./helm/$plugin \ --values=`basename ${helm_config}` \ $(helm-set-args "$plugin")" || @@ -586,6 +589,13 @@ helm-terminate() { # script API if [ "$?" != "0" ]; then return 0 fi + + # Ask the plugin for its coverage data while it is still there to ask. What + # it writes to $GOCOVERDIR on its way out is picked up as well, but only if + # it gets that far, so this is what makes the coverage of a test which + # launches a plugin more than once survive. + vm-coverage-snapshot + vm-command "helm uninstall -n kube-system test --wait --timeout 20s" vm-port-forward-disable } @@ -1142,6 +1152,8 @@ eval "${yaml_in_defaults}" TEST_FAILURES="" test_start_secs=$(vm-seconds-now) +vm-coverage-reset + test-user-code test_span_secs="$(vm-seconds-since $test_start_secs)" @@ -1152,6 +1164,12 @@ service="${k8scri}" since="$since" vm-pull-journal > "${TEST_OUTPUT_DIR}"/runtim host-command "$SCP $VM_HOSTNAME:nri-resource-policy.output.txt \"${TEST_OUTPUT_DIR}/\"" || out "copying \"$nri-resource-policy.output.txt\" from VM failed" +# Dump the coverage data of a plugin which is still running, then copy back +# everything collected for this test, including what any plugin which already +# exited wrote to $GOCOVERDIR. +vm-coverage-snapshot +vm-coverage-collect "${TEST_OUTPUT_DIR}/coverage" + # Summarize results exit_status=0 if [ -n "$TEST_FAILURES" ]; then diff --git a/test/e2e/run_tests.sh b/test/e2e/run_tests.sh index acb30f7a5..61e76a9e4 100755 --- a/test/e2e/run_tests.sh +++ b/test/e2e/run_tests.sh @@ -1,8 +1,15 @@ #!/bin/bash TESTS_DIR="$1" +# Where the output of the run goes. Given a directory, every topology writes +# into it; without one, each writes into a directory of its own under the +# current one. Discarding coverage data and reporting on it both cover the +# root, so that a run of several topologies is reported as a single run. +OUTPUT_DIR_ARG="${2:-}" +OUTPUT_ROOT="${OUTPUT_DIR_ARG:-$(pwd)}" SKIP_LONG_TESTS="${skip_long_tests:-yes}" RUN_SH="${0%/*}/run.sh" +REPORT_COVERAGE_SH="${0%/*}/report-coverage.sh" DEFAULT_DISTRO=${DEFAULT_DISTRO:-"fedora/43"} @@ -11,12 +18,16 @@ allow_var_override="" k8scri=${k8scri:="containerd"} efi=${efi:-} +# Discard all coverage data collected by earlier runs before running any tests, +# see vm-coverage-reset-requested in lib/vm.bash. +reset_coverage=${reset_coverage:-0} + proxy=${proxy:=$https_proxy} proxy=${proxy:=$HTTPS_PROXY} export proxy usage() { - echo "Usage: [skip_long_tests=no] run_tests.sh TESTS_DIR" + echo "Usage: [skip_long_tests=no] run_tests.sh TESTS_DIR [OUTPUT_DIR]" echo "TESTS_DIR is expected to be structured as POLICY/TOPOLOGY/TEST with files:" echo "POLICY/nri-resource_policy.cfg: configuration of nri-resource_policy" echo "POLICY/TOPOLOGY/topology.var.json: contents of the topology variable for run.sh" @@ -187,6 +198,11 @@ trap cleanup TERM EXIT QUIT summary_file="$summary_dir/summary.txt" echo -n "" > "$summary_file" +# Start the run from scratch if asked to. This is done once here, for all the +# tests, and never per test, as each test only ever overwrites the data of its +# own earlier run. +vm-coverage-discard-collected "$OUTPUT_ROOT" + # Shared test helpers are the root of the *.source.sh chain: they are sourced # before any test suite, policy, topology or test case level *.source.sh file, # so that any of those can override a helper. @@ -240,7 +256,7 @@ for POLICY_DIR in "$TESTS_ROOT_DIR"/*; do # Create ansible inventory file from a template ESCAPED_VM=$(printf '%s\n' "$vm_name" | sed -e 's/[\/]/-/g') - OUTPUT_DIR=$(realpath ${2:-"`pwd`/$ESCAPED_VM"}) + OUTPUT_DIR=$(realpath "${OUTPUT_DIR_ARG:-$OUTPUT_ROOT/$ESCAPED_VM}") for TEST_DIR in "$TOPOLOGY_DIR"/test*; do if ! [ -d "$TEST_DIR" ]; then @@ -313,6 +329,13 @@ done echo "" echo "Tests summary:" cat "$summary_file" + +# Report the total coverage of the tests, merging the data collected for each +# of them. This covers all data found under the output directory, including the +# data of tests this run did not rerun, unless reset_coverage asked us to start +# from scratch. +"$REPORT_COVERAGE_SH" "$OUTPUT_ROOT" + if grep -q ERROR "$summary_file" || grep -q FAIL "$summary_file"; then exit 1 fi