-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Copy e2es from master into v3.31 #11645
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release-v3.31
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 pull request merges end-to-end test configurations and scripts from the master branch into the current branch. The changes include significant updates to CI/CD pipeline scripts and the addition of multiple new pipeline configuration files for comprehensive testing across different platforms and scenarios.
Changes:
- Updated CI/CD scripts to support HCP (Hosted Control Plane) workflows, AKS migration, and improved DNS configuration
- Added 6 new comprehensive test pipeline configurations covering Windows, nftables, iptables, BPF, certification, and benchmarking scenarios
- Updated existing pipeline configurations (upgrade, test) with new script paths and private registry settings
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
.semaphore/end-to-end/scripts/global_prologue.sh |
Adds HCP support, changes BZ_HOME location from PWD to HOME, sets RELEASE_STREAM default to v3.31, adds DNS override logic |
.semaphore/end-to-end/scripts/global_epilogue.sh |
Refactors cleanup and reporting logic with new functions, adds HCP-specific workflows, improves artifact management |
.semaphore/end-to-end/scripts/body_standard.sh |
Adds HCP test execution paths, AKS migration support, conditional test execution based on stage |
.semaphore/end-to-end/pipelines/windows.yml |
New comprehensive Windows testing pipeline with multiple OS versions, dataplanes, and provisioners |
.semaphore/end-to-end/pipelines/upgrade.yml |
Updates script paths, adds private registry configuration, updates OpenShift version |
.semaphore/end-to-end/pipelines/test.yml |
Updates script paths and adds private registry configuration |
.semaphore/end-to-end/pipelines/patch-verification.yml |
New extensive patch verification pipeline covering multiple dimensions (installers, CNIs, platforms, features) |
.semaphore/end-to-end/pipelines/nftables.yml |
New nftables dataplane testing pipeline for ARM64, EKS, and KinD environments |
.semaphore/end-to-end/pipelines/iptables.yml |
New comprehensive iptables testing pipeline covering OpenShift, ARM64, AKS, dual-stack, and various features |
.semaphore/end-to-end/pipelines/certification.yml |
New OpenShift certification pipeline with standalone and HCP cluster testing |
.semaphore/end-to-end/pipelines/bpf.yml |
New extensive BPF dataplane testing covering ARM64, AWS, GCP, AKS, EKS, dual-stack, and various configurations |
.semaphore/end-to-end/pipelines/benchmarking.yml |
New performance benchmarking pipeline for Calico with multiple test configurations |
| - name: ENABLE_AUTO_HEP | ||
| value: "true" |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ENABLE_AUTO_HEP environment variable is defined twice in this job configuration (lines 482 and 488), both with the value "true". This is redundant and the duplicate definition at line 488 should be removed.
| - name: ENABLE_AUTO_HEP | |
| value: "true" |
| - name: K8S_VERSION | ||
| value: stable-1 | ||
| - name: DATAPLANE | ||
| value: CalicoNftables | ||
| - name: ENCAPSULATION_TYPE | ||
| value: VXLAN | ||
| - name: RKE_VERSION # Don't just update these versions, need k8s and rke versions to match. | ||
| value: "v1.8.6" # https://github.com/rancher/rke/releases/tag/v1.8.6 | ||
| - name: K8S_VERSION | ||
| value: "v1.32.7" # RKE 1.8.6 supports: v1.30.14-rancher1-1, v1.31.11-rancher1-1, v1.32.7-rancher1-1 |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The K8S_VERSION environment variable is defined twice in this job configuration (lines 99-100 and line 107-108). The first sets it to "stable-1" while the second overrides it to "v1.32.7". If the intention is to use "v1.32.7" for RKE compatibility, the first K8S_VERSION definition should be removed to avoid confusion.
| echo "$std"; eval "$std" | ||
|
|
||
| restore_hcp_hosting_home="echo \"[INFO] Setting BZ_HOME env var from restored cache\"" | ||
| restore_hcp_hosting_home="$restore_hcp_hosting_home; unset BZ_HOME; export BZ_HOME=$(cat ${BZ_LOGS_DIR}/restore.log | grep -oP 'Restored: \K(.*)(?=.)' || echo '')" |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The command at line 186 uses a grep pattern with a lookbehind that expects the matched text to end with a character (the dot in the pattern). However, this pattern may not correctly capture the full path if there are edge cases. Consider testing this regex extraction thoroughly or simplifying it to be more robust.
| restore_hcp_hosting_home="$restore_hcp_hosting_home; unset BZ_HOME; export BZ_HOME=$(cat ${BZ_LOGS_DIR}/restore.log | grep -oP 'Restored: \K(.*)(?=.)' || echo '')" | |
| restore_hcp_hosting_home="$restore_hcp_hosting_home; unset BZ_HOME; export BZ_HOME=$(cat ${BZ_LOGS_DIR}/restore.log | grep -oP '^Restored: \K.*' || echo '')" |
|
|
||
| echo "[INFO] exporting default env vars..." | ||
| export RELEASE_STREAM=${RELEASE_STREAM:-v3.31} | ||
|
|
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 48-49 have inconsistent formatting with surrounding code. Line 48 has the export statement, but line 49 is blank. The blank line placement is inconsistent with the formatting pattern seen elsewhere in the file where blank lines typically separate logical sections. Consider removing line 49 or ensuring consistent blank line usage throughout.
| export CALICOCTL_INSTALL_TYPE=${CALICOCTL_INSTALL_TYPE:-"binary"} | ||
| export BZ_LOGS_DIR=${BZ_LOGS_DIR:-$HOME/.bz/logs} | ||
| export BZ_HOME=${BZ_HOME:-"${PWD}/${SEMAPHORE_JOB_ID}"} | ||
| export BZ_HOME=${BZ_HOME:-"${HOME}/${SEMAPHORE_JOB_ID}"} |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The BZ_HOME variable has changed from using PWD to HOME. This is a significant behavioral change that may cause issues if any scripts or jobs expect BZ_HOME to be in the current working directory. Ensure this change is intentional and that all dependent code has been updated to handle BZ_HOME being in the HOME directory instead of the current working directory.
| export BZ_HOME=${BZ_HOME:-"${HOME}/${SEMAPHORE_JOB_ID}"} | |
| export BZ_HOME=${BZ_HOME:-"${PWD}/${SEMAPHORE_JOB_ID}"} |
| echo "[INFO] publish new semaphore test results" | ||
| test_publish=0 | ||
| test-results publish ${REPORT_DIR}/junit.xml --name "$SEMAPHORE_JOB_NAME" || test_publish=1 | ||
| echo "[INFO] Status of Publishing test results to Semaphore: ${test_publish}" |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test_publish variable is set but not used after being assigned. The result of the test-results publish command is captured but there's no subsequent action taken based on whether it succeeded or failed. Either remove this variable if it's not needed, or add logic to handle the failure case appropriately.
| echo "[INFO] Status of Publishing test results to Semaphore: ${test_publish}" | |
| if [[ "${test_publish}" -ne 0 ]]; then | |
| echo "[ERROR] Failed to publish test results to Semaphore (status: ${test_publish})" | |
| else | |
| echo "[INFO] Successfully published test results to Semaphore" | |
| fi |
| restore_hcp_hosting="echo \"[INFO] Restoring from ${SEMAPHORE_WORKFLOW_ID}-hosting-${HOSTING_CLUSTER} cache\"" | ||
| restore_hcp_hosting="$restore_hcp_hosting; cache restore ${SEMAPHORE_WORKFLOW_ID}-hosting-${HOSTING_CLUSTER} |& tee ${BZ_LOGS_DIR}/restore.log" | ||
|
|
||
| if [[ "${HCP_ENABLED}" == "true" ]]; then std=$hcp; elif [[ "${HCP_STAGE}" == "hosting" || "${HCP_STAGE}" == "destroy-hosting" ]]; then std=$restore_hcp_hosting; fi |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conditional logic for HCP_STAGE at line 182 uses a fallback pattern where std is reassigned based on conditions. However, the initial value of std (the non-HCP flow) is always set first, and then conditionally overridden. This could be clearer by using an if-elif-else structure instead of sequential assignments and conditional overrides.
| if [[ "${HCP_ENABLED}" == "true" ]]; then std=$hcp; elif [[ "${HCP_STAGE}" == "hosting" || "${HCP_STAGE}" == "destroy-hosting" ]]; then std=$restore_hcp_hosting; fi | |
| default_std="$std"; if [[ "${HCP_ENABLED}" == "true" ]]; then std=$hcp; elif [[ "${HCP_STAGE}" == "hosting" || "${HCP_STAGE}" == "destroy-hosting" ]]; then std=$restore_hcp_hosting; else std=$default_std; fi |
| value: "operator" | ||
| - name: USE_PRIVATE_REGISTRY | ||
| value: "true" | ||
| - name: ENABLE_AUTOHEP |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ENABLE_AUTO_HEP variable name at line 39 is inconsistent with ENABLE_AUTOHEP used elsewhere in the same file (line 40). One uses an underscore while the other doesn't. This inconsistency could lead to confusion and bugs. The variable names should be standardized across all pipeline files.
| - name: ENABLE_AUTOHEP | |
| - name: ENABLE_AUTO_HEP |
| value: "benchmark" | ||
| - name: BENCHMARKS | ||
| value: "benchmarker" | ||
| - name: BENCHMARKER_TEST_CONFIGS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this here because different clusters are getting created for iptables and ebpf?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand the question.
BENCHMARKER_TEST_CONFIGS contains json which defines a list of tests to run, which includes defining the dataplane. The benchmark tool takes care of reconfiguring the cluster to run each test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be included in a separate config file in that case, instead of calling it out in the CI pipeline file.
Description
This PR copies the contents of the .semaphore/end-to-end directory from master, plus a small change to change the default RELEASE_STREAM to v3.31.
This will allow us to run e2es against this branch, from pipelines in the branch.
This sort of PR should be unnecessary in future, since new releases are branched off master (which already has the e2es)
Related issues/PRs
Todos
Release Note
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.