fix: refactored layer incompatiblility testing

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,7 +2,6 @@ package values
 
 import (
 	"errors"
-	"fmt"
 	"time"
 )
 
@@ -21,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
@@ -61,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 &&
@@ -140,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,51 +31,51 @@ 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{
+				DownscalePeriod: timeSpans{&absoluteTimeSpan{
 					from: time.Now(),
 					to:   time.Now().Add(1 * time.Hour),
 				}},
-				UpscalePeriod: timeSpans{absoluteTimeSpan{ // overlapping
+				UpscalePeriod: timeSpans{&absoluteTimeSpan{ // overlapping
 					from: time.Now(),
 					to:   time.Now().Add(1 * time.Hour),
 				}},
@@ -85,11 +85,11 @@ func TestLayer_checkForIncompatibleFields(t *testing.T) {
 		{
 			name: "down- and upscale do not overlap",
 			layer: Layer{
-				DownscalePeriod: timeSpans{absoluteTimeSpan{
+				DownscalePeriod: timeSpans{&absoluteTimeSpan{
 					from: time.Now(),
 					to:   time.Now().Add(time.Hour),
 				}},
-				UpscalePeriod: timeSpans{absoluteTimeSpan{
+				UpscalePeriod: timeSpans{&absoluteTimeSpan{
 					from: time.Now().Add(2 * time.Hour),
 					to:   time.Now().Add(3 * time.Hour),
 				}},
@@ -99,16 +99,16 @@ func TestLayer_checkForIncompatibleFields(t *testing.T) {
 		{
 			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 {
@@ -120,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),
 		}}

internal/pkg/values/timespan.go

@@ -51,7 +51,7 @@ func (t *timeSpans) Set(value string) error {
 			if err != nil {
 				return fmt.Errorf("failed to parse absolute timespan: %w", err)
 			}
-			timespans = append(timespans, timespan)
+			timespans = append(timespans, &timespan)
 			continue
 		}
 
@@ -143,70 +143,80 @@ type relativeTimeSpan struct {
 }
 
 // isWeekdayInRange checks if the weekday falls into the weekday range
-func (t relativeTimeSpan) isWeekdayInRange(weekday time.Weekday) bool {
+func (t *relativeTimeSpan) isWeekdayInRange(weekday time.Weekday) bool {
 	if t.weekdayFrom <= t.weekdayTo { // check if range wraps across weeks
 		return weekday >= t.weekdayFrom && weekday <= t.weekdayTo
 	}
 	return weekday >= t.weekdayFrom || weekday <= t.weekdayTo
 }
 
 // isTimeOfDayInRange checks if the time falls into the time of day range
-func (t relativeTimeSpan) isTimeOfDayInRange(timeOfDay time.Time) bool {
+func (t *relativeTimeSpan) isTimeOfDayInRange(timeOfDay time.Time) bool {
 	if t.timeFrom.After(t.timeTo) { // check if range wraps across days
 		return timeOfDay.After(t.timeFrom) || timeOfDay.Equal(t.timeFrom) || timeOfDay.Before(t.timeTo)
 	}
 	return (t.timeFrom.Before(timeOfDay) || t.timeFrom.Equal(timeOfDay)) && t.timeTo.After(timeOfDay)
 }
 
 // isTimeInSpan checks if the time is in the span
-func (t relativeTimeSpan) isTimeInSpan(targetTime time.Time) bool {
+func (t *relativeTimeSpan) isTimeInSpan(targetTime time.Time) bool {
 	targetTime = targetTime.In(t.timezone)
 	timeOfDay := getTimeOfDay(targetTime)
 	weekday := targetTime.Weekday()
 	return t.isTimeOfDayInRange(timeOfDay) && t.isWeekdayInRange(weekday)
 }
 
 // inLocation returns an array of relative timespans matching the timespan converted to the given location
-func (t relativeTimeSpan) inLocation(timezone *time.Location) []relativeTimeSpan {
+func (t *relativeTimeSpan) inLocation(timezone *time.Location) []relativeTimeSpan {
 	var result []relativeTimeSpan
-	sameDays := relativeTimeSpan{
+	sameWeedays := relativeTimeSpan{
 		timezone:    timezone,
 		weekdayFrom: t.weekdayFrom,
 		weekdayTo:   t.weekdayTo,
 		timeFrom:    t.timeFrom.In(timezone),
 		timeTo:      t.timeTo.In(timezone),
 	}
-	result = append(result, sameDays)
-	if isTimeFromSkippedToPreviousDay(sameDays.timeFrom) {
-		daysBefore := relativeTimeSpan{
+	result = append(result, sameWeedays)
+	if sameWeedays.overlapsIntoPreviousDay() {
+		weekdaysBefore := relativeTimeSpan{
 			timezone:    timezone,
-			timeFrom:    sameDays.timeFrom.Add(24 * time.Hour),
-			timeTo:      sameDays.timeTo.Add(24 * time.Hour),
-			weekdayFrom: getWeekdayBefore(sameDays.weekdayFrom),
-			weekdayTo:   getWeekdayBefore(sameDays.weekdayTo),
+			timeFrom:    sameWeedays.timeFrom.Add(24 * time.Hour),
+			timeTo:      sameWeedays.timeTo.Add(24 * time.Hour),
+			weekdayFrom: getWeekdayBefore(sameWeedays.weekdayFrom),
+			weekdayTo:   getWeekdayBefore(sameWeedays.weekdayTo),
 		}
-		result = append(result, daysBefore)
+		result = append(result, weekdaysBefore)
 	}
-	if isTimeToSkippedToNextDay(sameDays.timeTo) {
-		daysAfter := relativeTimeSpan{
+	if sameWeedays.overlapsIntoNextDay() {
+		weekdaysAfter := relativeTimeSpan{
 			timezone:    timezone,
-			timeFrom:    sameDays.timeFrom.Add(-24 * time.Hour),
-			timeTo:      sameDays.timeTo.Add(-24 * time.Hour),
-			weekdayFrom: getWeekdayAfter(sameDays.weekdayFrom),
-			weekdayTo:   getWeekdayAfter(sameDays.weekdayTo),
+			timeFrom:    sameWeedays.timeFrom.Add(-24 * time.Hour),
+			timeTo:      sameWeedays.timeTo.Add(-24 * time.Hour),
+			weekdayFrom: getWeekdayAfter(sameWeedays.weekdayFrom),
+			weekdayTo:   getWeekdayAfter(sameWeedays.weekdayTo),
 		}
-		result = append(result, daysAfter)
+		result = append(result, weekdaysAfter)
 	}
 	return result
 }
 
+// overlapsIntoPreviousDay checks if timeFrom overlaps into the previous day
+func (r *relativeTimeSpan) overlapsIntoPreviousDay() bool {
+	return r.timeFrom.Year() == -1
+}
+
+// overlapsIntoNextDay checks if timeTo overlaps into the following day
+func (r *relativeTimeSpan) overlapsIntoNextDay() bool {
+	return asExclusiveTimestamp(r.timeTo).Day() == 2
+}
+
 type absoluteTimeSpan struct {
 	from time.Time
 	to   time.Time
 }
 
 // isTimeInSpan check if the time is in the span
-func (t absoluteTimeSpan) isTimeInSpan(targetTime time.Time) bool {
+func (t *absoluteTimeSpan) isTimeInSpan(targetTime time.Time) bool {
 	return (t.from.Before(targetTime) || t.from.Equal(targetTime)) && t.to.After(targetTime)
 }
 
@@ -268,18 +278,18 @@ func getTimeOfDay(targetTime time.Time) time.Time {
 // doTimespansOverlap checks if the given timespans overlap with each other
 func doTimespansOverlap(span1, span2 TimeSpan) bool {
 	switch s1 := span1.(type) {
-	case absoluteTimeSpan:
-		if s2, ok := span2.(absoluteTimeSpan); ok {
-			return absAndAbsOverlap(s1, s2)
+	case *absoluteTimeSpan:
+		if s2, ok := span2.(*absoluteTimeSpan); ok {
+			return absAndAbsOverlap(*s1, *s2)
 		}
-		return relAndAbsOverlap(span2.(relativeTimeSpan), s1)
-	case relativeTimeSpan:
-		if s2, ok := span2.(absoluteTimeSpan); ok {
-			return relAndAbsOverlap(s1, s2)
+		return relAndAbsOverlap(*span2.(*relativeTimeSpan), *s1)
+	case *relativeTimeSpan:
+		if s2, ok := span2.(*absoluteTimeSpan); ok {
+			return relAndAbsOverlap(*s1, *s2)
 		}
-		return relAndRelOverlap(s1, span2.(relativeTimeSpan))
+		return relAndRelOverlap(*s1, *span2.(*relativeTimeSpan))
 	}
-	return false // this shouldn't ever be reached
+	panic(fmt.Sprintf("Fatal error, the timespan does not match any of the known types. This should never happen! Type: %T", span1))
 }
 
 // relAndRelOverlap checks if two relative timespans overlap