Skip to content

test(vpc): add e2e coverage for ignore-field-drift on spec.tags - #361

Open
sapphirew wants to merge 3 commits into
aws-controllers-k8s:mainfrom
sapphirew:test/vpc-ignore-field-drift-e2e
Open

test(vpc): add e2e coverage for ignore-field-drift on spec.tags#361
sapphirew wants to merge 3 commits into
aws-controllers-k8s:mainfrom
sapphirew:test/vpc-ignore-field-drift-e2e

Conversation

@sapphirew

Copy link
Copy Markdown
Contributor

Description

Adds an e2e integration test exercising the services.k8s.aws/ignore-field-drift annotation (aws-controllers-k8s/runtime#256) against the EC2 VPC resource, using spec.tags as the ignored field.

EC2 is one of the controllers the runtime presubmit (runtime_presubmit_services) regenerates and e2e-tests, so this coverage runs on runtime PRs and guards the feature against regressions.

The IgnoreFieldDrift feature gate is Alpha and disabled by default, so the test self-enables it on the deployed controller for the duration of the module (by patching the deployment's FEATURE_GATES env var and waiting for rollout), then restores the prior value afterwards.

What the test asserts

On a VPC annotated to ignore spec.tags:

  • the declared tag is applied at create;
  • an externally-added tag survives — the controller does not call DeleteTags for drift on the ignored field;
  • the resource stays Synced even though spec.tags differs from the live AWS tag set;
  • an edit to spec.tags is retained in the CR spec but is not pushed to AWS.

Testing

Ran locally via make kind-test SERVICE=ec2 against a controller built from the runtime PR branch — 1 passed in 107.50s. No leaked VPCs or CRs (teardown fixture verified clean).

Files

  • test/e2e/tests/test_vpc_ignore_field_drift.py
  • test/e2e/resources/vpc_ignore_field_drift.yaml

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Adds an integration test exercising the services.k8s.aws/ignore-field-drift
annotation (aws-controllers-k8s/runtime#256) against the EC2 VPC resource.
The IgnoreFieldDrift feature gate is Alpha and disabled by default, so the
test self-enables it on the deployed controller for the module and restores
the prior FEATURE_GATES value afterwards.

EC2 is one of the controllers the runtime presubmit regenerates and e2e-tests,
so this coverage runs on runtime PRs.

The test asserts, on a VPC annotated to ignore spec.tags:
- the declared tag is applied at create;
- an externally-added tag survives (controller does not call DeleteTags);
- the resource stays Synced despite tag drift;
- an edit to spec.tags is retained in the spec but not pushed to AWS.
@ack-prow
ack-prow Bot requested a review from a-hilaly July 28, 2026 23:45
@ack-prow

ack-prow Bot commented Jul 28, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sapphirew

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ack-prow
ack-prow Bot requested a review from michaelhtm July 28, 2026 23:45
@ack-prow ack-prow Bot added the approved label Jul 28, 2026
rhaowang added 2 commits July 28, 2026 17:24
Extends the ignore-field-drift e2e coverage beyond the list-of-objects case
(spec.tags) to the two other field shapes the runtime treats differently:

- test_scalar_field_drift_ignored: ignore a scalar leaf
  (spec.enableDNSSupport) whose Delta path matches the ignored path exactly.
  Asserts an external modify_vpc_attribute flip is not reverted and a spec
  edit is retained but not pushed.
- test_nested_field_under_ignored_parent: drift on a nested child
  (a tag's value, Delta path Spec.Tags.N.Value) while only the parent
  spec.tags is ignored, exercising the runtime's path-prefix match rather
  than exact equality.

The VPC fixture is now parametrizable (indirect) on the ignored paths and the
baseline enableDNSSupport value; the existing test keeps its default behaviour.
Temporary go.mod replace pointing the ACK runtime dependency at
github.com/sapphirew/ack-runtime@selective-reconciliation (runtime PR aws-controllers-k8s#256,
the IgnoreFieldDrift feature gate) so the ec2-kind-e2e presubmit builds a
controller that recognizes the IgnoreFieldDrift gate and can run the new
ignore-field-drift e2e tests.

Without this, the released runtime (v0.61.0) does not know the gate; the
controller exits 1 at startup on FEATURE_GATES=IgnoreFieldDrift=true (strict
parsing in pkg/featuregate) and the tests error with 'did not roll out'.

This commit MUST be reverted before merge, once runtime aws-controllers-k8s#256 is released and
ec2-controller is bumped to a runtime version that includes the gate.
@sapphirew

Copy link
Copy Markdown
Contributor Author

Heads up: I pushed a temporary commit (41ed2d8, titled DO NOT MERGE) that adds a go.mod replace pointing the ACK runtime at my fork branch (runtime PR #256, the IgnoreFieldDrift gate).

Why: the ec2-kind-e2e check builds the controller from the checked-in go.mod, which pins the released runtime v0.61.0. That runtime doesn't know the IgnoreFieldDrift gate, and gate parsing is strict — the controller os.Exit(1)s at startup on FEATURE_GATES=IgnoreFieldDrift=true, so the new tests fail with "did not roll out". The replace lets CI build a controller that recognizes the gate and actually exercise the tests once.

Merge plan: runtime #256 merges & releases first → ec2-controller bumps to a runtime version containing the gate → I revert 41ed2d8 → this PR merges. The replace commit must not merge as-is.

@sapphirew

Copy link
Copy Markdown
Contributor Author

/hold

@ack-prow ack-prow Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 29, 2026
@ack-prow

ack-prow Bot commented Jul 29, 2026

Copy link
Copy Markdown

@sapphirew: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ec2-kind-e2e 41ed2d8 link true /test ec2-kind-e2e

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants