Ajay tripathy external label consistency (#695)

* Merge develop into master (#688)

* Split window.ToDurationOffset into DurationOffset and DurationOffsetStrings

* Check and register annotations collectors if enabled.

* add csv fallback (#641) (#646)

* add csv fallback

* log class match

* add counts by source

* add test for pricing source counter and make the names of sources public

* Fix the issue with empty pod name on annotation metric.

* Only Emit label_ and annotation_ metrics if they have values!

* Simplify label and annotation metric to labels.

* Add annotations costmodel

* Additional annotations additions

* Ajay tripathy pvc error fix (#644)

* add csv fallback

* log class match

* filter empty volumenames out

* Ajay tripathy fix e2custom (#623)

* pass offset to ccdr

* remove conflict

* add e2custom support

* revert cntext background change

* fix improperly named constant for govcloud lookup (#651)

* Use compatibility region implementation. (#653)

* Add filter by annotations to AggregateCostModelHandler

* WIP AWS idle investigation

* Fix bug with multiple filters on label and annotations

* Fix idle allocation bug for windows < 1h; improve DurationOffset string conversion

* Commit missing test file

* aggregate on annotations

* Merge master into develop. (#658)

* add csv fallback (#641)

* add csv fallback

* log class match

* add counts by source

* add test for pricing source counter and make the names of sources public

* Bump to version 1.71.1

* Bump version to 1.72.0 (#659)

* Aggregation by label

Updated AssetSet and AssetSetRange to aggregate by a new property aggStrings []string.
The props []AssetProperty value is still maintained on a given AssetSet, but rather than use this for aggregation,
we now use aggStrings, which can include values other than enumerated props. Specifically, strings prefixed with "label:"
are interpreted as labels and can be grouped on.

Strings in aggStrings which match the enumerated AssetProperty strings are stored in AssetSet.props, as before.

Also updated the relevant asset_test tests to call the Aggregation funcs with []string rather than []AssetProperty

* WIP add labels to nodes

* Now copying all labels into node objects

* Updates:

- Factored out `AssetSet.properties []AssetProperty`, which has been replaced by `AssetSet.aggregateBy []string`.

- Function `key()` now uses reserved word `__unallocated__` in keys for assets that do not have the given prop defined. Previous behavior was to omit that part of the key.

- Function `key()` now emits errors for given aggregation keys that are not either an `AssetProperty` or a string prefixed with `"label:"`.

- Updated tests to expect `__unallocated__` in relevant parts of keys.

* Fixes from Nikos comments

* Implement kubecost.AllocationSetRange.InsertRange and test; refactor custom approx implementations into util.IsApproximately for testing

* Returning single Error from key() func and disallowing grouping on empty label

* Capacity Optimizations (#664)

* Update to latest bingen read optimizations by allocating the required map space ahead of time. Apply to Properties as well.

* A few more smaller optimizations on Clone()

* simple pvfix (#663)

* Add annotation to allocation key

* go fmt

* Adds the concept of an AssetCredit to pkg/kubecost/asset

* undo go fmt

* Checking for errors from key() everywhere and made __undefined__ a constant

* process annotations in etl

* Simplified Cloud.Credit down to a simple float64 rather than objects containing credit metadata

* Allocation ETL: on-demand idle cost with unit testing

* Make AllocationSet.insert safer

* Ajay tripathy fix pvcalls (#667)

* simple pvfix

* Update to latest bingen read optimizations by allocating the required map space ahead of time. Apply to Properties as well.

* A few more smaller optimizations on Clone()

* fix extra PVlookups

Co-authored-by: Matt Bolt <mbolt35@gmail.com>

* Re-implemented 'propsEqual' check in accumulate(). Can't be exactly propsEqual() because they aren't props anymore...but wrote equivalent functions for []string

* Allocation ETL: on-demand external cost

* Allocation ETL: on-demand external cost; implement Properties.AggregationStrings

* Allocation ETL: on-demand external cost: fix Properties

* camel case json property

* refactor map merging function

* WIP logging for Allocation ETL: on-demand external cost

* Added a unit test for Aggregating by label.

* Added a comment explaining the nature of Credit

* Changed the label-key format to match allocation. Format for a label's key entry is now '/key=value/' rather than '/value/'

* undefined labels don't list key= before the __undefined__ value

* Now treating labels with value '' as unset labels

* Fix test broken in merge

* Ajay tripathy remove 2d cache (#668)

* simple pvfix

* Update to latest bingen read optimizations by allocating the required map space ahead of time. Apply to Properties as well.

* A few more smaller optimizations on Clone()

* fix extra PVlookups

* remove 2d cache

Co-authored-by: Matt Bolt <mbolt35@gmail.com>

* Active minutes query includes provider id

* Refactor ClusterNodes and use provider ID

See comments for more detailed explanation.

* Detailed comment describing buildNodeMap

* Unit test for mergeTypeMaps

* Initial simple test for buildNodeMap

* More buildNodeMap tests

* Renamed test file

* Removed check made obsolete by refactor

* Grammar and spelling fix in a comment

* Moved labels map into helper function

Also updated tests and followed the new query name
for preemptible, resNodeLabels -> resIsSpot.

* Allocation ETL: on-demand external cost: fix naming convention

* Fix bugs with external cost AggregateBy; fix test funcs; remove logs

* Update TODOs; add log

* Unit tests for E2 manual cost adjustment (#676)

* e2 fixes

* Added tests for CPU cost adjustment for e2

Co-authored-by: Ajay Tripathy <4tripathy@gmail.com>

* Bump version (#677)

* fix n2 prices (#679)

* Include adjustment in idle cost: write failing test

* Add adjustmentRate to ComputeIdleAllocations

* Add tests

* ComputeCostData gets CPU+RAM requests from k8s API

The intended result of this change is reducing load on
Prometheus for things it is not needed for. The one caveat
of this change is a modification of the output of data
for pods that stop existing in the cluster during the
query lookback window (which is currently 2 minutes).
Since we will be querying the k8s API at a time where
the pod does not exist, it will not have any information
about it. We choose to handle this by not outputting
request information, only usage information.

* Commented request emission special case better

* Better comment explaining units of memory and cpu

* K8s request stats include current timestamp

Necessary for the maxing op that occurs in
getContainerAllocation. The timestamps for
request and usage must be roughly equal.

* Read secret and save values to env variable

* logical fix

* Comment explaining manual timestamp

* OOC for azure on details

* add service account checks to Azure provider to notify front end of storage configuration

* fix bool name

* Update costmodelenv.go

Co-authored-by: Niko Kovacevic <nikovacevic@gmail.com>
Co-authored-by: Matt Bolt <mbolt35@gmail.com>
Co-authored-by: Sean Holcomb <seanholcomb@gmail.com>
Co-authored-by: Neal Ormsbee <neal.ormsbee@gmail.com>
Co-authored-by: Sean Holcomb <sean@kubecost.com>
Co-authored-by: Michael Dresser <michael@kubecost.com>

* fix govcloud issue where wrong capacitystatus would sometimes be used

* assets store transformed labels

Co-authored-by: Niko Kovacevic <nikovacevic@gmail.com>
Co-authored-by: Matt Bolt <mbolt35@gmail.com>
Co-authored-by: Sean Holcomb <seanholcomb@gmail.com>
Co-authored-by: Neal Ormsbee <neal.ormsbee@gmail.com>
Co-authored-by: Sean Holcomb <sean@kubecost.com>
Co-authored-by: Michael Dresser <michael@kubecost.com>

pkg/kubecost/asset.go

@@ -64,6 +64,35 @@ type Asset interface {
 // the properties to use to aggregate, and the mapping from Allocation property
 // to Asset label. For example, consider this asset:
 //
+// CURRENT: Asset ETL stores its data ALREADY MAPPED from label to k8s concept. This isn't ideal-- see the TOOD.
+//   Cloud {
+// 	   TotalCost: 10.00,
+// 	   Labels{
+//       "kubernetes_namespace":"monitoring",
+// 	     "env":"prod"
+// 	   }
+//   }
+//
+// Given the following parameters, we expect to return:
+//
+//   1) single-prop full match
+//   aggregateBy = ["namespace"]
+//   => Allocation{Name: "monitoring", ExternalCost: 10.00, TotalCost: 10.00}, nil
+//
+//   2) multi-prop full match
+//   aggregateBy = ["namespace", "label:env"]
+//   allocationPropertyLabels = {"namespace":"kubernetes_namespace"}
+//   => Allocation{Name: "monitoring/env=prod", ExternalCost: 10.00, TotalCost: 10.00}, nil
+//
+//   3) multi-prop partial match
+//   aggregateBy = ["namespace", "label:foo"]
+//   => Allocation{Name: "monitoring/__unallocated__", ExternalCost: 10.00, TotalCost: 10.00}, nil
+//
+//   4) no match
+//   aggregateBy = ["cluster"]
+//   => nil, err
+//
+// TODO:
 //   Cloud {
 // 	   TotalCost: 10.00,
 // 	   Labels{
@@ -95,7 +124,7 @@ type Asset interface {
 //   => nil, err
 //
 // (See asset_test.go for assertions of these examples and more.)
-func AssetToExternalAllocation(asset Asset, aggregateBy []string, allocationPropertyLabels map[string]string) (*Allocation, error) {
+func AssetToExternalAllocation(asset Asset, aggregateBy []string) (*Allocation, error) {
 	if asset == nil {
 		return nil, fmt.Errorf("asset is nil")
 	}
@@ -115,7 +144,7 @@ func AssetToExternalAllocation(asset Asset, aggregateBy []string, allocationProp
 		// labelName should be derived from the mapping of properties to
 		// label names, unless the aggBy is explicitly a label, in which
 		// case we should pull the label name from the aggBy string.
-		labelName := allocationPropertyLabels[aggBy]
+		labelName := aggBy
 		if strings.HasPrefix(aggBy, "label:") {
 			labelName = strings.TrimPrefix(aggBy, "label:")
 		}

pkg/kubecost/asset_test.go

@@ -1027,11 +1027,7 @@ func TestAssetToExternalAllocation(t *testing.T) {
 	var alloc *Allocation
 	var err error
 
-	// default allocationPropertyLabels, which should be compatible with result
-	// of LabelConfig.AllocationPropertyLabels()
-	apls := map[string]string{"namespace": "kubernetes_namespace"}
-
-	alloc, err = AssetToExternalAllocation(asset, []string{"namespace"}, apls)
+	alloc, err = AssetToExternalAllocation(asset, []string{"namespace"})
 	if err == nil {
 		t.Fatalf("expected error due to nil asset")
 	}
@@ -1046,19 +1042,17 @@ func TestAssetToExternalAllocation(t *testing.T) {
 	//   }
 	cloud := NewCloud(ComputeCategory, "abc123", start1, start2, windows[0])
 	cloud.SetLabels(map[string]string{
-		"kubernetes_namespace": "monitoring",
-		"env":                  "prod",
+		"namespace": "monitoring",
+		"env":       "prod",
 	})
 	cloud.Cost = 10.00
 	asset = cloud
 
-	// Providing nil params with a non-nil Asset should not panic, but it
-	// should return an error in both cases (no matching is possible).
-	alloc, err = AssetToExternalAllocation(asset, []string{"namespace"}, nil)
-	if err == nil {
-		t.Fatalf("expected error due to nil allocationPropertyLabels")
+	alloc, err = AssetToExternalAllocation(asset, []string{"namespace"})
+	if err != nil {
+		t.Fatalf("expected to not error")
 	}
-	alloc, err = AssetToExternalAllocation(asset, nil, apls)
+	alloc, err = AssetToExternalAllocation(asset, nil)
 	if err == nil {
 		t.Fatalf("expected error due to nil aggregateBy")
 	}
@@ -1086,7 +1080,7 @@ func TestAssetToExternalAllocation(t *testing.T) {
 	//   => nil, err
 
 	// 1) single-prop full match
-	alloc, err = AssetToExternalAllocation(asset, []string{"namespace"}, apls)
+	alloc, err = AssetToExternalAllocation(asset, []string{"namespace"})
 	if err != nil {
 		t.Fatalf("unexpected error: %s", err)
 	}
@@ -1104,7 +1098,7 @@ func TestAssetToExternalAllocation(t *testing.T) {
 	}
 
 	// 2) multi-prop full match
-	alloc, err = AssetToExternalAllocation(asset, []string{"namespace", "label:env"}, apls)
+	alloc, err = AssetToExternalAllocation(asset, []string{"namespace", "label:env"})
 	if err != nil {
 		t.Fatalf("unexpected error: %s", err)
 	}
@@ -1125,7 +1119,7 @@ func TestAssetToExternalAllocation(t *testing.T) {
 	}
 
 	// 3) multi-prop partial match
-	alloc, err = AssetToExternalAllocation(asset, []string{"namespace", "label:foo"}, apls)
+	alloc, err = AssetToExternalAllocation(asset, []string{"namespace", "label:foo"})
 	if err != nil {
 		t.Fatalf("unexpected error: %s", err)
 	}
@@ -1143,7 +1137,7 @@ func TestAssetToExternalAllocation(t *testing.T) {
 	}
 
 	// 3) no match
-	alloc, err = AssetToExternalAllocation(asset, []string{"cluster"}, apls)
+	alloc, err = AssetToExternalAllocation(asset, []string{"cluster"})
 	if err == nil {
 		t.Fatalf("expected 'no match' error")
 	}