Skip to content

Commit 2035d6b

Browse files
committed
.github: run all quarantined EKS tests in a single tolerated step
The tests that cannot pass on the EKS legs without prefix delegation were skipped outright there, which loses all signal on those legs, and each one was handled by its own hand written block. As more tests get quarantined that pattern grows a block, a run step and a warning step per test. Collect them in a QUARANTINED_TESTS list instead. The job variables step walks the list to exclude them from the gating connectivity run and exports the same list as an output, and a single tolerated continue-on-error step re-runs exactly those tests, so they keep running and uploading a JUnit report while their failures do not fail the workflow. One warning step reports it. Adding a test to the quarantine is now a single line, and re-arming one is deleting that line. Both currently affected tests are listed: north-south-loadbalancing-with-l7-policy, whose outside-to-nodeport reply egresses a secondary ENI while carrying the primary ENI's source IP so the VPC source/destination check drops it (issue 47391), and egress-gateway-excluded-cidrs, which asserts the node HostIP while excluded-CIDR traffic is correctly masqueraded to the owning secondary ENI's primary IP (issue 47530). The prefix-delegation legs keep pod IPs on the primary ENI, so they run and gate on both tests as usual and the tolerated step is skipped there. Every other test keeps gating everywhere, so nothing else is masked. AIL:3 Signed-off-by: André Martins <andre@cilium.io>
1 parent 82c7f10 commit 2035d6b

1 file changed

Lines changed: 46 additions & 26 deletions

File tree

.github/workflows/conformance-eks.yaml

Lines changed: 46 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -578,27 +578,38 @@ jobs:
578578
run: |
579579
CONNECTIVITY_TEST_DEFAULTS="${{ steps.e2e_config.outputs.test_flags }}"
580580
581-
# Quarantine: on legs without prefix delegation the node attaches
582-
# multiple secondary ENIs, and the outside-to-nodeport reply for the
583-
# L7 policy test egresses a secondary ENI while carrying the primary
584-
# ENI's source IP, so the VPC source/dest check drops it (curl 28).
585-
# This is a real, still-under-investigation datapath bug rather than a
586-
# flake; skip only this test on the affected legs so the failure is
587-
# not silently masked elsewhere. The prefix-delegation legs still run
588-
# and gate it. Remove once the ENI return-path routing is fixed.
589-
if [[ "${{ matrix.aws-eni-pd }}" != "true" ]]; then
590-
CONNECTIVITY_TEST_DEFAULTS+=" --test '!north-south-loadbalancing-with-l7-policy'"
591-
echo "::warning title=Test quarantined::north-south-loadbalancing-with-l7-policy/outside-to-nodeport is skipped on this leg (${{ join(matrix.*, ', ') }}) because it hits a known ENI return-path drop without prefix delegation. The test is quarantined here and still gates on the prefix-delegation legs; remove once the bug is fixed."
592-
fi
581+
# Tests quarantined on the legs without prefix delegation. On those
582+
# legs a node attaches several secondary ENIs and pod IPs spread over
583+
# them, which these tests do not cope with:
584+
#
585+
# north-south-loadbalancing-with-l7-policy: the outside-to-nodeport
586+
# reply egresses a secondary ENI while carrying the primary ENI's
587+
# source IP, so the VPC source/destination check drops it and the
588+
# client times out (curl 28). Issue 47391.
589+
# egress-gateway-excluded-cidrs: excluded-CIDR traffic is correctly
590+
# masqueraded to the owning secondary ENI's primary IP, but the
591+
# test asserts the node HostIP. Issue 47530.
592+
#
593+
# They are removed from the gating run below and re-run together in a
594+
# single tolerated step, so they keep running and collecting artifacts
595+
# without failing the workflow. The prefix-delegation legs keep pod
596+
# IPs on the primary ENI, so they run and gate on them as usual. To
597+
# quarantine another test add it to this list; to re-arm one, drop it.
598+
QUARANTINED_TESTS=(
599+
north-south-loadbalancing-with-l7-policy
600+
egress-gateway-excluded-cidrs
601+
)
593602
594-
# Quarantine egress-gateway-excluded-cidrs on non-PD legs (issue
595-
# 47530): remove it from the gating run here and run it tolerated
596-
# below. The PD legs run it inline and gate on it.
603+
QUARANTINED=""
597604
if [[ "${{ matrix.aws-eni-pd }}" != "true" ]]; then
598-
CONNECTIVITY_TEST_DEFAULTS+=" --test '!egress-gateway-excluded-cidrs'"
605+
for test in "${QUARANTINED_TESTS[@]}"; do
606+
CONNECTIVITY_TEST_DEFAULTS+=" --test '!${test}'"
607+
QUARANTINED+="${QUARANTINED:+,}${test}"
608+
done
599609
fi
600610
601611
echo connectivity_test_defaults=${CONNECTIVITY_TEST_DEFAULTS} >> $GITHUB_OUTPUT
612+
echo "quarantined_tests=${QUARANTINED}" >> $GITHUB_OUTPUT
602613
603614
- name: Check that AWS leftover iptables chains have been removed
604615
run: |
@@ -623,24 +634,33 @@ jobs:
623634
--junit-file "cilium-junits/${{ env.job_name }} (${{ join(matrix.*, ', ') }}) - 1.xml" \
624635
--junit-property github_job_step="Run connectivity test (${{ join(matrix.*, ', ') }})"
625636
626-
# Tolerated re-run of the quarantined egress-gateway-excluded-cidrs test
627-
# on non-PD legs (issue 47530): it still runs and uploads a JUnit report,
628-
# but its failure does not fail the workflow.
629-
- name: Run quarantined test egress-gateway-excluded-cidrs (${{ join(matrix.*, ', ') }})
637+
# Re-run the tests quarantined above (see the quarantine list in the job
638+
# variables step) in one tolerated step, so they keep running and
639+
# uploading a JUnit report while their failures do not fail the workflow.
640+
- name: Run quarantined tests (${{ join(matrix.*, ', ') }})
630641
id: run-tests-quarantined
631-
if: ${{ matrix.aws-eni-pd != true }}
642+
if: ${{ steps.vars-conn.outputs.quarantined_tests != '' }}
632643
continue-on-error: true
644+
env:
645+
QUARANTINED_TESTS: ${{ steps.vars-conn.outputs.quarantined_tests }}
633646
run: |
634-
cilium connectivity test ${{ steps.e2e_config.outputs.test_flags }} \
635-
--test 'egress-gateway-excluded-cidrs' \
647+
TEST_FLAGS=""
648+
for test in ${QUARANTINED_TESTS//,/ }; do
649+
TEST_FLAGS+=" --test ${test}"
650+
done
651+
652+
# shellcheck disable=SC2086
653+
cilium connectivity test ${{ steps.e2e_config.outputs.test_flags }} ${TEST_FLAGS} \
636654
--test-concurrency=${{ env.test_concurrency }} \
637655
--junit-file "cilium-junits/${{ env.job_name }} (${{ join(matrix.*, ', ') }}) - quarantined.xml" \
638-
--junit-property github_job_step="Run quarantined test egress-gateway-excluded-cidrs (${{ join(matrix.*, ', ') }})"
656+
--junit-property github_job_step="Run quarantined tests (${{ join(matrix.*, ', ') }})"
639657
640-
- name: Warn on quarantined egress-gateway-excluded-cidrs failure (${{ join(matrix.*, ', ') }})
658+
- name: Warn on quarantined test failure (${{ join(matrix.*, ', ') }})
641659
if: ${{ always() && steps.run-tests-quarantined.outcome == 'failure' }}
660+
env:
661+
QUARANTINED_TESTS: ${{ steps.vars-conn.outputs.quarantined_tests }}
642662
run: |
643-
echo "::warning title=Test quarantined::egress-gateway-excluded-cidrs failed on this leg (${{ join(matrix.*, ', ') }}) but is quarantined, so it did not fail the workflow. Without prefix delegation a client pod may be masqueraded to a secondary ENI's primary IP rather than the node HostIP the test asserts. Investigate the failure above; remove the quarantine once the test oracle is fixed."
663+
echo "::warning title=Test quarantined::One of the quarantined tests (${QUARANTINED_TESTS}) failed on this leg (${{ join(matrix.*, ', ') }}) but is quarantined, so it did not fail the workflow. These tests do not cope with pod IPs spread over secondary ENIs, which happens without prefix delegation. Investigate the failure above; remove a test from the quarantine list once its issue is fixed."
644664
645665
- name: Features tested
646666
if: ${{ always() && steps.install-cilium.outcome == 'success' }}

0 commit comments

Comments
 (0)