feat: add istio-upgrade tool for AKS-managed mesh canary upgrades - #271
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new shared tools/istio-upgrade Go module in ARO-Tools that encapsulates AKS-managed Istio canary upgrade orchestration (decisioning, ARM operations, in-cluster migration/restarts, health/verification), intended for reuse by ARO-HCP and other repos.
Changes:
- Adds an AKS service-mesh upgrade engine (
pkg/istio) covering decision logic, ARM client operations, revision-tag webhook management, workload migration/restarts, rollout waiting, health checks, and post-upgrade verification. - Adds a Cobra command tree (
istio run) wiring options → clients →RunUpgrade(but currently no binary entry point). - Adds extensive unit tests for decisioning, webhooks, workloads/restarts, health/verify, and upgrade flows.
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/istio-upgrade/pkg/istio/workloads.go | Namespace label/tag flipping, stale-sidecar detection, workload restarts, rollout waiting, ingress annotation helpers. |
| tools/istio-upgrade/pkg/istio/workloads_test.go | Unit tests for namespace selection/labeling, ingress status/annotations, restart logic, and rollout waiting. |
| tools/istio-upgrade/pkg/istio/webhooks.go | Revision-tag webhook creation/update logic (istioctl tag-set equivalent behavior). |
| tools/istio-upgrade/pkg/istio/webhooks_test.go | Unit tests covering tag webhook create/update/no-op and failure conditions. |
| tools/istio-upgrade/pkg/istio/upgrade.go | Primary upgrade orchestration state machine and safety/rollback handling. |
| tools/istio-upgrade/pkg/istio/upgrade_test.go | End-to-end-ish unit tests for install/upgrade/resume/cleanup/rollback/verification flows using fakes/reactors. |
| tools/istio-upgrade/pkg/istio/mesh-config.yaml | Embedded mesh ConfigMap payload used per revision. |
| tools/istio-upgrade/pkg/istio/inspect.go | Mesh state inspection logging used during upgrades for observability. |
| tools/istio-upgrade/pkg/istio/health.go | Health check + upgrade verification + orphaned workload detection helpers. |
| tools/istio-upgrade/pkg/istio/health_test.go | Unit tests for health/verify/orphan detection and helper behaviors. |
| tools/istio-upgrade/pkg/istio/decide.go | Decision engine to choose install/upgrade/resume/cleanup/skip actions. |
| tools/istio-upgrade/pkg/istio/decide_test.go | Unit tests validating decision outcomes and revision comparison. |
| tools/istio-upgrade/pkg/istio/configmap.go | Kube client creation + revision ConfigMap create/update/delete (with embedded mesh config). |
| tools/istio-upgrade/pkg/istio/aks.go | ARM client wrapper for mesh profile enable/start/complete + provisioning polling. |
| tools/istio-upgrade/cmd/run/options.go | CLI flag binding/validation and conversion into UpgradeOptions. |
| tools/istio-upgrade/cmd/run/cmd.go | run subcommand wiring options to AKS/K8s clients and RunUpgrade. |
| tools/istio-upgrade/cmd/cmd.go | Root Cobra command (istio) and subcommand registration. |
| tools/istio-upgrade/Makefile | Build/test targets for the new module. |
| tools/istio-upgrade/go.mod | New module definition + dependencies. |
| tools/istio-upgrade/go.sum | Dependency checksums for the new module. |
| tools/istio-upgrade/.gitignore | Ignores local build outputs. |
| go.work | Adds ./tools/istio-upgrade to the workspace. |
| go.work.sum | Workspace dependency checksum updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
trevorwilliams2025
force-pushed
the
istio-upgrade-tool
branch
from
July 20, 2026 09:20
c650ef1 to
49e725b
Compare
janboll
requested changes
Jul 20, 2026
Adds Step N/M markers to all orchestration functions so pipeline logs are self-documenting. runCanaryPostInstall shows Steps 1-9/9, runCleanupAndUpgrade shows Phases 1-3/3, runInitialInstall shows Steps 1-4/4. An operator reading failure logs can immediately see which step failed and where in the sequence they are.
The Step N/M markers are now in logger.Info calls (Task 3). The inline comments that previously carried step numbers now describe what the step does without numbering — the logger line is the canonical source for step sequence.
The scenarioNotReady case covered both "cluster provisioning failed" and "ARM upgrading to a different revision." These have different operational implications. The ARM-busy case now logs with configTarget and installed revisions so the operator immediately sees the mismatch without needing to correlate with cluster state.
Adds testClusterBuilder to reduce test setup boilerplate from 80-120 lines to 5-10 lines per test. Builder handles system namespaces, istiod deployments, gateway objects, webhooks, and AKS error injection. Migrates 4 tests as proof of concept. Remaining tests can be migrated incrementally — the builder and manual setup coexist.
The function was renamed from runActionSkip to runReconcile in the ActionReconcile split, but three error log messages still said "on skip." Updated to "on reconcile" for consistency.
…and upgrade tests Add error injection and edge-case tests to bring five key functions to 100% statement coverage: CreateRevisionConfigMap, VerifyUpgrade, CheckOrphanedWorkloads, migrateWorkloads, and runReconcile. Also adds stableRevisionFrom reverse-order tests and HealthCheck error path tests. Overall package coverage 75.5% → 79.2%.
trevorwilliams2025
force-pushed
the
istio-upgrade-tool
branch
from
July 23, 2026 02:44
36936a7 to
400f977
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 47 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
tools/istio-upgrade/pkg/istio/mesh-config.yaml:5
envoyExtAuthzHttp.serviceshould be a DNS name (typically<svc>.<ns>.svc.cluster.local) so Envoy/Istio can resolve it. The current value contains a/(mise/mise.mise.svc.cluster.local), which is not a valid service DNS name and will likely break ext-authz resolution. Also,portis expected to be numeric; quoting it makes it a string in YAML.
envoyExtAuthzHttp:
service: "mise/mise.mise.svc.cluster.local"
port: "8080"
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 47 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
tools/istio-upgrade/pkg/istio/mesh-config.yaml:5
- The
servicevalue contains a/("mise/mise.mise.svc.cluster.local"), which is not a valid DNS hostname for Envoy ext-authz. This will likely make the mesh ConfigMap invalid or cause the extension provider to fail to resolve the backend service.
envoyExtAuthzHttp:
service: "mise/mise.mise.svc.cluster.local"
port: "8080"
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 47 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
tools/istio-upgrade/pkg/istio/workloads.go:131
- GetIngressGatewayStatus only populates ExternalIP from LoadBalancer.Ingress[0].IP. For some AKS load balancers, the ingress address may be provided via Hostname instead of IP, which will incorrectly mark gateways unhealthy in HealthCheck/Verify flows.
tools/istio-upgrade/Makefile:11 - The PR description/test plan mention a reusable CLI (e.g.
istio run ...), but this module currently has nopackage mainentrypoint and the Makefile explicitly states there’s no main.go. As-is,go build ./...will not produce anistiobinary, so consumers can’t actually runistio runwithout adding their own main wrapper.
# Build all packages (no main.go — binary entry point is in sdp-pipelines)
build: $(SOURCES)
go build ./...
| - name: "ext-authz" | ||
| envoyExtAuthzHttp: | ||
| service: "mise/mise.mise.svc.cluster.local" | ||
| port: "8080" |
Comment on lines
+3
to
+7
| envoyExtAuthzHttp: | ||
| service: "mise/mise.mise.svc.cluster.local" | ||
| port: "8080" | ||
| includeRequestHeadersInCheck: ["x-ext-authz", "mise-inbound-policies-to-filter"] | ||
| pathPrefix: "/v1/EnvoyValidateRequest" |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 47 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
tools/istio-upgrade/pkg/istio/workloads.go:240
- The goroutine in this loop closes over the range variables
iandns. Because those variables are reused across iterations, concurrent goroutines can write to the wrongoutcomesindex / use the wrong namespace, and can also race by all writing the finalivalue. Capture per-iteration copies before starting the goroutine (or pass them as parameters).
tools/istio-upgrade/pkg/istio/workloads.go:477 - Same closure-capture issue as in ExecuteRestartAllNamespaces: the goroutine closes over
iandns, soerrs[i]may be written to the wrong slot (often the finali) and the wrong namespace may be checked. Capture iteration-local copies before spawning the goroutine.
tools/istio-upgrade/pkg/istio/mesh-config.yaml:5 - The
envoyExtAuthzHttp.servicevalue contains a/and theportis quoted. In Istio mesh config this service is expected to be a resolvable DNS name (no/), andportis a numeric field. As written, AKS/Istio may reject the mesh config or Envoy may fail to resolve the ext-authz destination.
service: "mise/mise.mise.svc.cluster.local"
port: "8080"
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 47 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
tools/istio-upgrade/pkg/istio/mesh-config.yaml:7
- The embedded mesh config looks malformed for an Istio meshConfig:
serviceincludes a slash (mise/mise...) andportis quoted as a string. This is likely to be rejected by Istio/AKS mesh config parsing and could break installs/upgrades that create the revision ConfigMap.
extensionProviders:
- name: "ext-authz"
envoyExtAuthzHttp:
service: "mise/mise.mise.svc.cluster.local"
port: "8080"
includeRequestHeadersInCheck: ["x-ext-authz", "mise-inbound-policies-to-filter"]
pathPrefix: "/v1/EnvoyValidateRequest"
Comment on lines
+31
to
+46
| //go:embed mesh-config.yaml | ||
| var meshConfig string | ||
|
|
||
| func revisionConfigMapName(revision string) string { | ||
| return fmt.Sprintf("istio-shared-configmap-%s", revision) | ||
| } | ||
|
|
||
| func istiodServiceName(revision string) string { | ||
| return fmt.Sprintf("istiod-%s", revision) | ||
| } | ||
|
|
||
| // https://learn.microsoft.com/en-us/azure/aks/istio-meshconfig#set-up-configuration-on-cluster | ||
| func CreateRevisionConfigMap(ctx context.Context, kubeClient *KubeClient, revision string) error { | ||
| logger := logr.FromContextOrDiscard(ctx).WithName("revision-configmap") | ||
| cmName := revisionConfigMapName(revision) | ||
| meshData := strings.TrimSpace(meshConfig) |
Comment on lines
+88
to
+96
| if o.Versions == "" { | ||
| return nil, fmt.Errorf("--versions is required") | ||
| } | ||
| if !istio.RevisionPattern.MatchString(o.Versions) { | ||
| return nil, fmt.Errorf("invalid --versions %q: must match %s", o.Versions, istio.RevisionPattern.String()) | ||
| } | ||
| if (o.IngressIPName == "") != (o.RegionRG == "") { | ||
| return nil, fmt.Errorf("--ingress-ip-name and --region-rg must both be set or both be empty (got ingress-ip-name=%q, region-rg=%q)", o.IngressIPName, o.RegionRG) | ||
| } |
janboll
approved these changes
Jul 27, 2026
janboll
enabled auto-merge (squash)
July 27, 2026 12:19
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.
Summary
tooling/templatizeinto a sharedtools/istio-upgrademodule in ARO-Tools.istio run) and library API for AKS-managed Istio canary upgrades (decide → install/canary → migrate workloads → verify).Why
The upgrade logic is infrastructure tooling, not HCP-service code. Keeping it in ARO-HCP blocked reuse from other repos and duplicated ownership. Moving it to ARO-Tools so that it lives along other shared tools (
cmdutils,grafanactl, etc.)What’s included
Test plan
make -C tools/istio-upgrade test(or package tests) passistio run ... --dry-rungithub.com/Azure/ARO-Tools/tools/istio-upgrade/...