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

roachprod: delete cluster config on GC #124604

Merged

Conversation

nameisbhaskar
Copy link
Contributor

@nameisbhaskar nameisbhaskar commented May 23, 2024

currently, prometheus cluster configs are not deleted on gc. This makes stale entries to remain. This PR deletes the cluster configs on GC.

There is another minor fix for promhelpers access. The access to cloud storage was failing due to missing credentials.

Fixes: #124599
Epic: none

@nameisbhaskar nameisbhaskar requested a review from a team as a code owner May 23, 2024 08:30
@nameisbhaskar nameisbhaskar requested review from DarrylWong and vidit-bhat and removed request for a team May 23, 2024 08:30
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@nameisbhaskar nameisbhaskar force-pushed the user/bhaskar/delete-cc-on-gc branch 2 times, most recently from c063079 to df671c9 Compare May 28, 2024 08:46
Copy link
Contributor

@DarrylWong DarrylWong left a comment

Choose a reason for hiding this comment

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

Nice! Left two non blocking nits and one small potential change but otherwise looks good. Have you been able to test this via roachprod gc?

pkg/roachprod/promhelperclient/promhelper_utils.go Outdated Show resolved Hide resolved
pkg/roachprod/roachprod.go Show resolved Hide resolved
pkg/roachprod/roachprod.go Outdated Show resolved Hide resolved
pkg/roachprod/cloud/gc.go Outdated Show resolved Hide resolved
@nameisbhaskar nameisbhaskar force-pushed the user/bhaskar/delete-cc-on-gc branch 3 times, most recently from bb82246 to 91074e1 Compare May 29, 2024 06:35
@nameisbhaskar nameisbhaskar requested a review from DarrylWong May 29, 2024 06:35
@nameisbhaskar nameisbhaskar force-pushed the user/bhaskar/delete-cc-on-gc branch from 91074e1 to a57f692 Compare May 29, 2024 06:37
Copy link
Collaborator

@herkolategan herkolategan left a comment

Choose a reason for hiding this comment

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

Raised one concern about the default project and destroy cluster, but happy to approve once that is confirmed to not be problematic.

pkg/roachprod/promhelperclient/promhelper_utils.go Outdated Show resolved Hide resolved
pkg/roachprod/cloud/cluster_cloud.go Outdated Show resolved Hide resolved
@nameisbhaskar
Copy link
Contributor Author

nameisbhaskar commented May 29, 2024

Nice! Left two non blocking nits and one small potential change but otherwise looks good. Have you been able to test this via roachprod gc?

yes, I have tested this! I created a cluster with --lifetime=1m and ran roachprod gc

currently, prometheus cluster configs are not deleted on
gc. This makes stale entries to remain. This PR deletes the
cluster configs on GC.

There is another minor fix for promhelpers access. The access
to cloud storage was failing due to missing credentials.

Fixes: cockroachdb#124599
Epic: none
@nameisbhaskar nameisbhaskar force-pushed the user/bhaskar/delete-cc-on-gc branch from a57f692 to e4cb5c1 Compare May 29, 2024 11:27
@nameisbhaskar
Copy link
Contributor Author

Thanks @DarrylWong and @herkolategan for your comments. I have resolved all. Please do take a look!

Copy link
Contributor

@DarrylWong DarrylWong 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 fixing all the comments! LGTM

Copy link
Collaborator

@herkolategan herkolategan 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 updating, LGTM!

@nameisbhaskar
Copy link
Contributor Author

bors r=@DarrylWong @herkolategan

@craig craig bot merged commit 99c0d27 into cockroachdb:master May 30, 2024
22 checks passed
@nameisbhaskar nameisbhaskar deleted the user/bhaskar/delete-cc-on-gc branch June 2, 2024 16:22
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.

roachprod: clean cluster config on GC
4 participants