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.

Sign up for GitHub

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

Jump to bottom

Update prometheus and grafana #1850

Merged
merged 2 commits into from
Oct 20, 2020
Merged

Update prometheus and grafana #1850

merged 2 commits into from
Oct 20, 2020

Conversation

8398a7
Copy link
Contributor

@8398a7 8398a7 commented Oct 18, 2020

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:

/kind breaking
/kind bug
/kind cleanup

/kind documentation

/kind feature
/kind hotfix

What this PR does / Why we need it:

This is because the prometheus and grafana charts are deprecated.

Which issue(s) this PR fixes:

Closes #1829

Special notes for your reviewer:

I used the agones chart in 1.9.0 to verify this.
Existing grafana dashboards also seem to work fine.

@google-cla google-cla bot added the cla: yes label Oct 18, 2020
8398a7
8398a7 commented Oct 18, 2020
View reviewed changes
build/grafana.yaml Outdated
@@ -37,6 +37,6 @@ datasources:
datasources:
- name: Prometheus
type: prometheus
url: http://prom-prometheus-server.metrics.svc.cluster.local:80/
url: http://prometheus-server.metrics.svc.cluster.local:80/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the effect of changing the name of the installation from prom to prometheus.

If there is no reason, I think the same as the chart name is good.
It is because the name can be shortened.

In the case of grafana, it's the same as the chart name.

Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit hesitant to change this name, because of backward compatibility.

If people are pulling this file down from release branches as part of their install process, this could break things for them, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's certainly going to be a problem in that case.
I'll change it to the original release name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your comment. Fixed.

8398a7
8398a7 commented Oct 18, 2020
View reviewed changes
site/content/en/docs/Guides/metrics.md
helm repo add grafana https://grafana.github.io/helm-charts
helm repo update

helm upgrade --install --wait grafana grafana/grafana --version=5.7.10 --namespace metrics \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The same commands can be used to install and upgrade, so it has been modified to use upgrade.

8398a7
8398a7 commented Oct 18, 2020
View reviewed changes
site/content/en/docs/Guides/metrics.md Outdated
helm repo add prometheus-community https://prometheus-community.github.io/helm-charts
helm repo update

helm upgrade --install --wait prometheus prometheus-community/prometheus --version 11.16.2 --namespace metrics \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The version was fixed in grafana, so I fixed this one too.

@8398a7
Copy link
Contributor Author

8398a7 commented Oct 18, 2020

Just out of curiosity, won't grafana and prometheus be tied to the agones chart?

...

grafana:
  enabled: true

  service:
    port: 3000
...
prometheus:
  enabled: true

  alertmanager:
    enabled: false
...

It's as if you can disable it by using enabled.
It would be easier to deploy if you could also include the dashboard configmap from the agones chart.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: ce37ee9c-d7bd-4c1c-b541-62cd8e0dc519

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/googleforgames/agones.git pull/1850/head:pr_1850 && git checkout pr_1850
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.10.0-0bdbbf7

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: ba225989-e2c7-4186-9881-71b5657e455e

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/googleforgames/agones.git pull/1850/head:pr_1850 && git checkout pr_1850
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.10.0-6edd7f6

markmandel
markmandel approved these changes Oct 20, 2020
View reviewed changes
Copy link
Member

@markmandel markmandel left a comment

Choose a reason for hiding this comment

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

Just tested! Working great!

@markmandel markmandel added this to the 1.10.0 milestone Oct 20, 2020
@google-oss-robot
Copy link

New changes are detected. LGTM label has been removed.

@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 8398a7, markmandel

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

1 similar comment
@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 8398a7, markmandel

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@markmandel markmandel added kind/cleanup Refactoring code, fixing up documentation, etc kind/documentation Documentation for Agones and removed approved labels Oct 20, 2020
markmandel added a commit to markmandel/agones that referenced this pull request Oct 20, 2020
@markmandel
Build Tools: Update Prometheus and Grafana
2726954 
Updated the install instructions for our dev tooling to match PR googleforgames#1850
@markmandel markmandel mentioned this pull request Oct 20, 2020
Merged
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 6beb140a-0126-4199-a1cc-d1a7d74c1854

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/googleforgames/agones.git pull/1850/head:pr_1850 && git checkout pr_1850
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.10.0-8696e11

markmandel added a commit to markmandel/agones that referenced this pull request Oct 20, 2020
@markmandel
Build Tools: Update Prometheus and Grafana
62387de 
Updated the install instructions for our dev tooling to match PR googleforgames#1850
@markmandel markmandel merged commit 58472aa into googleforgames:master Oct 20, 2020
@8398a7 8398a7 deleted the updatePrometheusAndGrafana branch October 21, 2020 02:10
roberthbailey added a commit that referenced this pull request Oct 21, 2020
@markmandel @roberthbailey
Build Tools: Update Prometheus and Grafana (#1857)
8872716 
Updated the install instructions for our dev tooling to match PR #1850

Co-authored-by: Robert Bailey <robertbailey@google.com>
ilkercelikyilmaz pushed a commit to ilkercelikyilmaz/agones that referenced this pull request Oct 23, 2020
@8398a7 @markmandel @ilkercelikyilmaz
Docs: Update prometheus and grafana chart (googleforgames#1829) (goog…
b3748f5 
…leforgames#1850)

Co-authored-by: Mark Mandel <markmandel@google.com>
ilkercelikyilmaz pushed a commit to ilkercelikyilmaz/agones that referenced this pull request Oct 23, 2020
@markmandel @roberthbailey @ilkercelikyilmaz
Build Tools: Update Prometheus and Grafana (googleforgames#1857)
6ad4026 
Updated the install instructions for our dev tooling to match PR googleforgames#1850

Co-authored-by: Robert Bailey <robertbailey@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markmandel markmandel markmandel approved these changes

@aLekSer aLekSer Awaiting requested review from aLekSer

@pooneh-m pooneh-m Awaiting requested review from pooneh-m

Assignees

@markmandel markmandel

Labels
cla: yes kind/cleanup Refactoring code, fixing up documentation, etc kind/documentation Documentation for Agones size/S
Projects
None yet
Milestone
1.10.0
Development

Successfully merging this pull request may close these issues.

Metrics: link to helm repository is deprecated, install command as well
4 participants
@8398a7 @agones-bot @google-oss-robot @markmandel