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 prepare shutdown #9175

Merged
merged 19 commits into from
Apr 19, 2023
Merged

Update prepare shutdown #9175

merged 19 commits into from
Apr 19, 2023

Conversation

MichelHollands
Copy link
Contributor

@MichelHollands MichelHollands commented Apr 18, 2023 •
edited
Loading

What this PR does / why we need it:

This updates the PrepareShutdown method so it supports GET and DELETE methods as well. This makes it similar to Mimir: grafana/mimir#4718.
The status is now stored in a local file. A new config setting had to be added for this file as there is no obvious place to store it.

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • CHANGELOG.md updated
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/upgrading/_index.md

@MichelHollands
Update PrepareShutdown endpoint
b56b03a 
Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
@MichelHollands
Fix duplicate import
8a55f04 
Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
@MichelHollands
Add unit tests
e37ad64 
Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
@MichelHollands
Add updated docs
d50c499 
Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
@github-actions github-actions bot added the type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories label Apr 18, 2023
MichelHollands and others added 5 commits April 18, 2023 11:31
@MichelHollands
Merge branch 'main' into update_prepare_shutdown
6905f70
@MichelHollands
Update changelog
4f716f4 
Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
@MichelHollands
Fix existing unit tests
3ba95cf 
Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
@MichelHollands
Use common path prefix
f199dbe 
Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
@MichelHollands
Merge branch 'main' into update_prepare_shutdown
07a7fac
@MichelHollands
Add upgrading instructions
01c075c 
Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
@MichelHollands
Update endpoint documentation
542ff35 
Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
@MichelHollands MichelHollands marked this pull request as ready for review April 18, 2023 13:28
@MichelHollands MichelHollands requested review from JStickler and a team as code owners April 18, 2023 13:28
JStickler
JStickler approved these changes Apr 18, 2023
View reviewed changes
Copy link
Contributor

@JStickler JStickler left a comment

Choose a reason for hiding this comment

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

[docs team] LGTM

DylanGuedes
DylanGuedes requested changes Apr 18, 2023
View reviewed changes
Copy link
Contributor

@DylanGuedes DylanGuedes left a comment

Choose a reason for hiding this comment

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

looks pretty solid, gj. I'm blocking the PR because of the flag suggestion (from using underscore to dashes) because if you merge this way and we decide to change it back it would be a breaking change.

pkg/ingester/ingester.go Outdated Show resolved Hide resolved
docs/sources/configuration/_index.md Outdated Show resolved Hide resolved
docs/sources/configuration/_index.md Outdated Show resolved Hide resolved
pkg/ingester/ingester.go Show resolved Hide resolved
JordanRushing
JordanRushing approved these changes Apr 18, 2023
View reviewed changes
Copy link
Contributor

@JordanRushing JordanRushing left a comment

Choose a reason for hiding this comment

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

LGTM!

MichelHollands and others added 6 commits April 19, 2023 09:06
@MichelHollands @DylanGuedes
Update pkg/ingester/ingester.go
b22f8c2 
Co-authored-by: Dylan Guedes <djmgguedes@gmail.com>
@MichelHollands @DylanGuedes
Update docs/sources/configuration/_index.md
4bddd33 
Co-authored-by: Dylan Guedes <djmgguedes@gmail.com>
@MichelHollands
Elaborate description of shutdown-marker-path if common.path_prefix i…
f9252e7 
…s set

Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
@MichelHollands
Merge branch 'main' into update_prepare_shutdown
31c1991
@MichelHollands
Fix endpoint name in changelog
09689bf 
Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
@MichelHollands
Merge branch 'main' into update_prepare_shutdown
64a1f8f
DylanGuedes
DylanGuedes requested changes Apr 19, 2023
View reviewed changes
Copy link
Contributor

@DylanGuedes DylanGuedes left a comment

Choose a reason for hiding this comment

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

one more 😄

pkg/loki/modules.go Show resolved Hide resolved
DylanGuedes
DylanGuedes requested changes Apr 19, 2023
View reviewed changes
pkg/loki/modules.go Show resolved Hide resolved
@MichelHollands
Do not fail startup when ShutDownMarkerPath is not set
7822bde 
Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
DylanGuedes
DylanGuedes previously approved these changes Apr 19, 2023
View reviewed changes
Copy link
Contributor

@DylanGuedes DylanGuedes left a comment

Choose a reason for hiding this comment

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

lgtm

@DylanGuedes DylanGuedes dismissed their stale review April 19, 2023 14:13

dismissing the review since the ci is broken

@MichelHollands
Add new behaviour to upgrading doc
6fe0199 
Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
@MichelHollands MichelHollands merged commit abd6131 into grafana:main Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JordanRushing JordanRushing JordanRushing approved these changes

@DylanGuedes DylanGuedes DylanGuedes left review comments

@JStickler JStickler JStickler approved these changes

Assignees
No one assigned
Labels
size/L type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@MichelHollands @JordanRushing @JStickler @DylanGuedes