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

prometheus: keep all grafana functions under grafana flag #79

Merged

Conversation

maryamtahhan
Copy link
Contributor

@maryamtahhan maryamtahhan commented Apr 29, 2024

Tested with local_dev_cluster using:

export PROMETHEUS_ENABLE=true
export GRAFANA_ENABLE=true
./main.sh prerequisites
./main.sh up

also tested with kepler by patching the local copy of the repo and running

make OPTS="PROMETHEUS_DEPLOY" cluster-up cluster-deploy

@maryamtahhan
Copy link
Contributor Author

It looks like the unit tests are all passing. so I'm marking this ready for review

@SamYuan1990 I think this addresses your concerns from #77

main.sh Outdated
return 0
}

GOOS="$(go env GOOS)"
Copy link
Contributor

Choose a reason for hiding this comment

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

golang is not pre installed in this repo, hence when integration on a new created ec2 instance, this will fail. Is there any possible to use uname to replace goarch? or install yq without golang/ or jq?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah - I can get it to install without a golang dep - let me update

Copy link
Contributor

@SamYuan1990 SamYuan1990 left a comment

Choose a reason for hiding this comment

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

https://github.com/mikefarah/yq?tab=readme-ov-file#wget maybe we can use curl or wget instead of golang? and add wget/curl with just apt/yum.

@maryamtahhan maryamtahhan force-pushed the hotfix-grafana branch 2 times, most recently from 0b8450c to b477b84 Compare April 29, 2024 15:43
@maryamtahhan
Copy link
Contributor Author

maryamtahhan commented Apr 29, 2024

Still updating this - will notify you when it's ready...

[UPDATE] - works in local_dev_cluster
for Kepler make OPTS="PROMETHEUS_DEPLOY" cluster-up cluster-deploy I need to update the tooling (tools.sh) in kepler to install yq, else the prometheus.sh script in local_dev_cluster will just skip the kepler-grafana-dashboard configuration part if yq isn't available (which is good, as it shows the functionality works and won't break ci if there's no yq :) )--> will post a PR to the kepler repo shortly - I will also add a curl command to vefify dashboard installation as part as the kepler verify.sh step as part of this new kepler PR

from local_dev_cluster POV - this is ready

Copy link
Contributor

@SamYuan1990 SamYuan1990 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 @maryamtahhan please create another PR, at https://github.com/sustainable-computing-io/local-dev-cluster/blob/main/main.sh#L125 changing the output message as hit to user and document.

@SamYuan1990 SamYuan1990 merged commit a68f078 into sustainable-computing-io:main May 1, 2024
10 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.

3 participants