fix some orphan pods cleaner bugs #878
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cofyc
commented
Sep 5, 2019
|
|
||
| // Start informer factories after all controller are initializated. | ||
| informerFactory.Start(controllerCtx.Done()) | ||
| kubeInformerFactory.Start(controllerCtx.Done()) |
There was a problem hiding this comment.
Start will start a new goroutine, so no need to use go ... here.
cofyc
commented
Sep 5, 2019
weekface
reviewed
Sep 5, 2019
| @@ -89,6 +118,9 @@ func (opc *orphanPodsCleaner) Clean(tc *v1alpha1.TidbCluster) (map[string]string | |||
| return skipReason, err | |||
| } | |||
|
|
|||
| // if the PVC is not found in apiserver (also informer cache) and the | |||
| // phrase of the Pod is Pending, delete it and let the stateful | |||
| // controller to create the pod and its PVC(s) again | |||
| err = opc.podControl.DeletePod(tc, pod) | |||
There was a problem hiding this comment.
only delete pod if the pod status is Pending
There was a problem hiding this comment.
sorry, I forgot it...added in 9791178
|
/run-e2e-in-kind |
onlymellb
reviewed
Sep 5, 2019
onlymellb
reviewed
Sep 5, 2019
tennix
reviewed
Sep 5, 2019
tennix
reviewed
Sep 5, 2019
weekface
previously approved these changes
Sep 5, 2019
|
/run-e2e-in-kind |
onlymellb
reviewed
Sep 5, 2019
| // if the PVC is not found in apiserver (also informer cache) and the | ||
| // phase of the Pod is Pending, delete it and let the stateful | ||
| // controller to create the pod and its PVC(s) again | ||
| apiPod, err := opc.kubeCli.CoreV1().Pods(pod.Namespace).Get(pod.Name, metav1.GetOptions{}) |
There was a problem hiding this comment.
Use ns and podName instead of pod.Namespace and pod.Name
|
/run-e2e-in-kind |
cofyc
commented
Sep 5, 2019
| return skipReason, err | ||
| } | ||
| // try our best to avoid deleting wrong object in apiserver | ||
| // TODO upgrade to use deleteOption.Preconditions.ResourceVersion in client-go 1.14+ |
There was a problem hiding this comment.
|
/run-e2e-tests |
weekface
approved these changes
Sep 5, 2019
|
/run-e2e-in-kind |
|
/run-e2e-tests |
|
/run-e2e-in-kind |
|
/run-e2e-in-kind |
|
/run-e2e-tests |
1 similar comment
|
/run-e2e-tests |
|
cherry pick to release-1.0 failed |
cofyc
added a commit
to cofyc/tidb-operator
that referenced
this pull request
Sep 6, 2019
* fix some orphan pods cleaner bugs * pod pending check * reword * fix typo * fast path * double check pod in apiserver
cofyc
added a commit
to cofyc/tidb-operator
that referenced
this pull request
Sep 6, 2019
* fix some orphan pods cleaner bugs * pod pending check * reword * fix typo * fast path * double check pod in apiserver
cofyc
added a commit
that referenced
this pull request
Sep 6, 2019
tennix
pushed a commit
that referenced
this pull request
Sep 6, 2019
yahonda
pushed a commit
that referenced
this pull request
Dec 27, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
What is changed and how does it work?
Check List
Tests
Code changes
Side effects
Related changes
Does this PR introduce a user-facing change?: