Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .argoci/cron/e2e-nftables.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ schedule: 10 21 * * 1
globalPrologue: |
cd "${CI_HOME}/${CI_GIT_DIR}"
export ARGO_WORKFLOW_NAME="{{workflow.name}}"
export ARGO_WORKFLOW_UID="{{workflow.uid}}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This exports ARGO_WORKFLOW_UID, but I can't find anything that reads it — the epilogue builds the artifact path from ARGO_WORKFLOW_NAME, not the UID, and there are no other references in .argoci/. Was it meant to be used somewhere (e.g. to keep artifact paths unique across workflow re-runs, since the name can repeat)? If not, it looks like a dead export and can be dropped. Also, it's only set in this one cron — if it's meant to be a standard var, it should probably live wherever the other crons pick up their shared exports.

export DATAPLANE="${DATAPLANE:-CalicoNftables}"
export ENABLE_AUTOHEP="${ENABLE_AUTOHEP:-true}"
export FUNCTIONAL_AREA="${FUNCTIONAL_AREA:-nftables.yml}"
Expand Down
142 changes: 142 additions & 0 deletions .argoci/scripts/body_flannel-migration.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
#!/usr/bin/env bash
# body_flannel-migration.sh - flannel-to-Calico migration test flow.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This script is added here, but nothing seems to invoke it yet — I don't see any cron or template referencing body_flannel-migration.sh. So as of this PR it's dead until something wires it up. Is it intentionally staged ahead of the job wiring, or should the wiring land in the same PR? Just flagging so it isn't left dangling by accident.

#
# Provisions a cluster, installs flannel + a CNI plugin helper, runs a basic
# connectivity smoke test, applies Calico + the flannel-migration job, waits
# for the migration to complete, then runs the full e2e suite on Calico.
#
# Uses the legacy `./bz.sh tests:run` test runner (not the in-repo binary).
# When the in-repo binary reaches parity, this script can migrate to
# `make e2e-run` like body_standard.sh's run_tests_local.sh phase.
set -exo pipefail

PHASES="$(cd "$(dirname "$0")" && pwd)/phases"

echo "[INFO] starting job..."

export CNI_VERSION=${CNI_VERSION:-"v1.1.1"}
export DOCS_BASE=${DOCS_BASE:-"https://github.com/projectcalico/calico"}
export DOWNLEVEL_MANIFEST=${DOWNLEVEL_MANIFEST:-"https://github.com/projectcalico/calico/raw/release-${RELEASE_STREAM}/manifests/canal.yaml"}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

DOWNLEVEL_MANIFEST hardcodes release-${RELEASE_STREAM}. On master (the default stream, with USE_HASH_RELEASE=true also the default) this expands to …/raw/release-master/manifests/canal.yaml — there is no release-master branch, so wget -O flannel.yaml (L112) 404s (or truncates the file) and the flannel install breaks. The DOCS_URL guard above only exit 1s on master when USE_HASH_RELEASE != true, so the hashrelease path still reaches this. Only release-vX.Y streams work today.

export CALICO_MANIFEST=${CALICO_MANIFEST:-"manifests/flannel-migration/calico.yaml"}
export MIGRATION_MANIFEST=${MIGRATION_MANIFEST:-"manifests/flannel-migration/migration-job.yaml"}

if [ "${USE_HASH_RELEASE}" == "true" ]; then
echo "[INFO] Using hash release for flannel migration"
LATEST_HASHREL="https://latest-os.docs.eng.tigera.net/${RELEASE_STREAM}.txt"
echo "Checking ${LATEST_HASHREL} for latest hash release url..."
DOCS_URL=$(curl --retry 9 --retry-all-errors -sS ${LATEST_HASHREL})
echo "Using $DOCS_URL for hash release base url"
else
if [[ "${RELEASE_STREAM}" == "master" ]]; then
echo "Cannot use latest release on master branch"
exit 1
else
echo "[INFO] Using latest release for flannel migration"
export DOCS_URL=$DOCS_BASE/raw/release-${RELEASE_STREAM}
fi
fi

export BZ_LOCAL=${BZ_HOME}/.local
export KUBECONFIG=$BZ_LOCAL/kubeconfig
export PATH=$PATH:$BZ_LOCAL/bin

# Modern OSes no longer include br_netfilter by default, which breaks flannel.
echo "[INFO] installing br_netfilter..."
sudo modprobe br_netfilter

mkdir -p "$BZ_LOGS_DIR"
cd "${BZ_HOME}"
source "${PHASES}/provision.sh"

# Install bridge CNI plugin (needed by kube-flannel manifest).
kubectl apply -f - <<EOF
apiVersion: apps/v1
kind: DaemonSet
metadata:
name: cni-installer
namespace: kube-system
labels:
app: cni-installer
spec:
selector:
matchLabels:
app: cni-installer
template:
metadata:
labels:
app: cni-installer
spec:
nodeSelector:
kubernetes.io/os: linux
hostNetwork: true
terminationGracePeriodSeconds: 0
tolerations:
- effect: NoSchedule
operator: Exists
- key: CriticalAddonsOnly
operator: Exists
- effect: NoExecute
operator: Exists
terminationGracePeriodSeconds: 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

terminationGracePeriodSeconds: 0 is set twice in this pod spec (also at the line above the tolerations block). Duplicate YAML map key — harmless value-wise (both 0), but strict kubectl field validation (default since ~1.25) can reject a duplicate field, and at best it's confusing. Suggest deleting this one.

Suggested change
terminationGracePeriodSeconds: 0
priorityClassName: system-node-critical

priorityClassName: system-node-critical
securityContext:
seccompProfile:
type: RuntimeDefault
initContainers:
- name: cni-installer
image: quay.io/dosmith/cni-plugins:gen4

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

quay.io/dosmith/cni-plugins:gen4 is a personal quay namespace with a mutable tag baked into shared CI. If that account/repo/tag is deleted, made private, or repushed, every flannel-migration run silently breaks or changes behavior with no provenance. If this was copied verbatim from Semaphore for parity, fine to confirm as intended — otherwise worth moving to an org-owned registry (e.g. quay.io/calico/…) with a pinned digest.

command: ["/bin/bash", "-c", "cp -f /usr/src/plugins/bin/* /opt/cni/bin"]
volumeMounts:
- name: bindir
mountPath: /opt/cni/bin
securityContext:
privileged: true
resources:
requests:
cpu: 10m
memory: 10Mi
containers:
- name: pause
image: registry.k8s.io/pause
resources:
requests:
cpu: 10m
memory: 10Mi
volumes:
- name: bindir
hostPath:
path: /opt/cni/bin
EOF

# Update flannel.yaml to use the podCIDR that CRC sets up.
wget -O flannel.yaml "$DOWNLEVEL_MANIFEST"
sed -i "s?10.244.0.0/16?192.168.0.0/16?g" ./flannel.yaml
kubectl apply -f - < ./flannel.yaml
sleep 30 # wait for flannel to come up
kubectl get po -A -owide

# Run a basic services test to check that flannel networking is working.
K8S_E2E_FLAGS='--ginkgo.focus=should.serve.a.basic.endpoint.from.pods' \
./bz.sh tests:run |& tee >(gzip --stdout > "${BZ_LOGS_DIR}/e2e-tests-pre.log")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nit: these tee >(gzip --stdout > …) sinks write gzip'd bytes into files named .log (also e2e-tests.log at the end of the script), whereas phases/provision.sh uses .log.gz for the same pattern. Anyone/anything opening e2e-tests*.log gets binary garbage, and it defeats tooling keyed on .gz. Suggest e2e-tests-pre.log.gz / e2e-tests.log.gz.


kubectl delete -n kube-system ds cni-installer || true # remove the CNI installer daemonset
kubectl apply -f "$DOCS_URL/$CALICO_MANIFEST"
wget -O calico-migration.yaml "$DOCS_URL/$MIGRATION_MANIFEST"
kubectl apply -f - < ./calico-migration.yaml
sleep 5 # make sure the job has started before we check its status
kubectl -n kube-system get jobs flannel-migration
kubectl -n kube-system describe jobs flannel-migration
kubectl get po -A -owide
kubectl wait --for=condition=complete --timeout=600s -n kube-system job/flannel-migration

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nit: kubectl wait --for=condition=complete only returns early when the Job reaches complete (or is deleted). A failed migration sets Failed=true, never complete=true, so this blocks the full --timeout=600s and only then errors — ~10 wasted minutes per broken run. Consider also racing a --for=condition=failed (or a jsonpath poll) to abort fast on failure.

kubectl -n kube-system get jobs flannel-migration
kubectl -n kube-system describe jobs flannel-migration
kubectl -n kube-system logs -l k8s-app=flannel-migration-controller
kubectl get po -A -owide

# Delete the migration job because the presence of a non-Running pod in
# kube-system upsets the e2es.
kubectl -n kube-system delete job/flannel-migration || true
kubectl -n kube-system delete po -l k8s-app=flannel-migration-controller || true

# Run e2e on uplevel calico.
./bz.sh tests:run |& tee >(gzip --stdout > "${BZ_LOGS_DIR}/e2e-tests.log")
2 changes: 1 addition & 1 deletion .argoci/scripts/global_epilogue.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ CI_EXIT_CODE=${CI_EXIT_CODE:-0}
ARTIFACT_DEST="gs://${GS_BUCKET}/${ARGO_WORKFLOW_NAME:-local}/${HOSTNAME:-pod}"

# Capture diags on failure (or always for cert runs).
if [[ "${CI_EXIT_CODE}" != "0" ]]; then
if [[ "${CI_EXIT_CODE}" != "0" || "${TEST_TYPE}" == "ocp-cert" ]]; then
echo "[INFO] capturing diags"
bz diags |& tee "${BZ_LOGS_DIR}/diagnostic.log" || true
gsutil cp "${BZ_LOCAL_DIR}/${DIAGS_ARCHIVE_FILENAME}" "${ARTIFACT_DEST}/diags.tgz" || true
Expand Down
76 changes: 73 additions & 3 deletions .argoci/scripts/global_prologue.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,53 @@ if [[ -f "${GOOGLE_APPLICATION_CREDENTIALS}" ]]; then
else
echo "[WARN] GOOGLE_APPLICATION_CREDENTIALS missing: ${GOOGLE_APPLICATION_CREDENTIALS}"
fi
# Azure auth for azr-* provisioners (ported from Semaphore prologue L146-150).
# Creds come from banzai-secrets via envFrom (AZ_SP_ID/AZ_SP_PASSWORD/AZ_TENANT_ID/
# AZ_SUBSCRIPTION_ID); bz's azure path needs an active `az` session, so log in the
# service principal. Gated on azr-* so non-azure jobs skip the network call.
if [[ "${PROVISIONER:-}" == azr-* ]]; then
if command -v az >/dev/null 2>&1 && [[ -n "${AZ_SP_ID:-}" ]]; then
az login --service-principal -u "${AZ_SP_ID}" -p "${AZ_SP_PASSWORD}" --tenant "${AZ_TENANT_ID}" >/dev/null \
&& az account set --subscription "${AZ_SUBSCRIPTION_ID}" \
&& echo "[INFO] az login OK (subscription ${AZ_SUBSCRIPTION_ID})" \
|| echo "[WARN] az login failed; azr-* provisioning will fail"
else
echo "[WARN] azr-* provisioner but az CLI or AZ_SP_ID missing; azure auth skipped"
fi
fi

export DOCKER_AUTH_FILE="${DOCKER_AUTH_FILE:-${HOME}/.docker/config.json}"
chmod 0600 "${HOME}"/.keys/* 2>/dev/null || true
if [[ -f "${HOME}/.keys/marvin" ]]; then eval "$(ssh-agent -s)" >/dev/null 2>&1 || true; ssh-add "${HOME}/.keys/marvin" 2>/dev/null || true; fi

# AWS auth for aws-* provisioners (EKS, aws-openshift, aws-talos). banzai-secrets
# ships the AWS creds as the `credentials` key — its value is a full
# ~/.aws/credentials ini blob ([default]\naws_access_key_id=...). It arrives via
# envFrom as $credentials, but the AWS SDK reads the FILE, so materialise it.
# Fall back to standard AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY env if present.
mkdir -p "${HOME}/.aws"
if [[ -n "${credentials:-}" ]]; then
printf '%s\n' "${credentials}" > "${HOME}/.aws/credentials"
chmod 0600 "${HOME}/.aws/credentials"
echo "[INFO] wrote ~/.aws/credentials from banzai-secrets 'credentials' key"
elif [[ -n "${AWS_ACCESS_KEY_ID:-}" && -n "${AWS_SECRET_ACCESS_KEY:-}" ]]; then
{
echo "[default]"
echo "aws_access_key_id=${AWS_ACCESS_KEY_ID}"
echo "aws_secret_access_key=${AWS_SECRET_ACCESS_KEY}"
[[ -n "${AWS_SESSION_TOKEN:-}" ]] && echo "aws_session_token=${AWS_SESSION_TOKEN}"
} > "${HOME}/.aws/credentials"
chmod 0600 "${HOME}/.aws/credentials"
echo "[INFO] wrote ~/.aws/credentials from AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY env"
else
echo "[WARN] no AWS creds in banzai-secrets ('credentials' or AWS_ACCESS_KEY_ID); aws-* provisioners will fail"
fi

# `bz init` installs the gobz binary via `gh`, which needs GH_TOKEN (not the
# GITHUB_ACCESS_TOKEN name banzai-secrets provides) — alias it so go-backed
# provisioners are available.
export GH_TOKEN="${GH_TOKEN:-${GITHUB_ACCESS_TOKEN:-}}"

git config --global user.name "${GITHUB_USER_NAME:-marvin-tigera}"
git config --global user.email "${GITHUB_USER_EMAIL:-marvin@tigera.io}"

Expand All @@ -43,13 +86,19 @@ RANDOM_TOKEN1=$(LC_ALL=C tr -dc 'a-z0-9' </dev/urandom | head -c 5 || true)

echo "[INFO] exporting default env vars..."
export PRODUCT=${PRODUCT:-calico}
# bz tags aws-eks / aws-openshift clusters with `createdBy: ${USER}`; the ArgoCI
# runner leaves USER unset, so the tag renders empty and CloudFormation rejects
# it (tag value must be non-empty). Attribution only — cloud-custodian reaps on
# cluster name (bz-${PRODUCT}-*), not createdBy, so this doesn't affect cleanup.
export USER="${USER:-argoci}"
export PROVISIONER=${PROVISIONER:-gcp-kubeadm}
export INSTALLER=${INSTALLER:-operator}
export INSTALLER=${INSTALLER:-"manual"}
export DATAPLANE=${DATAPLANE:-CalicoIptables}
export TEST_TYPE=${TEST_TYPE:-k8s-e2e}
export GOOGLE_PROJECT=${GOOGLE_PROJECT:-unique-caldron-775}
export GOOGLE_REGION=${GOOGLE_REGION:-us-central1}
export GOOGLE_ZONE=${GOOGLE_ZONE:-us-central1-a}
export GOOGLE_REGIONS=("us-central1" "us-west1")
export GOOGLE_REGION=${GOOGLE_REGION:-${GOOGLE_REGIONS[RANDOM%${#GOOGLE_REGIONS[@]}]}}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

GOOGLE_REGION is now randomly chosen from {us-central1, us-west1}, but GOOGLE_NETWORK stays the fixed semaphore-autotest VPC (L54). Jobs that don't pin a region (e.g. nftables-talos-native-v3-crds-gcp, kubevirt-e2e-tests) will land in us-west1 ~50% of the time — if semaphore-autotest is a custom-mode VPC with no us-west1 subnet, bz provision will fail to place the instance. Worth confirming a us-west1 subnet (and any region-specific machine types/images) exist before this rides.

export GOOGLE_ZONE=${GOOGLE_ZONE:-$(gcloud compute zones list --filter="region~'$GOOGLE_REGION'" --format="value(name)" | awk 'BEGIN {srand()} {a[NR]=$0} rand() * NR < 1 {zone=$0} END {print zone}')}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

GOOGLE_ZONE is derived from gcloud … | awk inside ${GOOGLE_ZONE:-$(…)}. The prologue runs under set -o pipefail but deliberately not set -e (it's sourced), and this assignment returns rc=0 even when the substitution is empty. So if gcloud fails (auth/API/quota), GOOGLE_ZONE is silently set to "" and GCP provisioning proceeds with an empty zone instead of failing fast with a clear error. Consider validating the result is non-empty (and erroring if not).

Minor, related: this gcloud compute zones list runs on every job including the AWS-provisioner ones (aws-talos/aws-kubeadm/aws-eks) that never use GOOGLE_ZONE — worth gating on a GCP provisioner.

export GOOGLE_NETWORK=${GOOGLE_NETWORK:-semaphore-autotest}
export AWS_DEFAULT_REGION=${AWS_DEFAULT_REGION:-us-west-2}

Expand All @@ -59,6 +108,11 @@ export AWS_DEFAULT_REGION=${AWS_DEFAULT_REGION:-us-west-2}
# BRANCH then master. release-vX.Y -> vX.Y, else master.
export RELEASE_STREAM=${RELEASE_STREAM:-$( _b="${CI_GIT_CLONED_BRANCH:-${BRANCH:-master}}"; [[ "${_b}" =~ ^release-(v[0-9]+\.[0-9]+)$ ]] && echo "${BASH_REMATCH[1]}" || echo "master" )}

export K8S_VERSION=${K8S_VERSION:-stable-3}
export K8S_E2E_EXTRA_FLAGS=${K8S_E2E_EXTRA_FLAGS:-" --e2ecfg.calicoctl-opensource-image=calico/ctl:release-${RELEASE_STREAM} "}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Latent trap: when RELEASE_STREAM=master (the default), this default becomes calico/ctl:release-master, which is almost certainly not a published tag (master builds are calico/ctl:master). It happens to be inert on the make e2e-run path (this var isn't forwarded into the container or onto the make line), so it'd only bite the bz tests (non-e2e) path — but worth fixing the master casing regardless.

export HELM_PATCH=${HELM_PATCH:-"0"}
export CALICOCTL_INSTALL_TYPE=${CALICOCTL_INSTALL_TYPE:-"binary"}

export CLUSTER_NAME=${CLUSTER_NAME:-bz-${PRODUCT}-${RANDOM_TOKEN1}}
export DIAGS_ARCHIVE_FILENAME=${DIAGS_ARCHIVE_FILENAME:-${PROVISIONER}-${CLUSTER_NAME}-diags.tgz}

Expand Down Expand Up @@ -105,6 +159,22 @@ if ! python3 -c 'import yaml' 2>/dev/null; then
|| echo "[WARN] could not install pyyaml; bz destroy for local-kind may fail"
fi

# --- Ensure python3.10-venv for bz's provisioner venv. bz builds the venv with
# python3.10, but the runner image ships only python3.7/3.8-venv, so venv creation
# fails ("No such file: .local/venv/include"; "apt install python3.10-venv").
# Guard on python3.10 specifically — the default `python3` is 3.7 and *has*
# ensurepip, so guarding on it would skip this and let bz hit the gap later (and
# race apt → dpkg lock). Install up front so bz's venv-create finds it present.
# Stopgap until the runner image adds it (cc-utils/argoci-images); remove once that
# lands. (Separate, image-level: a py3.7 venv missing certifi cacert also breaks
# some gcp-kubeadm pip steps — tracked for the image fix, not band-aided here.) ---
if command -v python3.10 >/dev/null 2>&1 && ! python3.10 -c 'import ensurepip' 2>/dev/null; then
echo "[INFO] installing python3.10-venv (runner image lacks it)..."
sudo apt-get update -qq 2>/dev/null || true
sudo apt-get install -y python3.10-venv 2>/dev/null \
|| echo "[WARN] could not install python3.10-venv; bz venv creation may fail"
fi

echo "[INFO] initialising bz profile..."
( cd "${HOME}" && bz init profile -n "${BZ_PROFILE_NAME}" --skip-prompt --secretsPath "${HOME}/secrets" ) \
|& tee "${BZ_LOGS_DIR}/initialize.log" || true
Expand Down
40 changes: 21 additions & 19 deletions .argoci/scripts/phases/run_tests.sh
Original file line number Diff line number Diff line change
@@ -1,18 +1,12 @@
#!/usr/bin/env bash
# run_tests.sh - acquire an e2e binary and run it, or defer to bz tests.
#
# Selection (automatic):
# - E2E_TEST_CONFIG set → structured label-based selection via `make e2e-run`.
# The e2e binary is acquired first: built from local source when
# RUN_LOCAL_TESTS is set (per-PR CI / GCP e2e block), otherwise downloaded
# from the hashrelease (scheduled CI).
# - Else → `bz tests`, which honours K8S_E2E_FLAGS (regex --ginkgo.focus/skip,
# read from the environment) for regex-selected e2e jobs, and also runs the
# non-e2e test types (benchmarks, certification, etc.).
#
# `make e2e-run` has no --ginkgo.focus, so K8S_E2E_FLAGS is inert there; regex
# selection must go through `bz tests` (matches the Semaphore flannel-migration
# body, which runs `K8S_E2E_FLAGS=... bz tests`).
# Selection (automatic, matches Semaphore):
# - TEST_TYPE == k8s-e2e → run the monorepo e2e binary via `make e2e-run`.
# The binary is acquired first: built from local source when RUN_LOCAL_TESTS
# is set (per-PR CI), otherwise downloaded from the hashrelease (scheduled
# CI). E2E_TEST_CONFIG selects specs and may be empty (default config).
# - Else (non-e2e test types: benchmarks, certification, etc.) → `bz tests`.
#
# Required env:
# BZ_LOCAL_DIR, BZ_LOGS_DIR, HOME, REPORT_DIR, TEST_TYPE
Expand All @@ -34,7 +28,7 @@ if [[ -n "${RUN_LOCAL_TESTS:-}" ]]; then
make -C e2e build |& tee >(gzip --stdout > "${BZ_LOGS_DIR}/${TEST_TYPE}-build.log.gz")
E2E_BINARY=/go/src/github.com/projectcalico/calico/e2e/bin/k8s/e2e.test
popd || exit
elif [[ "${TEST_TYPE}" == "k8s-e2e" && -n "${E2E_TEST_CONFIG:-}" ]]; then
elif [[ "${TEST_TYPE}" == "k8s-e2e" ]]; then
# Scheduled CI: download the pre-built e2e binary from the hashrelease.
echo "[INFO] downloading e2e binary from hashrelease..."
HASHREL_URL=$(curl --retry 9 --retry-all-errors -sS "https://latest-os.docs.eng.tigera.net/${RELEASE_STREAM}.txt")
Expand All @@ -48,10 +42,10 @@ elif [[ "${TEST_TYPE}" == "k8s-e2e" && -n "${E2E_TEST_CONFIG:-}" ]]; then
fi

# E2E_BINARY is a set/unset sentinel (its value is not used here -- make
# e2e-run locates the binary itself). Take the structured path only when a
# k8s-e2e binary was acquired above AND E2E_TEST_CONFIG selects specs; regex
# (K8S_E2E_FLAGS) and non-e2e jobs fall through to bz tests below.
if [[ -n "${E2E_BINARY:-}" && -n "${E2E_TEST_CONFIG:-}" ]]; then
# e2e-run locates the binary itself). Take the structured path whenever a
# k8s-e2e binary was acquired above; non-e2e test types fall through to bz
# tests below. E2E_TEST_CONFIG may be empty (selects the default config).
if [[ -n "${E2E_BINARY:-}" ]]; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This gate change (dropping && -n "${E2E_TEST_CONFIG:-}") correctly matches .semaphore/end-to-end/scripts/run_tests.sh — so as a parity change it's right. But it has two consequences worth confirming the e2e-nftables cron is migrated for in lockstep:

  1. K8S_E2E_FLAGS becomes dead config. make e2e-run never receives --ginkgo.focus/--ginkgo.skip (root Makefile e2e-run target passes only --calico.test-config), yet the cron still sets an elaborate K8S_E2E_FLAGS on 8/9 jobs (globalPrologue L13, plus the EKS-calico-cni per-job value at L122–123 with extra known-failure DNS/proxy skips). Those skips are now ignored, so previously-skipped tests will actually run and likely fail.
  2. Empty E2E_TEST_CONFIG overrides the Makefile default. Those same 8 jobs set no E2E_TEST_CONFIG, and this path passes E2E_TEST_CONFIG='' unconditionally — which overrides the Makefile's ?= e2e/config/kind.yaml and hands the binary an empty --calico.test-config= rather than the kind.yaml default.

If historical Semaphore ran these nftables jobs through make e2e-run (not bz+K8S_E2E_FLAGS), then the fix belongs in the cron: drop the now-dead K8S_E2E_FLAGS and give each job a proper E2E_TEST_CONFIG. Flagging since the cron isn't in this PR.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Agreed — I confirmed the same when checking against .semaphore/end-to-end/scripts/run_tests.sh (identical [[ -n "${E2E_BINARY:-}" ]] gate + make e2e-run with E2E_TEST_CONFIG=''), so this is a faithful reproduction of existing Semaphore behavior, not a regression introduced here. Deferring to DE-4390 makes sense, and sequencing #1 first with a before/after test-count check is exactly right — that's the one with real behavior change (dead K8S_E2E_FLAGS → proper per-job E2E_TEST_CONFIG). Thanks for filing all nine.

echo "[INFO] starting e2e tests..."
pushd "${CI_HOME}/${CI_GIT_DIR}" || exit

Expand Down Expand Up @@ -82,6 +76,14 @@ if [[ -n "${E2E_BINARY:-}" && -n "${E2E_TEST_CONFIG:-}" ]]; then
# the container, and we prepend that to PATH inside the bash -c below.
make kubectl

# EKS kubeconfigs exec aws-iam-authenticator (PATH lookup), which the stock
# golang image lacks, so client-go fails before any tests run. The aws-eks
# provisioner installs it on the host; bind-mount it when present (no-op otherwise).
auth_mount=()
if [[ -x "${BZ_LOCAL_DIR}/bin/aws-iam-authenticator" ]]; then
auth_mount=(-v "${BZ_LOCAL_DIR}/bin/aws-iam-authenticator:/usr/local/bin/aws-iam-authenticator:ro")
fi

# Capture the exit code so the JUnit copy below runs even when tests fail
# (set -e would otherwise bail out before the cp).
e2e_rc=0
Expand All @@ -92,6 +94,7 @@ if [[ -n "${E2E_BINARY:-}" && -n "${E2E_TEST_CONFIG:-}" ]]; then
-e KUBECONFIG=/kubeconfig \
-e PRODUCT=${PRODUCT:-calico} \
${K8S_E2E_DOCKER_EXTRA_FLAGS:-} \
"${auth_mount[@]}" \
-v "$(pwd)":/go/src/github.com/projectcalico/calico:rw \
-v "$(pwd)"/.go-pkg-cache:/go-cache:rw \
-v "${BZ_LOCAL_DIR}/kubeconfig:/kubeconfig:ro" \
Expand All @@ -115,8 +118,7 @@ if [[ -n "${E2E_BINARY:-}" && -n "${E2E_TEST_CONFIG:-}" ]]; then
# Propagate the original test exit code.
exit ${e2e_rc}
else
# Regex-selected e2e (K8S_E2E_FLAGS) or non-e2e test types -- defer to bz,
# which reads K8S_E2E_FLAGS from the environment.
# Non-e2e test types (benchmarks, certification, etc.) -- defer to bz.
echo "[INFO] starting bz testing (K8S_E2E_FLAGS=${K8S_E2E_FLAGS:-<none>})..."
bz tests ${VERBOSE} |& tee >(gzip --stdout > "${BZ_LOGS_DIR}/${TEST_TYPE}-tests.log.gz")
fi
Loading