Skip to content

fix(ci): run go mod tidy after storage-e2e replace to survive dependency graph drift#56

Draft
fastrapier wants to merge 1 commit into
mainfrom
fix/e2e-run-tests-tidy-after-replace
Draft

fix(ci): run go mod tidy after storage-e2e replace to survive dependency graph drift#56
fastrapier wants to merge 1 commit into
mainfrom
fix/e2e-run-tests-tidy-after-replace

Conversation

@fastrapier

Copy link
Copy Markdown
Member

Problem

.github/scripts/e2e-run-tests.sh (used by the reusable e2e.yml workflow) applies a directory replace pointing the consumer module at the checked-out storage-e2e@main and then runs go mod download. download does not re-resolve the dependency graph, so when the replaced checkout (or another local replace such as ../api in a PR merge commit) changes the graph relative to what the consumer's go.mod/go.sum were resolved against, MVS selects module versions that are missing from the consumer's go.sum and go test fails with:

missing go.sum entry for module providing package golang.org/x/net/http2 (imported by k8s.io/client-go/rest)

This happened in practice in deckhouse/sds-node-configurator#321.

Solution

  • After applying the replace, run go mod tidy instead of go mod download. tidy re-resolves go.mod/go.sum against the actual (replaced) graph, so consumer CI no longer breaks when the pinned revision and main drift apart.
  • Mutating go.mod/go.sum is safe: the script already backs both up and restores them via a trap on EXIT. Additionally, if the consumer had no go.sum, a go.sum created by tidy is now removed on exit so the working tree is left unchanged on self-hosted runners.
  • Rewrote the header comment explaining the rationale and updated .github/scripts/tests/test-run-tests.sh: asserts tidy (not download) is invoked after the replace, go.sum is restored, and a tidy-created go.sum is removed when none existed.

Testing

All script tests pass locally:

bash .github/scripts/tests/test-run-tests.sh       # 9/9 PASS
bash .github/scripts/tests/test-prune-workspace.sh # 4/4 PASS
bash .github/scripts/tests/test-resolve-labels.sh  # 7/7 PASS

…ncy graph drift

go mod download does not re-resolve the dependency graph after the
directory replace to the checked-out storage-e2e@main (or local replaces
like ../api in PR merge commits) changes it. MVS can then select module
versions absent from the consumer's go.sum, and go test fails with
"missing go.sum entry for module providing package ..." (seen in
deckhouse/sds-node-configurator PR #321).

Run go mod tidy instead: it re-resolves go.mod/go.sum against the actual
graph. Mutation is safe because both files are already backed up and
restored by the EXIT trap; additionally remove a go.sum created by tidy
when the consumer had none, so the working tree is left unchanged.

Update script tests for the new behavior.
Copilot AI review requested due to automatic review settings July 23, 2026 12:54
@github-code-quality

Copy link
Copy Markdown

Code Coverage Overview

Languages: Go

Go / code-coverage/go

The overall coverage in commit b8f13ac in the fix/e2e-run-tests-ti... branch remains at 24%, unchanged from commit 7b037e4 in the main branch.

@github-actions

Copy link
Copy Markdown

Code Coverage

Package Line Rate Health
github.com/deckhouse/storage-e2e/internal/cluster 0%
github.com/deckhouse/storage-e2e/internal/config 56%
github.com/deckhouse/storage-e2e/internal/infrastructure/ssh 0%
github.com/deckhouse/storage-e2e/internal/infrastructure/ssh/v2 80%
github.com/deckhouse/storage-e2e/internal/kubernetes/clusterlock 75%
github.com/deckhouse/storage-e2e/internal/kubernetes/commander 53%
github.com/deckhouse/storage-e2e/internal/kubernetes/deckhouse 0%
github.com/deckhouse/storage-e2e/internal/kubernetes/kubeaccess 78%
github.com/deckhouse/storage-e2e/internal/kubernetes/storage 0%
github.com/deckhouse/storage-e2e/internal/kubernetes/virtualization 0%
github.com/deckhouse/storage-e2e/internal/logger 53%
github.com/deckhouse/storage-e2e/internal/provisioning/commander 16%
github.com/deckhouse/storage-e2e/internal/provisioning/dvp 63%
github.com/deckhouse/storage-e2e/internal/provisioning/dvp/vm 78%
github.com/deckhouse/storage-e2e/pkg/cluster 4%
github.com/deckhouse/storage-e2e/pkg/clusterprovider 89%
github.com/deckhouse/storage-e2e/pkg/clusterprovider/registry 100%
github.com/deckhouse/storage-e2e/pkg/commander 0%
github.com/deckhouse/storage-e2e/pkg/e2e 41%
github.com/deckhouse/storage-e2e/pkg/kubernetes 5%
github.com/deckhouse/storage-e2e/pkg/retry 94%
github.com/deckhouse/storage-e2e/pkg/storage-e2e 0%
github.com/deckhouse/storage-e2e/pkg/testkit 5%
Summary 24% (3773 / 15757)

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the reusable e2e runner script to re-resolve a consumer module’s dependency graph after applying a local replace to the checked-out storage-e2e@main, preventing CI breakage caused by go.sum drift when the effective module graph differs from the pinned revision.

Changes:

  • Switch .github/scripts/e2e-run-tests.sh from go mod download to go mod tidy after applying the directory replace.
  • Extend the EXIT trap restore logic to restore go.sum when present, and remove a go.sum created by tidy when none existed originally.
  • Update .github/scripts/tests/test-run-tests.sh to assert tidy usage and validate go.sum restore/removal behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
.github/scripts/e2e-run-tests.sh Runs go mod tidy after applying the storage-e2e directory replace and restores (or removes) go.sum via EXIT trap.
.github/scripts/tests/test-run-tests.sh Updates stubbed go expectations and adds coverage for go.sum restoration/removal.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants