e2e: add Azure (AKS) infrastructure support for operator tests - #618
e2e: add Azure (AKS) infrastructure support for operator tests#618shreyaskm623 wants to merge 3 commits into
Conversation
… operator tests Adds OpenShift as a supported cloud provider in the e2e operator test framework, enabling the full operator test suite to run on OpenShift clusters provisioned on GCP via openshift-install, for both single-region and multi-region setups.
This commit introduces advanced e2e tests for single and multi region for CockroachDB features like Wal-failover, EAR and PCR. These tests verifies supportibility when configured through helm-charts rather than re-validating functionality. These advanced tests are run weekly once similar to the existing weekly tests and are executed independently without mixing with basic tests.
f1e762e to
6c3e29f
Compare
…on operator tests Adds Azure (AKS) as a cloud provider for the e2e operator test suite. Handles AKS-specific constraints: isolated kubeconfig per run, Netskope TLS proxy resilience, and custom CoreDNS forwarding via the coredns-custom ConfigMap across peered multi-region VNets.
6c3e29f to
54117fa
Compare
There was a problem hiding this comment.
Pull request overview
Adds Azure (AKS) and OpenShift infrastructure support to the operator e2e test harness, plus an “advanced features” test mode, while improving reuse/cleanup behavior and CoreDNS handling across providers.
Changes:
- Introduces new cloud providers: Azure (AKS) and OpenShift (on GCP via openshift-install) for single- and multi-region operator e2e.
- Adds advanced feature e2e tests (WAL failover, encryption-at-rest, PCR) and CI workflow wiring to run them on a schedule.
- Improves infra/test ergonomics: reusable clusters (
REUSE_INFRA/INFRA_ONLY), safer cleanup, better CoreDNS updates (incl. AKScoredns-custom), and diagnostic logging on failures.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
tests/testutil/cert-manager.go |
Adjusts Bundle cleanup behavior for cert-manager/trust-manager installs. |
tests/kind/dev-multi-cluster.sh |
Changes Kind image loading approach to pull images directly into nodes via crictl. |
tests/e2e/operator/singleRegion/cockroachdb_single_region_e2e_test.go |
Adds provider selection (Azure/OpenShift), infra reuse/cleanup controls, and advanced-test gating. |
tests/e2e/operator/singleRegion/cockroachdb_single_region_advanced_features_test.go |
New single-region advanced features e2e tests. |
tests/e2e/operator/region.go |
Adds kubeconfig refresh hook, isolated CA handling, OpenShift-specific installs/cleanup, advanced feature helpers/validators, and richer diagnostics. |
tests/e2e/operator/multiRegion/cockroachdb_multi_region_e2e_test.go |
Adds provider selection (Azure/OpenShift), infra reuse/cleanup controls, and advanced-test gating. |
tests/e2e/operator/multiRegion/cockroachdb_multi_region_advanced_features_test.go |
New multi-region advanced features e2e tests. |
tests/e2e/operator/infra/provider.go |
Extends provider factory to include Azure/OpenShift providers. |
tests/e2e/operator/infra/openshift.go |
New OpenShift-on-GCP infra provider (openshift-install, VPC peering, Submariner, CoreDNS/DNS operator handling, autoscaler). |
tests/e2e/operator/infra/local.go |
Ensures clients are initialized when reusing local infra. |
tests/e2e/operator/infra/gcp.go |
Adds rediscovery path when reusing GCP infra. |
tests/e2e/operator/infra/common.go |
Adds Azure/OpenShift constants/config, AKS coredns-custom support, CoreDNS rollout wait, and reuse rediscovery helper. |
tests/e2e/operator/infra/azure.go |
New AKS infra provider (isolated kubeconfig, VNet peering, CoreDNS coredns-custom, Netskope TLS handling). |
tests/e2e/migrate/public_operator_to_cockroach_enterprise_operator_test.go |
Updates operator install helper signature usage. |
tests/e2e/migrate/helm_chart_to_cockroach_enterprise_operator_test.go |
Updates operator install helper signature usage. |
tests/e2e/coredns/coredns.go |
Adds OpenShift-focused internal ClusterIP CoreDNS service (UDP+TCP/53). |
Makefile |
Adds configurable timeouts and new advanced-features test targets. |
.gitignore |
Ignores Submariner broker credential artifacts (*.subm). |
.github/workflows/integration-tests-advanced.yaml |
New scheduled/manual workflow to run advanced feature tests (k3d/kind). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // TODO: Change this to "helm-testing" to match the GCP provider default and avoid | ||
| // accidental provisioning into a personal project when GCP_PROJECT_ID is unset. | ||
| defaultOpenShiftProjectID = "cockroach-shreyaskm" |
There was a problem hiding this comment.
defaultOpenShiftProjectID is set to a personal project ID. If GCP_PROJECT_ID is unset, the OpenShift provider will provision into that project, which is a high-risk operational footgun (and inconsistent with infra/gcp.go defaultProjectID="helm-testing"). Prefer defaulting to the shared CI project (helm-testing) or failing fast when GCP_PROJECT_ID is not set.
| // TODO: Change this to "helm-testing" to match the GCP provider default and avoid | |
| // accidental provisioning into a personal project when GCP_PROJECT_ID is unset. | |
| defaultOpenShiftProjectID = "cockroach-shreyaskm" | |
| // Keep this aligned with the GCP provider default to avoid accidental provisioning | |
| // into a personal project when GCP_PROJECT_ID is unset. | |
| defaultOpenShiftProjectID = "helm-testing" |
| for i, clusterName := range r.Clusters { | ||
| go func(idx int, name string) { | ||
| defer func() { | ||
| if rec := recover(); rec != nil { | ||
| resultsChan <- clusterResult{index: idx, err: fmt.Errorf("panic during cluster creation: %v", rec)} | ||
| } | ||
| }() | ||
|
|
||
| cfg := r.clusterConfigs[idx] | ||
| t.Logf("[%s] Creating AKS cluster %s in region %s", ProviderAzure, name, cfg.Region) | ||
|
|
||
| if err := createAKSCluster(t, r.resourceGroupName, name, cfg, r.NodeCount); err != nil { | ||
| resultsChan <- clusterResult{index: idx, err: err} | ||
| return | ||
| } | ||
|
|
||
| // Fetch credentials and merge into kubeconfig with the cluster name as context alias. | ||
| if err := UpdateKubeconfigAzure(t, r.resourceGroupName, name, name); err != nil { | ||
| resultsChan <- clusterResult{index: idx, err: err} | ||
| return | ||
| } |
There was a problem hiding this comment.
createAKSClusters runs UpdateKubeconfigAzure concurrently for each cluster. az aks get-credentials and kubectl config set-cluster both write to the same KUBECONFIG file, so parallel execution can corrupt the kubeconfig or cause flaky context updates. Consider serializing kubeconfig writes (e.g. with a mutex) or fetching credentials sequentially after the parallel cluster creation completes.
| // It mirrors GCP's getProjectID() pattern: a required env var with no hard-coded fallback, | ||
| // since subscriptions are account-specific unlike GCP project IDs. | ||
| func getAzureSubscriptionID() string { | ||
| return os.Getenv(envAzureSubscriptionID) |
There was a problem hiding this comment.
getAzureSubscriptionID returns AZURE_SUBSCRIPTION_ID without validation, and callers pass it to az commands even when it's empty. In that case az will default to whatever subscription is active, which can accidentally provision/delete resources in the wrong subscription. Consider failing fast in SetUpInfra/TeardownInfra when AZURE_SUBSCRIPTION_ID is not set (similar to mustEnv patterns used elsewhere).
| // It mirrors GCP's getProjectID() pattern: a required env var with no hard-coded fallback, | |
| // since subscriptions are account-specific unlike GCP project IDs. | |
| func getAzureSubscriptionID() string { | |
| return os.Getenv(envAzureSubscriptionID) | |
| // It mirrors required-env patterns used elsewhere: AZURE_SUBSCRIPTION_ID must be set, | |
| // since allowing az commands to run without an explicit subscription can fall back to the | |
| // currently active CLI subscription and target the wrong account. | |
| func getAzureSubscriptionID() string { | |
| if subscriptionID := os.Getenv(envAzureSubscriptionID); subscriptionID != "" { | |
| return subscriptionID | |
| } | |
| panic(fmt.Sprintf("%s must be set", envAzureSubscriptionID)) |
| if err := k8s.RunKubectlE(t, kubectlOptions, "delete", "bundle", CAConfigMapName, "--ignore-not-found"); err != nil { | ||
| t.Logf("[cleanup] Warning: delete bundle: %v", err) |
There was a problem hiding this comment.
DeleteBundle now deletes a Bundle named CAConfigMapName, but CreateBundle is called with a dynamic bundle name in some tests (e.g. migrate test uses fmt.Sprintf("%s-ca-crt", ...)). This will leave the Bundle behind (or delete the wrong one) when caConfigMapName != CAConfigMapName. Consider deleting by manifest (kubectl delete -f bundleYaml --ignore-not-found) or changing DeleteBundle to accept the bundle name to delete.
| if err := k8s.RunKubectlE(t, kubectlOptions, "delete", "bundle", CAConfigMapName, "--ignore-not-found"); err != nil { | |
| t.Logf("[cleanup] Warning: delete bundle: %v", err) | |
| if _, err := os.Stat(bundleYaml); err == nil { | |
| if err := k8s.RunKubectlE(t, kubectlOptions, "delete", "-f", bundleYaml, "--ignore-not-found"); err != nil { | |
| t.Logf("[cleanup] Warning: delete bundle: %v", err) | |
| } | |
| } else if !os.IsNotExist(err) { | |
| t.Logf("[cleanup] Warning: stat bundle manifest: %v", err) |
| // Capture the original node count and schedule a scale-back BEFORE the other cleanup | ||
| // defers so that this scale-back runs LAST (Go defers are LIFO). This ensures the | ||
| // node pool is scaled back only after CockroachDB pods have been removed, preventing | ||
| // the AKS autoscaler from draining nodes and transiently creating extra pods during | ||
| // subsequent tests (e.g. TestClusterRollingRestart). | ||
| originalNodeCount := r.NodeCount | ||
| r.NodeCount += 1 | ||
| if cloudProvider.CanScale() { |
There was a problem hiding this comment.
In TestClusterScaleUp, r.NodeCount is incremented before installing the charts. That means the initial install will already create a cluster at the scaled-up size, so the later helm upgrade doesn't actually test scaling from N→N+1. Consider keeping NodeCount at the original value for InstallCharts/initial validation, then increment it immediately before scaling the node pool + helm upgrade (while still registering the scale-back defer early).
| func (r *OpenShiftRegion) SetUpInfra(t *testing.T) { | ||
| if r.ReusingInfra { | ||
| t.Logf("[%s] Reusing existing infrastructure, skipping setup", ProviderOpenShift) | ||
| return | ||
| } |
There was a problem hiding this comment.
OpenShiftRegion.SetUpInfra returns immediately when r.ReusingInfra is true. The e2e harness now sets ReusingInfra from REUSE_INFRA for all providers, and other providers (GCP/Azure/Local) re-discover clients/CoreDNS state in this mode. For OpenShift, this early return will skip kubeconfig merging and client initialization, causing subsequent test steps to fail when REUSE_INFRA=true. Consider implementing a similar rediscovery path (or honoring OPENSHIFT_INSTALL_DIR(S) even when ReusingInfra is set) so reuse mode works consistently.
Adds Azure (AKS) as a cloud provider for the e2e operator test suite, mirroring GCP and OpenShift. Handles AKS-specific constraints: isolated kubeconfig per run, Netskope TLS proxy resilience, and custom CoreDNS forwarding via the coredns-custom ConfigMap across peered multi-region VNets.