Repository navigation
smoke: install MSR in the modern and upgrade suites - #704
Open
james-nesbitt wants to merge 1 commit into
Open
james-nesbitt wants to merge 1 commit into
james-nesbitt wants to merge 1 commit into
Conversation
test/platforms.go has defined GetMSR() with zero callers. Every smoke suite set MSRVersion and smoke_test.go passed it to terraform as msr_version, but launchpad.tf:207 derives has_msr by counting msr-role nodegroups, so it was always false: :231 rendered `kind: mke`, the :273-281 msr block was skipped, and msr_version was discarded at the last gate. launchpad.tf:20 says as much -- "unused if you have no MSR hosts". The effect was that no smoke test has ever installed MSR, on any suite, while eight configurations read as though they covered it. That is how a review credited smoke-legacy with MSR 2.9.28 coverage it did not have. Three suites now carry an msr-role nodegroup on a platform already in their matrix: TestModernCluster (ubuntu24), TestUpgradeLegacyToModern (ubuntu22) and TestUpgradeModernClusterFromLegacy (ubuntu24). One m6a.2xlarge per run each. TestModernCluster's MSRVersion moves 3.1.18 -> 2.9.28. The terraform renders MSR 2.x only -- `kind: mke+msr` with --dtr-external-url and --ucp-insecure-tls -- and this launchpad has no MSR 3 support at all: no msr3 kind anywhere in pkg/, and the MSR phases are DTR-era. 3.1.18 would have produced a mke+msr document carrying a 3.x version. It had never been rendered, so nothing caught it. The upgrade suites now move MSR as well as MCR and MKE: 2.9.28 -> 2.9.30 across the MKE 3.8.8 -> 3.9.2 upgrade. bumpVersions takes an msrVersion and rewrites spec.msr.version beside the other two. An empty msrVersion leaves the document alone, which is the right shape for a cluster with no MSR node; a non-empty one against a document with no spec.msr is a configuration mistake and is reported rather than silently skipped. Verified against both document shapes: versions replaced, --san, --dtr-external-url and host addresses preserved verbatim, the no-MSR cluster untouched, and the mismatch rejected. go vet ./test/... clean. Written by AI: claude-opus-5 Signed-off-by: James Nesbitt <jnesbitt@mirantis.com>
james-nesbitt
force-pushed
the
smoke/msr-coverage
branch
from
November 11, 2026 12:39
728eaec to
ab590c3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Three smoke suites gain an MSR node, so that launchpad's MSR install path is exercised for the first time:
TestModernCluster2.9.28(was3.1.18)TestUpgradeLegacyToModern2.9.28→2.9.30TestUpgradeModernClusterFromLegacy2.9.28→2.9.30One
m6a.2xlargeper run each.Why
test/platforms.go:37has definedGetMSR()with zero callers. Every smoke suite setMSRVersion, andsmoke_test.go:110passed it to terraform asmsr_version, butexamples/terraform/aws-simple/launchpad.tf:207deriveshas_msrby counting msr-role nodegroups:With no
GetMSR()call that is alwaysfalse, so:231renderedkind: mkerather thankind: mke+msr, and the:273-281block that emitsspec.msrwas skipped.msr_versiontravelled the whole way and was discarded at the last gate.launchpad.tf:20documents the field as "unused if you have no MSR hosts".No smoke test has ever installed MSR, on any suite, for either 2.x or 3.x — while eight configurations read as though they covered it.
smoke-legacyin particular looks like it covers MSR 2.9.28 and does not. That appearance has already misled one review into crediting MSR coverage that did not exist.This is a property of
main; no open PR introduced it.TestModernCluster's version moves 3.1.18 → 2.9.28The terraform renders MSR 2.x only:
kind: mke+msrwith--dtr-external-urland--ucp-insecure-tls. This launchpad has no MSR 3 support at all — nomsr3kind anywhere inpkg/, and the MSR phases are DTR-era.So
3.1.18would have produced akind: mke+msrdocument carrying a 3.x version. Nothing caught it because the block was never rendered.Upgrading MSR
bumpVersionsnow takes anmsrVersionand rewritesspec.msr.versionbeside the MCR channel and MKE version, so the upgrade suites move MSR as part of the same in-place upgrade.msrVersionleaves the document alone — the correct shape for a cluster with no MSR nodespec.msris reported, not silently skipped: that combination can only be a configuration mistake2.9.30is the newestmirantis/dtr2.9.x tag on Docker Hub, checked against the registry.Testing
bumpVersionsexercised offline against both document shapes: versions replaced;--san,--dtr-external-urland host addresses preserved verbatim; the no-MSR cluster untouched; the mismatch rejected. Throwaway, not committed.go vet ./test/...andgofmt -l test/: cleanKnown unknowns
Both need a real run to settle, and neither is answerable from the repo:
TestModernClusteris where that surfaces.GetMSR()'sm6a.2xlargeis sized for MSR. It matchesGetManager(), presumably deliberately, but nothing has ever run on it.A failure in either direction is new information, not a regression.
Checklist
Written by AI: claude-opus-5