ENG-136: Exclude local storage provisioner by sig-storage-local-provisioner on node to show up in assets when param ASSET_INCLUDE_LOCAL_DISK_COST is set to false (#2882) (#2889)

* exclude local nvme storage from showing up in assets when ASSET_INCLUDE_LOCAL_DISK_COST is set to false



* add comment around the const



* update comment based on code review



* remove all continue statements and filter diskmap at the end before returning



* better function name, variable names and go docs



---------

Signed-off-by: Alan Rodrigues <alanr5691@yahoo.com>

pkg/costmodel/cluster.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"net"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/opencost/opencost/pkg/cloud/provider"
@@ -44,6 +45,15 @@ const (
 
 const MAX_LOCAL_STORAGE_SIZE = 1024 * 1024 * 1024 * 1024
 
+// When ASSET_INCLUDE_LOCAL_DISK_COST is set to false, local storage
+// provisioned by sig-storage-local-static-provisioner is excluded
+// by checking if the volume is prefixed by "local-pv-".
+//
+// This is based on the sig-storage-local-static-provisioner implementation,
+// which creates all PVs with the "local-pv-" prefix. For reference, see:
+// https://github.com/kubernetes-sigs/sig-storage-local-static-provisioner/blob/b6f465027bd059e92c0032c81dd1e1d90e35c909/pkg/discovery/discovery.go#L410-L417
+const SIG_STORAGE_LOCAL_PROVISIONER_PREFIX = "local-pv-"
+
 // Costs represents cumulative and monthly cluster costs over a given duration. Costs
 // are broken down by cores, memory, and storage.
 type ClusterCosts struct {
@@ -532,6 +542,10 @@ func ClusterDisks(client prometheus.Client, cp models.Provider, start, end time.
 		}
 	}
 
+	if !env.GetAssetIncludeLocalDiskCost() {
+		return filterOutLocalPVs(diskMap), nil
+	}
+
 	return diskMap, nil
 }
 
@@ -1548,11 +1562,13 @@ func pvCosts(diskMap map[DiskIdentifier]*Disk, resolution time.Duration, resActi
 				log.Debugf("ClusterDisks: pv claim data missing volumename")
 				continue
 			}
+
 			thatClaimName, err := thatRes.GetString("persistentvolumeclaim")
 			if err != nil {
 				log.Debugf("ClusterDisks: pv claim data missing persistentvolumeclaim")
 				continue
 			}
+
 			thatClaimNamespace, err := thatRes.GetString("namespace")
 			if err != nil {
 				log.Debugf("ClusterDisks: pv claim data missing namespace")
@@ -1589,6 +1605,7 @@ func pvCosts(diskMap map[DiskIdentifier]*Disk, resolution time.Duration, resActi
 			log.Debugf("ClusterDisks: pv usage data missing persistentvolumeclaim")
 			continue
 		}
+
 		claimNamespace, err := result.GetString("namespace")
 		if err != nil {
 			log.Debugf("ClusterDisks: pv usage data missing namespace")
@@ -1609,11 +1626,13 @@ func pvCosts(diskMap map[DiskIdentifier]*Disk, resolution time.Duration, resActi
 				log.Debugf("ClusterDisks: pv claim data missing volumename")
 				continue
 			}
+
 			thatClaimName, err := thatRes.GetString("persistentvolumeclaim")
 			if err != nil {
 				log.Debugf("ClusterDisks: pv claim data missing persistentvolumeclaim")
 				continue
 			}
+
 			thatClaimNamespace, err := thatRes.GetString("namespace")
 			if err != nil {
 				log.Debugf("ClusterDisks: pv claim data missing namespace")
@@ -1639,3 +1658,22 @@ func pvCosts(diskMap map[DiskIdentifier]*Disk, resolution time.Duration, resActi
 		diskMap[key].BytesUsedMaxPtr = &usage
 	}
 }
+
+// filterOutLocalPVs removes local Persistent Volumes (PVs) from the given disk map.
+// Local PVs are identified by the prefix "local-pv-" in their names, which is the
+// convention used by sig-storage-local-static-provisioner.
+//
+// Parameters:
+//   - diskMap: A map of DiskIdentifier to Disk pointers, representing all PVs.
+//
+// Returns:
+//   - A new map of DiskIdentifier to Disk pointers, containing only non-local PVs.
+func filterOutLocalPVs(diskMap map[DiskIdentifier]*Disk) map[DiskIdentifier]*Disk {
+	nonLocalPVDiskMap := map[DiskIdentifier]*Disk{}
+	for key, val := range diskMap {
+		if !strings.HasPrefix(key.Name, SIG_STORAGE_LOCAL_PROVISIONER_PREFIX) {
+			nonLocalPVDiskMap[key] = val
+		}
+	}
+	return nonLocalPVDiskMap
+}

pkg/costmodel/cluster_test.go

@@ -0,0 +1,54 @@
+package costmodel
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_filterOutLocalPVs(t *testing.T) {
+	testCases := []struct {
+		name     string
+		input    map[DiskIdentifier]*Disk
+		expected map[DiskIdentifier]*Disk
+	}{
+		{
+			name: "Filter out local PVs",
+			input: map[DiskIdentifier]*Disk{
+				{Cluster: "cluster1", Name: "pv1"}:              &Disk{Name: "pv1"},
+				{Cluster: "cluster1", Name: "local-pv-123"}:     &Disk{Name: "local-pv-123"},
+				{Cluster: "cluster2", Name: "pv2"}:              &Disk{Name: "pv2"},
+				{Cluster: "cluster2", Name: "local-pv-456"}:     &Disk{Name: "local-pv-456"},
+				{Cluster: "cluster3", Name: "not-local-pv-789"}: &Disk{Name: "not-local-pv-789"},
+			},
+			expected: map[DiskIdentifier]*Disk{
+				{Cluster: "cluster1", Name: "pv1"}:              &Disk{Name: "pv1"},
+				{Cluster: "cluster2", Name: "pv2"}:              &Disk{Name: "pv2"},
+				{Cluster: "cluster3", Name: "not-local-pv-789"}: &Disk{Name: "not-local-pv-789"},
+			},
+		},
+		{
+			name: "No local PVs to filter",
+			input: map[DiskIdentifier]*Disk{
+				{Cluster: "cluster1", Name: "pv1"}: &Disk{Name: "pv1"},
+				{Cluster: "cluster2", Name: "pv2"}: &Disk{Name: "pv2"},
+			},
+			expected: map[DiskIdentifier]*Disk{
+				{Cluster: "cluster1", Name: "pv1"}: &Disk{Name: "pv1"},
+				{Cluster: "cluster2", Name: "pv2"}: &Disk{Name: "pv2"},
+			},
+		},
+		{
+			name:     "Empty input",
+			input:    map[DiskIdentifier]*Disk{},
+			expected: map[DiskIdentifier]*Disk{},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			result := filterOutLocalPVs(tc.input)
+			assert.Equal(t, tc.expected, result)
+		})
+	}
+}