Explorar o código

Refactor time.go, clean out duplicate methods, check for whitespace, add

Sean Holcomb %!s(int64=4) %!d(string=hai) anos
pai
achega
53a9557e12

+ 5 - 4
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)
@@ -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)
 			}
@@ -1492,7 +1493,7 @@ func (a *Accesses) ComputeAggregateCostModel(promClient prometheusClient.Client,
 		}
 
 		// Convert to Prometheus-compatible strings
-		durStr, offStr := util.DurationOffsetStrings(dur, off)
+		durStr, offStr := timeutil.DurationOffsetStrings(dur, off)
 
 		idleCoefficients, err = a.ComputeIdleCoefficient(costData, promClient, a.CloudProvider, discount, customDiscount, durStr, offStr)
 		if err != nil {

+ 3 - 3
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"
 )
 
@@ -124,7 +124,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)
 
@@ -441,7 +441,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)
 

+ 10 - 8
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"
@@ -71,7 +71,8 @@ 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)
+	offset = timeutil.CleanDurationString(offset)
+	start, end, err := timeutil.ParseTimeRange(window, offset)
 	if err != nil {
 		return nil, err
 	}
@@ -94,10 +95,10 @@ func NewClusterCostsFromCumulative(cpu, gpu, ram, storage float64, window, offse
 		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
 
@@ -704,7 +705,8 @@ 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) {
 	// Compute number of minutes in the full interval, for use interpolating missed scrapes or scaling missing data
-	start, end, err := util.ParseTimeRange(window, offset)
+	offset = timeutil.CleanDurationString(offset)
+	start, end, err := timeutil.ParseTimeRange(window, offset)
 	if err != nil {
 		return nil, err
 	}
@@ -1003,7 +1005,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

+ 3 - 30
pkg/costmodel/router.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"flag"
 	"fmt"
+	"github.com/kubecost/cost-model/pkg/util/timeutil"
 	"net/http"
 	"reflect"
 	"strconv"
@@ -224,7 +225,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,34 +244,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) {
@@ -278,7 +251,7 @@ func ParseTimeRange(duration, offset string) (*time.Time, *time.Time, error) {
 	// in which case it shifts endTime back by given duration
 	endTime := time.Now()
 	if offset != "" {
-		o, err := ParseDuration(offset)
+		o, err := timeutil.ParseDuration(offset)
 		if err != nil {
 			return nil, nil, fmt.Errorf("error parsing offset (%s): %s", offset, err)
 		}

+ 3 - 3
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 {

+ 4 - 65
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,12 +397,12 @@ 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
 	}
 
-	return d
+	return *d
 }
 
 // GetList returns a string list which contains the value set by key split using the
@@ -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
-}

+ 91 - 75
pkg/util/time.go → pkg/util/timeutil/timeutil.go

@@ -1,8 +1,9 @@
-package util
+package timeutil
 
 import (
 	"fmt"
 	"strconv"
+	"strings"
 	"sync"
 	"time"
 )
@@ -64,80 +65,6 @@ func DurationOffsetStrings(duration, offset time.Duration) (string, string) {
 	return DurationString(duration), DurationString(offset)
 }
 
-// ParseDuration converts a Prometheus-style duration string into a Duration
-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 normalizeTimeParam(param string) (string, error) {
-	// convert days to hours
-	if param[len(param)-1:] == "d" {
-		count := param[:len(param)-1]
-		val, err := strconv.ParseInt(count, 10, 64)
-		if err != nil {
-			return "", err
-		}
-		val = val * 24
-		param = fmt.Sprintf("%dh", val)
-	}
-
-	return param, nil
-}
-
 // FormatStoreResolution provides a clean notation for ETL store resolutions.
 // e.g. daily => 1d; hourly => 1h
 func FormatStoreResolution(dur time.Duration) string {
@@ -223,3 +150,92 @@ func (jt *JobTicker) TickIn(d time.Duration) {
 		}
 	}(d)
 }
+
+
+
+// ParseDuration converts a Prometheus-style duration string into a Duration
+func ParseDuration(duration string) (*time.Duration, error) {
+	// Trim prefix of Prometheus format duration
+	duration = CleanDurationString(duration)
+	if len(duration) < 2 {
+		return nil, 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 {
+	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
+}
+
+// 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) {
+	// 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 normalizeTimeParam(param string) (string, error) {
+	// convert days to hours
+	if param[len(param)-1:] == "d" {
+		count := param[:len(param)-1]
+		val, err := strconv.ParseInt(count, 10, 64)
+		if err != nil {
+			return "", err
+		}
+		val = val * 24
+		param = fmt.Sprintf("%dh", val)
+	}
+
+	return param, nil
+}

+ 63 - 1
pkg/util/time_test.go → pkg/util/timeutil/timeutil_test.go

@@ -1,4 +1,4 @@
-package util
+package timeutil
 
 import (
 	"testing"
@@ -54,3 +54,65 @@ func TestDurationOffsetStrings(t *testing.T) {
 		t.Fatalf("DurationOffsetStrings: exp (%s %s); act (%s, %s)", "", "", dur, off)
 	}
 }
+
+
+func TestParseDuration(t *testing.T) {
+	testCases := map[string]struct {
+		input string
+		expected time.Duration
+		errorExpected bool
+	} {
+		"expected" : {
+			input: "3h",
+			expected: time.Hour * 3,
+			errorExpected: false,
+		},
+		"white space" : {
+			input: " 4s ",
+			expected: time.Second * 4,
+			errorExpected: false,
+		},
+		"prom prefix" : {
+			input: "offset 3m",
+			expected: time.Minute * 3,
+			errorExpected: false,
+		},
+		"prom prefix white space" : {
+			input: " offset 3d ",
+			expected: 24.0 * time.Hour * 3,
+			errorExpected: false,
+		},
+		"empty" : {
+			input: "",
+			expected: time.Second,
+			errorExpected: true,
+		},
+		"bad string" : {
+			input: "oqwd3dk5hk",
+			expected: time.Second,
+			errorExpected: true,
+		},
+		"digit" : {
+			input: "3",
+			expected: time.Second,
+			errorExpected: true,
+		},
+		"unit" : {
+			input: "h",
+			expected: time.Second,
+			errorExpected: true,
+		},
+	}
+	for name, test := range testCases {
+		t.Run(name, func(t *testing.T) {
+			dur, err := ParseDuration(test.input)
+			if err != nil && test.errorExpected{
+				return
+			}
+			if *dur != test.expected {
+				t.Errorf("Expected duration %v did not match result %v", test.expected, dur)
+			}
+		})
+	}
+
+}