From 7f0367fe9b2fc71149e5c83b5b3be1ce0f7d97e4 Mon Sep 17 00:00:00 2001 From: Yonah Dissen <47282577+yonahd@users.noreply.github.com> Date: Thu, 23 Nov 2023 17:50:27 +0200 Subject: [PATCH] Feat: Find unused pv (#163) * Feat: discover unused PVs * support multi and all --- README.md | 5 +- cmd/kor/pv.go | 31 +++++++++++ pkg/kor/all.go | 28 ++++++++-- pkg/kor/create_test_resources.go | 11 ++++ pkg/kor/delete.go | 7 +++ pkg/kor/kor.go | 2 +- pkg/kor/multi.go | 47 ++++++++++++----- pkg/kor/pv.go | 89 ++++++++++++++++++++++++++++++++ pkg/kor/pv_test.go | 79 ++++++++++++++++++++++++++++ 9 files changed, 280 insertions(+), 19 deletions(-) create mode 100644 cmd/kor/pv.go create mode 100644 pkg/kor/pv.go create mode 100644 pkg/kor/pv_test.go diff --git a/README.md b/README.md index a6e6cd5a..46365d4a 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,7 @@ Kor is a tool to discover unused Kubernetes resources. Currently, Kor can identi - Ingresses - PDBs - CRDs +- PVs ![Kor Screenshot](/images/screenshot.png) @@ -82,9 +83,10 @@ Kor provides various subcommands to identify and list unused resources. The avai - `role` - Gets unused Roles for the specified namespace or all namespaces. - `hpa` - Gets unused HPAs for the specified namespace or all namespaces. - `pvc` - Gets unused PVCs for the specified namespace or all namespaces. +- `pv` - Gets unused PVs in the cluster(non namespaced resource). - `ingress` - Gets unused Ingresses for the specified namespace or all namespaces. - `pdb` - Gets unused PDBs for the specified namespace or all namespaces. -- `crd` - Gets unused CRDs in the cluster. +- `crd` - Gets unused CRDs in the cluster(non namespaced resource). - `exporter` - Export Prometheus metrics. ### Supported Flags @@ -133,6 +135,7 @@ kor [subcommand] --help | Ingresses | Ingresses not pointing at any Service | | | 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 | | diff --git a/cmd/kor/pv.go b/cmd/kor/pv.go new file mode 100644 index 00000000..a473034b --- /dev/null +++ b/cmd/kor/pv.go @@ -0,0 +1,31 @@ +package kor + +import ( + "fmt" + + "github.com/spf13/cobra" + "github.com/yonahd/kor/pkg/kor" + "github.com/yonahd/kor/pkg/utils" +) + +var pvCmd = &cobra.Command{ + Use: "persistentvolume", + Aliases: []string{"pv", "persistentvolumes"}, + Short: "Gets unused pvs", + Args: cobra.NoArgs, + Run: func(cmd *cobra.Command, args []string) { + clientset := kor.GetKubeClient(kubeconfig) + + if response, err := kor.GetUnusedPvs(filterOptions, clientset, outputFormat, opts); err != nil { + fmt.Println(err) + } else { + utils.PrintLogo(outputFormat) + fmt.Println(response) + } + + }, +} + +func init() { + rootCmd.AddCommand(pvCmd) +} diff --git a/pkg/kor/all.go b/pkg/kor/all.go index ef338146..047d7356 100644 --- a/pkg/kor/all.go +++ b/pkg/kor/all.go @@ -121,12 +121,21 @@ func getUnusedPdbs(clientset kubernetes.Interface, namespace string, filterOpts } func getUnusedCrds(apiExtClient apiextensionsclientset.Interface, dynamicClient dynamic.Interface) ResourceDiff { - pdbDiff, err := processCrds(apiExtClient, dynamicClient) + crdDiff, err := processCrds(apiExtClient, dynamicClient) if err != nil { fmt.Fprintf(os.Stderr, "Failed to get %s: %v\n", "Crds", err) } - namespacePdbDiff := ResourceDiff{"Crd", pdbDiff} - return namespacePdbDiff + allCrdDiff := ResourceDiff{"Crd", crdDiff} + return allCrdDiff +} + +func getUnusedPvs(clientset kubernetes.Interface, filterOpts *FilterOptions) ResourceDiff { + pvDiff, err := processPvs(clientset, filterOpts) + if err != nil { + fmt.Fprintf(os.Stderr, "Failed to get %s: %v\n", "Pvs", err) + } + allPvDiff := ResourceDiff{"Pv", pvDiff} + return allPvDiff } func GetUnusedAll(includeExcludeLists IncludeExcludeLists, filterOpts *FilterOptions, clientset kubernetes.Interface, apiExtClient apiextensionsclientset.Interface, dynamicClient dynamic.Interface, outputFormat string, opts Opts) (string, error) { @@ -173,13 +182,24 @@ func GetUnusedAll(includeExcludeLists IncludeExcludeLists, filterOpts *FilterOpt } var allDiffs []ResourceDiff + noNamespaceResourceMap := make(map[string][]string) crdDiff := getUnusedCrds(apiExtClient, dynamicClient) - allDiffs = append(allDiffs, crdDiff) + crdOutput := FormatOutputAll("", []ResourceDiff{crdDiff}, opts) + outputBuffer.WriteString(crdOutput) + outputBuffer.WriteString("\n") + noNamespaceResourceMap[crdDiff.resourceType] = crdDiff.diff + + pvDiff := getUnusedPvs(clientset, filterOpts) + pvOutput := FormatOutputAll("", []ResourceDiff{pvDiff}, opts) + outputBuffer.WriteString(pvOutput) + outputBuffer.WriteString("\n") + noNamespaceResourceMap[pvDiff.resourceType] = pvDiff.diff output := FormatOutputAll("", allDiffs, opts) outputBuffer.WriteString(output) outputBuffer.WriteString("\n") + response[""] = noNamespaceResourceMap jsonResponse, err := json.MarshalIndent(response, "", " ") if err != nil { diff --git a/pkg/kor/create_test_resources.go b/pkg/kor/create_test_resources.go index 2f2e5ad5..2659f864 100644 --- a/pkg/kor/create_test_resources.go +++ b/pkg/kor/create_test_resources.go @@ -216,6 +216,17 @@ func CreateTestPvc(namespace, name string) *corev1.PersistentVolumeClaim { } } +func CreateTestPv(name, phase string) *corev1.PersistentVolume { + return &corev1.PersistentVolume{ + ObjectMeta: v1.ObjectMeta{ + Name: name, + }, + Status: corev1.PersistentVolumeStatus{ + Phase: corev1.PersistentVolumePhase(phase), + }, + } +} + func CreateTestPdb(namespace, name string, matchLabels map[string]string) *policyv1.PodDisruptionBudget { return &policyv1.PodDisruptionBudget{ ObjectMeta: v1.ObjectMeta{ diff --git a/pkg/kor/delete.go b/pkg/kor/delete.go index e63e2bc8..7c0dd241 100644 --- a/pkg/kor/delete.go +++ b/pkg/kor/delete.go @@ -52,6 +52,9 @@ func DeleteResourceCmd() map[string]func(clientset kubernetes.Interface, namespa "ServiceAccount": func(clientset kubernetes.Interface, namespace, name string) error { return clientset.CoreV1().ServiceAccounts(namespace).Delete(context.TODO(), name, metav1.DeleteOptions{}) }, + "PV": func(clientset kubernetes.Interface, namespace, name string) error { + return clientset.CoreV1().PersistentVolumes().Delete(context.TODO(), name, metav1.DeleteOptions{}) + }, } return deleteResourceApiMap @@ -103,6 +106,8 @@ func updateResource(clientset kubernetes.Interface, namespace, resourceType stri return clientset.AppsV1().StatefulSets(namespace).Update(context.TODO(), resource.(*appsv1.StatefulSet), metav1.UpdateOptions{}) case "ServiceAccount": return clientset.CoreV1().ServiceAccounts(namespace).Update(context.TODO(), resource.(*corev1.ServiceAccount), metav1.UpdateOptions{}) + case "PV": + return clientset.CoreV1().PersistentVolumes().Update(context.TODO(), resource.(*corev1.PersistentVolume), metav1.UpdateOptions{}) } return nil, fmt.Errorf("resource type '%s' is not supported", resourceType) } @@ -131,6 +136,8 @@ func getResource(clientset kubernetes.Interface, namespace, resourceType, resour return clientset.AppsV1().StatefulSets(namespace).Get(context.TODO(), resourceName, metav1.GetOptions{}) case "ServiceAccount": return clientset.CoreV1().ServiceAccounts(namespace).Get(context.TODO(), resourceName, metav1.GetOptions{}) + case "PV": + return clientset.CoreV1().PersistentVolumes().Get(context.TODO(), resourceName, metav1.GetOptions{}) } return nil, fmt.Errorf("resource type '%s' is not supported", resourceType) } diff --git a/pkg/kor/kor.go b/pkg/kor/kor.go index 4cb93725..f937fddd 100644 --- a/pkg/kor/kor.go +++ b/pkg/kor/kor.go @@ -210,7 +210,7 @@ func FormatOutputAll(namespace string, allDiffs []ResourceDiff, opts Opts) strin table.Render() if namespace == "" { - return fmt.Sprintf("Unused CRDs: \n%s", buf.String()) + return fmt.Sprintf("Unused %ss: \n%s", allDiffs[0].resourceType, buf.String()) } return fmt.Sprintf("Unused Resources in Namespace: %s\n%s", namespace, buf.String()) } diff --git a/pkg/kor/multi.go b/pkg/kor/multi.go index 5c7bc2d1..26bc206f 100644 --- a/pkg/kor/multi.go +++ b/pkg/kor/multi.go @@ -12,20 +12,33 @@ import ( "k8s.io/client-go/kubernetes" ) -func retrieveNoNamespaceDiff(apiExtClient apiextensionsclientset.Interface, dynamicClient dynamic.Interface, outputFormat string, opts Opts, resourceList []string) ([]ResourceDiff, []string) { +func retrieveNoNamespaceDiff(clientset kubernetes.Interface, apiExtClient apiextensionsclientset.Interface, dynamicClient dynamic.Interface, resourceList []string, filterOpts *FilterOptions) ([]ResourceDiff, []string) { var noNamespaceDiff []ResourceDiff + markedForRemoval := make([]bool, len(resourceList)) + updatedResourceList := resourceList + for counter, resource := range resourceList { - if resource == "crd" || resource == "customresourcedefinition" || resource == "customresourcedefinitions" { + switch resource { + case "crd", "customresourcedefinition", "customresourcedefinitions": crdDiff := getUnusedCrds(apiExtClient, dynamicClient) noNamespaceDiff = append(noNamespaceDiff, crdDiff) - updatedResourceList := append(resourceList[:counter], resourceList[counter+1:]...) - return noNamespaceDiff, updatedResourceList - } else { - resourceList[counter] = resource + markedForRemoval[counter] = true + case "pv", "persistentvolume", "persistentvolumes": + pvDiff := getUnusedPvs(clientset, filterOpts) + noNamespaceDiff = append(noNamespaceDiff, pvDiff) + markedForRemoval[counter] = true + } + } + + // Remove elements marked for removal + var clearedResourceList []string + for i, marked := range markedForRemoval { + if !marked { + clearedResourceList = append(clearedResourceList, updatedResourceList[i]) } } - return noNamespaceDiff, resourceList + return noNamespaceDiff, clearedResourceList } func retrieveNamespaceDiffs(clientset kubernetes.Interface, namespace string, resourceList []string, filterOpts *FilterOptions) []ResourceDiff { @@ -72,14 +85,22 @@ func GetUnusedMulti(includeExcludeLists IncludeExcludeLists, resourceNames strin response := make(map[string]map[string][]string) var err error - crdDiff, resourceList := retrieveNoNamespaceDiff(apiExtClient, dynamicClient, outputFormat, opts, resourceList) - if len(crdDiff) != 0 { - output := FormatOutputAll("", crdDiff, opts) - outputBuffer.WriteString(output) - outputBuffer.WriteString("\n") + noNamespaceDiff, resourceList := retrieveNoNamespaceDiff(clientset, apiExtClient, dynamicClient, resourceList, filterOpts) + if len(noNamespaceDiff) != 0 { + for _, diff := range noNamespaceDiff { + if len(diff.diff) != 0 { + output := FormatOutputAll("", []ResourceDiff{diff}, opts) + outputBuffer.WriteString(output) + outputBuffer.WriteString("\n") + + resourceMap := make(map[string][]string) + resourceMap[diff.resourceType] = diff.diff + response[""] = resourceMap + } + } resourceMap := make(map[string][]string) - for _, diff := range crdDiff { + for _, diff := range noNamespaceDiff { resourceMap[diff.resourceType] = diff.diff } response[""] = resourceMap diff --git a/pkg/kor/pv.go b/pkg/kor/pv.go new file mode 100644 index 00000000..7f4fbd6c --- /dev/null +++ b/pkg/kor/pv.go @@ -0,0 +1,89 @@ +package kor + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "os" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + _ "k8s.io/client-go/plugin/pkg/client/auth/oidc" +) + +func processPvs(clientset kubernetes.Interface, filterOpts *FilterOptions) ([]string, error) { + pvs, err := clientset.CoreV1().PersistentVolumes().List(context.TODO(), metav1.ListOptions{}) + if err != nil { + return nil, err + } + + var unusedPvs []string + + for _, pv := range pvs.Items { + if pv.Labels["kor/used"] == "true" { + continue + } + + if excluded, _ := HasExcludedLabel(pv.Labels, filterOpts.ExcludeLabels); excluded { + continue + } + + if included, _ := HasIncludedAge(pv.CreationTimestamp, filterOpts); !included { + continue + } + + if pv.Status.Phase != "Bound" { + unusedPvs = append(unusedPvs, pv.Name) + } + + } + + return unusedPvs, nil + +} + +func GetUnusedPvs(filterOpts *FilterOptions, clientset kubernetes.Interface, outputFormat string, opts Opts) (string, error) { + var outputBuffer bytes.Buffer + response := make(map[string]map[string][]string) + + diff, err := processPvs(clientset, filterOpts) + if err != nil { + fmt.Fprintf(os.Stderr, "Failed to process pvs: %v\n", err) + } + + if len(diff) > 0 { + // We consider cluster scope resources in "" (empty string) namespace, as it is common in k8s + if response[""] == nil { + response[""] = make(map[string][]string) + } + response[""]["Pv"] = diff + } + + if opts.DeleteFlag { + if diff, err = DeleteResource(diff, clientset, "", "PV", opts.NoInteractive); err != nil { + fmt.Fprintf(os.Stderr, "Failed to delete PV %s: %v\n", diff, err) + } + } + + output := FormatOutput("", diff, "PVs", opts) + if output != "" { + outputBuffer.WriteString(output) + outputBuffer.WriteString("\n") + + response[""]["Pv"] = diff + + } + + jsonResponse, err := json.MarshalIndent(response, "", " ") + if err != nil { + return "", err + } + + unusedPvs, err := unusedResourceFormatter(outputFormat, outputBuffer, opts, jsonResponse) + if err != nil { + fmt.Printf("err: %v\n", err) + } + + return unusedPvs, nil +} diff --git a/pkg/kor/pv_test.go b/pkg/kor/pv_test.go new file mode 100644 index 00000000..97f119b1 --- /dev/null +++ b/pkg/kor/pv_test.go @@ -0,0 +1,79 @@ +package kor + +import ( + "context" + "encoding/json" + "reflect" + "testing" + + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" +) + +func createTestPvs(t *testing.T) *fake.Clientset { + clientset := fake.NewSimpleClientset() + + pv1 := CreateTestPv("test-pv1", "Bound") + pv2 := CreateTestPv("test-pv2", "Available") + _, err := clientset.CoreV1().PersistentVolumes().Create(context.TODO(), pv1, v1.CreateOptions{}) + if err != nil { + t.Fatalf("Error creating fake %s: %v", "PV", err) + } + + _, err = clientset.CoreV1().PersistentVolumes().Create(context.TODO(), pv2, v1.CreateOptions{}) + if err != nil { + t.Fatalf("Error creating fake %s: %v", "PV", err) + } + + return clientset +} + +func TestProcessPvs(t *testing.T) { + clientset := createTestPvs(t) + usedPvs, err := processPvs(clientset, &FilterOptions{}) + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + if len(usedPvs) != 1 { + t.Errorf("Expected 1 used pv, got %d", len(usedPvs)) + } + + if usedPvs[0] != "test-pv2" { + t.Errorf("Expected 'test-pv2', got %s", usedPvs[0]) + } +} + +func TestGetUnusedPvs(t *testing.T) { + clientset := createTestPvs(t) + + opts := Opts{ + WebhookURL: "", + Channel: "", + Token: "", + DeleteFlag: false, + NoInteractive: true, + } + + output, err := GetUnusedPvs(&FilterOptions{}, clientset, "json", opts) + if err != nil { + t.Fatalf("Error calling GetUnusedPvs: %v", err) + } + + expectedOutput := map[string]map[string][]string{ + "": { + "Pv": {"test-pv2"}, + }, + } + + var actualOutput map[string]map[string][]string + if err := json.Unmarshal([]byte(output), &actualOutput); err != nil { + t.Fatalf("Error unmarshaling actual output: %v", err) + } + + if !reflect.DeepEqual(expectedOutput, actualOutput) { + t.Errorf("Expected output does not match actual output") + } +} + +