cluster-api-provider-aws icon indicating copy to clipboard operation
cluster-api-provider-aws copied to clipboard

Published 20 hours ago • verified publisher icon kubernetes-sigs

Add EventBridge ec2 event reconciliation and rule creation to eks managed control planes

Open cnmcavoy opened this issue 3 years ago • 9 comments
trafficstars

What type of PR is this? /kind bug

What this PR does / why we need it:

The EventBridge feature for importing ec2 events is not supported for EKS in the AWS managed control planes controller. The logic to reconcile the events and create any roles/policies needed is missing.

Special notes for your reviewer: Slack context: https://kubernetes.slack.com/archives/CD6U2V71N/p1663962497725469

Checklist:

  • [x] squashed commits
  • [ ] includes documentation
  • [ ] adds unit tests
  • [ ] adds or updates e2e tests

cnmcavoy avatar Sep 26 '22 15:09 cnmcavoy

Hi @cnmcavoy. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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/test-infra repository.

k8s-ci-robot avatar Sep 26 '22 15:09 k8s-ci-robot

/ok-to-test

Ankitasw avatar Sep 27 '22 10:09 Ankitasw

/test pull-cluster-api-provider-aws-e2e-eks

Ankitasw avatar Sep 28 '22 14:09 Ankitasw

/retest

cnmcavoy avatar Sep 28 '22 18:09 cnmcavoy

@cnmcavoy could you please take care of failing E2E test?

Ankitasw avatar Sep 29 '22 05:09 Ankitasw

/retest

cnmcavoy avatar Sep 29 '22 16:09 cnmcavoy

@Ankitasw not sure why there is a failing test, this PR is a dead code path for e2e, the event bridge feature gate is not active in any of the e2e eks tests as far as I can tell.

cnmcavoy avatar Sep 29 '22 16:09 cnmcavoy

@Ankitasw the e2e test succeeded on a retry, it is just flaky I guess.

cnmcavoy avatar Sep 29 '22 20:09 cnmcavoy

@cnmcavoy it would be good to test this in E2E. could you please enable EVENT_BRIDGE_INSTANCE_STATE in https://github.com/kubernetes-sigs/cluster-api-provider-aws/blob/f23771b6b1cb6022e755c18d9f3f8d07465806fc/test/e2e/data/e2e_eks_conf.yaml and run the eks tests again? Otherwise, LGTM.

Ankitasw avatar Oct 20 '22 08:10 Ankitasw

Rebased and added to the eks e2e.

cnmcavoy avatar Oct 24 '22 17:10 cnmcavoy

/test pull-cluster-api-provider-aws-e2e-eks

Ankitasw avatar Oct 25 '22 07:10 Ankitasw

/lgtm /approve

Ankitasw avatar Oct 25 '22 09:10 Ankitasw

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Ankitasw

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

The pull request process is described here

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

k8s-ci-robot avatar Oct 25 '22 09:10 k8s-ci-robot