fix: add ubuntu2204 azure cni with overlay since thats most popular#8389
fix: add ubuntu2204 azure cni with overlay since thats most popular#8389awesomenix merged 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an Ubuntu 22.04 E2E scenario targeting Azure CNI overlay networking, and adjusts Linux VM log collection to include containerd logs.
Changes:
- Add
Test_Ubuntu2204_AzureCNIscenario using an Azure CNI overlay cluster. - Update Linux VM log extraction list to collect
containerdsystemd logs (and stop collectinglocaldnslogs).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| e2e/vmss.go | Modifies the Linux log-collection command list (adds containerd.log, removes localdns.log). |
| e2e/scenario_test.go | Adds a new Ubuntu 22.04 + Azure CNI scenario using the overlay cluster factory and sets NBC network plugin to Azure. |
| Cluster: clusterAzureOverlayNetwork, | ||
| VHD: config.VHDUbuntu2204Gen2Containerd, |
There was a problem hiding this comment.
Cluster: clusterAzureOverlayNetwork bypasses the cached ClusterAzureOverlayNetwork wrapper (see e2e/cache.go) and will repeat the cluster lookup/create path for every scenario invocation. Switch to ClusterAzureOverlayNetwork so the cluster preparation is memoized and avoids extra Azure API calls and flakiness/cost across the test suite.
| func Test_Ubuntu2204_AzureCNI(t *testing.T) { | ||
| RunScenario(t, &Scenario{ | ||
| Description: "Ubuntu 22.04 scenario on a cluster configured with Azure CNI", |
There was a problem hiding this comment.
This scenario uses the Azure CNI overlay cluster (clusterAzureOverlayNetwork), but the test name/description only says "Azure CNI". Consider including "Overlay" in the test name and/or Description to avoid confusion with the existing non-overlay Azure CNI scenarios (e.g., Test_Flatcar_AzureCNI which uses ClusterAzureNetwork).
| func Test_Ubuntu2204_AzureCNI(t *testing.T) { | |
| RunScenario(t, &Scenario{ | |
| Description: "Ubuntu 22.04 scenario on a cluster configured with Azure CNI", | |
| func Test_Ubuntu2204_AzureCNIOverlay(t *testing.T) { | |
| RunScenario(t, &Scenario{ | |
| Description: "Ubuntu 22.04 scenario on a cluster configured with Azure CNI Overlay", |
| "kubelet.log": "sudo journalctl -u kubelet", | ||
| "aks-log-collector.log": "sudo journalctl -u aks-log-collector", | ||
| "localdns.log": "sudo journalctl -u localdns", | ||
| "containerd.log": "sudo journalctl -u containerd", |
There was a problem hiding this comment.
Dropping localdns.log collection here removes a useful debug artifact for failures, especially since localdns behavior is validated in several scenarios. If the goal is to add containerd logs, consider keeping both localdns.log and containerd.log entries (or conditionally collecting localdns only when supported) rather than replacing it outright.
| "containerd.log": "sudo journalctl -u containerd", | |
| "containerd.log": "sudo journalctl -u containerd", | |
| "localdns.log": "sudo journalctl -u localdns", |
Add a new ubuntu2204 + azurecni test since thats most popular right now.