Merge branch 'master' of https://github.com/kubecost/cost-model into AjayTripathy-gpu-allocation

pkg/cloud/awsprovider.go

@@ -314,7 +314,7 @@ var regionToBillingRegionCode = map[string]string{
 var loadedAWSSecret bool = false
 var awsSecret *AWSAccessKey = nil
 
-func (aws *AWS) GetLocalStorageQuery(window, offset string, rate bool, used bool) string {
+func (aws *AWS) GetLocalStorageQuery(window, offset time.Duration, rate bool, used bool) string {
 	return ""
 }
 
@@ -366,14 +366,17 @@ func (aws *AWS) GetManagementPlatform() (string, error) {
 
 func (aws *AWS) GetConfig() (*CustomPricing, error) {
 	c, err := aws.Config.GetCustomPricingData()
+	if err != nil {
+		return nil, err
+	}
 	if c.Discount == "" {
 		c.Discount = "0%"
 	}
 	if c.NegotiatedDiscount == "" {
 		c.NegotiatedDiscount = "0%"
 	}
-	if err != nil {
-		return nil, err
+	if c.ShareTenancyCosts == "" {
+		c.ShareTenancyCosts = defaultShareTenancyCost
 	}
 
 	return c, nil
@@ -1754,14 +1757,14 @@ func (a *AWS) GetSavingsPlanDataFromAthena() error {
 	end := tNow.Format("2006-01-02")
 	// Use Savings Plan Effective Rate as an estimation for cost, assuming the 1h most recent period got a fully loaded savings plan.
 	//
-	q := `SELECT   
+	q := `SELECT
 		line_item_usage_start_date,
 		savings_plan_savings_plan_a_r_n,
 		line_item_resource_id,
-		savings_plan_savings_plan_rate 
+		savings_plan_savings_plan_rate
 	FROM %s as cost_data
 	WHERE line_item_usage_start_date BETWEEN date '%s' AND date '%s'
-	AND line_item_line_item_type = 'SavingsPlanCoveredUsage' ORDER BY 
+	AND line_item_line_item_type = 'SavingsPlanCoveredUsage' ORDER BY
 	line_item_usage_start_date DESC`
 
 	page := 0
@@ -1844,14 +1847,14 @@ func (a *AWS) GetReservationDataFromAthena() error {
 		tOneDayAgo := tNow.Add(time.Duration(-25) * time.Hour) // Also get files from one day ago to avoid boundary conditions
 		start := tOneDayAgo.Format("2006-01-02")
 		end := tNow.Format("2006-01-02")
-		q := `SELECT   
+		q := `SELECT
 		line_item_usage_start_date,
 		reservation_reservation_a_r_n,
 		line_item_resource_id,
 		reservation_effective_cost
 	FROM %s as cost_data
 	WHERE line_item_usage_start_date BETWEEN date '%s' AND date '%s'
-	AND reservation_reservation_a_r_n <> '' ORDER BY 
+	AND reservation_reservation_a_r_n <> '' ORDER BY
 	line_item_usage_start_date DESC`
 		query := fmt.Sprintf(q, cfg.AthenaTable, start, end)
 		op, err := a.QueryAthenaBillingData(query)
@@ -1967,19 +1970,19 @@ func (a *AWS) ExternalAllocations(start string, end string, aggregators []string
 	if filterType != "kubernetes_" { // This gets appended upstream and is equivalent to no filter.
 		lastIdx = len(formattedAggregators) + 3
 		groupby := generateAWSGroupBy(lastIdx)
-		query = fmt.Sprintf(`SELECT   
+		query = fmt.Sprintf(`SELECT
 			CAST(line_item_usage_start_date AS DATE) as start_date,
 			%s,
 			line_item_product_code,
 			%s,
 			SUM(line_item_blended_cost) as blended_cost
 		FROM %s as cost_data
-		WHERE (%s='%s') AND line_item_usage_start_date BETWEEN date '%s' AND date '%s' AND (%s) 
+		WHERE (%s='%s') AND line_item_usage_start_date BETWEEN date '%s' AND date '%s' AND (%s)
 		GROUP BY %s`, aggregatorNames, filter_column_name, customPricing.AthenaTable, filter_column_name, filterValue, start, end, aggregatorOr, groupby)
 	} else {
 		lastIdx = len(formattedAggregators) + 2
 		groupby := generateAWSGroupBy(lastIdx)
-		query = fmt.Sprintf(`SELECT   
+		query = fmt.Sprintf(`SELECT
 			CAST(line_item_usage_start_date AS DATE) as start_date,
 			%s,
 			line_item_product_code,
@@ -2359,42 +2362,3 @@ func (a *AWS) ServiceAccountStatus() *ServiceAccountStatus {
 func (aws *AWS) CombinedDiscountForNode(instanceType string, isPreemptible bool, defaultDiscount, negotiatedDiscount float64) float64 {
 	return 1.0 - ((1.0 - defaultDiscount) * (1.0 - negotiatedDiscount))
 }
-
-func (aws *AWS) ParseID(id string) string {
-	// It's of the form aws:///us-east-2a/i-0fea4fd46592d050b and we want i-0fea4fd46592d050b, if it exists
-	rx := regexp.MustCompile("aws://[^/]*/[^/]*/([^/]+)")
-	match := rx.FindStringSubmatch(id)
-	if len(match) < 2 {
-		if id != "" {
-			log.Infof("awsprovider.ParseID: failed to parse %s", id)
-		}
-		return id
-	}
-
-	return match[1]
-}
-
-func (aws *AWS) ParsePVID(id string) string {
-	rx := regexp.MustCompile("aws:/[^/]*/[^/]*/([^/]+)") // Capture "vol-0fc54c5e83b8d2b76" from "aws://us-east-2a/vol-0fc54c5e83b8d2b76"
-	match := rx.FindStringSubmatch(id)
-	if len(match) < 2 {
-		if id != "" {
-			log.Infof("awsprovider.ParseID: failed to parse %s", id)
-		}
-		return id
-	}
-
-	return match[1]
-}
-
-func (aws *AWS) ParseLBID(id string) string {
-	rx := regexp.MustCompile("^([^-]+)-.+$") // Capture "ad9d88195b52a47c89b5055120f28c58" from "ad9d88195b52a47c89b5055120f28c58-1037804914.us-east-2.elb.amazonaws.com"
-	match := rx.FindStringSubmatch(id)
-	if len(match) < 2 {
-		if id != "" {
-			log.Infof("awsprovider.ParseLBID: failed to parse %s, %v", id, match)
-		}
-		return id
-	}
-	return match[1]
-}

pkg/cloud/azureprovider.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"encoding/csv"
 	"fmt"
-	"github.com/kubecost/cost-model/pkg/kubecost"
 	"io"
 	"io/ioutil"
 	"regexp"
@@ -15,6 +14,7 @@ import (
 
 	"github.com/kubecost/cost-model/pkg/clustercache"
 	"github.com/kubecost/cost-model/pkg/env"
+	"github.com/kubecost/cost-model/pkg/kubecost"
 	"github.com/kubecost/cost-model/pkg/util"
 	"github.com/kubecost/cost-model/pkg/util/json"
 
@@ -265,9 +265,10 @@ func (k *azureKey) GetGPUCount() string {
 
 // Represents an azure storage config
 type AzureStorageConfig struct {
-	AccountName   string `json:"azureStorageAccount"`
-	AccessKey     string `json:"azureStorageAccessKey"`
-	ContainerName string `json:"azureStorageContainer"`
+	SubscriptionId string `json:"azureSubscriptionID"`
+	AccountName    string `json:"azureStorageAccount"`
+	AccessKey      string `json:"azureStorageAccessKey"`
+	ContainerName  string `json:"azureStorageContainer"`
 }
 
 // Represents an azure app key
@@ -756,13 +757,13 @@ func (az *Azure) NodePricing(key Key) (*Node, error) {
 	if err != nil {
 		return nil, fmt.Errorf("No default pricing data available")
 	}
-	if azKey.isValidGPUNode()  {
+	if azKey.isValidGPUNode() {
 		return &Node{
-			VCPUCost: c.CPU,
-			RAMCost:  c.RAM,
+			VCPUCost:         c.CPU,
+			RAMCost:          c.RAM,
 			UsesBaseCPUPrice: true,
-			GPUCost:  c.GPU,
-			GPU:      azKey.GetGPUCount(),
+			GPUCost:          c.GPU,
+			GPU:              azKey.GetGPUCount(),
 		}, nil
 	}
 	return &Node{
@@ -942,6 +943,9 @@ func (az *Azure) UpdateConfig(r io.Reader, updateType string) (*CustomPricing, e
 }
 func (az *Azure) GetConfig() (*CustomPricing, error) {
 	c, err := az.Config.GetCustomPricingData()
+	if err != nil {
+		return nil, err
+	}
 	if c.Discount == "" {
 		c.Discount = "0%"
 	}
@@ -954,8 +958,8 @@ func (az *Azure) GetConfig() (*CustomPricing, error) {
 	if c.AzureBillingRegion == "" {
 		c.AzureBillingRegion = "US"
 	}
-	if err != nil {
-		return nil, err
+	if c.ShareTenancyCosts == "" {
+		c.ShareTenancyCosts = defaultShareTenancyCost
 	}
 	return c, nil
 }
@@ -1127,7 +1131,7 @@ func (az *Azure) PVPricing(pvk PVKey) (*PV, error) {
 	return pricing.PV, nil
 }
 
-func (az *Azure) GetLocalStorageQuery(window, offset string, rate bool, used bool) string {
+func (az *Azure) GetLocalStorageQuery(window, offset time.Duration, rate bool, used bool) string {
 	return ""
 }
 
@@ -1152,15 +1156,3 @@ func (*Azure) ClusterManagementPricing() (string, float64, error) {
 func (az *Azure) CombinedDiscountForNode(instanceType string, isPreemptible bool, defaultDiscount, negotiatedDiscount float64) float64 {
 	return 1.0 - ((1.0 - defaultDiscount) * (1.0 - negotiatedDiscount))
 }
-
-func (az *Azure) ParseID(id string) string {
-	return id
-}
-
-func (az *Azure) ParsePVID(id string) string {
-	return id
-}
-
-func (az *Azure) ParseLBID(id string) string {
-	return id
-}

pkg/cloud/csvprovider.go

@@ -366,15 +366,3 @@ func (*CSVProvider) ClusterManagementPricing() (string, float64, error) {
 func (c *CSVProvider) CombinedDiscountForNode(instanceType string, isPreemptible bool, defaultDiscount, negotiatedDiscount float64) float64 {
 	return 1.0 - ((1.0 - defaultDiscount) * (1.0 - negotiatedDiscount))
 }
-
-func (c *CSVProvider) ParseID(id string) string {
-	return id
-}
-
-func (c *CSVProvider) ParsePVID(id string) string {
-	return id
-}
-
-func (c *CSVProvider) ParseLBID(id string) string {
-	return id
-}

pkg/cloud/customprovider.go

@@ -5,6 +5,7 @@ import (
 	"strconv"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/kubecost/cost-model/pkg/clustercache"
 	"github.com/kubecost/cost-model/pkg/env"
@@ -42,7 +43,7 @@ func (*CustomProvider) ClusterManagementPricing() (string, float64, error) {
 	return "", 0.0, nil
 }
 
-func (*CustomProvider) GetLocalStorageQuery(window, offset string, rate bool, used bool) string {
+func (*CustomProvider) GetLocalStorageQuery(window, offset time.Duration, rate bool, used bool) string {
 	return ""
 }
 
@@ -311,15 +312,3 @@ func (cp *CustomProvider) PricingSourceStatus() map[string]*PricingSource {
 func (cp *CustomProvider) CombinedDiscountForNode(instanceType string, isPreemptible bool, defaultDiscount, negotiatedDiscount float64) float64 {
 	return 1.0 - ((1.0 - defaultDiscount) * (1.0 - negotiatedDiscount))
 }
-
-func (cp *CustomProvider) ParseID(id string) string {
-	return id
-}
-
-func (cp *CustomProvider) ParsePVID(id string) string {
-	return id
-}
-
-func (cp *CustomProvider) ParseLBID(id string) string {
-	return id
-}

pkg/cloud/gcpprovider.go

@@ -13,22 +13,21 @@ import (
 	"sync"
 	"time"
 
-	"k8s.io/klog"
-
-	"cloud.google.com/go/bigquery"
-	"cloud.google.com/go/compute/metadata"
-
 	"github.com/kubecost/cost-model/pkg/clustercache"
 	"github.com/kubecost/cost-model/pkg/env"
 	"github.com/kubecost/cost-model/pkg/log"
 	"github.com/kubecost/cost-model/pkg/util"
 	"github.com/kubecost/cost-model/pkg/util/json"
+	"github.com/kubecost/cost-model/pkg/util/timeutil"
 
+	"cloud.google.com/go/bigquery"
+	"cloud.google.com/go/compute/metadata"
 	"golang.org/x/oauth2"
 	"golang.org/x/oauth2/google"
 	compute "google.golang.org/api/compute/v1"
 	"google.golang.org/api/iterator"
 	v1 "k8s.io/api/core/v1"
+	"k8s.io/klog"
 )
 
 const GKE_GPU_TAG = "cloud.google.com/gke-accelerator"
@@ -124,7 +123,7 @@ func gcpAllocationToOutOfClusterAllocation(gcpAlloc gcpAllocation) *OutOfCluster
 
 // GetLocalStorageQuery returns the cost of local storage for the given window. Setting rate=true
 // returns hourly spend. Setting used=true only tracks used storage, not total.
-func (gcp *GCP) GetLocalStorageQuery(window, offset string, rate bool, used bool) string {
+func (gcp *GCP) GetLocalStorageQuery(window, offset time.Duration, rate bool, used bool) string {
 	// TODO Set to the price for the appropriate storage class. It's not trivial to determine the local storage disk type
 	// See https://cloud.google.com/compute/disks-image-pricing#persistentdisk
 	localStorageCost := 0.04
@@ -134,10 +133,7 @@ func (gcp *GCP) GetLocalStorageQuery(window, offset string, rate bool, used bool
 		baseMetric = "container_fs_usage_bytes"
 	}
 
-	fmtOffset := ""
-	if offset != "" {
-		fmtOffset = fmt.Sprintf("offset %s", offset)
-	}
+	fmtOffset := timeutil.DurationToPromOffsetString(offset)
 
 	fmtCumulativeQuery := `sum(
 		sum_over_time(%s{device!="tmpfs", id="/"}[%s:1m]%s)
@@ -151,8 +147,9 @@ func (gcp *GCP) GetLocalStorageQuery(window, offset string, rate bool, used bool
 	if rate {
 		fmtQuery = fmtMonthlyQuery
 	}
+	fmtWindow := timeutil.DurationString(window)
 
-	return fmt.Sprintf(fmtQuery, baseMetric, window, fmtOffset, env.GetPromClusterLabel(), localStorageCost)
+	return fmt.Sprintf(fmtQuery, baseMetric, fmtWindow, fmtOffset, env.GetPromClusterLabel(), localStorageCost)
 }
 
 func (gcp *GCP) GetConfig() (*CustomPricing, error) {
@@ -169,6 +166,9 @@ func (gcp *GCP) GetConfig() (*CustomPricing, error) {
 	if c.CurrencyCode == "" {
 		c.CurrencyCode = "USD"
 	}
+	if c.ShareTenancyCosts == "" {
+		c.ShareTenancyCosts = defaultShareTenancyCost
+	}
 	return c, nil
 }
 
@@ -1472,26 +1472,3 @@ func sustainedUseDiscount(class string, defaultDiscount float64, isPreemptible b
 	}
 	return discount
 }
-
-func (gcp *GCP) ParseID(id string) string {
-	// gce://guestbook-227502/us-central1-a/gke-niko-n1-standard-2-wljla-8df8e58a-hfy7
-	//  => gke-niko-n1-standard-2-wljla-8df8e58a-hfy7
-	rx := regexp.MustCompile("gce://[^/]*/[^/]*/([^/]+)")
-	match := rx.FindStringSubmatch(id)
-	if len(match) < 2 {
-		if id != "" {
-			log.Infof("gcpprovider.ParseID: failed to parse %s", id)
-		}
-		return id
-	}
-
-	return match[1]
-}
-
-func (gcp *GCP) ParsePVID(id string) string {
-	return id
-}
-
-func (gcp *GCP) ParseLBID(id string) string {
-	return id
-}

pkg/cloud/provider.go

@@ -5,7 +5,9 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"regexp"
 	"strings"
+	"time"
 
 	"k8s.io/klog"
 
@@ -19,6 +21,7 @@ import (
 
 const authSecretPath = "/var/secrets/service-key.json"
 const storageConfigSecretPath = "/var/azure-storage-config/azure-storage-config.json"
+const defaultShareTenancyCost = "true"
 
 var createTableStatements = []string{
 	`CREATE TABLE IF NOT EXISTS names (
@@ -174,6 +177,7 @@ type CustomPricing struct {
 	SharedNamespaces             string            `json:"sharedNamespaces"`
 	SharedLabelNames             string            `json:"sharedLabelNames"`
 	SharedLabelValues            string            `json:"sharedLabelValues"`
+	ShareTenancyCosts            string            `json:"shareTenancyCosts"` // TODO clean up configuration so we can use a type other that string (this should be a bool, but the app panics if it's not a string)
 	ReadOnly                     string            `json:"readOnly"`
 	KubecostToken                string            `json:"kubecostToken"`
 }
@@ -185,7 +189,7 @@ type ServiceAccountStatus struct {
 type ServiceAccountCheck struct {
 	Message        string `json:"message"`
 	Status         bool   `json:"status"`
-	AdditionalInfo string `json:additionalInfo`
+	AdditionalInfo string `json:"additionalInfo"`
 }
 
 type PricingSources struct {
@@ -232,16 +236,13 @@ type Provider interface {
 	UpdateConfigFromConfigMap(map[string]string) (*CustomPricing, error)
 	GetConfig() (*CustomPricing, error)
 	GetManagementPlatform() (string, error)
-	GetLocalStorageQuery(string, string, bool, bool) string
+	GetLocalStorageQuery(time.Duration, time.Duration, bool, bool) string
 	ExternalAllocations(string, string, []string, string, string, bool) ([]*OutOfClusterAllocation, error)
 	ApplyReservedInstancePricing(map[string]*Node)
 	ServiceAccountStatus() *ServiceAccountStatus
 	PricingSourceStatus() map[string]*PricingSource
 	ClusterManagementPricing() (string, float64, error)
 	CombinedDiscountForNode(string, bool, float64, float64) float64
-	ParseID(string) string
-	ParsePVID(string) string
-	ParseLBID(string) string
 }
 
 // ClusterName returns the name defined in cluster info, defaulting to the
@@ -335,6 +336,17 @@ func SharedLabels(p Provider) ([]string, []string) {
 	return names, values
 }
 
+// ShareTenancyCosts returns true if the application settings specify to share
+// tenancy costs by default.
+func ShareTenancyCosts(p Provider) bool {
+	config, err := p.GetConfig()
+	if err != nil {
+		return false
+	}
+
+	return config.ShareTenancyCosts == "true"
+}
+
 func NewCrossClusterProvider(ctype string, overrideConfigPath string, cache clustercache.ClusterCache) (Provider, error) {
 	if ctype == "aws" {
 		return &AWS{
@@ -346,6 +358,11 @@ func NewCrossClusterProvider(ctype string, overrideConfigPath string, cache clus
 			Clientset: cache,
 			Config:    NewProviderConfig(overrideConfigPath),
 		}, nil
+	} else if ctype == "azure" {
+		return &Azure{
+			Clientset: cache,
+			Config:    NewProviderConfig(overrideConfigPath),
+		}, nil
 	}
 	return &CustomProvider{
 		Clientset: cache,
@@ -501,3 +518,52 @@ func GetOrCreateClusterMeta(cluster_id, cluster_name string) (string, string, er
 
 	return id, name, nil
 }
+
+// ParseID attempts to parse a ProviderId from a string based on formats from the various providers and
+// returns the string as is if it cannot find a match
+func ParseID(id string) string {
+	// It's of the form aws:///us-east-2a/i-0fea4fd46592d050b and we want i-0fea4fd46592d050b, if it exists
+	rx := regexp.MustCompile("aws://[^/]*/[^/]*/([^/]+)")
+	match := rx.FindStringSubmatch(id)
+	if len(match) >= 2 {
+		return match[1]
+	}
+
+	// gce://guestbook-227502/us-central1-a/gke-niko-n1-standard-2-wljla-8df8e58a-hfy7
+	//  => gke-niko-n1-standard-2-wljla-8df8e58a-hfy7
+	rx = regexp.MustCompile("gce://[^/]*/[^/]*/([^/]+)")
+	match = rx.FindStringSubmatch(id)
+	if len(match) >= 2 {
+		return match[1]
+	}
+
+	// Return id for Azure Provider, CSV Provider and Custom Provider
+	return id
+}
+
+// ParsePVID attempts to parse a PV ProviderId from a string based on formats from the various providers and
+// returns the string as is if it cannot find a match
+func ParsePVID(id string) string {
+	// Capture "vol-0fc54c5e83b8d2b76" from "aws://us-east-2a/vol-0fc54c5e83b8d2b76"
+	rx := regexp.MustCompile("aws:/[^/]*/[^/]*/([^/]+)")
+	match := rx.FindStringSubmatch(id)
+	if len(match) >= 2 {
+		return match[1]
+	}
+
+	// Return id for GCP Provider, Azure Provider, CSV Provider and Custom Provider
+	return id
+}
+
+// ParseLBID attempts to parse a LB ProviderId from a string based on formats from the various providers and
+// returns the string as is if it cannot find a match
+func ParseLBID(id string) string {
+	rx := regexp.MustCompile("^([^-]+)-.+amazonaws\\.com$") // Capture "ad9d88195b52a47c89b5055120f28c58" from "ad9d88195b52a47c89b5055120f28c58-1037804914.us-east-2.elb.amazonaws.com"
+	match := rx.FindStringSubmatch(id)
+	if len(match) >= 2 {
+		return match[1]
+	}
+
+	// Return id for GCP Provider, Azure Provider, CSV Provider and Custom Provider
+	return id
+}

pkg/cloud/providerconfig.go

@@ -92,6 +92,11 @@ func (pc *ProviderConfig) loadConfig(writeIfNotExists bool) (*CustomPricing, err
 	if pc.customPricing.SpotGPU == "" {
 		pc.customPricing.SpotGPU = DefaultPricing().SpotGPU // Migration for users without this value set by default.
 	}
+
+	if pc.customPricing.ShareTenancyCosts == "" {
+		pc.customPricing.ShareTenancyCosts = defaultShareTenancyCost
+	}
+
 	return pc.customPricing, nil
 }
 
@@ -177,6 +182,7 @@ func DefaultPricing() *CustomPricing {
 		RegionNetworkEgress:   "0.01",
 		InternetNetworkEgress: "0.12",
 		CustomPricesEnabled:   "false",
+		ShareTenancyCosts:     "true",
 	}
 }
 

pkg/costmodel/aggregation.go

@@ -2,6 +2,7 @@ package costmodel
 
 import (
 	"fmt"
+	"github.com/kubecost/cost-model/pkg/util/timeutil"
 	"math"
 	"net/http"
 	"regexp"
@@ -118,9 +119,9 @@ func (a *Aggregation) RateCoefficient(rateStr string, resolutionHours float64) f
 	coeff := 1.0
 	switch rateStr {
 	case "daily":
-		coeff = util.HoursPerDay
+		coeff = timeutil.HoursPerDay
 	case "monthly":
-		coeff = util.HoursPerMonth
+		coeff = timeutil.HoursPerMonth
 	}
 
 	return coeff / a.TotalHours(resolutionHours)
@@ -195,7 +196,7 @@ func GetTotalContainerCost(costData map[string]*CostData, rate string, cp cloud.
 	return totalContainerCost
 }
 
-func (a *Accesses) ComputeIdleCoefficient(costData map[string]*CostData, cli prometheusClient.Client, cp cloud.Provider, discount float64, customDiscount float64, windowString, offset string) (map[string]float64, error) {
+func (a *Accesses) ComputeIdleCoefficient(costData map[string]*CostData, cli prometheusClient.Client, cp cloud.Provider, discount float64, customDiscount float64, window, offset time.Duration) (map[string]float64, error) {
 	coefficients := make(map[string]float64)
 
 	profileName := "ComputeIdleCoefficient: ComputeClusterCosts"
@@ -203,12 +204,12 @@ func (a *Accesses) ComputeIdleCoefficient(costData map[string]*CostData, cli pro
 
 	var clusterCosts map[string]*ClusterCosts
 	var err error
-
-	key := fmt.Sprintf("%s:%s", windowString, offset)
+	fmtWindow, fmtOffset := timeutil.DurationOffsetStrings(window, offset)
+	key := fmt.Sprintf("%s:%s", fmtWindow, fmtOffset)
 	if data, valid := a.ClusterCostsCache.Get(key); valid {
 		clusterCosts = data.(map[string]*ClusterCosts)
 	} else {
-		clusterCosts, err = a.ComputeClusterCosts(cli, cp, windowString, offset, false)
+		clusterCosts, err = a.ComputeClusterCosts(cli, cp, window, offset, false)
 		if err != nil {
 			return nil, err
 		}
@@ -224,7 +225,7 @@ func (a *Accesses) ComputeIdleCoefficient(costData map[string]*CostData, cli pro
 		}
 
 		if costs.TotalCumulative == 0 {
-			return nil, fmt.Errorf("TotalCumulative cluster cost for cluster '%s' returned 0 over window '%s' offset '%s'", cid, windowString, offset)
+			return nil, fmt.Errorf("TotalCumulative cluster cost for cluster '%s' returned 0 over window '%s' offset '%s'", cid, fmtWindow, fmtOffset)
 		}
 
 		totalContainerCost := 0.0
@@ -1452,7 +1453,7 @@ func (a *Accesses) ComputeAggregateCostModel(promClient prometheusClient.Client,
 	if !disableSharedOverhead {
 		for key, val := range c.SharedCosts {
 			cost, err := strconv.ParseFloat(val, 64)
-			durationCoefficient := window.Hours() / util.HoursPerMonth
+			durationCoefficient := window.Hours() / timeutil.HoursPerMonth
 			if err != nil {
 				return nil, "", fmt.Errorf("unable to parse shared cost %s: %s", val, err)
 			}
@@ -1491,11 +1492,9 @@ func (a *Accesses) ComputeAggregateCostModel(promClient prometheusClient.Client,
 			}
 		}
 
-		// Convert to Prometheus-compatible strings
-		durStr, offStr := util.DurationOffsetStrings(dur, off)
-
-		idleCoefficients, err = a.ComputeIdleCoefficient(costData, promClient, a.CloudProvider, discount, customDiscount, durStr, offStr)
+		idleCoefficients, err = a.ComputeIdleCoefficient(costData, promClient, a.CloudProvider, discount, customDiscount, dur, off)
 		if err != nil {
+			durStr, offStr := timeutil.DurationOffsetStrings(dur, off)
 			log.Errorf("ComputeAggregateCostModel: error computing idle coefficient: duration=%s, offset=%s, err=%s", durStr, offStr, err)
 			return nil, "", err
 		}
@@ -1743,10 +1742,16 @@ func (a *Accesses) warmAggregateCostModelCache() {
 	// for the given duration. Cache is intentionally set to expire (i.e. noExpireCache=false) so that
 	// if the default parameters change, the old cached defaults with eventually expire. Thus, the
 	// timing of the cache expiry/refresh is the only mechanism ensuring 100% cache warmth.
-	warmFunc := func(duration, durationHrs, offset string, cacheEfficiencyData bool) (error, error) {
+	warmFunc := func(duration, offset time.Duration, cacheEfficiencyData bool) (error, error) {
+		if a.ThanosClient != nil {
+			duration = thanos.OffsetDuration()
+			log.Infof("Setting Offset to %s", duration)
+		}
+		fmtDuration, fmtOffset := timeutil.DurationOffsetStrings(duration, offset)
+		durationHrs, err := timeutil.FormatDurationStringDaysToHours(fmtDuration)
 		promClient := a.GetPrometheusClient(true)
 
-		windowStr := fmt.Sprintf("%s offset %s", duration, offset)
+		windowStr := fmt.Sprintf("%s offset %s", fmtDuration, fmtOffset)
 		window, err := kubecost.ParseWindowUTC(windowStr)
 		if err != nil {
 			return nil, fmt.Errorf("invalid window from window string: %s", windowStr)
@@ -1777,17 +1782,15 @@ func (a *Accesses) warmAggregateCostModelCache() {
 
 		aggKey := GenerateAggKey(window, field, subfields, aggOpts)
 		log.Infof("aggregation: cache warming defaults: %s", aggKey)
-		key := fmt.Sprintf("%s:%s", durationHrs, offset)
+		key := fmt.Sprintf("%s:%s", durationHrs, fmtOffset)
 
 		_, _, aggErr := a.ComputeAggregateCostModel(promClient, window, field, subfields, aggOpts)
 		if aggErr != nil {
 			log.Infof("Error building cache %s: %s", window, aggErr)
 		}
-		if a.ThanosClient != nil {
-			offset = thanos.Offset()
-			log.Infof("Setting offset to %s", offset)
-		}
-		totals, err := a.ComputeClusterCosts(promClient, a.CloudProvider, durationHrs, offset, cacheEfficiencyData)
+
+
+		totals, err := a.ComputeClusterCosts(promClient, a.CloudProvider, duration, offset, cacheEfficiencyData)
 		if err != nil {
 			log.Infof("Error building cluster costs cache %s", key)
 		}
@@ -1799,9 +1802,9 @@ func (a *Accesses) warmAggregateCostModelCache() {
 		}
 		if len(totals) > 0 && maxMinutesWithData > clusterCostsCacheMinutes {
 			a.ClusterCostsCache.Set(key, totals, a.GetCacheExpiration(window.Duration()))
-			log.Infof("caching %s cluster costs for %s", duration, a.GetCacheExpiration(window.Duration()))
+			log.Infof("caching %s cluster costs for %s", fmtDuration, a.GetCacheExpiration(window.Duration()))
 		} else {
-			log.Warningf("not caching %s cluster costs: no data or less than %f minutes data ", duration, clusterCostsCacheMinutes)
+			log.Warningf("not caching %s cluster costs: no data or less than %f minutes data ", fmtDuration, clusterCostsCacheMinutes)
 		}
 		return aggErr, err
 	}
@@ -1810,18 +1813,16 @@ func (a *Accesses) warmAggregateCostModelCache() {
 	go func(sem *util.Semaphore) {
 		defer errors.HandlePanic()
 
-		duration := "1d"
-		offset := "1m"
-		durHrs := "24h"
-		dur := 24 * time.Hour
+		offset := time.Minute
+		duration := 24 * time.Hour
 
 		for {
 			sem.Acquire()
-			warmFunc(duration, durHrs, offset, true)
+			warmFunc(duration, offset, true)
 			sem.Return()
 
-			log.Infof("aggregation: warm cache: %s", duration)
-			time.Sleep(a.GetCacheRefresh(dur))
+			log.Infof("aggregation: warm cache: %s", timeutil.DurationString(duration))
+			time.Sleep(a.GetCacheRefresh(duration))
 		}
 	}(sem)
 
@@ -1830,18 +1831,16 @@ func (a *Accesses) warmAggregateCostModelCache() {
 		go func(sem *util.Semaphore) {
 			defer errors.HandlePanic()
 
-			duration := "2d"
-			offset := "1m"
-			durHrs := "48h"
-			dur := 2 * 24 * time.Hour
+			offset := time.Minute
+			duration := 2 * 24 * time.Hour
 
 			for {
 				sem.Acquire()
-				warmFunc(duration, durHrs, offset, false)
+				warmFunc(duration, offset, false)
 				sem.Return()
 
-				log.Infof("aggregation: warm cache: %s", duration)
-				time.Sleep(a.GetCacheRefresh(dur))
+				log.Infof("aggregation: warm cache: %s", timeutil.DurationString(duration))
+				time.Sleep(a.GetCacheRefresh(duration))
 			}
 		}(sem)
 
@@ -1849,19 +1848,17 @@ func (a *Accesses) warmAggregateCostModelCache() {
 		go func(sem *util.Semaphore) {
 			defer errors.HandlePanic()
 
-			duration := "7d"
-			offset := "1m"
-			durHrs := "168h"
-			dur := 7 * 24 * time.Hour
+			offset := time.Minute
+			duration := 7 * 24 * time.Hour
 
 			for {
 				sem.Acquire()
-				aggErr, err := warmFunc(duration, durHrs, offset, false)
+				aggErr, err := warmFunc(duration, offset, false)
 				sem.Return()
 
-				log.Infof("aggregation: warm cache: %s", duration)
+				log.Infof("aggregation: warm cache: %s", timeutil.DurationString(duration))
 				if aggErr == nil && err == nil {
-					time.Sleep(a.GetCacheRefresh(dur))
+					time.Sleep(a.GetCacheRefresh(duration))
 				} else {
 					time.Sleep(5 * time.Minute)
 				}
@@ -1873,16 +1870,14 @@ func (a *Accesses) warmAggregateCostModelCache() {
 			defer errors.HandlePanic()
 
 			for {
-				duration := "30d"
-				offset := "1m"
-				durHrs := "720h"
-				dur := 30 * 24 * time.Hour
+				offset := time.Minute
+				duration := 30 * 24 * time.Hour
 
 				sem.Acquire()
-				aggErr, err := warmFunc(duration, durHrs, offset, false)
+				aggErr, err := warmFunc(duration, offset, false)
 				sem.Return()
 				if aggErr == nil && err == nil {
-					time.Sleep(a.GetCacheRefresh(dur))
+					time.Sleep(a.GetCacheRefresh(duration))
 				} else {
 					time.Sleep(5 * time.Minute)
 				}

pkg/costmodel/allocation.go

@@ -2,6 +2,7 @@ package costmodel
 
 import (
 	"fmt"
+	"github.com/kubecost/cost-model/pkg/util/timeutil"
 	"math"
 	"strconv"
 	"strings"
@@ -12,7 +13,6 @@ import (
 	"github.com/kubecost/cost-model/pkg/kubecost"
 	"github.com/kubecost/cost-model/pkg/log"
 	"github.com/kubecost/cost-model/pkg/prom"
-	"github.com/kubecost/cost-model/pkg/util"
 	"k8s.io/apimachinery/pkg/labels"
 )
 
@@ -125,7 +125,7 @@ func (cm *CostModel) ComputeAllocation(start, end time.Time, resolution time.Dur
 	}
 
 	// Convert resolution duration to a query-ready string
-	resStr := util.DurationString(resolution)
+	resStr := timeutil.DurationString(resolution)
 
 	ctx := prom.NewContext(cm.PrometheusClient)
 
@@ -338,9 +338,9 @@ func (cm *CostModel) ComputeAllocation(start, end time.Time, resolution time.Dur
 	// for converting resource allocation data to cumulative costs.
 	nodeMap := map[nodeKey]*NodePricing{}
 
-	applyNodeCostPerCPUHr(nodeMap, resNodeCostPerCPUHr, cm.Provider.ParseID)
-	applyNodeCostPerRAMGiBHr(nodeMap, resNodeCostPerRAMGiBHr, cm.Provider.ParseID)
-	applyNodeCostPerGPUHr(nodeMap, resNodeCostPerGPUHr, cm.Provider.ParseID)
+	applyNodeCostPerCPUHr(nodeMap, resNodeCostPerCPUHr)
+	applyNodeCostPerRAMGiBHr(nodeMap, resNodeCostPerRAMGiBHr)
+	applyNodeCostPerGPUHr(nodeMap, resNodeCostPerGPUHr)
 	applyNodeSpot(nodeMap, resNodeIsSpot)
 	applyNodeDiscount(nodeMap, cm)
 
@@ -446,7 +446,7 @@ func (cm *CostModel) buildPodMap(window kubecost.Window, resolution, maxBatchSiz
 	start, end := *window.Start(), *window.End()
 
 	// Convert resolution duration to a query-ready string
-	resStr := util.DurationString(resolution)
+	resStr := timeutil.DurationString(resolution)
 
 	ctx := prom.NewContext(cm.PrometheusClient)
 
@@ -1360,8 +1360,7 @@ func applyControllersToPods(podMap map[podKey]*Pod, podControllerMap map[podKey]
 	}
 }
 
-func applyNodeCostPerCPUHr(nodeMap map[nodeKey]*NodePricing, resNodeCostPerCPUHr []*prom.QueryResult,
-	providerIDParser func(string) string) {
+func applyNodeCostPerCPUHr(nodeMap map[nodeKey]*NodePricing, resNodeCostPerCPUHr []*prom.QueryResult) {
 	for _, res := range resNodeCostPerCPUHr {
 		cluster, err := res.GetString(env.GetPromClusterLabel())
 		if err != nil {
@@ -1391,7 +1390,7 @@ func applyNodeCostPerCPUHr(nodeMap map[nodeKey]*NodePricing, resNodeCostPerCPUHr
 			nodeMap[key] = &NodePricing{
 				Name:       node,
 				NodeType:   instanceType,
-				ProviderID: providerIDParser(providerID),
+				ProviderID: cloud.ParseID(providerID),
 			}
 		}
 
@@ -1399,8 +1398,7 @@ func applyNodeCostPerCPUHr(nodeMap map[nodeKey]*NodePricing, resNodeCostPerCPUHr
 	}
 }
 
-func applyNodeCostPerRAMGiBHr(nodeMap map[nodeKey]*NodePricing, resNodeCostPerRAMGiBHr []*prom.QueryResult,
-	providerIDParser func(string) string) {
+func applyNodeCostPerRAMGiBHr(nodeMap map[nodeKey]*NodePricing, resNodeCostPerRAMGiBHr []*prom.QueryResult) {
 	for _, res := range resNodeCostPerRAMGiBHr {
 		cluster, err := res.GetString(env.GetPromClusterLabel())
 		if err != nil {
@@ -1430,7 +1428,7 @@ func applyNodeCostPerRAMGiBHr(nodeMap map[nodeKey]*NodePricing, resNodeCostPerRA
 			nodeMap[key] = &NodePricing{
 				Name:       node,
 				NodeType:   instanceType,
-				ProviderID: providerIDParser(providerID),
+				ProviderID: cloud.ParseID(providerID),
 			}
 		}
 
@@ -1438,8 +1436,7 @@ func applyNodeCostPerRAMGiBHr(nodeMap map[nodeKey]*NodePricing, resNodeCostPerRA
 	}
 }
 
-func applyNodeCostPerGPUHr(nodeMap map[nodeKey]*NodePricing, resNodeCostPerGPUHr []*prom.QueryResult,
-	providerIDParser func(string) string) {
+func applyNodeCostPerGPUHr(nodeMap map[nodeKey]*NodePricing, resNodeCostPerGPUHr []*prom.QueryResult) {
 	for _, res := range resNodeCostPerGPUHr {
 		cluster, err := res.GetString(env.GetPromClusterLabel())
 		if err != nil {
@@ -1469,7 +1466,7 @@ func applyNodeCostPerGPUHr(nodeMap map[nodeKey]*NodePricing, resNodeCostPerGPUHr
 			nodeMap[key] = &NodePricing{
 				Name:       node,
 				NodeType:   instanceType,
-				ProviderID: providerIDParser(providerID),
+				ProviderID: cloud.ParseID(providerID),
 			}
 		}
 

pkg/costmodel/cluster.go

@@ -2,13 +2,13 @@ package costmodel
 
 import (
 	"fmt"
+	"github.com/kubecost/cost-model/pkg/util/timeutil"
 	"time"
 
 	"github.com/kubecost/cost-model/pkg/cloud"
 	"github.com/kubecost/cost-model/pkg/env"
 	"github.com/kubecost/cost-model/pkg/log"
 	"github.com/kubecost/cost-model/pkg/prom"
-	"github.com/kubecost/cost-model/pkg/util"
 
 	prometheus "github.com/prometheus/client_golang/api"
 	"k8s.io/klog"
@@ -70,15 +70,12 @@ type ClusterCostsBreakdown struct {
 
 // NewClusterCostsFromCumulative takes cumulative cost data over a given time range, computes
 // the associated monthly rate data, and returns the Costs.
-func NewClusterCostsFromCumulative(cpu, gpu, ram, storage float64, window, offset string, dataHours float64) (*ClusterCosts, error) {
-	start, end, err := util.ParseTimeRange(window, offset)
-	if err != nil {
-		return nil, err
-	}
+func NewClusterCostsFromCumulative(cpu, gpu, ram, storage float64, window, offset time.Duration, dataHours float64) (*ClusterCosts, error) {
+	start, end := timeutil.ParseTimeRange(window, offset)
 
 	// If the number of hours is not given (i.e. is zero) compute one from the window and offset
 	if dataHours == 0 {
-		dataHours = end.Sub(*start).Hours()
+		dataHours = end.Sub(start).Hours()
 	}
 
 	// Do not allow zero-length windows to prevent divide-by-zero issues
@@ -87,17 +84,17 @@ func NewClusterCostsFromCumulative(cpu, gpu, ram, storage float64, window, offse
 	}
 
 	cc := &ClusterCosts{
-		Start:             start,
-		End:               end,
+		Start:             &start,
+		End:               &end,
 		CPUCumulative:     cpu,
 		GPUCumulative:     gpu,
 		RAMCumulative:     ram,
 		StorageCumulative: storage,
 		TotalCumulative:   cpu + gpu + ram + storage,
-		CPUMonthly:        cpu / dataHours * (util.HoursPerMonth),
-		GPUMonthly:        gpu / dataHours * (util.HoursPerMonth),
-		RAMMonthly:        ram / dataHours * (util.HoursPerMonth),
-		StorageMonthly:    storage / dataHours * (util.HoursPerMonth),
+		CPUMonthly:        cpu / dataHours * (timeutil.HoursPerMonth),
+		GPUMonthly:        gpu / dataHours * (timeutil.HoursPerMonth),
+		RAMMonthly:        ram / dataHours * (timeutil.HoursPerMonth),
+		StorageMonthly:    storage / dataHours * (timeutil.HoursPerMonth),
 	}
 	cc.TotalMonthly = cc.CPUMonthly + cc.GPUMonthly + cc.RAMMonthly + cc.StorageMonthly
 
@@ -195,7 +192,7 @@ func ClusterDisks(client prometheus.Client, provider cloud.Provider, duration, o
 		diskMap[key].Cost += cost
 		providerID, _ := result.GetString("provider_id") // just put the providerID set up here, it's the simplest query.
 		if providerID != "" {
-			diskMap[key].ProviderID = provider.ParsePVID(providerID)
+			diskMap[key].ProviderID = cloud.ParsePVID(providerID)
 		}
 	}
 
@@ -522,18 +519,18 @@ func ClusterNodes(cp cloud.Provider, client prometheus.Client, duration, offset
 		return nil, requiredCtx.ErrorCollection()
 	}
 
-	activeDataMap := buildActiveDataMap(resActiveMins, resolution, cp.ParseID)
+	activeDataMap := buildActiveDataMap(resActiveMins, resolution)
 
-	gpuCountMap := buildGPUCountMap(resNodeGPUCount, cp.ParseID)
+	gpuCountMap := buildGPUCountMap(resNodeGPUCount)
 
-	cpuCostMap, clusterAndNameToType1 := buildCPUCostMap(resNodeCPUHourlyCost, cp.ParseID)
-	ramCostMap, clusterAndNameToType2 := buildRAMCostMap(resNodeRAMHourlyCost, cp.ParseID)
-	gpuCostMap, clusterAndNameToType3 := buildGPUCostMap(resNodeGPUHourlyCost, gpuCountMap, cp.ParseID)
+	cpuCostMap, clusterAndNameToType1 := buildCPUCostMap(resNodeCPUHourlyCost)
+	ramCostMap, clusterAndNameToType2 := buildRAMCostMap(resNodeRAMHourlyCost)
+	gpuCostMap, clusterAndNameToType3 := buildGPUCostMap(resNodeGPUHourlyCost, gpuCountMap)
 
 	clusterAndNameToTypeIntermediate := mergeTypeMaps(clusterAndNameToType1, clusterAndNameToType2)
 	clusterAndNameToType := mergeTypeMaps(clusterAndNameToTypeIntermediate, clusterAndNameToType3)
 
-	cpuCoresMap := buildCPUCoresMap(resNodeCPUCores, clusterAndNameToType)
+	cpuCoresMap := buildCPUCoresMap(resNodeCPUCores)
 
 	ramBytesMap := buildRAMBytesMap(resNodeRAMBytes)
 
@@ -541,7 +538,7 @@ func ClusterNodes(cp cloud.Provider, client prometheus.Client, duration, offset
 	ramSystemPctMap := buildRAMSystemPctMap(resNodeRAMSystemPct)
 
 	cpuBreakdownMap := buildCPUBreakdownMap(resNodeCPUModeTotal)
-	preemptibleMap := buildPreemptibleMap(resIsSpot, cp.ParseID)
+	preemptibleMap := buildPreemptibleMap(resIsSpot)
 	labelsMap := buildLabelsMap(resLabels)
 
 	costTimesMinuteAndCount(activeDataMap, cpuCostMap, cpuCoresMap)
@@ -595,7 +592,7 @@ type LoadBalancer struct {
 	Minutes    float64
 }
 
-func ClusterLoadBalancers(cp cloud.Provider, client prometheus.Client, duration, offset time.Duration) (map[string]*LoadBalancer, error) {
+func ClusterLoadBalancers(client prometheus.Client, duration, offset time.Duration) (map[string]*LoadBalancer, error) {
 	durationStr := fmt.Sprintf("%dm", int64(duration.Minutes()))
 	offsetStr := fmt.Sprintf(" offset %dm", int64(offset.Minutes()))
 	if offset < time.Minute {
@@ -655,7 +652,7 @@ func ClusterLoadBalancers(cp cloud.Provider, client prometheus.Client, duration,
 			loadBalancerMap[key] = &LoadBalancer{
 				Cluster:    cluster,
 				Name:       namespace + "/" + serviceName,
-				ProviderID: cp.ParseLBID(providerID),
+				ProviderID: cloud.ParseLBID(providerID),
 			}
 		}
 		// Fill in Provider ID if it is available and missing in the loadBalancerMap
@@ -702,13 +699,11 @@ func ClusterLoadBalancers(cp cloud.Provider, client prometheus.Client, duration,
 }
 
 // ComputeClusterCosts gives the cumulative and monthly-rate cluster costs over a window of time for all clusters.
-func (a *Accesses) ComputeClusterCosts(client prometheus.Client, provider cloud.Provider, window, offset string, withBreakdown bool) (map[string]*ClusterCosts, error) {
+func (a *Accesses) ComputeClusterCosts(client prometheus.Client, provider cloud.Provider, window, offset time.Duration, withBreakdown bool) (map[string]*ClusterCosts, error) {
 	// Compute number of minutes in the full interval, for use interpolating missed scrapes or scaling missing data
-	start, end, err := util.ParseTimeRange(window, offset)
-	if err != nil {
-		return nil, err
-	}
-	mins := end.Sub(*start).Minutes()
+	start, end := timeutil.ParseTimeRange(window, offset)
+
+	mins := end.Sub(start).Minutes()
 
 	// minsPerResolution determines accuracy and resource use for the following
 	// queries. Smaller values (higher resolution) result in better accuracy,
@@ -777,10 +772,7 @@ func (a *Accesses) ComputeClusterCosts(client prometheus.Client, provider cloud.
 		queryTotalLocalStorage = fmt.Sprintf(" + %s", queryTotalLocalStorage)
 	}
 
-	fmtOffset := ""
-	if offset != "" {
-		fmtOffset = fmt.Sprintf("offset %s", offset)
-	}
+	fmtOffset := timeutil.DurationToPromOffsetString(offset)
 
 	queryDataCount := fmt.Sprintf(fmtQueryDataCount, env.GetPromClusterLabel(), window, minsPerResolution, fmtOffset, minsPerResolution)
 	queryTotalGPU := fmt.Sprintf(fmtQueryTotalGPU, window, minsPerResolution, fmtOffset, hourlyToCumulative, env.GetPromClusterLabel())
@@ -1003,7 +995,7 @@ func (a *Accesses) ComputeClusterCosts(client prometheus.Client, provider cloud.
 			dataMins = mins
 			klog.V(3).Infof("[Warning] cluster cost data count not found for cluster %s", id)
 		}
-		costs, err := NewClusterCostsFromCumulative(cd["cpu"], cd["gpu"], cd["ram"], cd["storage"]+cd["localstorage"], window, offset, dataMins/util.MinsPerHour)
+		costs, err := NewClusterCostsFromCumulative(cd["cpu"], cd["gpu"], cd["ram"], cd["storage"]+cd["localstorage"], window, offset, dataMins/timeutil.MinsPerHour)
 		if err != nil {
 			klog.V(3).Infof("[Warning] Failed to parse cluster costs on %s (%s) from cumulative data: %+v", window, offset, cd)
 			return nil, err
@@ -1054,8 +1046,8 @@ func resultToTotals(qrs []*prom.QueryResult) ([][]string, error) {
 }
 
 // ClusterCostsOverTime gives the full cluster costs over time
-func ClusterCostsOverTime(cli prometheus.Client, provider cloud.Provider, startString, endString, windowString, offset string) (*Totals, error) {
-	localStorageQuery := provider.GetLocalStorageQuery(windowString, offset, true, false)
+func ClusterCostsOverTime(cli prometheus.Client, provider cloud.Provider, startString, endString string, window, offset time.Duration) (*Totals, error) {
+	localStorageQuery := provider.GetLocalStorageQuery(window, offset, true, false)
 	if localStorageQuery != "" {
 		localStorageQuery = fmt.Sprintf("+ %s", localStorageQuery)
 	}
@@ -1064,28 +1056,27 @@ func ClusterCostsOverTime(cli prometheus.Client, provider cloud.Provider, startS
 
 	start, err := time.Parse(layout, startString)
 	if err != nil {
-		klog.V(1).Infof("Error parsing time " + startString + ". Error: " + err.Error())
+		klog.V(1).Infof("Error parsing time %s. Error: %s", startString, err.Error())
 		return nil, err
 	}
 	end, err := time.Parse(layout, endString)
 	if err != nil {
-		klog.V(1).Infof("Error parsing time " + endString + ". Error: " + err.Error())
+		klog.V(1).Infof("Error parsing time %s. Error: %s", endString, err.Error())
 		return nil, err
 	}
-	window, err := time.ParseDuration(windowString)
-	if err != nil {
-		klog.V(1).Infof("Error parsing time " + windowString + ". Error: " + err.Error())
+	fmtWindow := timeutil.DurationString(window)
+
+	if fmtWindow == "" {
+		err := fmt.Errorf("window value invalid or missing")
+		klog.V(1).Infof("Error parsing time %v. Error: %s", window, err.Error())
 		return nil, err
 	}
 
-	// turn offsets of the format "[0-9+]h" into the format "offset [0-9+]h" for use in query templatess
-	if offset != "" {
-		offset = fmt.Sprintf("offset %s", offset)
-	}
+	fmtOffset := timeutil.DurationToPromOffsetString(offset)
 
-	qCores := fmt.Sprintf(queryClusterCores, windowString, offset, env.GetPromClusterLabel(), windowString, offset, env.GetPromClusterLabel(), windowString, offset, env.GetPromClusterLabel(), env.GetPromClusterLabel())
-	qRAM := fmt.Sprintf(queryClusterRAM, windowString, offset, env.GetPromClusterLabel(), windowString, offset, env.GetPromClusterLabel(), env.GetPromClusterLabel())
-	qStorage := fmt.Sprintf(queryStorage, windowString, offset, env.GetPromClusterLabel(), windowString, offset, env.GetPromClusterLabel(), env.GetPromClusterLabel(), localStorageQuery)
+	qCores := fmt.Sprintf(queryClusterCores, fmtWindow, fmtOffset, env.GetPromClusterLabel(), fmtWindow, fmtOffset, env.GetPromClusterLabel(), fmtWindow, fmtOffset, env.GetPromClusterLabel(), env.GetPromClusterLabel())
+	qRAM := fmt.Sprintf(queryClusterRAM, fmtWindow, fmtOffset, env.GetPromClusterLabel(), fmtWindow, fmtOffset, env.GetPromClusterLabel(), env.GetPromClusterLabel())
+	qStorage := fmt.Sprintf(queryStorage, fmtWindow, fmtOffset, env.GetPromClusterLabel(), fmtWindow, fmtOffset, env.GetPromClusterLabel(), env.GetPromClusterLabel(), localStorageQuery)
 	qTotal := fmt.Sprintf(queryTotal, env.GetPromClusterLabel(), env.GetPromClusterLabel(), env.GetPromClusterLabel(), env.GetPromClusterLabel(), localStorageQuery)
 
 	ctx := prom.NewContext(cli)

pkg/costmodel/cluster_helpers.go

@@ -1,6 +1,7 @@
 package costmodel
 
 import (
+	"github.com/kubecost/cost-model/pkg/cloud"
 	"time"
 
 	"github.com/kubecost/cost-model/pkg/env"
@@ -27,7 +28,6 @@ func mergeTypeMaps(clusterAndNameToType1, clusterAndNameToType2 map[nodeIdentifi
 
 func buildCPUCostMap(
 	resNodeCPUCost []*prom.QueryResult,
-	providerIDParser func(string) string,
 ) (
 	map[NodeIdentifier]float64,
 	map[nodeIdentifierNoProviderID]string,
@@ -56,7 +56,7 @@ func buildCPUCostMap(
 		key := NodeIdentifier{
 			Cluster:    cluster,
 			Name:       name,
-			ProviderID: providerIDParser(providerID),
+			ProviderID: cloud.ParseID(providerID),
 		}
 		keyNon := nodeIdentifierNoProviderID{
 			Cluster: cluster,
@@ -73,7 +73,6 @@ func buildCPUCostMap(
 
 func buildRAMCostMap(
 	resNodeRAMCost []*prom.QueryResult,
-	providerIDParser func(string) string,
 ) (
 	map[NodeIdentifier]float64,
 	map[nodeIdentifierNoProviderID]string,
@@ -102,7 +101,7 @@ func buildRAMCostMap(
 		key := NodeIdentifier{
 			Cluster:    cluster,
 			Name:       name,
-			ProviderID: providerIDParser(providerID),
+			ProviderID: cloud.ParseID(providerID),
 		}
 		keyNon := nodeIdentifierNoProviderID{
 			Cluster: cluster,
@@ -119,7 +118,6 @@ func buildRAMCostMap(
 func buildGPUCostMap(
 	resNodeGPUCost []*prom.QueryResult,
 	gpuCountMap map[NodeIdentifier]float64,
-	providerIDParser func(string) string,
 ) (
 	map[NodeIdentifier]float64,
 	map[nodeIdentifierNoProviderID]string,
@@ -148,7 +146,7 @@ func buildGPUCostMap(
 		key := NodeIdentifier{
 			Cluster:    cluster,
 			Name:       name,
-			ProviderID: providerIDParser(providerID),
+			ProviderID: cloud.ParseID(providerID),
 		}
 		keyNon := nodeIdentifierNoProviderID{
 			Cluster: cluster,
@@ -171,7 +169,6 @@ func buildGPUCostMap(
 
 func buildGPUCountMap(
 	resNodeGPUCount []*prom.QueryResult,
-	providerIDParser func(string) string,
 ) map[NodeIdentifier]float64 {
 
 	gpuCountMap := make(map[NodeIdentifier]float64)
@@ -194,7 +191,7 @@ func buildGPUCountMap(
 		key := NodeIdentifier{
 			Cluster:    cluster,
 			Name:       name,
-			ProviderID: providerIDParser(providerID),
+			ProviderID: cloud.ParseID(providerID),
 		}
 		gpuCountMap[key] = gpuCount
 	}
@@ -204,7 +201,6 @@ func buildGPUCountMap(
 
 func buildCPUCoresMap(
 	resNodeCPUCores []*prom.QueryResult,
-	clusterAndNameToType map[nodeIdentifierNoProviderID]string,
 ) map[nodeIdentifierNoProviderID]float64 {
 
 	m := make(map[nodeIdentifierNoProviderID]float64)
@@ -403,7 +399,7 @@ type activeData struct {
 	minutes float64
 }
 
-func buildActiveDataMap(resActiveMins []*prom.QueryResult, resolution time.Duration, providerIDParser func(string) string) map[NodeIdentifier]activeData {
+func buildActiveDataMap(resActiveMins []*prom.QueryResult, resolution time.Duration) map[NodeIdentifier]activeData {
 
 	m := make(map[NodeIdentifier]activeData)
 
@@ -424,7 +420,7 @@ func buildActiveDataMap(resActiveMins []*prom.QueryResult, resolution time.Durat
 		key := NodeIdentifier{
 			Cluster:    cluster,
 			Name:       name,
-			ProviderID: providerIDParser(providerID),
+			ProviderID: cloud.ParseID(providerID),
 		}
 
 		if len(result.Values) == 0 {
@@ -450,7 +446,6 @@ func buildActiveDataMap(resActiveMins []*prom.QueryResult, resolution time.Durat
 // node id -> is preemptible?
 func buildPreemptibleMap(
 	resIsSpot []*prom.QueryResult,
-	providerIDParser func(string) string,
 ) map[NodeIdentifier]bool {
 
 	m := make(map[NodeIdentifier]bool)
@@ -474,7 +469,7 @@ func buildPreemptibleMap(
 		key := NodeIdentifier{
 			Cluster:    cluster,
 			Name:       nodeName,
-			ProviderID: providerIDParser(providerID),
+			ProviderID: cloud.ParseID(providerID),
 		}
 
 		// TODO(michaelmdresser): check this condition at merge time?
@@ -503,7 +498,7 @@ func buildLabelsMap(
 		if err != nil {
 			cluster = env.GetClusterID()
 		}
-		node, err := result.GetString("kubernetes_node")
+		node, err := result.GetString("node")
 		if err != nil {
 			log.DedupedWarningf(5, "ClusterNodes: label data missing node")
 			continue

pkg/costmodel/cluster_helpers_test.go

@@ -699,7 +699,6 @@ func TestBuildNodeMap(t *testing.T) {
 }
 
 func TestBuildGPUCostMap(t *testing.T) {
-	providerIDParser := func(s string) string { return s }
 	cases := []struct {
 		name       string
 		promResult []*prom.QueryResult
@@ -850,7 +849,7 @@ func TestBuildGPUCostMap(t *testing.T) {
 
 	for _, testCase := range cases {
 		t.Run(testCase.name, func(t *testing.T) {
-			result, _ := buildGPUCostMap(testCase.promResult, testCase.countMap, providerIDParser)
+			result, _ := buildGPUCostMap(testCase.promResult, testCase.countMap)
 			if !reflect.DeepEqual(result, testCase.expected) {
 				t.Errorf("buildGPUCostMap case %s failed. Got %+v but expected %+v", testCase.name, result, testCase.expected)
 			}

pkg/costmodel/router.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"flag"
 	"fmt"
+	"github.com/kubecost/cost-model/pkg/util/timeutil"
 	"net/http"
 	"reflect"
 	"strconv"
@@ -123,16 +124,18 @@ func (a *Accesses) GetCacheRefresh(dur time.Duration) time.Duration {
 func (a *Accesses) ClusterCostsFromCacheHandler(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
 	w.Header().Set("Content-Type", "application/json")
 
+	duration := 24 * time.Hour
+	offset := time.Minute
 	durationHrs := "24h"
-	offset := "1m"
+	fmtOffset := "1m"
 	pClient := a.GetPrometheusClient(true)
 
-	key := fmt.Sprintf("%s:%s", durationHrs, offset)
+	key := fmt.Sprintf("%s:%s", durationHrs, fmtOffset)
 	if data, valid := a.ClusterCostsCache.Get(key); valid {
 		clusterCosts := data.(map[string]*ClusterCosts)
 		w.Write(WrapDataWithMessage(clusterCosts, nil, "clusterCosts cache hit"))
 	} else {
-		data, err := a.ComputeClusterCosts(pClient, a.CloudProvider, durationHrs, offset, true)
+		data, err := a.ComputeClusterCosts(pClient, a.CloudProvider, duration, offset, true)
 		w.Write(WrapDataWithMessage(data, err, fmt.Sprintf("clusterCosts cache miss: %s", key)))
 	}
 }
@@ -224,7 +227,7 @@ func normalizeTimeParam(param string) (string, error) {
 	return param, nil
 }
 
-// parsePercentString takes a string of expected format "N%" and returns a floating point 0.0N.
+// ParsePercentString takes a string of expected format "N%" and returns a floating point 0.0N.
 // If the "%" symbol is missing, it just returns 0.0N. Empty string is interpreted as "0%" and
 // return 0.0.
 func ParsePercentString(percentStr string) (float64, error) {
@@ -243,66 +246,6 @@ func ParsePercentString(percentStr string) (float64, error) {
 	return discount, nil
 }
 
-// parseDuration converts a Prometheus-style duration string into a Duration
-// TODO:CLEANUP delete this. do it now.
-func ParseDuration(duration string) (*time.Duration, error) {
-	unitStr := duration[len(duration)-1:]
-	var unit time.Duration
-	switch unitStr {
-	case "s":
-		unit = time.Second
-	case "m":
-		unit = time.Minute
-	case "h":
-		unit = time.Hour
-	case "d":
-		unit = 24.0 * time.Hour
-	default:
-		return nil, fmt.Errorf("error parsing duration: %s did not match expected format [0-9+](s|m|d|h)", duration)
-	}
-
-	amountStr := duration[:len(duration)-1]
-	amount, err := strconv.ParseInt(amountStr, 10, 64)
-	if err != nil {
-		return nil, fmt.Errorf("error parsing duration: %s did not match expected format [0-9+](s|m|d|h)", duration)
-	}
-
-	dur := time.Duration(amount) * unit
-	return &dur, nil
-}
-
-// ParseTimeRange returns a start and end time, respectively, which are converted from
-// a duration and offset, defined as strings with Prometheus-style syntax.
-func ParseTimeRange(duration, offset string) (*time.Time, *time.Time, error) {
-	// endTime defaults to the current time, unless an offset is explicity declared,
-	// in which case it shifts endTime back by given duration
-	endTime := time.Now()
-	if offset != "" {
-		o, err := ParseDuration(offset)
-		if err != nil {
-			return nil, nil, fmt.Errorf("error parsing offset (%s): %s", offset, err)
-		}
-		endTime = endTime.Add(-1 * *o)
-	}
-
-	// if duration is defined in terms of days, convert to hours
-	// e.g. convert "2d" to "48h"
-	durationNorm, err := normalizeTimeParam(duration)
-	if err != nil {
-		return nil, nil, fmt.Errorf("error parsing duration (%s): %s", duration, err)
-	}
-
-	// convert time duration into start and end times, formatted
-	// as ISO datetime strings
-	dur, err := time.ParseDuration(durationNorm)
-	if err != nil {
-		return nil, nil, fmt.Errorf("errorf parsing duration (%s): %s", durationNorm, err)
-	}
-	startTime := endTime.Add(-1 * dur)
-
-	return &startTime, &endTime, nil
-}
-
 func WrapData(data interface{}, err error) []byte {
 	var resp []byte
 
@@ -438,6 +381,27 @@ func (a *Accesses) ClusterCosts(w http.ResponseWriter, r *http.Request, ps httpr
 	window := r.URL.Query().Get("window")
 	offset := r.URL.Query().Get("offset")
 
+	if window == "" {
+		w.Write(WrapData(nil, fmt.Errorf("missing window arguement")))
+		return
+	}
+	windowDur, err := timeutil.ParseDuration(window)
+	if err != nil {
+		w.Write(WrapData(nil, fmt.Errorf("error parsing window (%s): %s", window, err)))
+		return
+	}
+
+	// offset is not a required parameter
+	var offsetDur time.Duration
+	if offset != "" {
+		offsetDur, err = timeutil.ParseDuration(offset)
+		if err != nil {
+			w.Write(WrapData(nil, fmt.Errorf("error parsing offset (%s): %s", offset, err)))
+			return
+		}
+	}
+
+
 	useThanos, _ := strconv.ParseBool(r.URL.Query().Get("multi"))
 
 	if useThanos && !thanos.IsEnabled() {
@@ -448,12 +412,13 @@ func (a *Accesses) ClusterCosts(w http.ResponseWriter, r *http.Request, ps httpr
 	var client prometheusClient.Client
 	if useThanos {
 		client = a.ThanosClient
-		offset = thanos.Offset()
+		offsetDur = thanos.OffsetDuration()
+
 	} else {
 		client = a.PrometheusClient
 	}
 
-	data, err := a.ComputeClusterCosts(client, a.CloudProvider, window, offset, true)
+	data, err := a.ComputeClusterCosts(client, a.CloudProvider, windowDur, offsetDur, true)
 	w.Write(WrapData(data, err))
 }
 
@@ -466,7 +431,27 @@ func (a *Accesses) ClusterCostsOverTime(w http.ResponseWriter, r *http.Request,
 	window := r.URL.Query().Get("window")
 	offset := r.URL.Query().Get("offset")
 
-	data, err := ClusterCostsOverTime(a.PrometheusClient, a.CloudProvider, start, end, window, offset)
+	if window == "" {
+		w.Write(WrapData(nil, fmt.Errorf("missing window arguement")))
+		return
+	}
+	windowDur, err := timeutil.ParseDuration(window)
+	if err != nil {
+		w.Write(WrapData(nil, fmt.Errorf("error parsing window (%s): %s", window, err)))
+		return
+	}
+
+	// offset is not a required parameter
+	var offsetDur time.Duration
+	if offset != "" {
+		offsetDur, err = timeutil.ParseDuration(offset)
+		if err != nil {
+			w.Write(WrapData(nil, fmt.Errorf("error parsing offset (%s): %s", offset, err)))
+			return
+		}
+	}
+
+	data, err := ClusterCostsOverTime(a.PrometheusClient, a.CloudProvider, start, end, windowDur, offsetDur)
 	w.Write(WrapData(data, err))
 }
 

pkg/env/costmodelenv.go

@@ -76,7 +76,7 @@ const (
 // GetAWSAccessKeyID returns the environment variable value for AWSAccessKeyIDEnvVar which represents
 // the AWS access key for authentication
 func GetAppVersion() string {
-	return Get(AppVersionEnvVar, "1.82.2")
+	return Get(AppVersionEnvVar, "1.83.1")
 }
 
 // IsEmitNamespaceAnnotationsMetric returns true if cost-model is configured to emit the kube_namespace_annotations metric

pkg/kubecost/allocation.go

@@ -563,6 +563,11 @@ func (a *Allocation) IsUnallocated() bool {
 	return strings.Contains(a.Name, UnallocatedSuffix)
 }
 
+// IsUnmounted is true if the given Allocation represents unmounted volume costs.
+func (a *Allocation) IsUnmounted() bool {
+	return strings.Contains(a.Name, UnmountedSuffix)
+}
+
 // Minutes returns the number of minutes the Allocation represents, as defined
 // by the difference between the end and start times.
 func (a *Allocation) Minutes() float64 {
@@ -1124,7 +1129,9 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 		for _, alloc := range aggSet.allocations {
 			for _, sharedAlloc := range shareSet.allocations {
 				if _, ok := shareCoefficients[alloc.Name]; !ok {
-					log.Warningf("AllocationSet.AggregateBy: error getting share coefficienct for '%s'", alloc.Name)
+					if !alloc.IsIdle() {
+						log.Warningf("AllocationSet.AggregateBy: error getting share coefficienct for '%s'", alloc.Name)
+					}
 					continue
 				}
 
@@ -1184,6 +1191,10 @@ func computeShareCoeffs(aggregateBy []string, options *AllocationAggregationOpti
 			// Skip idle allocations in coefficient calculation
 			continue
 		}
+		if alloc.IsUnmounted() {
+			// Skip unmounted allocations in coefficient calculation
+			continue
+		}
 
 		// Determine the post-aggregation key under which the allocation will
 		// be shared.

pkg/kubecost/allocation_test.go

@@ -1498,8 +1498,8 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 			start: start,
 			aggBy: []string{AllocationNamespaceProp},
 			aggOpts: &AllocationAggregationOptions{
-				ShareIdle:   ShareEven,
-				IdleByNode:  true,
+				ShareIdle:  ShareEven,
+				IdleByNode: true,
 			},
 			numResults: 3,
 			totalCost:  112.00,

pkg/kubecost/mock.go

@@ -4,8 +4,10 @@ import (
 	"fmt"
 	"time"
 )
+
 const gb = 1024 * 1024 * 1024
 const day = 24 * time.Hour
+
 var disk = PVKey{}
 
 // NewMockUnitAllocation creates an *Allocation with all of its float64 values set to 1 and generic properties if not provided in arg
@@ -304,7 +306,7 @@ func GenerateMockAllocationSet(start time.Time) *AllocationSet {
 }
 
 // GenerateMockAllocationSetWithAssetProperties with no idle and connections to Assets in properties
-func GenerateMockAllocationSetWithAssetProperties(start time.Time)  *AllocationSet {
+func GenerateMockAllocationSetWithAssetProperties(start time.Time) *AllocationSet {
 	as := GenerateMockAllocationSet(start)
 	disk1 := PVKey{
 		Cluster: "cluster2",

pkg/kubecost/window.go

@@ -3,6 +3,7 @@ package kubecost
 import (
 	"bytes"
 	"fmt"
+	"github.com/kubecost/cost-model/pkg/util/timeutil"
 	"math"
 	"regexp"
 	"strconv"
@@ -10,7 +11,6 @@ import (
 
 	"github.com/kubecost/cost-model/pkg/env"
 	"github.com/kubecost/cost-model/pkg/thanos"
-	"github.com/kubecost/cost-model/pkg/util"
 )
 
 const (
@@ -611,7 +611,7 @@ func (w Window) DurationOffsetForPrometheus() (string, string, error) {
 		offset = 0
 	}
 
-	durStr, offStr := util.DurationOffsetStrings(duration, offset)
+	durStr, offStr := timeutil.DurationOffsetStrings(duration, offset)
 	if offset < time.Minute {
 		offStr = ""
 	} else {
@@ -630,7 +630,7 @@ func (w Window) DurationOffsetStrings() (string, string) {
 		return "", ""
 	}
 
-	return util.DurationOffsetStrings(dur, off)
+	return timeutil.DurationOffsetStrings(dur, off)
 }
 
 type BoundaryError struct {

pkg/util/mapper/mapper.go

@@ -1,7 +1,7 @@
 package mapper
 
 import (
-	"fmt"
+	"github.com/kubecost/cost-model/pkg/util/timeutil"
 	"strconv"
 	"strings"
 	"time"
@@ -397,7 +397,7 @@ func (rom *readOnlyMapper) GetBool(key string, defaultValue bool) bool {
 func (rom *readOnlyMapper) GetDuration(key string, defaultValue time.Duration) time.Duration {
 	r := rom.getter.Get(key)
 
-	d, err := parseDuration(r)
+	d, err := timeutil.ParseDuration(r)
 	if err != nil {
 		return defaultValue
 	}
@@ -488,7 +488,7 @@ func (wom *writeOnlyMapper) SetBool(key string, value bool) error {
 
 // SetDuration sets the map to a string formatted bool value.
 func (wom *writeOnlyMapper) SetDuration(key string, value time.Duration) error {
-	return wom.setter.Set(key, durationString(value))
+	return wom.setter.Set(key, timeutil.DurationString(value))
 }
 
 // SetList sets the map's value at key to a string consistent of each value in the list separated
@@ -497,66 +497,5 @@ func (wom *writeOnlyMapper) SetList(key string, values []string, delimiter strin
 	return wom.setter.Set(key, strings.Join(values, delimiter))
 }
 
-const (
-	secsPerMin  = 60.0
-	secsPerHour = 3600.0
-	secsPerDay  = 86400.0
-)
-
-// durationString converts duration to a string of the form "4d", "4h", "4m", or "4s" if
-// the number of seconds in the string is evenly divisible into an integer number of
-// days, hours, minutes, or seconds respectively.
-func durationString(duration time.Duration) string {
-	durSecs := int64(duration.Seconds())
-
-	durStr := ""
-	if durSecs > 0 {
-		if durSecs%secsPerDay == 0 {
-			// convert to days
-			durStr = fmt.Sprintf("%dd", durSecs/secsPerDay)
-		} else if durSecs%secsPerHour == 0 {
-			// convert to hours
-			durStr = fmt.Sprintf("%dh", durSecs/secsPerHour)
-		} else if durSecs%secsPerMin == 0 {
-			// convert to mins
-			durStr = fmt.Sprintf("%dm", durSecs/secsPerMin)
-		} else if durSecs > 0 {
-			// default to mins, as long as duration is positive
-			durStr = fmt.Sprintf("%ds", durSecs)
-		}
-	}
 
-	return durStr
-}
-
-func parseDuration(duration string) (time.Duration, error) {
-	var amountStr string
-	var unit time.Duration
-	switch {
-	case strings.HasSuffix(duration, "s"):
-		unit = time.Second
-		amountStr = strings.TrimSuffix(duration, "s")
-	case strings.HasSuffix(duration, "m"):
-		unit = time.Minute
-		amountStr = strings.TrimSuffix(duration, "m")
-	case strings.HasSuffix(duration, "h"):
-		unit = time.Hour
-		amountStr = strings.TrimSuffix(duration, "h")
-	case strings.HasSuffix(duration, "d"):
-		unit = 24.0 * time.Hour
-		amountStr = strings.TrimSuffix(duration, "d")
-	default:
-		return 0, fmt.Errorf("error parsing duration: %s did not match expected format [0-9+](s|m|d|h)", duration)
-	}
 
-	if len(amountStr) == 0 {
-		return 0, fmt.Errorf("error parsing duration: %s did not match expected format [0-9+](s|m|d|h)", duration)
-	}
-
-	amount, err := strconv.ParseInt(amountStr, 10, 64)
-	if err != nil {
-		return 0, fmt.Errorf("error parsing duration: %s did not match expected format [0-9+](s|m|d|h)", duration)
-	}
-
-	return time.Duration(amount) * unit, nil
-}

pkg/util/time_test.go

@@ -1,56 +0,0 @@
-package util
-
-import (
-	"testing"
-	"time"
-)
-
-func TestDurationOffsetStrings(t *testing.T) {
-	dur, off := "", ""
-
-	dur, off = DurationOffsetStrings(0, 0)
-	if dur != "" || off != "" {
-		t.Fatalf("DurationOffsetStrings: exp (%s %s); act (%s, %s)", "", "", dur, off)
-	}
-
-	dur, off = DurationOffsetStrings(24*time.Hour, 0)
-	if dur != "1d" || off != "" {
-		t.Fatalf("DurationOffsetStrings: exp (%s %s); act (%s, %s)", "1d", "", dur, off)
-	}
-
-	dur, off = DurationOffsetStrings(24*time.Hour+5*time.Minute, 0)
-	if dur != "1445m" || off != "" {
-		t.Fatalf("DurationOffsetStrings: exp (%s %s); act (%s, %s)", "1445m", "", dur, off)
-	}
-
-	dur, off = DurationOffsetStrings(25*time.Hour, 5*time.Minute)
-	if dur != "25h" || off != "5m" {
-		t.Fatalf("DurationOffsetStrings: exp (%s %s); act (%s, %s)", "25h", "5m", dur, off)
-	}
-
-	dur, off = DurationOffsetStrings(25*time.Hour, 60*time.Minute)
-	if dur != "25h" || off != "1h" {
-		t.Fatalf("DurationOffsetStrings: exp (%s %s); act (%s, %s)", "25h", "1h", dur, off)
-	}
-
-	dur, off = DurationOffsetStrings(72*time.Hour, 1440*time.Minute)
-	if dur != "3d" || off != "1d" {
-		t.Fatalf("DurationOffsetStrings: exp (%s %s); act (%s, %s)", "3d", "1d", dur, off)
-	}
-
-	dur, off = DurationOffsetStrings(25*time.Hour, 1*time.Second)
-	if dur != "25h" || off != "1s" {
-		t.Fatalf("DurationOffsetStrings: exp (%s %s); act (%s, %s)", "25h", "1s", dur, off)
-	}
-
-	dur, off = DurationOffsetStrings(24*time.Hour+time.Second, 1*time.Second)
-	if dur != "86401s" || off != "1s" {
-		t.Fatalf("DurationOffsetStrings: exp (%s %s); act (%s, %s)", "86401s", "1s", dur, off)
-	}
-
-	// Expect empty strings if durations are negative
-	dur, off = DurationOffsetStrings(-25*time.Hour, -1*time.Second)
-	if dur != "" || off != "" {
-		t.Fatalf("DurationOffsetStrings: exp (%s %s); act (%s, %s)", "", "", dur, off)
-	}
-}

pkg/util/time.go → pkg/util/timeutil/timeutil.go

@@ -1,8 +1,10 @@
-package util
+package timeutil
 
 import (
 	"fmt"
+	"regexp"
 	"strconv"
+	"strings"
 	"sync"
 	"time"
 )
@@ -50,7 +52,7 @@ func DurationString(duration time.Duration) string {
 			// convert to mins
 			durStr = fmt.Sprintf("%dm", durSecs/SecsPerMin)
 		} else if durSecs > 0 {
-			// default to mins, as long as duration is positive
+			// default to secs, as long as duration is positive
 			durStr = fmt.Sprintf("%ds", durSecs)
 		}
 	}
@@ -58,14 +60,39 @@ func DurationString(duration time.Duration) string {
 	return durStr
 }
 
+// DurationToPromOffsetString returns a Prometheus formatted string with leading offset or empty string if given a negative duration
+func DurationToPromOffsetString(duration time.Duration) string {
+	dirStr := DurationString(duration)
+	if dirStr != "" {
+		dirStr = fmt.Sprintf("offset %s", dirStr)
+	}
+	return dirStr
+}
+
 // DurationOffsetStrings converts a (duration, offset) pair to Prometheus-
 // compatible strings in terms of days, hours, minutes, or seconds.
 func DurationOffsetStrings(duration, offset time.Duration) (string, string) {
 	return DurationString(duration), DurationString(offset)
 }
 
+// FormatStoreResolution provides a clean notation for ETL store resolutions.
+// e.g. daily => 1d; hourly => 1h
+func FormatStoreResolution(dur time.Duration) string {
+	if dur >= 24*time.Hour {
+		return fmt.Sprintf("%dd", int(dur.Hours()/24.0))
+	} else if dur >= time.Hour {
+		return fmt.Sprintf("%dh", int(dur.Hours()))
+	}
+	return fmt.Sprint(dur)
+}
+
 // ParseDuration converts a Prometheus-style duration string into a Duration
-func ParseDuration(duration string) (*time.Duration, error) {
+func ParseDuration(duration string) (time.Duration, error) {
+	// Trim prefix of Prometheus format duration
+	duration = CleanDurationString(duration)
+	if len(duration) < 2 {
+		return 0, fmt.Errorf("error parsing duration: %s did not match expected format [0-9+](s|m|d|h)", duration)
+	}
 	unitStr := duration[len(duration)-1:]
 	var unit time.Duration
 	switch unitStr {
@@ -78,52 +105,51 @@ func ParseDuration(duration string) (*time.Duration, error) {
 	case "d":
 		unit = 24.0 * time.Hour
 	default:
-		return nil, fmt.Errorf("error parsing duration: %s did not match expected format [0-9+](s|m|d|h)", duration)
+		return 0, fmt.Errorf("error parsing duration: %s did not match expected format [0-9+](s|m|d|h)", duration)
 	}
 
 	amountStr := duration[:len(duration)-1]
 	amount, err := strconv.ParseInt(amountStr, 10, 64)
 	if err != nil {
-		return nil, fmt.Errorf("error parsing duration: %s did not match expected format [0-9+](s|m|d|h)", duration)
+		return 0, fmt.Errorf("error parsing duration: %s did not match expected format [0-9+](s|m|d|h)", duration)
 	}
 
-	dur := time.Duration(amount) * unit
-	return &dur, nil
+	return time.Duration(amount) * unit, nil
+}
+
+// CleanDurationString removes prometheus formatted prefix "offset " allong with leading a trailing whitespace
+// from duration string, leaving behind a string with format [0-9+](s|m|d|h)
+func CleanDurationString(duration string) string {
+	duration = strings.TrimSpace(duration)
+	duration = strings.TrimPrefix(duration, "offset ")
+	return duration
 }
 
 // ParseTimeRange returns a start and end time, respectively, which are converted from
 // a duration and offset, defined as strings with Prometheus-style syntax.
-func ParseTimeRange(duration, offset string) (*time.Time, *time.Time, error) {
+func ParseTimeRange(duration, offset time.Duration) (time.Time, time.Time) {
 	// endTime defaults to the current time, unless an offset is explicity declared,
 	// in which case it shifts endTime back by given duration
 	endTime := time.Now()
-	if offset != "" {
-		o, err := ParseDuration(offset)
-		if err != nil {
-			return nil, nil, fmt.Errorf("error parsing offset (%s): %s", offset, err)
-		}
-		endTime = endTime.Add(-1 * *o)
+	if offset > 0 {
+		endTime = endTime.Add(-1 * offset)
 	}
 
-	// if duration is defined in terms of days, convert to hours
-	// e.g. convert "2d" to "48h"
-	durationNorm, err := normalizeTimeParam(duration)
-	if err != nil {
-		return nil, nil, fmt.Errorf("error parsing duration (%s): %s", duration, err)
-	}
+	startTime := endTime.Add(-1 * duration)
 
-	// convert time duration into start and end times, formatted
-	// as ISO datetime strings
-	dur, err := time.ParseDuration(durationNorm)
-	if err != nil {
-		return nil, nil, fmt.Errorf("errorf parsing duration (%s): %s", durationNorm, err)
-	}
-	startTime := endTime.Add(-1 * dur)
-
-	return &startTime, &endTime, nil
+	return startTime, endTime
 }
 
-func normalizeTimeParam(param string) (string, error) {
+// FormatDurationStringDaysToHours converts string from format [0-9+]d to [0-9+]h
+func FormatDurationStringDaysToHours(param string) (string, error) {
+	//check that input matches format
+	ok, err := regexp.MatchString("[0-9+]d", param)
+	if !ok {
+		return param, fmt.Errorf("FormatDurationStringDaysToHours: input string (%s) not formatted as [0-9+]d", param)
+	}
+	if err != nil {
+		return "", err
+	}
 	// convert days to hours
 	if param[len(param)-1:] == "d" {
 		count := param[:len(param)-1]
@@ -138,17 +164,6 @@ func normalizeTimeParam(param string) (string, error) {
 	return param, nil
 }
 
-// FormatStoreResolution provides a clean notation for ETL store resolutions.
-// e.g. daily => 1d; hourly => 1h
-func FormatStoreResolution(dur time.Duration) string {
-	if dur >= 24*time.Hour {
-		return fmt.Sprintf("%dd", int(dur.Hours()/24.0))
-	} else if dur >= time.Hour {
-		return fmt.Sprintf("%dh", int(dur.Hours()))
-	}
-	return fmt.Sprint(dur)
-}
-
 // JobTicker is a ticker used to synchronize the next run of a repeating
 // process. The designated use-case is for infinitely-looping selects,
 // where a timeout or an exit channel might cancel the process, but otherwise
@@ -222,4 +237,4 @@ func (jt *JobTicker) TickIn(d time.Duration) {
 			jt.ch <- time.Now()
 		}
 	}(d)
-}
+}

pkg/util/timeutil/timeutil_test.go

@@ -0,0 +1,381 @@
+package timeutil
+
+import (
+	"testing"
+	"time"
+)
+
+func Test_DurationString(t *testing.T) {
+	testCases := map[string]struct {
+		duration time.Duration
+		expectedDuration string
+	}{
+		"1a": {
+			duration:         0,
+			expectedDuration: "",
+		},
+		"1b": {
+			duration:         24*time.Hour,
+			expectedDuration: "1d",
+		},
+		"1c": {
+			duration:         24*time.Hour+5*time.Minute,
+			expectedDuration: "1445m",
+		},
+		"1d": {
+			duration:         25*time.Hour,
+			expectedDuration: "25h",
+		},
+		"1e": {
+			duration:         25*time.Hour,
+			expectedDuration: "25h",
+		},
+		"1f": {
+			duration:         72*time.Hour,
+			expectedDuration: "3d",
+		},
+		"1g": {
+			duration:         25*time.Hour,
+			expectedDuration: "25h",
+		},
+		"1h": {
+			duration:         24*time.Hour+time.Second,
+			expectedDuration: "86401s",
+		},
+		// Expect empty strings if durations are negative
+		"1i": {
+			duration:         -25*time.Hour,
+			expectedDuration: "",
+		},
+	}
+
+	for name, test := range testCases {
+		t.Run(name, func(t *testing.T) {
+			dur := DurationString(test.duration)
+			if dur != test.expectedDuration {
+				t.Fatalf("DurationOffsetStrings: exp (%s); act (%s)", test.expectedDuration, dur)
+			}
+		})
+	}
+}
+
+func Test_DurationToPromOffsetString(t *testing.T) {
+	testCases := map[string]struct {
+		duration time.Duration
+		expectedDuration string
+	}{
+		"1a": {
+			duration:         0,
+			expectedDuration: "",
+		},
+		"1b": {
+			duration:         24*time.Hour,
+			expectedDuration: "offset 1d",
+		},
+		"1c": {
+			duration:         24*time.Hour+5*time.Minute,
+			expectedDuration: "offset 1445m",
+		},
+		"1d": {
+			duration:         25*time.Hour,
+			expectedDuration: "offset 25h",
+		},
+		"1e": {
+			duration:         25*time.Hour,
+			expectedDuration: "offset 25h",
+		},
+		"1f": {
+			duration:         72*time.Hour,
+			expectedDuration: "offset 3d",
+		},
+		"1g": {
+			duration:         25*time.Hour,
+			expectedDuration: "offset 25h",
+		},
+		"1h": {
+			duration:         24*time.Hour+time.Second,
+			expectedDuration: "offset 86401s",
+		},
+		// Expect empty strings if durations are negative
+		"1i": {
+			duration:         -25*time.Hour,
+			expectedDuration: "",
+		},
+	}
+
+	for name, test := range testCases {
+		t.Run(name, func(t *testing.T) {
+			dur := DurationToPromOffsetString(test.duration)
+			if dur != test.expectedDuration {
+				t.Fatalf("DurationOffsetStrings: exp (%s); act (%s)", test.expectedDuration, dur)
+			}
+		})
+	}
+}
+
+func Test_FormatStoreResolution(t *testing.T) {
+	testCases := map[string]struct {
+		duration time.Duration
+		expectedDuration string
+	}{
+		"1a": {
+			duration:         0,
+			expectedDuration: "0s",
+		},
+		"1b": {
+			duration:         24*time.Hour,
+			expectedDuration: "1d",
+		},
+		"1c": {
+			duration:         24*time.Hour+5*time.Minute,
+			expectedDuration: "1d",
+		},
+		"1d": {
+			duration:         25*time.Hour,
+			expectedDuration: "1d",
+		},
+		"1e": {
+			duration:         25*time.Hour,
+			expectedDuration: "1d",
+		},
+		"1f": {
+			duration:         72*time.Hour,
+			expectedDuration: "3d",
+		},
+		"1g": {
+			duration:         25*time.Hour,
+			expectedDuration: "1d",
+		},
+		"1h": {
+			duration:         24*time.Hour+time.Second,
+			expectedDuration: "1d",
+		},
+		// Expect empty strings if durations are negative
+		"1i": {
+			duration:         -25*time.Hour,
+			expectedDuration: "-25h0m0s",
+		},
+	}
+
+	for name, test := range testCases {
+		t.Run(name, func(t *testing.T) {
+			dur := FormatStoreResolution(test.duration)
+			if dur != test.expectedDuration {
+				t.Fatalf("DurationOffsetStrings: exp (%s); act (%s)", test.expectedDuration, dur)
+			}
+		})
+	}
+}
+
+func Test_DurationOffsetStrings(t *testing.T) {
+	testCases := map[string]struct {
+		duration time.Duration
+		offset time.Duration
+		expectedDuration string
+		expectedOffset string
+	}{
+		"1a": {
+			duration:         0,
+			offset:           0,
+			expectedDuration: "",
+			expectedOffset:   "",
+		},
+		"1b": {
+			duration:         24*time.Hour,
+			offset:           0,
+			expectedDuration: "1d",
+			expectedOffset:   "",
+		},
+		"1c": {
+			duration:         24*time.Hour+5*time.Minute,
+			offset:           0,
+			expectedDuration: "1445m",
+			expectedOffset:   "",
+		},
+		"1d": {
+			duration:         25*time.Hour,
+			offset:           5*time.Minute,
+			expectedDuration: "25h",
+			expectedOffset:   "5m",
+		},
+		"1e": {
+			duration:         25*time.Hour,
+			offset:           60*time.Minute,
+			expectedDuration: "25h",
+			expectedOffset:   "1h",
+		},
+		"1f": {
+			duration:         72*time.Hour,
+			offset:           1440*time.Minute,
+			expectedDuration: "3d",
+			expectedOffset:   "1d",
+		},
+		"1g": {
+			duration:         25*time.Hour,
+			offset:           1*time.Second,
+			expectedDuration: "25h",
+			expectedOffset:   "1s",
+		},
+		"1h": {
+			duration:         24*time.Hour+time.Second,
+			offset:           1*time.Second,
+			expectedDuration: "86401s",
+			expectedOffset:   "1s",
+		},
+		// Expect empty strings if durations are negative
+		"1i": {
+			duration:         -25*time.Hour,
+			offset:           -1*time.Second,
+			expectedDuration: "",
+			expectedOffset:   "",
+		},
+	}
+
+	for name, test := range testCases {
+		t.Run(name, func(t *testing.T) {
+			dur, off:= DurationOffsetStrings(test.duration, test.offset)
+			if dur != test.expectedDuration || off != test.expectedOffset {
+				t.Fatalf("DurationOffsetStrings: exp (%s %s); act (%s, %s)", test.expectedDuration, test.expectedOffset, dur, off)
+			}
+		})
+	}
+}
+
+func Test_ParseDuration(t *testing.T) {
+	testCases := map[string]struct {
+		input    string
+		expected time.Duration
+	}{
+		"expected": {
+			input:    "3h",
+			expected: time.Hour * 3,
+		},
+		"white space": {
+			input:    " 4s ",
+			expected: time.Second * 4,
+		},
+		"prom prefix": {
+			input:    "offset 3m",
+			expected: time.Minute * 3,
+		},
+		"prom prefix white space": {
+			input:    " offset 3d ",
+			expected: 24.0 * time.Hour * 3,
+		},
+		"zero": {
+			input:    "0h",
+			expected: time.Duration(0),
+		},
+		"empty": {
+			input:    "",
+			expected: time.Duration(0),
+		},
+		"bad string": {
+			input:    "oqwd3dk5hk",
+			expected: time.Duration(0),
+		},
+		"digit": {
+			input:    "3",
+			expected: time.Duration(0),
+		},
+		"unit": {
+			input:    "h",
+			expected: time.Duration(0),
+		},
+	}
+	for name, test := range testCases {
+		t.Run(name, func(t *testing.T) {
+			dur, _ := ParseDuration(test.input)
+			if dur != test.expected {
+				t.Errorf("Expected duration %v did not match result %v", test.expected, dur)
+			}
+		})
+	}
+}
+
+func Test_CleanDurationString(t *testing.T) {
+	testCases := map[string]struct {
+		input    string
+		expected string
+	}{
+		"white space": {
+			input:    " 1d ",
+			expected: "1d",
+		},
+		"no change": {
+			input:    "1d",
+			expected: "1d",
+		},
+		"prefix": {
+			input:    "offset 1d",
+			expected: "1d",
+		},
+		"prefix white space": {
+			input:    " offset 1d ",
+			expected: "1d",
+		},
+		"empty": {
+			input:    "",
+			expected: "",
+		},
+		"random": {
+			input:    "oqwd3dk5hk",
+			expected: "oqwd3dk5hk",
+		},
+
+
+	}
+	for name, test := range testCases {
+		t.Run(name, func(t *testing.T) {
+			res := CleanDurationString(test.input)
+			if res != test.expected {
+				t.Errorf("Expected output %s did not match result %s", test.expected, res)
+			}
+		})
+	}
+}
+
+func Test_FormatDurationStringDaysToHours(t *testing.T) {
+	testCases := map[string]struct {
+		input    string
+		expected string
+	}{
+		"1 day": {
+			input:    "1d",
+			expected: "24h",
+		},
+		"2 days": {
+			input:    "1d",
+			expected: "24h",
+		},
+		"500 days": {
+			input:    "500d",
+			expected: "12000h",
+		},
+		"1h": {
+			input:    "1h",
+			expected: "1h",
+		},
+		"empty": {
+			input:    "",
+			expected: "",
+		},
+		"no unit": {
+			input:    "1",
+			expected: "1",
+		},
+		"random": {
+			input:    "oqwd3dk5hk",
+			expected: "oqwd3dk5hk",
+		},
+
+	}
+	for name, test := range testCases {
+		t.Run(name, func(t *testing.T) {
+			res, _ := FormatDurationStringDaysToHours(test.input)
+			if res != test.expected {
+				t.Errorf("Expected output %s did not match result %s", test.expected, res)
+			}
+		})
+	}
+}