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

Use annotations for the http port and path #52

Merged
merged 4 commits into from
Apr 25, 2023
Merged

Use annotations for the http port and path #52

merged 4 commits into from
Apr 25, 2023

Conversation

MichelHollands
Copy link
Contributor

@MichelHollands MichelHollands commented Apr 24, 2023 •
edited

Previously these used labels which cannot contain the / character.

@MichelHollands
Use annotations for the http port and path
0cce706 
Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
@MichelHollands
Fix messages
cfd9468 
Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
@MichelHollands MichelHollands marked this pull request as ready for review April 24, 2023 19:12
@MichelHollands MichelHollands requested a review from a team as a code owner April 24, 2023 19:12
aknuds1
aknuds1 requested changes Apr 25, 2023
View reviewed changes
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

I think there's a risk of dereferencing a nil pointer.

pkg/admission/prep_downscale.go Outdated Show resolved Hide resolved
@MichelHollands @aknuds1
Update pkg/admission/prep_downscale.go
39358a3 
Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>
aknuds1
aknuds1 approved these changes Apr 25, 2023
View reviewed changes
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

LGTM

aknuds1
aknuds1 requested changes Apr 25, 2023
View reviewed changes
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

Could you also add this PR to the following changelog entry, please?

* [ENHANCEMENT] Add an `/admission/prepare-downscale` mutating admission webhook that prepares pods for shutdown before downscaling. #47

@MichelHollands
Add this PR to existing changelog entry
7fbc54a 
Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
aknuds1
aknuds1 approved these changes Apr 25, 2023
View reviewed changes
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for applying my suggestions :)

@aknuds1
Copy link
Contributor

aknuds1 commented Apr 25, 2023 •
edited

I will merge after CI finishes, since @MichelHollands lacks the permission.

@aknuds1 aknuds1 merged commit 02ea947 into grafana:main Apr 25, 2023
4 checks passed
@MichelHollands MichelHollands deleted the use_annotations_for_downscale_path branch April 25, 2023 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aknuds1 aknuds1 aknuds1 approved these changes

@colega colega colega approved these changes

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

Successfully merging this pull request may close these issues.

None yet
3 participants
@MichelHollands @aknuds1 @colega