Skip to content

Commit

Permalink
refactor: Support empty selectors & arbitrary pods
Browse files Browse the repository at this point in the history
  • Loading branch information
@doronkg
doronkg committed Oct 26, 2024
1 parent 306dfab commit 63f6aab
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 28 deletions.
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ kor [subcommand] --help
| Hpas | HPAs not used in Deployments<br/> HPAs not used in StatefulSets | |
| CRDs | CRDs not used the cluster | |
| Pvs | PVs not bound to a PVC | |
| Pdbs | PDBs not used in Deployments<br/> PDBs not used in StatefulSets | |
| Pdbs | PDBs not used in Deployments / StatefulSets (templates) or in arbitrary Pods<br/>PDBs with empty selectors (match every pod) but no running pods in namespace | |
| Jobs | Jobs status is completed<br/> Jobs status is suspended<br/> Jobs failed with backoff limit exceeded (including indexed jobs) <br/> Jobs failed with dedaline exceeded | |
| ReplicaSets | replicaSets that specify replicas to 0 and has already completed it's work |
| DaemonSets | DaemonSets not scheduled on any nodes |
Expand Down
77 changes: 63 additions & 14 deletions pkg/kor/pdbs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,32 +53,67 @@ func processNamespacePdbs(clientset kubernetes.Interface, namespace string, filt
}

selector := pdb.Spec.Selector
var hasMatchingTemplates, hasMatchingWorkloads bool

// Validate empty selector
if selector == nil || len(selector.MatchLabels) == 0 {
reason := "Pdb has no selector"
unusedPdbs = append(unusedPdbs, ResourceInfo{Name: pdb.Name, Reason: reason})
continue
}
hasRunningPods, err := validateRunningPods(clientset, namespace)
if err != nil {
return nil, err
}

labelSelector, err := metav1.LabelSelectorAsSelector(selector)
if err != nil {
return nil, err
}
if !hasRunningPods {
reason := "Pdb matches every pod (empty selector) but 0 pods run"
unusedPdbs = append(unusedPdbs, ResourceInfo{Name: pdb.Name, Reason: reason})
}

isPdbUsed, err := hasMatchedResources(clientset, namespace, filterOpts, labelSelector)
if err != nil {
return nil, err
continue
} else {
hasMatchingTemplates, err = validateMatchingTemplates(clientset, namespace, selector)
if err != nil {
return nil, err
}

hasMatchingWorkloads, err = validateMatchingWorkloads(clientset, namespace, selector)
if err != nil {
return nil, err
}
}

if !isPdbUsed {
reason := "Pdb is not referencing any deployments or statefulsets"
if !hasMatchingTemplates && !hasMatchingWorkloads {
reason := "Pdb is not referencing any deployments, statefulsets or pods"
unusedPdbs = append(unusedPdbs, ResourceInfo{Name: pdb.Name, Reason: reason})
}
}

return unusedPdbs, nil
}

func hasMatchedResources(clientset kubernetes.Interface, namespace string, filterOpts *filters.Options, labelSelector labels.Selector) (bool, error) {
func validateRunningPods(clientset kubernetes.Interface, namespace string) (bool, error) {
pods, err := clientset.CoreV1().Pods(namespace).List(context.TODO(), metav1.ListOptions{
FieldSelector: "status.phase=Running",
})
if err != nil {
return false, err
}

// Field status.phase=Running can still reference Terminating pods
// Return true if at least one pod is running
for _, pod := range pods.Items {
if pod.ObjectMeta.DeletionTimestamp == nil {
return true, nil
}
}

return false, nil
}

func validateMatchingTemplates(clientset kubernetes.Interface, namespace string, selector *metav1.LabelSelector) (bool, error) {
labelSelector, err := metav1.LabelSelectorAsSelector(selector)
if err != nil {
return false, err
}

deployments, err := clientset.AppsV1().Deployments(namespace).List(context.TODO(), metav1.ListOptions{})
if err != nil {
return false, err
Expand Down Expand Up @@ -106,6 +141,20 @@ func hasMatchedResources(clientset kubernetes.Interface, namespace string, filte
return false, nil
}

func validateMatchingWorkloads(clientset kubernetes.Interface, namespace string, selector *metav1.LabelSelector) (bool, error) {
pods, err := clientset.CoreV1().Pods(namespace).List(context.TODO(), metav1.ListOptions{
LabelSelector: metav1.FormatLabelSelector(selector),
})
if err != nil {
return false, err
}
if len(pods.Items) > 0 {
return true, nil
}

return false, nil
}

func GetUnusedPdbs(filterOpts *filters.Options, clientset kubernetes.Interface, outputFormat string, opts common.Opts) (string, error) {
resources := make(map[string]map[string][]ResourceInfo)
for _, namespace := range filterOpts.Namespaces(clientset) {
Expand Down
74 changes: 61 additions & 13 deletions pkg/kor/pdbs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,34 @@ import (
"github.com/yonahd/kor/pkg/filters"
)

var testNamespace2 = "test-namespace2"

func createTestPdbs(t *testing.T) *fake.Clientset {
clientset := fake.NewSimpleClientset()
namespaces := []string{testNamespace, testNamespace2}
var err error

_, err := clientset.CoreV1().Namespaces().Create(context.TODO(), &corev1.Namespace{
ObjectMeta: v1.ObjectMeta{Name: testNamespace},
}, v1.CreateOptions{})
for _, ns := range namespaces {
_, err := clientset.CoreV1().Namespaces().Create(context.TODO(), &corev1.Namespace{
ObjectMeta: v1.ObjectMeta{Name: ns},
}, v1.CreateOptions{})

if err != nil {
t.Fatalf("Error creating namespace %s: %v", testNamespace, err)
if err != nil {
t.Fatalf("Error creating namespace %s: %v", ns, err)
}
}

appLabels1 := map[string]string{
"app": "my-app",
}

appLabels2 := map[string]string{
"unused-app": "my-unused-app",
}

// Empty selector
appLabels3 := map[string]string{}

pdb1 := CreateTestPdb(testNamespace, "test-pdb1", appLabels1, AppLabels)
_, err = clientset.PolicyV1().PodDisruptionBudgets(testNamespace).Create(context.TODO(), pdb1, v1.CreateOptions{})
if err != nil {
Expand All @@ -41,7 +54,8 @@ func createTestPdbs(t *testing.T) *fake.Clientset {
t.Fatalf("Error creating fake %s: %v", "Pdb", err)
}

pdb3 := CreateTestPdb(testNamespace, "test-pdb3", AppLabels, AppLabels)
// Unused PDB - no matching templates / workloads
pdb3 := CreateTestPdb(testNamespace, "test-pdb3", appLabels2, AppLabels)
_, err = clientset.PolicyV1().PodDisruptionBudgets(testNamespace).Create(context.TODO(), pdb3, v1.CreateOptions{})
if err != nil {
t.Fatalf("Error creating fake %s: %v", "Pdb", err)
Expand All @@ -53,12 +67,26 @@ func createTestPdbs(t *testing.T) *fake.Clientset {
t.Fatalf("Error creating fake %s: %v", "Pdb", err)
}

// Unused PDB - kor/used: false
pdb5 := CreateTestPdb(testNamespace, "test-pdb5", AppLabels, UnusedLabels)
_, err = clientset.PolicyV1().PodDisruptionBudgets(testNamespace).Create(context.TODO(), pdb5, v1.CreateOptions{})
if err != nil {
t.Fatalf("Error creating fake %s: %v", "Pdb", err)
}

pdb6 := CreateTestPdb(testNamespace, "test-pdb6", appLabels3, AppLabels)
_, err = clientset.PolicyV1().PodDisruptionBudgets(testNamespace).Create(context.TODO(), pdb6, v1.CreateOptions{})
if err != nil {
t.Fatalf("Error creating fake %s: %v", "Pdb", err)
}

// Unused PDB - empty selector with 0 pods running
pdb7 := CreateTestPdb(testNamespace2, "test-pdb7", appLabels3, AppLabels)
_, err = clientset.PolicyV1().PodDisruptionBudgets(testNamespace2).Create(context.TODO(), pdb7, v1.CreateOptions{})
if err != nil {
t.Fatalf("Error creating fake %s: %v", "Pdb", err)
}

deployment1 := CreateTestDeployment(testNamespace, "test-deployment2", 1, appLabels1)
_, err = clientset.AppsV1().Deployments(testNamespace).Create(context.TODO(), deployment1, v1.CreateOptions{})
if err != nil {
Expand All @@ -71,23 +99,38 @@ func createTestPdbs(t *testing.T) *fake.Clientset {
t.Fatalf("Error creating fake %s: %v", "StatefulSet", err)
}

pod1 := CreateTestPod(testNamespace, "test-arbitrary-pod", "", nil, appLabels1)
_, err = clientset.CoreV1().Pods(testNamespace).Create(context.TODO(), pod1, v1.CreateOptions{})
if err != nil {
t.Fatalf("Error creating fake %s: %v", "Pod", err)
}

return clientset
}

func TestProcessNamespacePdbs(t *testing.T) {
clientset := createTestPdbs(t)
namespaces := []string{testNamespace, testNamespace2}
expectedUnusedPdbs := []string{"test-pdb3", "test-pdb5", "test-pdb7"}
totalUnusedPdbs := []ResourceInfo{}

unusedPdbs, err := processNamespacePdbs(clientset, testNamespace, &filters.Options{})
if err != nil {
t.Errorf("Expected no error, got %v", err)
for _, ns := range namespaces {
unusedPdbs, err := processNamespacePdbs(clientset, ns, &filters.Options{})
if err != nil {
t.Errorf("Expected no error, got %v", err)
}

totalUnusedPdbs = append(totalUnusedPdbs, unusedPdbs...)
}

if len(unusedPdbs) != 2 {
t.Errorf("Expected 2 unused pdb, got %d", len(unusedPdbs))
if len(totalUnusedPdbs) != len(expectedUnusedPdbs) {
t.Errorf("Expected %d unused pdbs, got %d", len(expectedUnusedPdbs), len(totalUnusedPdbs))
}

if unusedPdbs[0].Name != "test-pdb3" && unusedPdbs[1].Name != "test-pdb5" {
t.Errorf("Expected 'test-pdb3', got %s", unusedPdbs[0])
for i, expected := range expectedUnusedPdbs {
if totalUnusedPdbs[i].Name != expected {
t.Errorf("Expected '%s' in unused pdbs, got '%s'", expected, totalUnusedPdbs[i].Name)
}
}
}

Expand Down Expand Up @@ -115,6 +158,11 @@ func TestGetUnusedPdbsStructured(t *testing.T) {
"test-pdb5",
},
},
testNamespace2: {
"Pdb": {
"test-pdb7",
},
},
}

var actualOutput map[string]map[string][]string
Expand Down

0 comments on commit 63f6aab

Please sign in to comment.