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

Add EKS Istio Dashboards #194

Merged
merged 57 commits into from
Jul 27, 2023
Merged

Add EKS Istio Dashboards #194

merged 57 commits into from
Jul 27, 2023

Conversation

dms486
Copy link
Contributor

@dms486 dms486 commented Jul 19, 2023

What does this PR do?

Adding the EKS Istio dashboards example

🛑 Please open an issue first to discuss any significant work and flesh out details/direction - we would hate for your time to be wasted. Consult the CONTRIBUTING guide for submitting pull-requests.

Motivation

More

  • 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
  • Yes, I have added a new example under examples to support my PR (when applicable)
  • Yes, I have updated the Pages for this feature

Note: Not all the PRs required examples and docs.

For Moderators

  • E2E Test successfully complete before merge?

Additional Notes

@awsdabra awsdabra requested a review from bonclay7 July 19, 2023 23:44
@awsdabra
Copy link
Contributor

@bonclay7 Can you please review this request?

Copy link
Member

@bonclay7 bonclay7 left a comment

Choose a reason for hiding this comment

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

Can you guys install https://pre-commit.com/ and run pre-commit run -a ?

@dms486
Copy link
Contributor Author

dms486 commented Jul 20, 2023

@bonclay7 sure, this is complete

docs/eks/istio.md Outdated Show resolved Hide resolved
@bonclay7
Copy link
Member

thanks let me re-run the tests

Copy link
Member

@bonclay7 bonclay7 left a comment

Choose a reason for hiding this comment

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

Nice job! few minor changes, and we'll be able to ship

modules/eks-monitoring/.variables.tf.swp Outdated Show resolved Hide resolved
docs/eks/istio.md Outdated Show resolved Hide resolved
examples/eks-istio/README.md Show resolved Hide resolved
docs/eks/istio.md Outdated Show resolved Hide resolved
@bonclay7 bonclay7 self-requested a review July 26, 2023 14:03
docs/eks/istio.md Outdated Show resolved Hide resolved
docs/eks/istio.md Outdated Show resolved Hide resolved
Added istioctl prereq, updated dashboards image
Updated Istio Bookinfo sample app instructions, included clean up of Bookinfo resources
@bonclay7 bonclay7 self-requested a review July 27, 2023 08:31
Copy link
Member

@bonclay7 bonclay7 left a comment

Choose a reason for hiding this comment

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

@dms486 thanks for your patience on this. Please address my new comments, run pre-commit and we'll get this merged today

docs/images/istio-dashboards.png Outdated Show resolved Hide resolved
docs/eks/istio.md Outdated Show resolved Hide resolved
-Switching image to Github generated link
-Removing advanced configuration section
@bonclay7 bonclay7 self-requested a review July 27, 2023 16:43
Copy link
Member

@bonclay7 bonclay7 left a comment

Choose a reason for hiding this comment

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

Thanks a LOT!

@bonclay7 bonclay7 merged commit 1ceecb3 into aws-observability:main Jul 27, 2023
21 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.

4 participants