fix: default to scriptless phase 2, since its default in prod now - #8966
Merged
Conversation
awesomenix
requested review from
AbelHu,
Devinwong,
SriHarsha001,
calvin197,
cameronmeissner,
djsly,
ganeshkumarashok,
karenychen,
lilypan26,
mxj220,
pdamianov-dev,
phealy,
r2k1,
runzhen,
sulixu,
surajssd,
timmy-wright,
titilambert,
xuexu6666 and
zachary-bailey
as code owners
July 16, 2026 21:58
awesomenix
force-pushed
the
nishp/scriptlessonly
branch
from
July 16, 2026 22:02
3762bce to
e8ef822
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the E2E harness to make the “scriptless phase 2” (scriptless_nbc) path the default subtest, removing the old per-scenario opt-out (SkipScriptlessNBC) and updating scenarios that previously forced the legacy/default path.
Changes:
- Removed
Config.SkipScriptlessNBCand corresponding uses across multiple E2E scenarios. - Updated
RunScenarioto run onlyscriptless_nbcby default (falling back todefaultonly when scriptless is unsupported). - Cleaned up scenario configs in GPU, daemonset, and CSE performance tests to no longer set
SkipScriptlessNBC.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| e2e/types.go | Removes SkipScriptlessNBC from the E2E scenario config. |
| e2e/test_helpers.go | Changes scenario execution to prefer scriptless_nbc and introduces scriptlessUnsupported gating. |
| e2e/scenario_test.go | Removes SkipScriptlessNBC from multiple scenarios (but one usage remains). |
| e2e/scenario_gpu_managed_experience_test.go | Removes SkipScriptlessNBC from GPU managed experience scenarios. |
| e2e/scenario_gpu_daemonset_test.go | Removes SkipScriptlessNBC from GPU daemonset scenario. |
| e2e/scenario_cse_perf_test.go | Removes SkipScriptlessNBC from CSE performance scenarios. |
Comments suppressed due to low confidence (1)
e2e/types.go:242
SkipScriptlessNBCwas removed fromConfig, but there is still at least one remaining usage (e2e/scenario_test.gostill setsSkipScriptlessNBC: true, e.g. inTest_AzureLinuxV2). This will not compile until that last reference is removed or replaced with a new opt-out mechanism.
// The assertion is performed inside the scenario's subtest.
ExpectedError string
// UseNVMe indicates whether to use NVMe-based disk placement/controller. This is required for certain VM sizes (e.g., v6 and v7 series) which only support NVMe disk controllers.
UseNVMe bool
// EagerCSETimingExtraction when true causes CSE timing events to be extracted
// immediately after SSH is established, before other validators run.
// This prevents the Guest Agent from sweeping events before they can be read.
// Only set this on CSE performance test scenarios.
EagerCSETimingExtraction bool
awesomenix
force-pushed
the
nishp/scriptlessonly
branch
from
July 16, 2026 22:14
e8ef822 to
c1fc27e
Compare
Comment on lines
+108
to
110
| func scriptlessUnsupported(s *Scenario) bool { | ||
| return s.IsWindows() || len(s.Config.CustomDataWriteFiles) > 0 || s.VHDCaching || config.Config.TestPreProvision || s.VHD.Distro == datamodel.AKSAzureLinuxV2Gen2 | ||
| } |
djsly
approved these changes
Jul 17, 2026
Devinwong
approved these changes
Jul 17, 2026
awesomenix
force-pushed
the
nishp/scriptlessonly
branch
from
July 17, 2026 03:00
c1fc27e to
efa23e7
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
e2e/cluster.go:188
- The error from EnsureDebugDaemonsets is now ignored. If daemonset creation or proxy ConfigMap setup fails, the cluster will still be considered ready and later failures will be harder to diagnose (and GetProxyURL may not work as expected). Handle and propagate the error here, like before.
k.EnsureDebugDaemonsets(ctx, isNetworkIsolated, config.GetPrivateACRName(true, *cluster.Location))
if isNetworkIsolated {
return "", nil
}
return k.GetProxyURL(ctx)
Comment on lines
+109
to
111
| func scriptlessUnsupported(s *Scenario) bool { | ||
| return s.IsWindows() || len(s.Config.CustomDataWriteFiles) > 0 || s.VHDCaching || config.Config.TestPreProvision || s.VHD.Distro == datamodel.AKSAzureLinuxV2Gen2 | ||
| } |
awesomenix
force-pushed
the
nishp/scriptlessonly
branch
from
July 17, 2026 03:42
efa23e7 to
49303f9
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
e2e/cluster.go:189
- The error returned by EnsureDebugDaemonsets is currently ignored. If daemonset creation or proxy config setup fails, prepareCluster will still proceed and may later fail in less obvious ways (or return a Cluster missing debug/proxy functionality). Propagate the error so the DAG fails fast with the real root cause.
proxyURL := dag.Go1(g, kubeForDebug, func(ctx context.Context, k *Kubeclient) (string, error) {
k.EnsureDebugDaemonsets(ctx, isNetworkIsolated, config.GetPrivateACRName(true, *cluster.Location))
if isNetworkIsolated {
return "", nil
}
return k.GetProxyURL(ctx)
}, debugDeps...)
timmy-wright
approved these changes
Jul 17, 2026
awesomenix
added a commit
that referenced
this pull request
Jul 17, 2026
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.
scriptless phase 2 is default, dont run other tests