diff --git a/Gopkg.lock b/Gopkg.lock index 6a753d59..df256664 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -483,6 +483,7 @@ "github.com/kelseyhightower/envconfig", "github.com/phayes/freeport", "github.com/satori/go.uuid", + "github.com/stretchr/testify/assert", "github.com/stretchr/testify/require", "golang.org/x/net/http/httpguts", "golang.org/x/net/http2", diff --git a/README.md b/README.md index 8e05f2a4..d1a6b9cb 100644 --- a/README.md +++ b/README.md @@ -101,7 +101,7 @@ The following table lists the configurable parameters of the Helm chart and thei | Parameter | Description | Default | | --------- | ----------- | ------- | -| `zeroscaler.metricsCheckInterval` | The interval in which the zeroScaler would repeatedly track the pod http request metrics. The value is the number of seconds of the interval. | `150` | +| `zeroscaler.metricsCheckInterval` | The interval in which the zeroScaler would repeatedly track the pod http request metrics. The value is the number of seconds of the interval. Note that this can also be set on a per-deployment basis, with an annotation. | `150` | Example of installation with Helm and a custom configuration: @@ -178,6 +178,7 @@ The following table lists the supported annotations for Kubernetes `Deployments` | ---------- | ----------- | ------- | | `osiris.deislabs.io/enabled` | Enable Osiris for this deployment. Allowed values: `y`, `yes`, `true`, `on`, `1`. | _no value_ (= disabled) | | `osiris.deislabs.io/minReplicas` | The minimum number of replicas to set on the deployment when Osiris will scale up. If you set `2`, Osiris will scale the deployment from `0` to `2` replicas directly. Osiris won't collect metrics from deployments which have more than `minReplicas` replicas - to avoid useless collections of metrics. | `1` | +| `osiris.deislabs.io/metricsCheckInterval` | The interval in which Osiris would repeatedly track the pod http request metrics. The value is the number of seconds of the interval. Note that this value override the global value defined by the `zeroscaler.metricsCheckInterval` Helm value. | _value of the `zeroscaler.metricsCheckInterval` Helm value_ | #### Service Annotations diff --git a/example/hello-osiris.yaml b/example/hello-osiris.yaml index 53c945d0..b0727a2e 100644 --- a/example/hello-osiris.yaml +++ b/example/hello-osiris.yaml @@ -45,6 +45,7 @@ metadata: annotations: osiris.deislabs.io/enabled: "true" osiris.deislabs.io/minReplicas: "1" + osiris.deislabs.io/metricsCheckInterval: "120" # seconds spec: replicas: 1 selector: diff --git a/pkg/deployments/zeroscaler/zeroscaler.go b/pkg/deployments/zeroscaler/zeroscaler.go index d48b9cfc..ef42e991 100644 --- a/pkg/deployments/zeroscaler/zeroscaler.go +++ b/pkg/deployments/zeroscaler/zeroscaler.go @@ -131,17 +131,43 @@ func (z *zeroscaler) ensureMetricsCollection(deployment *appsv1.Deployment) { if ok { collector.stop() } + metricsCheckInterval, err := k8s.GetMetricsCheckInterval( + deployment.Annotations, + ) + if err != nil { + glog.Warningf( + "There was an error getting custom metrics check interval value "+ + "in deployment %s, falling back to the default value of %d "+ + "seconds; error: %s", + deployment.Name, + z.cfg.MetricsCheckInterval, + err, + ) + metricsCheckInterval = z.cfg.MetricsCheckInterval + } + if metricsCheckInterval <= 0 { + glog.Warningf( + "Invalid custom metrics check interval value %d in deployment %s,"+ + " falling back to the default value of %d seconds", + metricsCheckInterval, + deployment.Name, + z.cfg.MetricsCheckInterval, + ) + metricsCheckInterval = z.cfg.MetricsCheckInterval + } glog.Infof( - "Using new metrics collector for deployment %s in namespace %s", + "Using new metrics collector for deployment %s in namespace %s "+ + "with metrics check interval of %d seconds", deployment.Name, deployment.Namespace, + metricsCheckInterval, ) collector := newMetricsCollector( z.kubeClient, deployment.Name, deployment.Namespace, selector, - time.Duration(z.cfg.MetricsCheckInterval)*time.Second, + time.Duration(metricsCheckInterval)*time.Second, ) go func() { collector.run(z.ctx) diff --git a/pkg/kubernetes/osiris.go b/pkg/kubernetes/osiris.go index b9c713fb..5e098fc6 100644 --- a/pkg/kubernetes/osiris.go +++ b/pkg/kubernetes/osiris.go @@ -1,10 +1,15 @@ package kubernetes import ( + "fmt" "strconv" "strings" ) +const ( + metricsCheckIntervalAnnotationName = "osiris.deislabs.io/metricsCheckInterval" +) + // ResourceIsOsirisEnabled checks the annotations to see if the // kube resource is enabled for osiris or not. func ResourceIsOsirisEnabled(annotations map[string]string) bool { @@ -34,3 +39,27 @@ func GetMinReplicas(annotations map[string]string, defaultVal int32) int32 { } return int32(minReplicas) } + +// GetMetricsCheckInterval gets the interval in which the zeroScaler would +// repeatedly track the pod http request metrics. The value is the number +// of seconds of the interval. If it fails to do so, it returns an error. +func GetMetricsCheckInterval(annotations map[string]string) (int, error) { + if len(annotations) == 0 { + return 0, nil + } + val, ok := annotations[metricsCheckIntervalAnnotationName] + if !ok { + return 0, nil + } + metricsCheckInterval, err := strconv.Atoi(val) + if err != nil { + return 0, fmt.Errorf("invalid int value '%s' for '%s' annotation: %s", + val, metricsCheckIntervalAnnotationName, err) + } + if metricsCheckInterval <= 0 { + return 0, fmt.Errorf("metricsCheckInterval should be positive, "+ + "'%d' is not a valid value", + metricsCheckInterval) + } + return metricsCheckInterval, nil +} diff --git a/pkg/kubernetes/osiris_test.go b/pkg/kubernetes/osiris_test.go index b7e223bc..9fb4a3b0 100644 --- a/pkg/kubernetes/osiris_test.go +++ b/pkg/kubernetes/osiris_test.go @@ -1,6 +1,11 @@ package kubernetes -import "testing" +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) func TestResourceIsOsirisEnabled(t *testing.T) { testcases := []struct { @@ -113,3 +118,82 @@ func TestGetMinReplicas(t *testing.T) { }) } } + +func TestGetMetricsCheckInterval(t *testing.T) { + testcases := []struct { + name string + annotations map[string]string + expectedResult int + expectedError string + }{ + { + name: "nil map", + annotations: nil, + expectedResult: 0, + expectedError: "", + }, + { + name: "empty map", + annotations: map[string]string{}, + expectedResult: 0, + expectedError: "", + }, + { + name: "map with no metrics check interval entry", + annotations: map[string]string{ + "whatever": "60", + }, + expectedResult: 0, + expectedError: "", + }, + { + name: "map with invalid metrics check interval entry", + annotations: map[string]string{ + "osiris.deislabs.io/metricsCheckInterval": "invalid", + }, + expectedResult: 0, + expectedError: "invalid int value 'invalid' for " + + "'osiris.deislabs.io/metricsCheckInterval' annotation: " + + "strconv.Atoi: parsing \"invalid\": invalid syntax", + }, + { + name: "map with negative metrics check interval entry", + annotations: map[string]string{ + "osiris.deislabs.io/metricsCheckInterval": "-1", + }, + expectedResult: 0, + expectedError: "metricsCheckInterval should be positive, " + + "'-1' is not a valid value", + }, + { + name: "map with zero metrics check interval entry", + annotations: map[string]string{ + "osiris.deislabs.io/metricsCheckInterval": "0", + }, + expectedResult: 0, + expectedError: "metricsCheckInterval should be positive, " + + "'0' is not a valid value", + }, + { + name: "map with valid metrics check interval entry", + annotations: map[string]string{ + "osiris.deislabs.io/metricsCheckInterval": "60", + }, + expectedResult: 60, + expectedError: "", + }, + } + + for _, test := range testcases { + t.Run(test.name, func(t *testing.T) { + actual, err := GetMetricsCheckInterval(test.annotations) + if len(test.expectedError) > 0 { + require.EqualError(t, err, test.expectedError, "") + } else { + require.NoError(t, err) + } + + assert.Equal(t, test.expectedResult, actual) + }) + } +}