Repository navigation
fix(e2e): add parameters to Component CR in verify_cms_bridge_reconciliation - #1276
Conversation
…liation
Include `parameters: {"authCheck": "true"}` in the `e2e-cms-auth-check`
Component manifest in `verify_cms_bridge_reconciliation` so that it satisfies
the Kubernetes CRD OpenAPI CEL validation rule requiring either `wrapperRef`
or `parameters` when `componentType` is `COMPONENT_TYPE_CUSTOM_3P_WRAPPER`.
Also add hermetic unit test coverage in `test_e2e_flow_test.py`.
Bug: b/569895291
TAG=agy
CONV=7e782875-965d-4097-a513-2b68b39b5ca3
…e2e_flow_test Inline the single-caller `_poll_component_disabled` helper back into `verify_cms_bridge_reconciliation` to eliminate the unused `timeout_sec` default parameter and keep the production test diff minimal. Also simplify mock subprocess return values and direct parameter assertions in `test_e2e_flow_test.py`. Bug: b/569895291 TAG=agy CONV=7e782875-965d-4097-a513-2b68b39b5ca3
There was a problem hiding this comment.
Code Review
This pull request introduces a new end-to-end flow test target in Bazel, extracts the component polling logic into a helper function _poll_component_disabled in test_e2e_flow.py, and adds comprehensive unit tests in test_e2e_flow_test.py. The feedback recommends using time.monotonic() instead of time.time() to measure deadlines robustly against system clock changes, adding a timeout to the subprocess.run call to prevent hangs, and updating the unit tests to mock time.monotonic accordingly.
I am having trouble creating individual review comments. Click here to see my feedback.
test/e2e/test_e2e_flow.py (490-497)
Using time.time() to measure elapsed time or deadlines can be problematic if the system clock is adjusted (e.g., via NTP synchronization or manual changes). It is highly recommended to use time.monotonic() instead, as it is guaranteed to never go backwards. Additionally, adding a timeout to subprocess.run prevents the test from hanging indefinitely if the kubectl command hangs.
deadline, last_yaml = time.monotonic() + timeout_sec, ""
while time.monotonic() < deadline:
try:
res = subprocess.run(get_cmd, capture_output=True, text=True, check=False, timeout=5.0)
last_yaml = res.stdout or ""
except subprocess.SubprocessError as e:
last_yaml = f"kubectl execution failed: {e}"
if "state: COMPONENT_STATE_DISABLED" in last_yaml:
logger.info("✓ xmc-bridge authenticated to CMS and reconciled Component CR.")
return
time.sleep(1.0)
test/e2e/test_e2e_flow_test.py (85)
Since verify_cms_bridge_reconciliation is updated to use time.monotonic() for measuring deadlines, we should patch time.monotonic instead of time.time in this test to ensure the timeout behavior is correctly mocked.
@mock.patch("test.e2e.test_e2e_flow.time.monotonic", side_effect=[100.0, 100.5, 131.0])
Drop `test_e2e_flow_test.py` and its `BUILD.bazel` target since `test_e2e_flow.py` is the E2E scenario script itself and is verified directly against the Kubernetes CRD OpenAPI CEL admission rule in the headless KinD E2E suite. Bug: b/569895291 TAG=agy CONV=7e782875-965d-4097-a513-2b68b39b5ca3
Summary
Fixes the failing headless E2E test stage
[5/5] Verifying Authenticated xmc-bridge -> CMS Reconciliationonmainby addingparametersto thee2e-cms-auth-checkComponentCR manifest.Motivation & Context
Commit
c5bb4a7a(#1219) introducedverify_cms_bridge_reconciliation()intest/e2e/test_e2e_flow.py, which applies aComponentCR (e2e-cms-auth-check) withcomponentType: COMPONENT_TYPE_CUSTOM_3P_WRAPPERwhile omitting bothwrapperRefandparameters. TheComponentCRD (deploy/helm/templates/components/crd.yaml) enforces the OpenAPI CEL admission rule:This caused
kubectl applyto fail immediately at Kubernetes API server admission on everymerge_groupE2E run starting fromc5bb4a7a. Providingparameters: {"authCheck": "true"}satisfies the CEL rule while keeping the operatorless disabled-component reconciliation path intact.Key Changes
test/e2e/test_e2e_flow.py):"parameters": {"authCheck": "true"}to thee2e-cms-auth-checkComponentCR spec inverify_cms_bridge_reconciliation().Assumptions
Verification & Testing
./.githooks/pre-commitbazel test //test/e2e/..../tools/analyze_diff_size.py --base-ref origin/main...gh workflow run e2e-tests.yml --ref fix/e2e-cms-bridge-crd-validation//test/e2e/...passed.38570364368(Job115873658863) passed in 3m 51s on commit5d5e2390:Related Issues / Bugs
Fixes bug 569895291
TAG=agy
CONV=7e782875-965d-4097-a513-2b68b39b5ca3