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

Add event in webhook #2305

Merged
merged 17 commits into from
Apr 27, 2020
Merged

Add event in webhook #2305

merged 17 commits into from
Apr 27, 2020

Conversation

Yisaer
Copy link
Contributor

@Yisaer Yisaer commented Apr 26, 2020

What problem does this PR solve?

Add event in webhook before the pod is admitted to delete. It would help us know the tidbcluster running process when pod admissionwebhook is enabled.

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation

Does this PR introduce a user-facing change?:

Add useful `Event` in `TidbCluster` during upgrading and scaling when `admissionWebhook.validation.pods` in operator configuration is enabled.

pkg/webhook/admission_hooks.go Outdated Show resolved Hide resolved
pkg/webhook/pod/pods.go Outdated Show resolved Hide resolved
Song Gao and others added 3 commits April 27, 2020 11:16
Co-Authored-By: weekface <weekface@gmail.com>
@Yisaer Yisaer requested a review from weekface April 27, 2020 03:19
weekface
weekface previously approved these changes Apr 27, 2020
pkg/webhook/pod/pods.go Outdated Show resolved Hide resolved
}

const (
stsControllerServiceAccounts = "system:serviceaccount:kube-system:statefulset-controller"
podDeleteMsgPattern = "pod[%s] deleted"
Copy link
Contributor

Choose a reason for hiding this comment

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

Add namespace?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the namespace here is not necessary as the event belongs to a certain tidbcluster.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then add tc info?
Is the tc info already shown in the event?

pkg/webhook/pod/tikv_deleter.go Outdated Show resolved Hide resolved
pkg/webhook/pod/pods.go Outdated Show resolved Hide resolved
Co-Authored-By: DanielZhangQD <36026334+DanielZhangQD@users.noreply.github.com>
Co-Authored-By: DanielZhangQD <36026334+DanielZhangQD@users.noreply.github.com>
@Yisaer Yisaer requested a review from DanielZhangQD April 27, 2020 06:25
Copy link
Contributor

@DanielZhangQD DanielZhangQD left a comment

Choose a reason for hiding this comment

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

LGTM

@Yisaer
Copy link
Contributor Author

Yisaer commented Apr 27, 2020

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Apr 27, 2020

Your auto merge job has been accepted, waiting for:

  • 2308

@Yisaer Yisaer merged commit 3a91c8e into pingcap:master Apr 27, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Apr 27, 2020

cherry pick to release-1.1 in PR #2311

sre-bot added a commit that referenced this pull request Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants