Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions e2e/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,27 @@ func (c *Configuration) IsLocalBuild() bool {
return c.BuildID == "local"
}

// SIGVersionTagNameBuildID is the value .pipelines/scripts/e2e_run.sh exports as
// SIG_VERSION_TAG_NAME when VHD_BUILD_ID is set, which selects the VHD produced by that
// specific build rather than the newest VHD tagged branch=refs/heads/main.
const SIGVersionTagNameBuildID = "buildId"

// VHDBuiltFromSourceUnderTest reports whether the VHD under test was built from the same
// AgentBaker source these tests are running from.
//
// The VHD builder pipelines run their e2e stage with VHD_BUILD_ID: $(Build.BuildId) (see
// .vsts-vhd-builder.yaml, .vsts-vhd-builder-release.yaml), so the VHD selected by that tag is
// the one built earlier in the same pipeline run, from the same commit. e2e_run.sh turns that
// into SIG_VERSION_TAG_NAME=buildId.
//
// The standalone e2e check leaves VHD_BUILD_ID unset, so SIGVersionTagName keeps its
// "branch"/"refs/heads/main" default and the PR's code runs against a VHD built from main.
// Validations that compare VHD-baked state against source-generated state are only meaningful
// when this returns true.
func (c *Configuration) VHDBuiltFromSourceUnderTest() bool {
return c.SIGVersionTagName == SIGVersionTagNameBuildID
}

func (c *Configuration) BlobStorageAccountURL() string {
return "https://" + c.BlobStorageAccount() + ".blob.core.windows.net"
}
Expand Down
44 changes: 29 additions & 15 deletions e2e/validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -3488,16 +3488,16 @@ func ValidateScriptlessNBCCSECmd(ctx context.Context, s *Scenario) error {
// source under test. The nbc-cmd side is generated from the source under test instead, so
// the two sides only agree when the VHD was built from that same source.
//
// E2E deliberately does not do that: it runs the PR's code against the newest VHD tagged
// branch=refs/heads/main (see config.SIGVersionTagValue, which defaults to "refs/heads/main").
// So any PR that changes a GPU driver version in parts/common/components.json will
// legitimately differ on these vars until a VHD is rebuilt from that PR, which cannot happen
// before merge. Failing on them makes GPU driver bumps unmergeable for a reason unrelated to
// the change under test.
// The standalone e2e check deliberately does not do that: it runs the PR's code against the
// newest VHD tagged branch=refs/heads/main (see config.SIGVersionTagValue). So any PR that
// changes a GPU driver version in parts/common/components.json will legitimately differ on
// these vars until a VHD is rebuilt from that PR, which cannot happen before merge. Failing
// on them makes GPU driver bumps unmergeable for a reason unrelated to the change under test.
//
// Differences limited to these vars are therefore tolerated here. Any other differing var
// still fails, and aks-node-controller still reports the full diff (including these vars) as
// a guest agent event for Kusto, so production observability is unchanged.
// Differences limited to these vars are therefore tolerated, but only when the VHD was not
// built from the source under test — see unexpectedEnvCompareDiffVars. aks-node-controller
// still reports the full diff (including these vars) as a guest agent event for Kusto, so
// production observability is unchanged.
var vhdSourcedEnvVars = map[string]struct{}{
"GPU_DRIVER_VERSION": {},
"GPU_IMAGE_SHA": {},
Expand Down Expand Up @@ -3525,14 +3525,20 @@ func parseEnvCompareDiffVars(grepOutput string) []string {
return names
}

// unexpectedEnvCompareDiffVars returns the differing env vars that are not explained by
// VHD-vs-source skew.
func unexpectedEnvCompareDiffVars(diffVars []string) []string {
// unexpectedEnvCompareDiffVars returns the differing env vars that phase 3 should fail on.
//
// When the VHD under test was built from the source under test, both sides of the comparison
// come from the same commit, so every difference is real and nothing is tolerated. Otherwise
// the vars in vhdSourcedEnvVars are read from the VHD on the provision-config side while the
// nbc-cmd side is generated from source, so they differ for a reason unrelated to the change
// under test and are excluded.
func unexpectedEnvCompareDiffVars(diffVars []string, vhdBuiltFromSourceUnderTest bool) []string {
var unexpected []string
for _, name := range diffVars {
if _, ok := vhdSourcedEnvVars[name]; !ok {
unexpected = append(unexpected, name)
if _, isVHDSourced := vhdSourcedEnvVars[name]; isVHDSourced && !vhdBuiltFromSourceUnderTest {
continue
}
unexpected = append(unexpected, name)
}
return unexpected
}
Expand Down Expand Up @@ -3565,7 +3571,15 @@ func ValidateScriptlessPhase3(ctx context.Context, s *Scenario) error {
"but the success marker is absent and no diff entries were found in %s:\n%s", logFile, result.stdout)
}

if unexpected := unexpectedEnvCompareDiffVars(diffVars); len(unexpected) > 0 {
builtFromSource := config.Config.VHDBuiltFromSourceUnderTest()

if unexpected := unexpectedEnvCompareDiffVars(diffVars, builtFromSource); len(unexpected) > 0 {
if builtFromSource {
return fmt.Errorf("expected no env var differences between provision-config and nbc-cmd, but found differences in %v. "+
"The VHD under test was built from the source under test (%s=%s), so both sides come from the same commit "+
"and every difference is real:\n%s",
unexpected, config.Config.SIGVersionTagName, config.Config.SIGVersionTagValue, result.stdout)
}
return fmt.Errorf("expected no env var differences between provision-config and nbc-cmd, but found differences in %v:\n%s",
unexpected, result.stdout)
}
Expand Down
95 changes: 76 additions & 19 deletions e2e/validators_envcompare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package e2e
import (
"testing"

"github.com/Azure/agentbaker/e2e/config"
"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -52,43 +53,99 @@ func TestParseEnvCompareDiffVars(t *testing.T) {

func TestUnexpectedEnvCompareDiffVars(t *testing.T) {
for _, tc := range []struct {
name string
diffVars []string
expected []string
name string
// vhdBuiltFromSourceUnderTest mirrors config.Configuration.VHDBuiltFromSourceUnderTest:
// true in the VHD builder pipelines (VHD_BUILD_ID set), false for the standalone e2e check.
vhdBuiltFromSourceUnderTest bool
diffVars []string
expected []string
}{
{
name: "GPU vars alone are explained by VHD skew",
diffVars: []string{"GPU_DRIVER_VERSION", "GPU_IMAGE_SHA"},
expected: nil,
name: "GPU vars alone are explained by VHD skew when the VHD is not from this source",
vhdBuiltFromSourceUnderTest: false,
diffVars: []string{"GPU_DRIVER_VERSION", "GPU_IMAGE_SHA"},
expected: nil,
},
{
name: "a non-GPU var is still reported",
diffVars: []string{"GPU_DRIVER_VERSION", "KUBELET_FLAGS"},
expected: []string{"KUBELET_FLAGS"},
name: "GPU vars are NOT tolerated when the VHD was built from this source",
vhdBuiltFromSourceUnderTest: true,
diffVars: []string{"GPU_DRIVER_VERSION", "GPU_IMAGE_SHA"},
expected: []string{"GPU_DRIVER_VERSION", "GPU_IMAGE_SHA"},
},
{
name: "only non-GPU vars are reported",
diffVars: []string{"KUBELET_FLAGS", "NETWORK_PLUGIN"},
expected: []string{"KUBELET_FLAGS", "NETWORK_PLUGIN"},
name: "a non-GPU var is still reported",
vhdBuiltFromSourceUnderTest: false,
diffVars: []string{"GPU_DRIVER_VERSION", "KUBELET_FLAGS"},
expected: []string{"KUBELET_FLAGS"},
},
{
name: "no diffs yields none",
diffVars: nil,
expected: nil,
name: "only non-GPU vars are reported",
vhdBuiltFromSourceUnderTest: false,
diffVars: []string{"KUBELET_FLAGS", "NETWORK_PLUGIN"},
expected: []string{"KUBELET_FLAGS", "NETWORK_PLUGIN"},
},
{
name: "non-GPU vars are reported regardless of VHD provenance",
vhdBuiltFromSourceUnderTest: true,
diffVars: []string{"KUBELET_FLAGS", "NETWORK_PLUGIN"},
expected: []string{"KUBELET_FLAGS", "NETWORK_PLUGIN"},
},
{
name: "no diffs yields none",
vhdBuiltFromSourceUnderTest: false,
diffVars: nil,
expected: nil,
},
} {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.expected, unexpectedEnvCompareDiffVars(tc.diffVars))
assert.Equal(t, tc.expected, unexpectedEnvCompareDiffVars(tc.diffVars, tc.vhdBuiltFromSourceUnderTest))
})
}
}

// TestVHDBuiltFromSourceUnderTest asserts the provenance signal matches what
// .pipelines/scripts/e2e_run.sh exports: SIG_VERSION_TAG_NAME=buildId only when VHD_BUILD_ID
// is set, which the VHD builder pipelines set to $(Build.BuildId).
func TestVHDBuiltFromSourceUnderTest(t *testing.T) {
for _, tc := range []struct {
name string
tagName string
expected bool
}{
{
name: "buildId tag means the VHD came from this pipeline run",
tagName: "buildId",
expected: true,
},
{
name: "default branch tag means the VHD came from main, not this source",
tagName: "branch",
expected: false,
},
{
name: "empty tag is not treated as same-source",
tagName: "",
expected: false,
},
} {
t.Run(tc.name, func(t *testing.T) {
c := &config.Configuration{SIGVersionTagName: tc.tagName}
assert.Equal(t, tc.expected, c.VHDBuiltFromSourceUnderTest())
})
}
}

// TestEnvCompareGPUBumpIsTolerated asserts the end-to-end decision for the exact failure that
// blocked the aks-gpu-grid 570.237 bump: the diff parses to only VHD-sourced vars, so it must
// not fail the scenario.
// blocked the aks-gpu-grid 570.237 bump: on the standalone e2e check the diff parses to only
// VHD-sourced vars, so it must not fail the scenario. The same diff must still fail when the
// VHD was built from the source under test, where the two sides are expected to agree.
func TestEnvCompareGPUBumpIsTolerated(t *testing.T) {
diffVars := parseEnvCompareDiffVars(realGPUBumpLogLine)
assert.Equal(t, []string{"GPU_DRIVER_VERSION", "GPU_IMAGE_SHA"}, diffVars)
assert.Empty(t, unexpectedEnvCompareDiffVars(diffVars),

assert.Empty(t, unexpectedEnvCompareDiffVars(diffVars, false),
"a GPU driver version bump must not fail phase 3 when the VHD is built from a different source")

assert.Equal(t, diffVars, unexpectedEnvCompareDiffVars(diffVars, true),
"the same diff must still fail once the VHD is built from the source under test")
}
Loading