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

feat!: Promote managed_prometheus to GA #1505

Merged

Conversation

IIBenII
Copy link
Contributor

@IIBenII IIBenII commented Dec 19, 2022

The managed_prometheus option is now GA since the version 4.46.0

@IIBenII IIBenII requested review from a team and Jberlinsky as code owners December 19, 2022 17:20
@IIBenII
Copy link
Contributor Author

IIBenII commented Dec 20, 2022

@Jberlinsky Can I have more detail about the failing tests? Thanks

@ericyz
Copy link
Collaborator

ericyz commented Dec 28, 2022

Triggering test again

@bharathkkb bharathkkb changed the title feat: Promote managed_prometheus to GA feat!: Promote managed_prometheus to GA Dec 28, 2022
@IIBenII
Copy link
Contributor Author

IIBenII commented Dec 29, 2022

@ericyz Tests still fail, can you give me more informations about the error? Sadly I can't run tests on my side due to organisation constraint about public cluster 😢

@ericyz
Copy link
Collaborator

ericyz commented Dec 29, 2022

@IIBenII - The test passed. Could you please resolve the conflict? Thanks

@IIBenII
Copy link
Contributor Author

IIBenII commented Dec 30, 2022

@ericyz done

autogen/main/main.tf.tmpl Outdated Show resolved Hide resolved
autogen/main/variables.tf.tmpl Show resolved Hide resolved
@IIBenII
Copy link
Contributor Author

IIBenII commented Jan 3, 2023

@ericyz Thanks for all your feedbacks. According to this documentation https://cloud.google.com/kubernetes-engine/docs/resources/autopilot-standard-feature-comparison I also updated how we handle logging/monitoring for autopilot clusters.

Copy link
Collaborator

@ericyz ericyz left a comment

Choose a reason for hiding this comment

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

Minor comments. LGTM overall, please address the comments and resolve the conflicts

autogen/main/main.tf.tmpl Outdated Show resolved Hide resolved
autogen/main/variables.tf.tmpl Show resolved Hide resolved
autogen/main/variables.tf.tmpl Outdated Show resolved Hide resolved
@IIBenII IIBenII force-pushed the promote_prometheus_ga branch 2 times, most recently from 5ed1ebe to a05430b Compare January 4, 2023 16:17
@IIBenII
Copy link
Contributor Author

IIBenII commented Jan 4, 2023

@ericyz I cleaned the code and took your feedbacks, thanks a lot

Copy link
Collaborator

@ericyz ericyz left a comment

Choose a reason for hiding this comment

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

Thanks for the updates. LGTM, with minor improvement for house-keeping formatting.

autogen/main/main.tf.tmpl Outdated Show resolved Hide resolved
autogen/main/cluster.tf.tmpl Show resolved Hide resolved
Copy link
Contributor Author

@IIBenII IIBenII left a comment

Choose a reason for hiding this comment

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

@ericyz updated

autogen/main/cluster.tf.tmpl Show resolved Hide resolved
@comment-bot-dev
Copy link

@IIBenII
Thanks for the PR! 🚀
✅ Lint checks have passed.

@ericyz
Copy link
Collaborator

ericyz commented Jan 6, 2023

Re-trigger the checks

Copy link
Collaborator

@ericyz ericyz left a comment

Choose a reason for hiding this comment

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

LGTM and will approve once all checks are passed

@ericyz ericyz self-requested a review January 6, 2023 01:05
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.

None yet

4 participants