Skip to content

Conversation

@lwr20
Copy link
Member

@lwr20 lwr20 commented Dec 10, 2025

Description

Add an e2e test run to exercise the process in https://docs.tigera.io/calico/latest/getting-started/kubernetes/managed-public-cloud/aks-migrate

Related issues/PRs

Todos

  • Tests
  • Documentation
  • Release note

Release Note

TBD

Reminder for the reviewer

Make sure that this PR has the correct labels and milestone set.

Every PR needs one docs-* label.

  • docs-pr-required: This change requires a change to the documentation that has not been completed yet.
  • docs-completed: This change has all necessary documentation completed.
  • docs-not-required: This change has no user-facing impact and requires no docs.

Every PR needs one release-note-* label.

  • release-note-required: This PR has user-facing changes. Most PRs should have this label.
  • release-note-not-required: This PR has no user-facing changes.

Other optional labels:

  • cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.
  • needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.

@marvin-tigera marvin-tigera added this to the Calico v3.32.0 milestone Dec 10, 2025
@marvin-tigera marvin-tigera added release-note-required Change has user-facing impact (no matter how small) docs-pr-required Change is not yet documented labels Dec 10, 2025
@lwr20 lwr20 added docs-not-required Docs not required for this change release-note-not-required Change has no user-facing impact skip-bot-cherry-pick and removed docs-pr-required Change is not yet documented release-note-required Change has user-facing impact (no matter how small) labels Dec 10, 2025
@lwr20 lwr20 marked this pull request as ready for review December 10, 2025 14:32
@lwr20 lwr20 requested a review from a team as a code owner December 10, 2025 14:32
Copilot AI review requested due to automatic review settings December 10, 2025 14:32
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds an end-to-end test to exercise the AKS migration process from vendored Calico CNI to unmanaged Calico CNI, following the documentation at https://docs.tigera.io/calico/latest/getting-started/kubernetes/managed-public-cloud/aks-migrate.

Key Changes:

  • Added conditional logic in the test script to trigger AKS migration via bz addons run aks-migrate:setup
  • Created a new test job "AKS migrate from vendored Calico CNI to Calico CNI" with 7-hour execution limit
  • Configured test environment with vendored CNI setup (USE_VENDORED_CNI: true, CNI_PLUGIN: Calico) and migration trigger (DESIRED_POLICY: Calico)

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
.semaphore/end-to-end/scripts/body_standard.sh Added conditional block to execute AKS migration setup when DESIRED_POLICY is set
.semaphore/end-to-end/pipelines/iptables.yml Added new AKS migration test job configuration under the AKS block with appropriate environment variables


# Perform the AKS migration following the instructions here:
# https://docs.tigera.io/calico/latest/getting-started/kubernetes/managed-public-cloud/aks-migrate
if [[ -n "$DESIRED_POLICY" ]]; then
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable name DESIRED_POLICY is ambiguous for triggering AKS migration. The condition only checks if the variable is non-empty (-n), making it function as a boolean flag, but the name suggests it's a configuration value. Consider using a more explicit name like AKS_MIGRATE to match the pattern of OPERATOR_MIGRATE on line 46, or if the value matters, document why and ensure the bz addons run aks-migrate:setup command uses it.

Suggested change
if [[ -n "$DESIRED_POLICY" ]]; then
if [[ -n "$AKS_MIGRATE" ]]; then

Copilot uses AI. Check for mistakes.
Comment on lines +171 to +172
- name: INSTALLER
value: operator
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The INSTALLER environment variable is set to "operator" at the job level (line 171-172) but is also defined as "helmerator" at the task level (line 185-186). While the job-level value will take precedence, this duplication could cause confusion. Consider removing the job-level definition if the task-level default is incorrect, or document why both are needed.

Suggested change
- name: INSTALLER
value: operator

Copilot uses AI. Check for mistakes.
@lwr20 lwr20 merged commit 05b4cfe into projectcalico:master Dec 12, 2025
16 checks passed
@lwr20 lwr20 deleted the lwr-aks-migrate branch December 12, 2025 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs-not-required Docs not required for this change release-note-not-required Change has no user-facing impact skip-bot-cherry-pick

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants