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

Control Plane Logging as a default in all patterns #129

Merged
merged 6 commits into from
Dec 19, 2023

Conversation

5herlocked
Copy link
Contributor

@5herlocked 5herlocked commented Nov 6, 2023

Issue #, if available: #118

Description of changes: Added control plane logging through upstream changes and minor modifications to blueprints.
Included documentation changes + instructions on how to access Control Plane logs in CloudWatch.
Control Plane logging is an in-place change to EKS clusters and should not be a breaking change.

Hard dependency on aws-quickstart/cdk-eks-blueprints@d826223 being packaged and released.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link
Contributor

@elamaran11 elamaran11 left a comment

Choose a reason for hiding this comment

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

@5herlocked Nice work on this PR and blueprints PR. Have some tactical and doc comments. Please fix these when you get chance. We can merge this only after blueprints release so we should also factor for changes to package.json later.

…r all existing deployments, enabling control plane logging remains a user choice.
@5herlocked 5herlocked requested a review from elamaran11 November 7, 2023 04:54
Copy link
Contributor

@elamaran11 elamaran11 left a comment

Choose a reason for hiding this comment

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

@5herlocked Looks great, fix these minor issues. We will block the e2e and merge until we have blueprints release.

@5herlocked
Copy link
Contributor Author

Resolved the comments in the docs, ready to GTM after deployment of the upstream changes.

@elamaran11
Copy link
Contributor

@5herlocked GH Failure needs to be fixed.

@5herlocked
Copy link
Contributor Author

Error has nothing to do with my code.

@elamaran11
Copy link
Contributor

Error has nothing to do with my code.

This is not because of your code but did you pull from main, that could be the reason. I had one PR yesterday and we never had that error.

@elamaran11
Copy link
Contributor

/do-e2e-test single-new-eks-awsnative-observability deploy

@elamaran11
Copy link
Contributor

/do-e2e-test single-new-eks-awsnative-observability destroy

Copy link
Contributor

@elamaran11 elamaran11 left a comment

Choose a reason for hiding this comment

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

LGTM

@elamaran11 elamaran11 merged commit 69f19cf into aws-observability:main Dec 19, 2023
3 checks passed
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.

2 participants