Made small adjustments to CostModel methods to fulfil TODOs

core/go.mod

@@ -22,7 +22,6 @@ require (
 	github.com/prometheus/common v0.63.0
 	github.com/rs/zerolog v1.26.1
 	github.com/spf13/viper v1.8.1
-	github.com/stretchr/testify v1.10.0
 	golang.org/x/exp v0.0.0-20221031165847-c99f073a8326
 	golang.org/x/oauth2 v0.25.0
 	golang.org/x/sync v0.12.0
@@ -87,7 +86,6 @@ require (
 	github.com/oklog/run v1.1.0 // indirect
 	github.com/pelletier/go-toml v1.9.3 // indirect
 	github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect
-	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/prometheus/client_model v0.6.1 // indirect
 	github.com/rs/xid v1.6.0 // indirect
 	github.com/spf13/afero v1.6.0 // indirect
@@ -110,7 +108,6 @@ require (
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20240221002015-b0ce06bbee7c // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/ini.v1 v1.67.0 // indirect
-	gopkg.in/yaml.v3 v3.0.1 // indirect
 	k8s.io/klog/v2 v2.80.0 // indirect
 	k8s.io/utils v0.0.0-20220728103510-ee6ede2d64ed // indirect
 	sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2 // indirect

pkg/costmodel/costmodel.go

@@ -197,7 +197,7 @@ func (cm *CostModel) ComputeCostData(start, end time.Time) (map[string]*CostData
 
 	defer measureTime(time.Now(), profileThreshold, "ComputeCostData: Processing Query Data")
 
-	nodes, err := cm.GetNodeCost(cp)
+	nodes, err := cm.GetNodeCost()
 	if err != nil {
 		log.Warnf("GetNodeCost: no node cost model available: %s", err)
 		return nil, err
@@ -210,7 +210,7 @@ func (cm *CostModel) ComputeCostData(start, end time.Time) (map[string]*CostData
 		log.Warnf("GetPVInfo: unable to get PV data: %s", err.Error())
 	}
 	if pvClaimMapping != nil {
-		err = addPVData(cm.Cache, pvClaimMapping, cp)
+		err = cm.addPVData(pvClaimMapping)
 		if err != nil {
 			return nil, err
 		}
@@ -755,7 +755,9 @@ func getContainerAllocation(req *util.Vector, used *util.Vector, allocationType
 	return result
 }
 
-func addPVData(cache clustercache.ClusterCache, pvClaimMapping map[string]*PersistentVolumeClaimData, cloud costAnalyzerCloud.Provider) error {
+func (cm *CostModel) addPVData(pvClaimMapping map[string]*PersistentVolumeClaimData) error {
+	cache := cm.Cache
+	cloud := cm.Provider
 	cfg, err := cloud.GetConfig()
 	if err != nil {
 		return err
@@ -797,7 +799,7 @@ func addPVData(cache clustercache.ClusterCache, pvClaimMapping map[string]*Persi
 			Region:     region,
 			Parameters: parameters,
 		}
-		err := GetPVCost(cacPv, pv, cloud, region)
+		err := cm.GetPVCost(cacPv, pv, region)
 		if err != nil {
 			return err
 		}
@@ -818,7 +820,8 @@ func addPVData(cache clustercache.ClusterCache, pvClaimMapping map[string]*Persi
 	return nil
 }
 
-func GetPVCost(pv *costAnalyzerCloud.PV, kpv *clustercache.PersistentVolume, cp costAnalyzerCloud.Provider, defaultRegion string) error {
+func (cm *CostModel) GetPVCost(pv *costAnalyzerCloud.PV, kpv *clustercache.PersistentVolume, defaultRegion string) error {
+	cp := cm.Provider
 	cfg, err := cp.GetConfig()
 	if err != nil {
 		return err
@@ -846,7 +849,8 @@ func (cm *CostModel) GetPricingSourceCounts() (*costAnalyzerCloud.PricingMatchMe
 	}
 }
 
-func (cm *CostModel) GetNodeCost(cp costAnalyzerCloud.Provider) (map[string]*costAnalyzerCloud.Node, error) {
+func (cm *CostModel) GetNodeCost() (map[string]*costAnalyzerCloud.Node, error) {
+	cp := cm.Provider
 	cfg, err := cp.GetConfig()
 	if err != nil {
 		return nil, err
@@ -1247,13 +1251,13 @@ func (cm *CostModel) GetNodeCost(cp costAnalyzerCloud.Provider) (map[string]*cos
 }
 
 // TODO: drop some logs
-func (cm *CostModel) GetLBCost(cp costAnalyzerCloud.Provider) (map[serviceKey]*costAnalyzerCloud.LoadBalancer, error) {
+func (cm *CostModel) GetLBCost() (map[serviceKey]*costAnalyzerCloud.LoadBalancer, error) {
 	// for fetching prices from cloud provider
 	// cfg, err := cp.GetConfig()
 	// if err != nil {
 	// 	return nil, err
 	// }
-
+	cp := cm.Provider
 	servicesList := cm.Cache.GetAllServices()
 	loadBalancerMap := make(map[serviceKey]*costAnalyzerCloud.LoadBalancer)
 

pkg/costmodel/metrics.go

@@ -471,8 +471,7 @@ func (cmme *CostModelMetricsEmitter) Start() bool {
 				data = map[string]*CostData{}
 			}
 
-			// TODO: Pass CloudProvider into CostModel on instantiation so this isn't so awkward
-			nodes, err := cmme.Model.GetNodeCost(cmme.CloudProvider)
+			nodes, err := cmme.Model.GetNodeCost()
 			if err != nil {
 				log.Warnf("Error getting Node cost: %s", err)
 			}
@@ -571,8 +570,7 @@ func (cmme *CostModelMetricsEmitter) Start() bool {
 				nodeSeen[labelKey] = true
 			}
 
-			// TODO: Pass CloudProvider into CostModel on instantiation so this isn't so awkward
-			loadBalancers, err := cmme.Model.GetLBCost(cmme.CloudProvider)
+			loadBalancers, err := cmme.Model.GetLBCost()
 			if err != nil {
 				log.Warnf("Error getting LoadBalancer cost: %s", err)
 			}
@@ -682,7 +680,7 @@ func (cmme *CostModelMetricsEmitter) Start() bool {
 				}
 
 				// TODO: GetPVCost should be a method in CostModel?
-				GetPVCost(cacPv, pv, cmme.CloudProvider, region)
+				cmme.Model.GetPVCost(cacPv, pv, region)
 				c, _ := strconv.ParseFloat(cacPv.Cost, 64)
 				cmme.PersistentVolumePriceRecorder.WithLabelValues(pv.Name, pv.Name, cacPv.ProviderID).Set(c)
 				labelKey := getKeyFromLabelStrings(pv.Name, pv.Name, cacPv.ProviderID)

test/cloud_test.go

@@ -265,9 +265,9 @@ func TestNodePriceFromCSVWithGPULabels(t *testing.T) {
 	fm := FakeClusterMap{}
 	d, _ := time.ParseDuration("1m")
 
-	model := costmodel.NewCostModel(nil, nil, fc, fm, d)
+	model := costmodel.NewCostModel(nil, c, fc, fm, d)
 
-	nodeMap, err := model.GetNodeCost(c)
+	nodeMap, err := model.GetNodeCost()
 	if err != nil {
 		t.Errorf("Error in NodePricing: %s", err.Error())
 	} else {
@@ -311,9 +311,9 @@ func TestRKE2NodePriceFromCSVWithGPULabels(t *testing.T) {
 	fm := FakeClusterMap{}
 	d, _ := time.ParseDuration("1m")
 
-	model := costmodel.NewCostModel(nil, nil, fc, fm, d)
+	model := costmodel.NewCostModel(nil, c, fc, fm, d)
 
-	nodeMap, err := model.GetNodeCost(c)
+	nodeMap, err := model.GetNodeCost()
 	if err != nil {
 		t.Errorf("Error in NodePricing: %s", err.Error())
 	} else {
@@ -574,9 +574,9 @@ func TestNodePriceFromCSVWithBadConfig(t *testing.T) {
 	fm := FakeClusterMap{}
 	d, _ := time.ParseDuration("1m")
 
-	model := costmodel.NewCostModel(nil, nil, fc, fm, d)
+	model := costmodel.NewCostModel(nil, c, fc, fm, d)
 
-	_, err := model.GetNodeCost(c)
+	_, err := model.GetNodeCost()
 	if err != nil {
 		t.Errorf("Error in node pricing: %s", err)
 	}
@@ -633,9 +633,9 @@ func TestSourceMatchesFromCSV(t *testing.T) {
 	fm := FakeClusterMap{}
 	d, _ := time.ParseDuration("1m")
 
-	model := costmodel.NewCostModel(nil, nil, fc, fm, d)
+	model := costmodel.NewCostModel(nil, c, fc, fm, d)
 
-	_, err = model.GetNodeCost(c)
+	_, err = model.GetNodeCost()
 	if err != nil {
 		t.Errorf("Error in node pricing: %s", err)
 	}