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 filter to remove deleted pods #153

Merged
merged 4 commits into from
Oct 24, 2019
Merged

Conversation

pims
Copy link
Contributor

@pims pims commented Oct 22, 2019

The filterByPhase function:

// filterByPhase filters a list of pods by a given PodPhase, e.g. Running.
func filterByPhase(pods []v1.Pod, phase v1.PodPhase) []v1.Pod {
	filteredList := []v1.Pod{}

	for _, pod := range pods {
		if pod.Status.Phase == phase {
			filteredList = append(filteredList, pod)
		}
	}

	return filteredList
}

does not filter out lingering pods in "terminating state", which is not a real PodPhase, but a state computed from the presence of a DeletionTimestamp: https://sourcegraph.com/github.com/kubernetes/kubernetes@v1.12.7/-/blob/pkg/printers/internalversion/printers.go#L640-642

Any lingering pod in this state keeps on being selected by ChaosKube for termination.
If it is fair to assume that filtering by phase was meant to only select active pods and that deleted pods should not be included, this PR should address the issue.

@linki
Copy link
Owner

linki commented Oct 23, 2019

@pims It certainly was. Thank you!

I'm curious as to what the pod phase is in such cases, still "Running"?

@@ -368,6 +369,18 @@ func filterByPhase(pods []v1.Pod, phase v1.PodPhase) []v1.Pod {
return filteredList
}

// filterDeletedPods removes pod which have a non nil DeletionTimestamp
func filterDeletedPods(pods []v1.Pod) []v1.Pod {
Copy link
Owner

Choose a reason for hiding this comment

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

Let's rename this to filterTerminatingPods.

filtered := filterDeletedPods(pods)
suite.Equal(len(filtered), 1)
suite.Equal(pods[0].Name, "running")
}
Copy link
Owner

Choose a reason for hiding this comment

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

👍

@pims
Copy link
Contributor Author

pims commented Oct 23, 2019

@linki yes, in this case the pode phase is still running.

@linki linki force-pushed the filter-terminating-pods branch from 67ac04f to 2e65fa5 Compare October 24, 2019 09:39
@linki linki merged commit 037d834 into linki:master Oct 24, 2019
@linki
Copy link
Owner

linki commented Oct 24, 2019

@pims Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants