diff --git a/README.md b/README.md
index 31aec8e9..9024b1a1 100644
--- a/README.md
+++ b/README.md
@@ -180,7 +180,7 @@ kor [subcommand] --help
| Hpas | HPAs not used in Deployments
HPAs not used in StatefulSets | |
| CRDs | CRDs not used the cluster | |
| Pvs | PVs not bound to a PVC | |
-| Pdbs | PDBs not used in Deployments
PDBs not used in StatefulSets | |
+| Pdbs | PDBs not used in Deployments / StatefulSets (templates) or in arbitrary Pods
PDBs with empty selectors (match every pod) but no running pods in namespace | |
| Jobs | Jobs status is completed
Jobs status is suspended
Jobs failed with backoff limit exceeded (including indexed jobs)
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 |
diff --git a/pkg/kor/pdbs.go b/pkg/kor/pdbs.go
index 5a37e827..d7d5ce5b 100644
--- a/pkg/kor/pdbs.go
+++ b/pkg/kor/pdbs.go
@@ -53,24 +53,35 @@ 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})
}
}
@@ -78,7 +89,31 @@ func processNamespacePdbs(clientset kubernetes.Interface, namespace string, filt
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
@@ -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) {
diff --git a/pkg/kor/pdbs_test.go b/pkg/kor/pdbs_test.go
index d9183656..36c4461d 100644
--- a/pkg/kor/pdbs_test.go
+++ b/pkg/kor/pdbs_test.go
@@ -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 {
@@ -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)
@@ -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 {
@@ -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)
+ }
}
}
@@ -115,6 +158,11 @@ func TestGetUnusedPdbsStructured(t *testing.T) {
"test-pdb5",
},
},
+ testNamespace2: {
+ "Pdb": {
+ "test-pdb7",
+ },
+ },
}
var actualOutput map[string]map[string][]string