Merge branch 'develop' into sean/fix-aws-catalog-load

README.md

@@ -11,10 +11,10 @@ OpenCost was originally developed and open sourced by [Kubecost](https://kubecos
 To see the full functionality of OpenCost you can view [OpenCost features](https://opencost.io). Here is a summary of features enabled:
 
 - Real-time cost allocation by Kubernetes cluster, node, namespace, controller kind, controller, service, or pod
-- Dynamic onDemand asset pricing enabled by integrations with AWS, Azure, and GCP billing APIs
+- Dynamic on-demand asset pricing enabled by integrations with AWS, Azure, and GCP billing APIs
 - Supports on-prem k8s clusters with custom CSV pricing
 - Allocation for in-cluster resources like CPU, GPU, memory, and persistent volumes.
-- Easily export pricing data to Prometheus with /metrics endpoint ([learn more](PROMETHEUS.md))
+- Easily export pricing data to Prometheus with /metrics endpoint ([learn more](https://www.opencost.io/docs/installation/prometheus))
 - Free and open source distribution (Apache2 license)
 
 ## Getting Started

pkg/cloud/aws/provider.go

@@ -70,6 +70,13 @@ var (
 	usageTypeRegx = regexp.MustCompile(".*(-|^)(EBS.+)")
 	versionRx     = regexp.MustCompile(`^#Version: (\\d+)\\.\\d+$`)
 	regionRx      = regexp.MustCompile("([a-z]+-[a-z]+-[0-9])")
+
+	// StorageClassProvisionerDefaults specifies the default storage class types depending upon the provisioner
+	StorageClassProvisionerDefaults = map[string]string{
+		"kubernetes.io/aws-ebs": "gp2",
+		"ebs.csi.aws.com":       "gp3",
+		// TODO: add efs provisioner
+	}
 )
 
 func (aws *AWS) PricingSourceStatus() map[string]*models.PricingSource {
@@ -666,6 +673,9 @@ func (k *awsKey) Features() string {
 // If the instance is a spot instance, it will return PreemptibleType
 // Otherwise returns an empty string
 func (k *awsKey) getUsageType(labels map[string]string) string {
+	if kLabel, ok := labels[k.SpotLabelName]; ok && kLabel == k.SpotLabelValue {
+		return PreemptibleType
+	}
 	if eksLabel, ok := labels[EKSCapacityTypeLabel]; ok && eksLabel == EKSCapacitySpotTypeValue {
 		// We currently write out spot instances as "preemptible" in the pricing data, so these need to match
 		return PreemptibleType
@@ -720,7 +730,12 @@ func (key *awsPVKey) GetStorageClass() string {
 }
 
 func (key *awsPVKey) Features() string {
-	storageClass := key.StorageClassParameters["type"]
+	storageClass, ok := key.StorageClassParameters["type"]
+	if !ok {
+		log.Debugf("storage class %s doesn't have a 'type' parameter", key.Name)
+		storageClass = getStorageClassTypeFrom(key.StorageClassParameters["provisioner"])
+	}
+
 	if storageClass == "standard" {
 		storageClass = "gp2"
 	}
@@ -738,6 +753,22 @@ func (key *awsPVKey) Features() string {
 	return region + "," + class
 }
 
+// getStorageClassTypeFrom returns the default ebs volume type for a provider provisioner
+func getStorageClassTypeFrom(provisioner string) string {
+	// if there isn't any provided provisioner, return empty volume type
+	if provisioner == "" {
+		return ""
+	}
+
+	scType, ok := StorageClassProvisionerDefaults[provisioner]
+	if ok {
+		log.Debugf("using default voltype %s for provisioner %s", scType, provisioner)
+		return scType
+	}
+
+	return ""
+}
+
 // GetKey maps node labels to information needed to retrieve pricing data
 func (aws *AWS) GetKey(labels map[string]string, n *v1.Node) models.Key {
 	return &awsKey{
@@ -862,6 +893,7 @@ func (aws *AWS) DownloadPricingData() error {
 	storageClassMap := make(map[string]map[string]string)
 	for _, storageClass := range storageClasses {
 		params := storageClass.Parameters
+		params["provisioner"] = storageClass.Provisioner
 		storageClassMap[storageClass.ObjectMeta.Name] = params
 		if storageClass.GetAnnotations()["storageclass.kubernetes.io/is-default-class"] == "true" || storageClass.GetAnnotations()["storageclass.beta.kubernetes.io/is-default-class"] == "true" {
 			storageClassMap["default"] = params

pkg/cloud/aws/provider_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 
 	"github.com/opencost/opencost/pkg/cloud/models"
+	v1 "k8s.io/api/core/v1"
 )
 
 func Test_awsKey_getUsageType(t *testing.T) {
@@ -492,5 +493,71 @@ func Test_populate_pricing(t *testing.T) {
 	if !reflect.DeepEqual(expectedPricing, awsTest.Pricing) {
 		t.Fatalf("expected parsed pricing did not match actual parsed result (cn)")
 	}
+}
+
+func TestFeatures(t *testing.T) {
+	testCases := map[string]struct {
+		aws      awsKey
+		expected string
+	}{
+		"Spot from custom labels": {
+			aws: awsKey{
+				SpotLabelName:  "node-type",
+				SpotLabelValue: "node-spot",
+				Labels: map[string]string{
+					"node-type":                "node-spot",
+					v1.LabelOSStable:           "linux",
+					v1.LabelHostname:           "my-hostname",
+					v1.LabelTopologyRegion:     "us-west-2",
+					v1.LabelTopologyZone:       "us-west-2b",
+					v1.LabelInstanceTypeStable: "m5.large",
+				},
+			},
+			expected: "us-west-2,m5.large,linux,preemptible",
+		},
+	}
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			features := tc.aws.Features()
+			if features != tc.expected {
+				t.Errorf("expected %s, got %s", tc.expected, features)
+			}
+		})
+	}
+}
 
+func Test_getStorageClassTypeFrom(t *testing.T) {
+	tests := []struct {
+		name        string
+		provisioner string
+		want        string
+	}{
+		{
+			name:        "empty-provisioner",
+			provisioner: "",
+			want:        "",
+		},
+		{
+			name:        "ebs-default-provisioner",
+			provisioner: "kubernetes.io/aws-ebs",
+			want:        "gp2",
+		},
+		{
+			name:        "ebs-csi-provisioner",
+			provisioner: "ebs.csi.aws.com",
+			want:        "gp3",
+		},
+		{
+			name:        "unknown-provisioner",
+			provisioner: "unknown",
+			want:        "",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := getStorageClassTypeFrom(tt.provisioner); got != tt.want {
+				t.Errorf("getStorageClassTypeFrom() = %v, want %v", got, tt.want)
+			}
+		})
+	}
 }

pkg/cloud/scaleway/provider.go

@@ -149,7 +149,7 @@ func (c *Scaleway) NodePricing(key models.Key) (*models.Node, models.PricingMeta
 				RAM:         fmt.Sprintf("%d", info.RAM),
 				// This is tricky, as instances can have local volumes or not
 				Storage:      fmt.Sprintf("%d", info.PerVolumeConstraint.LSSD.MinSize),
-				GPU:          fmt.Sprintf("%d", info.Gpu),
+				GPU:          fmt.Sprintf("%d", *info.Gpu),
 				InstanceType: split[1],
 				Region:       split[0],
 				GPUName:      key.GPUType(),

pkg/costmodel/allocation.go

@@ -295,8 +295,9 @@ func (cm *CostModel) ComputeAllocation(start, end time.Time, resolution time.Dur
 // it supposed to be a good indicator of available allocation data
 func (cm *CostModel) DateRange() (time.Time, time.Time, error) {
 	ctx := prom.NewNamedContext(cm.PrometheusClient, prom.AllocationContextName)
+	exportCsvDaysFmt := fmt.Sprintf("%dd", env.GetExportCSVMaxDays())
 
-	resOldest, _, err := ctx.QuerySync(fmt.Sprintf(queryFmtOldestSample, env.GetPromClusterFilter(), "90d", "1h"))
+	resOldest, _, err := ctx.QuerySync(fmt.Sprintf(queryFmtOldestSample, env.GetPromClusterFilter(), exportCsvDaysFmt, "1h"))
 	if err != nil {
 		return time.Time{}, time.Time{}, fmt.Errorf("querying oldest sample: %w", err)
 	}
@@ -305,7 +306,7 @@ func (cm *CostModel) DateRange() (time.Time, time.Time, error) {
 	}
 	oldest := time.Unix(int64(resOldest[0].Values[0].Value), 0)
 
-	resNewest, _, err := ctx.QuerySync(fmt.Sprintf(queryFmtNewestSample, env.GetPromClusterFilter(), "90d", "1h"))
+	resNewest, _, err := ctx.QuerySync(fmt.Sprintf(queryFmtNewestSample, env.GetPromClusterFilter(), exportCsvDaysFmt, "1h"))
 	if err != nil {
 		return time.Time{}, time.Time{}, fmt.Errorf("querying newest sample: %w", err)
 	}

pkg/env/costmodelenv.go

@@ -106,6 +106,7 @@ const (
 	ExportCSVFile       = "EXPORT_CSV_FILE"
 	ExportCSVLabelsList = "EXPORT_CSV_LABELS_LIST"
 	ExportCSVLabelsAll  = "EXPORT_CSV_LABELS_ALL"
+	ExportCSVMaxDays    = "EXPORT_CSV_MAX_DAYS"
 )
 
 const DefaultConfigMountPath = "/var/configs"
@@ -128,6 +129,10 @@ func GetExportCSVLabelsList() []string {
 	return GetList(ExportCSVLabelsList, ",")
 }
 
+func GetExportCSVMaxDays() int {
+	return GetInt(ExportCSVMaxDays, 90)
+}
+
 // GetKubecostConfigBucket returns a file location for a mounted bucket configuration which is used to store
 // a subset of kubecost configurations that require sharing via remote storage.
 func GetKubecostConfigBucket() string {

pkg/env/costmodelenv_test.go

@@ -41,3 +41,47 @@ func TestIsCacheDisabled(t *testing.T) {
 		})
 	}
 }
+
+func TestGetExportCSVMaxDays(t *testing.T) {
+	tests := []struct {
+		name string
+		want int
+		pre  func()
+	}{
+		{
+			name: "Ensure the default value is 90d",
+			want: 90,
+		},
+		{
+			name: "Ensure the value is 30 when EXPORT_CSV_MAX_DAYS is set to 30",
+			want: 30,
+			pre: func() {
+				os.Setenv("EXPORT_CSV_MAX_DAYS", "30")
+			},
+		},
+		{
+			name: "Ensure the value is 90 when EXPORT_CSV_MAX_DAYS is set to empty string",
+			want: 90,
+			pre: func() {
+				os.Setenv("EXPORT_CSV_MAX_DAYS", "")
+			},
+		},
+		{
+			name: "Ensure the value is 90 when EXPORT_CSV_MAX_DAYS is set to invalid value",
+			want: 90,
+			pre: func() {
+				os.Setenv("EXPORT_CSV_MAX_DAYS", "foo")
+			},
+		},
+	}
+	for _, tt := range tests {
+		if tt.pre != nil {
+			tt.pre()
+		}
+		t.Run(tt.name, func(t *testing.T) {
+			if got := GetExportCSVMaxDays(); got != tt.want {
+				t.Errorf("GetExportCSVMaxDays() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

pkg/kubecost/asset.go

@@ -1589,6 +1589,10 @@ func (b *Breakdown) Clone() *Breakdown {
 
 // Equal returns true if the two Breakdowns are exact matches
 func (b *Breakdown) Equal(that *Breakdown) bool {
+	if b == nil && that == nil {
+		return true
+	}
+
 	if b == nil || that == nil {
 		return false
 	}
@@ -1889,6 +1893,32 @@ func (n *NodeOverhead) SanitizeNaN() {
 	}
 }
 
+func (n *NodeOverhead) Equal(other *NodeOverhead) bool {
+	if n == nil && other != nil {
+		return false
+	}
+	if n != nil && other == nil {
+		return false
+	}
+	if n == nil && other == nil {
+		return true
+	}
+
+	// This is okay because everything in NodeOverhead is a value type.
+	return *n == *other
+}
+
+func (n *NodeOverhead) Clone() *NodeOverhead {
+	if n == nil {
+		return nil
+	}
+	return &NodeOverhead{
+		CpuOverheadFraction:  n.CpuOverheadFraction,
+		RamOverheadFraction:  n.RamOverheadFraction,
+		OverheadCostFraction: n.OverheadCostFraction,
+	}
+}
+
 // Node is an Asset representing a single node in a cluster
 type Node struct {
 	Properties   *AssetProperties
@@ -2171,6 +2201,7 @@ func (n *Node) Clone() Asset {
 		GPUCount:     n.GPUCount,
 		RAMCost:      n.RAMCost,
 		Preemptible:  n.Preemptible,
+		Overhead:     n.Overhead.Clone(),
 		Discount:     n.Discount,
 	}
 }
@@ -2233,6 +2264,9 @@ func (n *Node) Equal(a Asset) bool {
 	if n.Preemptible != that.Preemptible {
 		return false
 	}
+	if !n.Overhead.Equal(that.Overhead) {
+		return false
+	}
 
 	return true
 }

pkg/kubecost/asset_test.go

@@ -548,7 +548,37 @@ func TestNode_Add(t *testing.T) {
 }
 
 func TestNode_Clone(t *testing.T) {
-	// TODO
+	cases := []struct {
+		name string
+
+		input *Node
+	}{
+		{
+			name: "overhead nil",
+			input: &Node{
+				Overhead: nil,
+			},
+		},
+		{
+			name: "overhead non-nil",
+			input: &Node{
+				Overhead: &NodeOverhead{
+					CpuOverheadFraction:  3,
+					RamOverheadFraction:  7,
+					OverheadCostFraction: 6,
+				},
+			},
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			result := c.input.Clone()
+			if !result.Equal(c.input) {
+				t.Errorf("clone result doesn't equal input")
+			}
+		})
+	}
 }
 
 func TestNode_MarshalJSON(t *testing.T) {

pkg/kubecost/assetprops.go

@@ -199,8 +199,12 @@ func (ap *AssetProperties) Clone() *AssetProperties {
 	return clone
 }
 
-// Equal returns true only if both AssetProperties are non-nil exact matches
+// Equal returns true only if both AssetProperties are matches
 func (ap *AssetProperties) Equal(that *AssetProperties) bool {
+	if ap == nil && that == nil {
+		return true
+	}
+
 	if ap == nil || that == nil {
 		return false
 	}

pkg/metrics/kubemetrics.go

@@ -156,7 +156,7 @@ func getPersistentVolumeClaimClass(claim *v1.PersistentVolumeClaim) string {
 	}
 
 	// Special non-empty string to indicate absence of storage class.
-	return "<none>"
+	return ""
 }
 
 // toResourceUnitValue accepts a resource name and quantity and returns the sanitized resource, the unit, and the value in the units.