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 monitoring to GCP #219

Merged
merged 1 commit into from
Oct 2, 2019
Merged

Add monitoring to GCP #219

merged 1 commit into from
Oct 2, 2019

Conversation

juadk
Copy link
Contributor

@juadk juadk commented Oct 1, 2019

Enable monitoring for GCP with an option to disable it

GCP documentation needs to be rework as is in AWS or Azure (future PR).

@juadk juadk added the WIP Work in Progress label Oct 1, 2019
@MalloZup MalloZup mentioned this pull request Oct 1, 2019
14 tasks
@juadk juadk removed the WIP Work in Progress label Oct 1, 2019
Copy link
Contributor

@MalloZup MalloZup left a comment

Choose a reason for hiding this comment

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

Looks good to me nice pr 🐧🐧

Copy link
Member

@ldevulder ldevulder left a comment

Choose a reason for hiding this comment

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

2 small comments, otherwise LGTM!

Tested!

gcp/outputs.tf Outdated Show resolved Hide resolved
gcp/outputs.tf Show resolved Hide resolved
Copy link
Collaborator

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

@juadk Great! Just a comment about the disk creation, I think we could avoid it is the monitoring stack is not needed. What do you think?

gcp/disks.tf Outdated Show resolved Hide resolved
@juadk juadk requested a review from arbulu89 October 2, 2019 08:23
Copy link
Collaborator

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

Without testing personally the changes it looks good and ready to be merged! Thanks @juadk

@juadk
Copy link
Contributor Author

juadk commented Oct 2, 2019

Without testing personally the changes it looks good and ready to be merged! Thanks @juadk

Thanks @arbulu89 , I've tested it quickly with and without the monitoring part and it worked well.

@MalloZup MalloZup merged commit 11a61de into SUSE:master Oct 2, 2019
@juadk juadk deleted the feature/gcp_monitoring branch October 24, 2019 12:29
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