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

updated probe handler types descriptions #106285

Merged
merged 2 commits into from
Nov 15, 2021
Merged

updated probe handler types descriptions #106285

merged 2 commits into from
Nov 15, 2021

Conversation

SergeyKanzhelev
Copy link
Member

Follow up of #105979

What type of PR is this?

Comments added may not be needed as the consistency long term will be guaranteed by back compatibility requirement.

Also removed todo and false statements.

/kind cleanup
/sig node
/priority backlog
/label api-review

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. sig/node Categorizes an issue or PR as relevant to SIG Node. priority/backlog Higher priority than priority/awaiting-more-evidence. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. api-review Categorizes an issue or PR as actively needing an API review. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Nov 10, 2021
@SergeyKanzhelev SergeyKanzhelev mentioned this pull request Nov 10, 2021
Merged
@SergeyKanzhelev
Copy link
Member Author

cc: @thockin

@k8s-ci-robot k8s-ci-robot added the sig/apps Categorizes an issue or PR as relevant to SIG Apps. label Nov 10, 2021
@wgahnagl wgahnagl added this to Triage in SIG Node PR Triage Nov 10, 2021
@wgahnagl
Copy link
Contributor

/assign @ehashman
/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Nov 10, 2021
@wgahnagl wgahnagl moved this from Triage to Needs Reviewer in SIG Node PR Triage Nov 10, 2021
@SergeyKanzhelev
Copy link
Member Author

/retest

SergeyKanzhelev
SergeyKanzhelev commented Nov 10, 2021
View reviewed changes
Copy link
Member Author

@SergeyKanzhelev SergeyKanzhelev left a comment

Choose a reason for hiding this comment

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

added comments for reviewers

pkg/apis/core/types.go Outdated Show resolved Hide resolved
pkg/apis/core/types.go Show resolved Hide resolved
pkg/apis/core/types.go Show resolved Hide resolved
pkg/apis/core/types.go
@@ -2235,9 +2229,8 @@ type Lifecycle struct {
// PreStop is called immediately before a container is terminated due to an
// API request or management event such as liveness/startup probe failure,
// preemption, resource contention, etc. The handler is not called if the
// container crashes or exits. The reason for termination is passed to the
Copy link
Member Author

Choose a reason for hiding this comment

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

The reason for termination is passed to the handler.

I don't know what this is referring to. I didn't find evidence that this is true or was true at some point

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixes #99250.

@liggitt liggitt added this to Assigned in API Reviews Nov 11, 2021
@liggitt liggitt added this to the v1.23 milestone Nov 11, 2021
thockin
thockin reviewed Nov 11, 2021
View reviewed changes
Copy link
Member

@thockin thockin left a comment

Choose a reason for hiding this comment

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

LGTM overall

pkg/apis/core/types.go Outdated Show resolved Hide resolved
pkg/apis/core/types.go Outdated Show resolved Hide resolved
pkg/apis/core/types.go Show resolved Hide resolved
pkg/apis/core/types.go Outdated Show resolved Hide resolved
@SergeyKanzhelev SergeyKanzhelev mentioned this pull request Nov 14, 2021
Closed
@SataQiu SataQiu mentioned this pull request Nov 15, 2021
Merged
thockin
thockin reviewed Nov 15, 2021
View reviewed changes
Copy link
Member

@thockin thockin left a comment

Choose a reason for hiding this comment

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

Thanks!

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 15, 2021
@thockin thockin removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 15, 2021
@thockin
Copy link
Member

thockin commented Nov 15, 2021

Removed LGTM

Is the goal to make these changes user-visible? The equivalent changes need to be made in staging/src/k8s.io/api/... and then a million hack/update-* need to run.

@SergeyKanzhelev
Copy link
Member Author

Removed LGTM

Is the goal to make these changes user-visible? The equivalent changes need to be made in staging/src/k8s.io/api/... and then a million hack/update-* need to run.

Yes, I was about to send another PR =). Updating this one

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 15, 2021
@thockin
Copy link
Member

thockin commented Nov 15, 2021

Thanks!

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 15, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SergeyKanzhelev, thockin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 058ce89 into kubernetes:master Nov 15, 2021
SIG Node PR Triage automation moved this from Needs Reviewer to Done Nov 15, 2021
@liggitt liggitt moved this from Assigned to API review completed, 1.23 in API Reviews Dec 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cbandy cbandy cbandy left review comments

@thockin thockin thockin left review comments

@caesarxuchao caesarxuchao Awaiting requested review from caesarxuchao

@piosz piosz Awaiting requested review from piosz

Assignees

@ehashman ehashman

@thockin thockin

Labels
api-review Categorizes an issue or PR as actively needing an API review. approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/backlog Higher priority than priority/awaiting-more-evidence. release-note-none Denotes a PR that doesn't merit a release note. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/node Categorizes an issue or PR as relevant to SIG Node. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
API Reviews
Status: API review completed, 1.23
SIG Node PR Triage
Archived in project
SIG Node PR Triage
Done
Milestone
v1.23
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@SergeyKanzhelev @wgahnagl @thockin @k8s-ci-robot @cbandy @liggitt @ehashman