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

Re-arrange directory structure and docs for GKE to support multiple Providers #425

Merged
merged 5 commits into from
Aug 19, 2020
Merged

Re-arrange directory structure and docs for GKE to support multiple Providers #425

merged 5 commits into from
Aug 19, 2020

Conversation

weastel
Copy link
Contributor

@weastel weastel commented Aug 6, 2020 •
edited
Loading

Signed-off-by: Drumil Patel [email protected]

This pull request introduces the following changes :

  • Move PROJECT_ID to GKE_PROJECT_ID.
  • Filename changes to make it more specific to gke(i.e cluster.yaml => cluster_gke.yaml, nodepools.yaml =>nodes_gke.yaml)
  • nodepools to nodes in \infra to make it more general.
  • Add separate documentation for gke(prombench/docs/gke.md)
  • Make prombench documentation more general

@weastel
Add gke specific changes
66355db 
Signed-off-by: Drumil Patel <[email protected]>
geekodour
geekodour reviewed Aug 7, 2020
View reviewed changes
Copy link
Contributor

@geekodour geekodour left a comment

Choose a reason for hiding this comment

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

Nice! This will make the #384 easier to follow. :)

Added a few small suggestions. Once this merged we must also make changes to the workflow files at : https://github.com/prometheus/prometheus/tree/master/.github/workflows

prombench/manifests/prombench/nodepools.yaml Show resolved Hide resolved
prombench/Makefile Outdated Show resolved Hide resolved
funcbench/manifests/cluster.yaml Show resolved Hide resolved
@geekodour
Copy link
Contributor

Another thing I would expect to be in this PR is the creation of docs directory and moving the GKE specific docs to /docs/gke.md, link to it from the prombench README. So the /prometheus/README.md will become significantly smaller.

weastel
weastel commented Aug 14, 2020
View reviewed changes
prombench/README.md Outdated Show resolved Hide resolved
@weastel weastel requested a review from geekodour August 14, 2020 22:17
geekodour
geekodour reviewed Aug 15, 2020
View reviewed changes
Copy link
Contributor

@geekodour geekodour left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for adding the changes and the description changes :) ✨

Added some suggestion, next thing to do with this PR is to update the commands in the funcbench Makefile as it also infra cli.

prombench/README.md Outdated Show resolved Hide resolved
prombench/README.md Show resolved Hide resolved
@geekodour geekodour changed the title Add gke specific changes for #384 Re-arrange directory structure and docs for GKE to support multiple Providers Aug 15, 2020
@weastel weastel requested a review from geekodour August 15, 2020 16:27
@weastel
Add PROVIDER variable in funcbench and prombench make file, change do…
71b17c9 
…cumentation to make it more general

Signed-off-by: Drumil Patel <[email protected]>
@weastel weastel requested a review from geekodour August 15, 2020 17:25
@weastel weastel mentioned this pull request Aug 15, 2020
Merged
geekodour
geekodour reviewed Aug 15, 2020
View reviewed changes
Copy link
Contributor

@geekodour geekodour left a comment

Choose a reason for hiding this comment

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

small doc changes, I tested this. will merge once changes are addressed :)

prombench/docs/gke.md Outdated Show resolved Hide resolved
prombench/docs/gke.md Outdated Show resolved Hide resolved
prombench/docs/gke.md Outdated Show resolved Hide resolved
@weastel weastel requested a review from geekodour August 15, 2020 21:43
@geekodour
Copy link
Contributor

@krasi-georgiev can you take a final look before merging? :)

@geekodour
Copy link
Contributor

@weastel you should make a PR to update the prometheus/prometheus github action workflow files aswell that will be affected by changes in this PR.

@weastel weastel mentioned this pull request Aug 16, 2020
Merged
@geekodour geekodour merged commit c209418 into prometheus:master Aug 19, 2020
@geekodour
Copy link
Contributor

@krasi-georgiev I merged this as it was blocking few other PRs. Please still take a look at it, if we need to change anything back.

@weastel thanks for this! :)

@krasi-georgiev
Copy link
Contributor

no problem :)

kushalShukla-web pushed a commit to kushalShukla-web/test-infra that referenced this pull request Feb 8, 2025
@weastel
Re-arrange directory structure and docs for GKE to support multiple p…
0706933 
…roviders (prometheus#425)

* Add documentation for GKE and update file names in both prombench and funcbench
* Add and update variables in funcbench and prombench Makefiles

Signed-off-by: Drumil Patel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geekodour geekodour Awaiting requested review from geekodour

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@weastel @geekodour @krasi-georgiev