Merge branch 'develop' into fix-panic-csv

configs/alibaba.json

@@ -1,12 +1,16 @@
 {
     "provider": "Alibaba",
-    "description": "Default prices used to compute allocation between RAM and CPU. Alibaba Cloud pricing API data still used for total node cost.",
-    "alibabaServiceKeyName": "ABC",
-    "alibabaServiceKeySecret": "XYZ",
+    "description": "Default prices used to compute allocation between RAM and CPU. Alibaba Cloud pricing API is used for total node and PV cost.",
+    "alibabaServiceKeyName": "",
+    "alibabaServiceKeySecret": "",
     "CPU": "0.031611",
     "spotCPU": "0.006655",
     "RAM": "0.004237",
     "GPU": "0.95",
     "spotRAM": "0.000892",
-    "storage": "0.00005479452"
+    "storage": "0.00005479452",
+    "zoneNetworkEgress": "0.02",
+    "regionNetworkEgress": "0.08",
+    "internetNetworkEgress": "0.123",
+    "defaultLBPrice": "0.007"
 }

pkg/cloud/aliyunprovider.go

@@ -6,6 +6,7 @@ import (
 	"io"
 	"os"
 	"regexp"
+	"strconv"
 	"strings"
 	"sync"
 	"time"
@@ -366,7 +367,10 @@ func (alibaba *Alibaba) GetAlibabaAccessKey() (*credentials.AccessKeyCredential,
 		return nil, fmt.Errorf("failed to get the access key for the current alibaba account")
 	}
 
-	alibaba.accessKey = &credentials.AccessKeyCredential{AccessKeyId: env.GetAlibabaAccessKeyID(), AccessKeySecret: env.GetAlibabaAccessKeySecret()}
+	// At this point either user is using the alibaba key and secret from secret passed in helm config if not he will use the secret that is passed in custom pricing
+	// There's no check at this time for if the custom pricing key and secret is valid and that's on the user else there will be errors recorded.
+	// Key and secret passed in config will supersede key and secret passed while installing Closed source helm chart.
+	alibaba.accessKey = &credentials.AccessKeyCredential{AccessKeyId: config.AlibabaServiceKeyName, AccessKeySecret: config.AlibabaServiceKeySecret}
 
 	return alibaba.accessKey, nil
 }
@@ -544,19 +548,46 @@ func (alibaba *Alibaba) PVPricing(pvk PVKey) (*PV, error) {
 	return pricing.PV, nil
 }
 
-// Stubbed NetworkPricing for Alibaba Cloud. Will look at this in Next PR
+// Inter zone and Inter region network cost are defaulted based on https://www.alibabacloud.com/help/en/cloud-data-transmission/latest/cross-region-data-transfers
+// Internet cost is default based on https://www.alibabacloud.com/help/en/elastic-compute-service/latest/public-bandwidth to $0.123
 func (alibaba *Alibaba) NetworkPricing() (*Network, error) {
+	cpricing, err := alibaba.Config.GetCustomPricingData()
+	if err != nil {
+		return nil, err
+	}
+	znec, err := strconv.ParseFloat(cpricing.ZoneNetworkEgress, 64)
+	if err != nil {
+		return nil, err
+	}
+	rnec, err := strconv.ParseFloat(cpricing.RegionNetworkEgress, 64)
+	if err != nil {
+		return nil, err
+	}
+	inec, err := strconv.ParseFloat(cpricing.InternetNetworkEgress, 64)
+	if err != nil {
+		return nil, err
+	}
+
 	return &Network{
-		ZoneNetworkEgressCost:     0.0,
-		RegionNetworkEgressCost:   0.0,
-		InternetNetworkEgressCost: 0.0,
+		ZoneNetworkEgressCost:     znec,
+		RegionNetworkEgressCost:   rnec,
+		InternetNetworkEgressCost: inec,
 	}, nil
 }
 
-// Stubbed LoadBalancerPricing for Alibaba Cloud. Will look at this in Next PR
+// Alibaba loadbalancer has three different types https://www.alibabacloud.com/product/server-load-balancer,
+// defaulted price to classic load balancer https://www.alibabacloud.com/help/en/server-load-balancer/latest/pay-as-you-go.
 func (alibaba *Alibaba) LoadBalancerPricing() (*LoadBalancer, error) {
+	cpricing, err := alibaba.Config.GetCustomPricingData()
+	if err != nil {
+		return nil, err
+	}
+	lbPricing, err := strconv.ParseFloat(cpricing.DefaultLBPrice, 64)
+	if err != nil {
+		return nil, err
+	}
 	return &LoadBalancer{
-		Cost: 0.0,
+		Cost: lbPricing,
 	}, nil
 }
 
@@ -747,7 +778,16 @@ func (alibaba *Alibaba) CombinedDiscountForNode(string, bool, float64, float64)
 }
 
 func (alibaba *Alibaba) accessKeyisLoaded() bool {
-	return alibaba.accessKey != nil
+	if alibaba.accessKey == nil {
+		return false
+	}
+	if alibaba.accessKey.AccessKeyId == "" {
+		return false
+	}
+	if alibaba.accessKey.AccessKeySecret == "" {
+		return false
+	}
+	return true
 }
 
 type AlibabaNodeKey struct {

pkg/cloud/provider.go

@@ -234,6 +234,7 @@ type CustomPricing struct {
 	KubecostToken                string `json:"kubecostToken"`
 	GoogleAnalyticsTag           string `json:"googleAnalyticsTag"`
 	ExcludeProviderID            string `json:"excludeProviderID"`
+	DefaultLBPrice               string `json:"defaultLBPrice"`
 }
 
 // GetSharedOverheadCostPerMonth parses and returns a float64 representation

pkg/cloud/providerconfig.go

@@ -2,6 +2,8 @@ package cloud
 
 import (
 	"fmt"
+	"io/ioutil"
+	"os"
 	gopath "path"
 	"reflect"
 	"strconv"
@@ -14,6 +16,8 @@ import (
 	"github.com/opencost/opencost/pkg/util/json"
 )
 
+const closedSourceConfigMount = "models/"
+
 var sanitizePolicy = bluemonday.UGCPolicy()
 
 // ProviderConfig is a utility class that provides a thread-safe configuration storage/cache for all Provider
@@ -91,6 +95,15 @@ func (pc *ProviderConfig) loadConfig(writeIfNotExists bool) (*CustomPricing, err
 	if !exists {
 		log.Infof("Could not find Custom Pricing file at path '%s'", pc.configFile.Path())
 		pc.customPricing = DefaultPricing()
+		// If config file is not present use the contents from mount models/ as pricing data
+		// in closed source rather than from from  DefaultPricing as first source of truth.
+		// since most images will already have a mount, to avail this facility user needs to delete the
+		// config file manually from configpath else default pricing still holds good.
+		fileName := filenameInConfigPath(pc.configFile.Path())
+		defaultPricing, err := ReturnPricingFromConfigs(fileName)
+		if err == nil {
+			pc.customPricing = defaultPricing
+		}
 
 		// Only write the file if flag enabled
 		if writeIfNotExists {
@@ -273,3 +286,32 @@ func configPathFor(filename string) string {
 	path := env.GetConfigPathWithDefault("/models/")
 	return gopath.Join(path, filename)
 }
+
+// Gives the config file name in a full qualified file name
+func filenameInConfigPath(fqfn string) string {
+	_, fileName := gopath.Split(fqfn)
+	return fileName
+}
+
+// ReturnPricingFromConfigs is a safe function to return pricing from configs of opensource to the closed source
+// before defaulting it with the above function DefaultPricing
+func ReturnPricingFromConfigs(filename string) (*CustomPricing, error) {
+	if _, err := os.Stat(closedSourceConfigMount); os.IsNotExist(err) {
+		return &CustomPricing{}, fmt.Errorf("ReturnPricingFromConfigs: %s likely running in provider config in opencost itself with err: %v", closedSourceConfigMount, err)
+	}
+	providerConfigFile := gopath.Join(closedSourceConfigMount, filename)
+	if _, err := os.Stat(providerConfigFile); err != nil {
+		return &CustomPricing{}, fmt.Errorf("ReturnPricingFromConfigs: unable to find file %s with err: %v", providerConfigFile, err)
+	}
+	configFile, err := ioutil.ReadFile(providerConfigFile)
+	if err != nil {
+		return &CustomPricing{}, fmt.Errorf("ReturnPricingFromConfigs: unable to open file %s with err: %v", providerConfigFile, err)
+	}
+
+	defaultPricing := &CustomPricing{}
+	err = json.Unmarshal(configFile, defaultPricing)
+	if err != nil {
+		return &CustomPricing{}, fmt.Errorf("ReturnPricingFromConfigs: unable to open file %s with err: %v", providerConfigFile, err)
+	}
+	return defaultPricing, nil
+}

pkg/costmodel/allocation_helpers.go

@@ -1588,7 +1588,7 @@ func (cm *CostModel) applyNodesToPod(podMap map[podKey]*pod, nodeMap map[nodeKey
 
 // getCustomNodePricing converts the CostModel's configured custom pricing
 // values into a nodePricing instance.
-func (cm *CostModel) getCustomNodePricing(spot bool) *nodePricing {
+func (cm *CostModel) getCustomNodePricing(spot bool, providerID string) *nodePricing {
 	customPricingConfig, err := cm.Provider.GetConfig()
 	if err != nil {
 		return nil
@@ -1603,7 +1603,10 @@ func (cm *CostModel) getCustomNodePricing(spot bool) *nodePricing {
 		ramCostStr = customPricingConfig.SpotRAM
 	}
 
-	node := &nodePricing{Source: "custom"}
+	node := &nodePricing{
+		Source:     "custom",
+		ProviderID: providerID,
+	}
 
 	costPerCPUHr, err := strconv.ParseFloat(cpuCostStr, 64)
 	if err != nil {
@@ -1639,7 +1642,7 @@ func (cm *CostModel) getNodePricing(nodeMap map[nodeKey]*nodePricing, nodeKey no
 		if nodeKey.Node != "" {
 			log.DedupedWarningf(5, "CostModel: failed to find node for %s", nodeKey)
 		}
-		return cm.getCustomNodePricing(false)
+		return cm.getCustomNodePricing(false, "")
 	}
 
 	// If custom pricing is enabled and can be retrieved, override detected
@@ -1649,7 +1652,7 @@ func (cm *CostModel) getNodePricing(nodeMap map[nodeKey]*nodePricing, nodeKey no
 		log.Warnf("CostModel: failed to load custom pricing: %s", err)
 	}
 	if cloud.CustomPricesEnabled(cm.Provider) && customPricingConfig != nil {
-		return cm.getCustomNodePricing(node.Preemptible)
+		return cm.getCustomNodePricing(node.Preemptible, node.ProviderID)
 	}
 
 	node.Source = "prometheus"

pkg/kubecost/allocation_test.go

@@ -4,9 +4,11 @@ import (
 	"fmt"
 	"math"
 	"reflect"
+	"strings"
 	"testing"
 	"time"
 
+	"github.com/davecgh/go-spew/spew"
 	"github.com/opencost/opencost/pkg/util"
 	"github.com/opencost/opencost/pkg/util/json"
 )
@@ -2767,3 +2769,82 @@ func TestAllocationSet_Accumulate_Equals_AllocationSetRange_Accumulate(t *testin
 		}
 	}
 }
+
+func Test_AggregateByService_UnmountedLBs(t *testing.T) {
+	end := time.Now().UTC().Truncate(day)
+	start := end.Add(-day)
+
+	normalProps := &AllocationProperties{
+		Cluster:        "cluster-one",
+		Container:      "nginx-plus-nginx-ingress",
+		Controller:     "nginx-plus-nginx-ingress",
+		ControllerKind: "deployment",
+		Namespace:      "nginx-plus",
+		Pod:            "nginx-plus-nginx-ingress-123a4b5678-ab12c",
+		ProviderID:     "test",
+		Node:           "testnode",
+		Services: []string{
+			"nginx-plus-nginx-ingress",
+		},
+	}
+
+	problematicProps := &AllocationProperties{
+		Cluster:    "cluster-one",
+		Container:  UnmountedSuffix,
+		Namespace:  UnmountedSuffix,
+		Pod:        UnmountedSuffix,
+		ProviderID: "test",
+		Node:       "testnode",
+		Services: []string{
+			"nginx-plus-nginx-ingress",
+			"ingress-nginx-controller",
+			"pacman",
+		},
+	}
+
+	idle := NewMockUnitAllocation(fmt.Sprintf("cluster-one/%s", IdleSuffix), start, day, &AllocationProperties{
+		Cluster: "cluster-one",
+	})
+	// this allocation is the main point of the test; an unmounted LB that has services
+	problematicAllocation := NewMockUnitAllocation("cluster-one//__unmounted__/__unmounted__/__unmounted__", start, day, problematicProps)
+
+	two := NewMockUnitAllocation("cluster-one//nginx-plus/nginx-plus-nginx-ingress-123a4b5678-ab12c/nginx-plus-nginx-ingress", start, day, normalProps)
+	three := NewMockUnitAllocation("cluster-one//nginx-plus/nginx-plus-nginx-ingress-123a4b5678-ab12c/nginx-plus-nginx-ingress", start, day, normalProps)
+	four := NewMockUnitAllocation("cluster-one//nginx-plus/nginx-plus-nginx-ingress-123a4b5678-ab12c/nginx-plus-nginx-ingress", start, day, normalProps)
+
+	problematicAllocation.ExternalCost = 2.35
+	two.ExternalCost = 1.35
+	three.ExternalCost = 2.60
+	four.ExternalCost = 4.30
+	set := NewAllocationSet(start, start.Add(day), problematicAllocation, two, three, four)
+
+	set.Insert(idle)
+
+	set.AggregateBy([]string{AllocationServiceProp}, &AllocationAggregationOptions{
+		Filter: AllocationFilterCondition{Field: FilterServices, Op: FilterContains, Value: "nginx-plus-nginx-ingress"},
+	})
+
+	for _, alloc := range set.Allocations {
+		if !strings.Contains(UnmountedSuffix, alloc.Name) {
+			props := alloc.Properties
+			if props.Cluster == UnmountedSuffix {
+				t.Error("cluster unmounted")
+			}
+			if props.Container == UnmountedSuffix {
+				t.Error("container unmounted")
+			}
+			if props.Namespace == UnmountedSuffix {
+				t.Error("namespace unmounted")
+			}
+			if props.Pod == UnmountedSuffix {
+				t.Error("pod unmounted")
+			}
+			if props.Controller == UnmountedSuffix {
+				t.Error("controller unmounted")
+			}
+		}
+	}
+
+	spew.Config.DisableMethods = true
+	t.Logf("%s", spew.Sdump(set.Allocations))
+}

pkg/kubecost/allocationprops.go

@@ -288,10 +288,18 @@ func (p *AllocationProperties) GenerateKey(aggregateBy []string, labelConfig *La
 				// Indicate that allocation has no services
 				names = append(names, UnallocatedSuffix)
 			} else {
-				// This just uses the first service
-				for _, service := range services {
-					names = append(names, service)
-					break
+				// Unmounted load balancers lead to __unmounted__ Allocations whose
+				// services field is populated. If we don't have a special case, the
+				// __unmounted__ Allocation will be transformed into a regular Allocation,
+				// causing issues with AggregateBy and drilldown
+				if p.Pod == UnmountedSuffix || p.Namespace == UnmountedSuffix || p.Container == UnmountedSuffix {
+					names = append(names, UnmountedSuffix)
+				} else {
+					// This just uses the first service
+					for _, service := range services {
+						names = append(names, service)
+						break
+					}
 				}
 			}
 		case strings.HasPrefix(agg, "label:"):

pkg/kubecost/allocationprops_test.go

@@ -96,6 +96,32 @@ func TestAllocationPropsIntersection(t *testing.T) {
 				Annotations:        map[string]string{"key2": "val2"},
 			},
 		},
+		"test services are nulled when intersecting": {
+			allocationProps1: &AllocationProperties{
+				AggregatedMetadata: false,
+				Container:          UnmountedSuffix,
+				Namespace:          "ns1",
+				Services: []string{
+					"cool",
+				},
+				Labels:      map[string]string{},
+				Annotations: map[string]string{},
+			},
+			allocationProps2: &AllocationProperties{
+				AggregatedMetadata: true,
+				Container:          "container3",
+				Namespace:          "ns1",
+				Labels:             map[string]string{"key1": "val1"},
+				Annotations:        map[string]string{"key2": "val2"},
+			},
+			expected: &AllocationProperties{
+				AggregatedMetadata: true,
+				Namespace:          "ns1",
+				ControllerKind:     "",
+				Labels:             map[string]string{"key1": "val1"},
+				Annotations:        map[string]string{"key2": "val2"},
+			},
+		},
 	}
 
 	for name, tc := range cases {