Feat/overlapping timespan check

README.md

@@ -222,16 +222,25 @@ Mon-Tue 20:00-00:00 Europe/Amsterdam # On Monday and Tuesday: from 20:00 to midn
 
 Valid Values:
 
-- Weekdays: (case-insensitive)
-  - Mon
-  - Tue
-  - Wed
-  - Thu
-  - Fri
-  - Sat
-  - Sun
-- Timezones: all from the [IANA Time Zone database](https://www.iana.org/time-zones)
-- Time of day: 00:00 - 24:00
+Weekdays: (case-insensitive)
+
+- Mon
+- Tue
+- Wed
+- Thu
+- Fri
+- Sat
+- Sun
+
+Timezones:
+
+- all from the [IANA Time Zone database](https://www.iana.org/time-zones)
+
+> [!Note]
+> The IANA Time Zone database mainly supports regional/city timezones (example: `Europe/Berlin`, `America/Los_Angeles`) instead of abbreviations (example: `CEST`, `PST`, `PDT`).
+> It supports some abbreviations like `CET`, `MET` and `PST8PDT` but these (not including `UTC`) shouldn't be used, and only exist for backwards compatibility.
+
+Time of day: 00:00 - 24:00
 
 #### Multiple/Complex Timespans
 

cmd/kubedownscaler/main.go

@@ -71,6 +71,10 @@ func main() {
 	if debug {
 		slog.SetLogLoggerLevel(slog.LevelDebug)
 	}
+	if err := layerCli.CheckForIncompatibleFields(); err != nil {
+		slog.Error("found incompatible fields", "error", err)
+		os.Exit(1)
+	}
 	ctx := context.Background()
 
 	client, err := kubernetes.NewClient(kubeconfig)
@@ -147,10 +151,6 @@ func scanWorkload(workload scalable.Workload, client kubernetes.Client, ctx cont
 		slog.Error("failed to get current scaling for workload", "error", err, "workload", workload.GetName(), "namespace", workload.GetNamespace())
 		return false
 	}
-	if scaling == values.ScalingIncompatible {
-		slog.Error("scaling is incompatible, skipping", "workload", workload.GetName(), "namespace", workload.GetNamespace())
-		return false
-	}
 	if scaling == values.ScalingIgnore {
 		slog.Debug("scaling is ignored, skipping", "workload", workload.GetName(), "namespace", workload.GetNamespace())
 		return true

internal/api/kubernetes/resourceLogger.go

@@ -7,7 +7,7 @@ import (
 	"github.com/caas-team/gokubedownscaler/internal/pkg/scalable"
 )
 
-const reasonInvalidAnnotation = "InvalidAnnotation"
+const reasonInvalidConfiguration = "InvalidConfiguration"
 
 func NewResourceLogger(client Client, workload scalable.Workload) resourceLogger {
 	logger := resourceLogger{
@@ -22,9 +22,9 @@ type resourceLogger struct {
 	client   Client
 }
 
-// ErrorInvalidAnnotation adds an error on the resource
-func (r resourceLogger) ErrorInvalidAnnotation(id, message string, ctx context.Context) {
-	err := r.client.AddErrorEvent(reasonInvalidAnnotation, id, message, r.workload, ctx)
+// ErrorInvalidConfiguration adds an error on the resource
+func (r resourceLogger) ErrorInvalidConfiguration(id, message string, ctx context.Context) {
+	err := r.client.AddErrorEvent(reasonInvalidConfiguration, id, message, r.workload, ctx)
 	if err != nil {
 		slog.Error("failed to add error event to workload", "workload", r.workload.GetName(), "error", err)
 		return

internal/pkg/values/layer.go

@@ -2,16 +2,16 @@ package values
 
 import (
 	"errors"
-	"fmt"
 	"time"
 )
 
 var (
-	errForceUpAndDownTime       = errors.New("error: both forceUptime and forceDowntime are defined")
-	errUpAndDownTime            = errors.New("error: both uptime and downtime are defined")
-	errTimeAndPeriod            = errors.New("error: both a time and a period is defined")
-	errInvalidDownscaleReplicas = errors.New("error: downscale replicas value is invalid")
-	errValueNotSet              = errors.New("error: no layer implements this value")
+	errForceUpAndDownTime        = errors.New("error: both forceUptime and forceDowntime are defined")
+	errUpAndDownTime             = errors.New("error: both uptime and downtime are defined")
+	errTimeAndPeriod             = errors.New("error: both a time and a period is defined")
+	errInvalidDownscaleReplicas  = errors.New("error: downscale replicas value is invalid")
+	errValueNotSet               = errors.New("error: no layer implements this value")
+	errUpAndDownscaleOverlapping = errors.New("error: up- and downscale periods are overlapping")
 )
 
 const Undefined = -1 // Undefined represents an undefined integer value
@@ -20,11 +20,10 @@ const Undefined = -1 // Undefined represents an undefined integer value
 type scaling int
 
 const (
-	scalingNone         scaling = iota // no scaling set in this layer, go to next layer
-	ScalingIncompatible                // incompatible scaling fields set, error
-	ScalingIgnore                      // not scaling
-	ScalingDown                        // scaling down
-	ScalingUp                          // scaling up
+	scalingNone   scaling = iota // no scaling set in this layer, go to next layer
+	ScalingIgnore                // not scaling
+	ScalingDown                  // scaling down
+	ScalingUp                    // scaling up
 )
 
 // NewLayer gets a new layer with the default values
@@ -60,8 +59,8 @@ func (l Layer) isScalingExcluded() *bool {
 	return nil
 }
 
-// checkForIncompatibleFields checks if there are incompatible fields
-func (l Layer) checkForIncompatibleFields() error {
+// CheckForIncompatibleFields checks if there are incompatible fields
+func (l Layer) CheckForIncompatibleFields() error {
 	// force down and uptime
 	if l.ForceDowntime.isSet &&
 		l.ForceDowntime.value &&
@@ -82,6 +81,14 @@ func (l Layer) checkForIncompatibleFields() error {
 		(l.UpscalePeriod != nil || l.DownscalePeriod != nil) {
 		return errTimeAndPeriod
 	}
+	// up- and downscale periods overlapping
+	for _, upscale := range l.UpscalePeriod {
+		for _, downscale := range l.DownscalePeriod {
+			if doTimespansOverlap(upscale, downscale) {
+				return errUpAndDownscaleOverlapping
+			}
+		}
+	}
 	return nil
 }
 
@@ -131,13 +138,6 @@ type Layers []Layer
 
 // GetCurrentScaling gets the current scaling of the first layer that implements scaling
 func (l Layers) GetCurrentScaling() (scaling, error) {
-	// check for incompatibilities
-	for _, layer := range l {
-		err := layer.checkForIncompatibleFields()
-		if err != nil {
-			return ScalingIncompatible, fmt.Errorf("error found incompatible fields: %w", err)
-		}
-	}
 	// check for forced scaling
 	for _, layer := range l {
 		forcedScaling := layer.getForcedScaling()

internal/pkg/values/layer_test.go

@@ -31,56 +31,84 @@ func TestLayer_checkForIncompatibleFields(t *testing.T) {
 		{
 			name: "up- and downtime",
 			layer: Layer{
-				UpTime:   timeSpans{relativeTimeSpan{}},
-				DownTime: timeSpans{relativeTimeSpan{}},
+				UpTime:   timeSpans{&relativeTimeSpan{}},
+				DownTime: timeSpans{&relativeTimeSpan{}},
 			},
 			wantErr: true,
 		},
 		{
 			name: "uptime an upscaleperiod",
 			layer: Layer{
-				UpTime:        timeSpans{relativeTimeSpan{}},
-				UpscalePeriod: timeSpans{relativeTimeSpan{}},
+				UpTime:        timeSpans{&relativeTimeSpan{}},
+				UpscalePeriod: timeSpans{&relativeTimeSpan{}},
 			},
 			wantErr: true,
 		},
 		{
 			name: "uptime an downscaleperiod",
 			layer: Layer{
-				UpTime:          timeSpans{relativeTimeSpan{}},
-				DownscalePeriod: timeSpans{relativeTimeSpan{}},
+				UpTime:          timeSpans{&relativeTimeSpan{}},
+				DownscalePeriod: timeSpans{&relativeTimeSpan{}},
 			},
 			wantErr: true,
 		},
 		{
 			name: "downtime an upscaleperiod",
 			layer: Layer{
-				DownTime:      timeSpans{relativeTimeSpan{}},
-				UpscalePeriod: timeSpans{relativeTimeSpan{}},
+				DownTime:      timeSpans{&relativeTimeSpan{}},
+				UpscalePeriod: timeSpans{&relativeTimeSpan{}},
 			},
 			wantErr: true,
 		},
 		{
 			name: "downtime an downscaleperiod",
 			layer: Layer{
-				DownTime:        timeSpans{relativeTimeSpan{}},
-				DownscalePeriod: timeSpans{relativeTimeSpan{}},
+				DownTime:        timeSpans{&relativeTimeSpan{}},
+				DownscalePeriod: timeSpans{&relativeTimeSpan{}},
 			},
 			wantErr: true,
 		},
+		{
+			name: "down- and upscale periods overlapping",
+			layer: Layer{
+				DownscalePeriod: timeSpans{&absoluteTimeSpan{
+					from: time.Now(),
+					to:   time.Now().Add(1 * time.Hour),
+				}},
+				UpscalePeriod: timeSpans{&absoluteTimeSpan{ // overlapping
+					from: time.Now(),
+					to:   time.Now().Add(1 * time.Hour),
+				}},
+			},
+			wantErr: true,
+		},
+		{
+			name: "down- and upscale do not overlap",
+			layer: Layer{
+				DownscalePeriod: timeSpans{&absoluteTimeSpan{
+					from: time.Now(),
+					to:   time.Now().Add(time.Hour),
+				}},
+				UpscalePeriod: timeSpans{&absoluteTimeSpan{
+					from: time.Now().Add(2 * time.Hour),
+					to:   time.Now().Add(3 * time.Hour),
+				}},
+			},
+			wantErr: false,
+		},
 		{
 			name: "valid",
 			layer: Layer{
-				DownTime:        timeSpans{relativeTimeSpan{}},
-				DownscalePeriod: timeSpans{relativeTimeSpan{}},
+				DownTime:        timeSpans{&relativeTimeSpan{}},
+				DownscalePeriod: timeSpans{&relativeTimeSpan{}},
 			},
 			wantErr: true,
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			err := test.layer.checkForIncompatibleFields()
+			err := test.layer.CheckForIncompatibleFields()
 			if test.wantErr {
 				assert.Error(t, err)
 			} else {
@@ -92,11 +120,11 @@ func TestLayer_checkForIncompatibleFields(t *testing.T) {
 
 func TestLayer_getCurrentScaling(t *testing.T) {
 	var (
-		inTimeSpan = timeSpans{absoluteTimeSpan{
+		inTimeSpan = timeSpans{&absoluteTimeSpan{
 			from: time.Now().Add(-time.Hour),
 			to:   time.Now().Add(time.Hour),
 		}}
-		outOfTimeSpan = timeSpans{absoluteTimeSpan{
+		outOfTimeSpan = timeSpans{&absoluteTimeSpan{
 			from: time.Now().Add(-2 * time.Hour),
 			to:   time.Now().Add(-time.Hour),
 		}}