Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Enable EventBridge rules/targets before EKS cluster #1728

Merged
merged 2 commits into from
Aug 14, 2023
Merged

fix: Enable EventBridge rules/targets before EKS cluster #1728

merged 2 commits into from
Aug 14, 2023

Conversation

vchintal
Copy link
Contributor

Description

The eventbridge target for the rule eks-api-endpoint-create wasn't getting created with the first terraform apply command and it doesn't get created until after the EKS cluster is completely setup causing the initial ENI create events to be missed.

This PR ensures that the first terraform apply command:

  1. Includes the target module.eventbridge that takes care of creating both modules module.create_eni_lambda and module.delete_eni_lambda
  2. For the above to be possible, it removes the dependency of module.delete_eni_lambda on module.eks

How was this change tested?

  • Yes, I have tested the PR using my local account setup (Provide any test evidence report under Additional Notes)
  • Yes, I ran pre-commit run -a with this PR

@vchintal vchintal requested a review from a team as a code owner August 14, 2023 16:09
@vchintal vchintal temporarily deployed to EKS Blueprints Test August 14, 2023 16:09 — with GitHub Actions Inactive
@vchintal vchintal changed the title bug: Enable EventBridge rules/targets before EKS cluster fix: Enable EventBridge rules/targets before EKS cluster Aug 14, 2023
Copy link
Contributor

@askulkarni2 askulkarni2 left a comment

Choose a reason for hiding this comment

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

@vchintal please see the error in the CI check.

@vchintal
Copy link
Contributor Author

vchintal commented Aug 14, 2023

@vchintal please see the error in the CI check.

Hi @askulkarni2, it is a known issue unfortunately. Since we do not run all of the TF at once, we cannot expect the TF plan to succeed.

If you see in the README we have broken this up into two TF apply commands. If the first command is run and then the TF plan is run, it will succeed. Let me know if this explanation makes sense.

@askulkarni2
Copy link
Contributor

Gotcha. Can you please update the github workflow to skip this blueprint then?

@vchintal
Copy link
Contributor Author

@askulkarni2 Sure, can do! Can you please point me to where I have to make the necessary changes? I haven't done this before and do not want to assume.

@askulkarni2
Copy link
Contributor

@vchintal vchintal temporarily deployed to EKS Blueprints Test August 14, 2023 18:36 — with GitHub Actions Inactive
@vchintal
Copy link
Contributor Author

@askulkarni2 Think I have done it right, please check the latest commit. Thanks 🙏🏼

@askulkarni2 askulkarni2 merged commit 145e6e2 into aws-ia:main Aug 14, 2023
42 of 43 checks passed
@vchintal vchintal deleted the privatelink branch August 15, 2023 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants