From 2125f6c6e27ca88aac8d75026c061078c7263541 Mon Sep 17 00:00:00 2001 From: Mike Ditton Date: Fri, 13 Feb 2026 15:04:36 +0100 Subject: [PATCH 1/2] Fix maintenance schedule mutation and day rollover bugs This fixes mutation and maintenance scheduling on wrong days by replacing AddTime() with AddDuration() and moving offset logic into SetCustomMaintenanceSchedule --- apis/vshn/v1/common_types.go | 33 +++- apis/vshn/v1/common_types_test.go | 159 +++++++++++++++--- .../functions/common/postgresql.go | 17 +- .../functions/vshnkeycloak/deploy.go | 2 +- .../functions/vshnnextcloud/deploy.go | 5 +- 5 files changed, 182 insertions(+), 34 deletions(-) diff --git a/apis/vshn/v1/common_types.go b/apis/vshn/v1/common_types.go index a2fd17689e..d1439fdf9a 100644 --- a/apis/vshn/v1/common_types.go +++ b/apis/vshn/v1/common_types.go @@ -1,6 +1,7 @@ package v1 import ( + "strings" "time" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" @@ -311,8 +312,32 @@ func (a *TimeOfDay) SetTime(t time.Time) { *a = TimeOfDay(t.Format(time.TimeOnly)) } -// AddTime adds duration to current time -func (a *TimeOfDay) AddTime(d time.Duration) TimeOfDay { - a.SetTime(a.GetTime().Add(d)) - return *a +// AddDuration adds duration to current time and returns the new time plus day offset. +func (a TimeOfDay) AddDuration(d time.Duration) (TimeOfDay, int) { + startTime := a.GetTime() + newTime := startTime.Add(d) + + dayOffset := int(d.Hours() / 24) + + if dayOffset == 0 && d != 0 { + if d > 0 && newTime.Hour() < startTime.Hour() { + dayOffset = 1 + } + if d < 0 && newTime.Hour() > startTime.Hour() { + dayOffset = -1 + } + } + + return TimeOfDay(newTime.Format(time.TimeOnly)), dayOffset +} + +// AddDaysToWeekday adds days to a weekday and returns the new weekday. +func AddDaysToWeekday(weekday string, days int) string { + for wd := time.Sunday; wd <= time.Saturday; wd++ { + if strings.ToLower(wd.String()) == weekday { + newWd := (int(wd) + days%7 + 7) % 7 + return strings.ToLower(time.Weekday(newWd).String()) + } + } + return weekday } diff --git a/apis/vshn/v1/common_types_test.go b/apis/vshn/v1/common_types_test.go index 8db11e8e6f..e122adc792 100644 --- a/apis/vshn/v1/common_types_test.go +++ b/apis/vshn/v1/common_types_test.go @@ -42,40 +42,36 @@ func Test_GetTime(t *testing.T) { } } -func Test_AddTime(t *testing.T) { +func Test_AddDuration_BasicCases(t *testing.T) { tests := []struct { - name string - scheduleSpec VSHNDBaaSMaintenanceScheduleSpec - d time.Duration - want string + name string + timeOfDay TimeOfDay + d time.Duration + want string }{ { - name: "GivenNormalSchedule_ThenExpectAddedTimeOfDay", - scheduleSpec: VSHNDBaaSMaintenanceScheduleSpec{ - TimeOfDay: TimeOfDay("00:30:00"), - }, - d: 60 * time.Minute, - want: "01:30:00", + name: "GivenNormalSchedule_ThenExpectAddedTimeOfDay", + timeOfDay: TimeOfDay("00:30:00"), + d: 60 * time.Minute, + want: "01:30:00", }, { - name: "GivenEmptyTimeOfDay_ThenExpectAddedTimeOfDay", - scheduleSpec: VSHNDBaaSMaintenanceScheduleSpec{ - TimeOfDay: TimeOfDay(""), - }, - d: 10 * time.Minute, - want: "00:10:00", + name: "GivenEmptyTimeOfDay_ThenExpectAddedTimeOfDay", + timeOfDay: TimeOfDay(""), + d: 10 * time.Minute, + want: "00:10:00", }, { - name: "GivenNoTimeOfDay_ThenExpectAddedTimeOfDay", - scheduleSpec: VSHNDBaaSMaintenanceScheduleSpec{}, - d: 5 * time.Minute, - want: "00:05:00", + name: "GivenNoTimeOfDay_ThenExpectAddedTimeOfDay", + timeOfDay: TimeOfDay(""), + d: 5 * time.Minute, + want: "00:05:00", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - actualTime := tt.scheduleSpec.TimeOfDay.AddTime(tt.d) - assert.Equal(t, tt.want, string(actualTime)) + resultTime, _ := tt.timeOfDay.AddDuration(tt.d) + assert.Equal(t, tt.want, string(resultTime)) }) } } @@ -164,3 +160,120 @@ func Test_IsAppcatReleaseDisabled(t *testing.T) { }) } } + +func Test_AddDuration(t *testing.T) { + tests := []struct { + name string + timeOfDay TimeOfDay + duration time.Duration + wantTime string + wantDayOffset int + }{ + { + name: "GivenMidnightRollover_ThenExpectNextDay", + timeOfDay: TimeOfDay("23:50:00"), + duration: 20 * time.Minute, + wantTime: "00:10:00", + wantDayOffset: 1, + }, + { + name: "GivenSameDay_ThenExpectNoDayOffset", + timeOfDay: TimeOfDay("10:00:00"), + duration: 30 * time.Minute, + wantTime: "10:30:00", + wantDayOffset: 0, + }, + { + name: "GivenExactMidnight_ThenExpectNoDayOffset", + timeOfDay: TimeOfDay("23:00:00"), + duration: 1 * time.Hour, + wantTime: "00:00:00", + wantDayOffset: 1, + }, + { + name: "GivenNegativeDuration_ThenExpectPreviousDay", + timeOfDay: TimeOfDay("00:10:00"), + duration: -20 * time.Minute, + wantTime: "23:50:00", + wantDayOffset: -1, + }, + { + name: "GivenLargePositiveDuration_ThenExpectPositiveDayOffset", + timeOfDay: TimeOfDay("10:00:00"), + duration: 15 * time.Hour, + wantTime: "01:00:00", + wantDayOffset: 1, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + resultTime, dayOffset := tt.timeOfDay.AddDuration(tt.duration) + assert.Equal(t, tt.wantTime, string(resultTime), "Result time should match expected") + assert.Equal(t, tt.wantDayOffset, dayOffset, "Day offset should match expected") + }) + } +} + +func Test_AddDaysToWeekday(t *testing.T) { + tests := []struct { + name string + weekday string + days int + want string + }{ + { + name: "GivenMondayPlus1_ThenExpectTuesday", + weekday: "monday", + days: 1, + want: "tuesday", + }, + { + name: "GivenMondayPlus0_ThenExpectMonday", + weekday: "monday", + days: 0, + want: "monday", + }, + { + name: "GivenSundayPlus1_ThenExpectMonday", + weekday: "sunday", + days: 1, + want: "monday", + }, + { + name: "GivenMondayPlus7_ThenExpectMonday", + weekday: "monday", + days: 7, + want: "monday", + }, + { + name: "GivenMondayMinus1_ThenExpectSunday", + weekday: "monday", + days: -1, + want: "sunday", + }, + { + name: "GivenSaturdayPlus2_ThenExpectMonday", + weekday: "saturday", + days: 2, + want: "monday", + }, + { + name: "GivenInvalidWeekday_ThenExpectUnchanged", + weekday: "invalid", + days: 1, + want: "invalid", + }, + { + name: "GivenWednesdayPlus10_ThenExpectSaturday", + weekday: "wednesday", + days: 10, + want: "saturday", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := AddDaysToWeekday(tt.weekday, tt.days) + assert.Equal(t, tt.want, result) + }) + } +} diff --git a/pkg/comp-functions/functions/common/postgresql.go b/pkg/comp-functions/functions/common/postgresql.go index 7e1e99abee..52a8338459 100644 --- a/pkg/comp-functions/functions/common/postgresql.go +++ b/pkg/comp-functions/functions/common/postgresql.go @@ -3,6 +3,7 @@ package common import ( "encoding/json" "fmt" + "time" "dario.cat/mergo" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" @@ -27,6 +28,7 @@ type PostgreSQLDependencyBuilder struct { pgBouncerConfig map[string]string pgSettings map[string]string timeOfDayMaintenance vshnv1.TimeOfDay + dayOfWeekOverride string restore *vshnv1.VSHNPostgreSQLRestore } @@ -60,8 +62,16 @@ func (a *PostgreSQLDependencyBuilder) AddPGSettings(pgSettings map[string]string return a } -func (a *PostgreSQLDependencyBuilder) SetCustomMaintenanceSchedule(timeOfDayMaintenance vshnv1.TimeOfDay) *PostgreSQLDependencyBuilder { - a.timeOfDayMaintenance = timeOfDayMaintenance +// SetCustomMaintenanceSchedule offsets the maintenance schedule by the given duration. +func (a *PostgreSQLDependencyBuilder) SetCustomMaintenanceSchedule(duration time.Duration) *PostgreSQLDependencyBuilder { + parentSchedule := a.comp.GetFullMaintenanceSchedule() + newTime, dayOffset := parentSchedule.TimeOfDay.AddDuration(duration) + a.timeOfDayMaintenance = newTime + + if dayOffset != 0 { + a.dayOfWeekOverride = vshnv1.AddDaysToWeekday(parentSchedule.DayOfWeek, dayOffset) + } + return a } @@ -145,6 +155,9 @@ func (a *PostgreSQLDependencyBuilder) CreateDependency() (string, error) { if a.timeOfDayMaintenance != "" { params.Maintenance.TimeOfDay = a.timeOfDayMaintenance } + if a.dayOfWeekOverride != "" { + params.Maintenance.DayOfWeek = a.dayOfWeekOverride + } // We need to set this after the merge, as the default instance count for PostgreSQL is always 1 // and would therefore override any value we set before the merge. params.Instances = a.comp.GetInstances() diff --git a/pkg/comp-functions/functions/vshnkeycloak/deploy.go b/pkg/comp-functions/functions/vshnkeycloak/deploy.go index c36c33076e..2f6d5072d3 100644 --- a/pkg/comp-functions/functions/vshnkeycloak/deploy.go +++ b/pkg/comp-functions/functions/vshnkeycloak/deploy.go @@ -96,7 +96,7 @@ func DeployKeycloak(ctx context.Context, comp *vshnv1.VSHNKeycloak, svc *runtime AddParameters(comp.Spec.Parameters.Service.PostgreSQLParameters). AddPGBouncerConfig(pgBuncerConfig). AddRestore(comp.Spec.Parameters.Restore, comp.Kind). - SetCustomMaintenanceSchedule(comp.Spec.Parameters.Maintenance.TimeOfDay.AddTime(20 * time.Minute)). + SetCustomMaintenanceSchedule(20 * time.Minute). CreateDependency() if err != nil { return runtime.NewWarningResult(fmt.Sprintf("cannot create postgresql instance: %s", err)) diff --git a/pkg/comp-functions/functions/vshnnextcloud/deploy.go b/pkg/comp-functions/functions/vshnnextcloud/deploy.go index 3ff2ca8af7..682c26f98c 100644 --- a/pkg/comp-functions/functions/vshnnextcloud/deploy.go +++ b/pkg/comp-functions/functions/vshnnextcloud/deploy.go @@ -137,9 +137,6 @@ func configureDatabase(ctx context.Context, comp *vshnv1.VSHNNextcloud, svc *run } func createNewPGService(comp *vshnv1.VSHNNextcloud, svc *runtime.ServiceRuntime) (pgSecret string, err error) { - var pgTime vshnv1.TimeOfDay - pgTime.SetTime(comp.GetMaintenanceTimeOfDay().GetTime().Add(20 * time.Minute)) - pgBouncerConfig, pgSettings, pgDiskSize, err := getObservedPostgresSettings(svc, comp) if err != nil { return "", fmt.Errorf("cannot get observed postgres settings: %s", err) @@ -151,7 +148,7 @@ func createNewPGService(comp *vshnv1.VSHNNextcloud, svc *runtime.ServiceRuntime) AddParameters(comp.Spec.Parameters.Service.PostgreSQLParameters). AddPGBouncerConfig(pgBouncerConfig). AddPGSettings(pgSettings). - SetCustomMaintenanceSchedule(pgTime) + SetCustomMaintenanceSchedule(20 * time.Minute) if pgDiskSize != "" { pgBuilder.SetDiskSize(pgDiskSize) From 42cd5915eb0c166d39433933b65eb2ce598418f6 Mon Sep 17 00:00:00 2001 From: Mike Ditton Date: Fri, 13 Feb 2026 16:44:22 +0100 Subject: [PATCH 2/2] Fix edgecase where duration >= 24 hours This fixes an edge case with the rollover if the duration specified is greater or equal than 24 hours --- apis/vshn/v1/common_types.go | 13 ++++++------- apis/vshn/v1/common_types_test.go | 30 +++++++++++++++++++++++++++++- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/apis/vshn/v1/common_types.go b/apis/vshn/v1/common_types.go index d1439fdf9a..e7fd3324d1 100644 --- a/apis/vshn/v1/common_types.go +++ b/apis/vshn/v1/common_types.go @@ -319,13 +319,12 @@ func (a TimeOfDay) AddDuration(d time.Duration) (TimeOfDay, int) { dayOffset := int(d.Hours() / 24) - if dayOffset == 0 && d != 0 { - if d > 0 && newTime.Hour() < startTime.Hour() { - dayOffset = 1 - } - if d < 0 && newTime.Hour() > startTime.Hour() { - dayOffset = -1 - } + // Adjust for additional day rollover from the time component + if d > 0 && newTime.Hour() < startTime.Hour() { + dayOffset++ + } + if d < 0 && newTime.Hour() > startTime.Hour() { + dayOffset-- } return TimeOfDay(newTime.Format(time.TimeOnly)), dayOffset diff --git a/apis/vshn/v1/common_types_test.go b/apis/vshn/v1/common_types_test.go index e122adc792..744ba42480 100644 --- a/apis/vshn/v1/common_types_test.go +++ b/apis/vshn/v1/common_types_test.go @@ -184,7 +184,7 @@ func Test_AddDuration(t *testing.T) { wantDayOffset: 0, }, { - name: "GivenExactMidnight_ThenExpectNoDayOffset", + name: "GivenRolloverToMidnight_ThenExpectNextDay", timeOfDay: TimeOfDay("23:00:00"), duration: 1 * time.Hour, wantTime: "00:00:00", @@ -204,6 +204,34 @@ func Test_AddDuration(t *testing.T) { wantTime: "01:00:00", wantDayOffset: 1, }, + { + name: "GivenExactly24Hours_ThenExpectOneDayOffset", + timeOfDay: TimeOfDay("10:00:00"), + duration: 24 * time.Hour, + wantTime: "10:00:00", + wantDayOffset: 1, + }, + { + name: "Given25HoursWithRollover_ThenExpectTwoDayOffset", + timeOfDay: TimeOfDay("23:00:00"), + duration: 25 * time.Hour, + wantTime: "00:00:00", + wantDayOffset: 2, + }, + { + name: "Given36HoursWithRollover_ThenExpectTwoDayOffset", + timeOfDay: TimeOfDay("12:00:00"), + duration: 36 * time.Hour, + wantTime: "00:00:00", + wantDayOffset: 2, + }, + { + name: "Given50HoursWithRollover_ThenExpectThreeDayOffset", + timeOfDay: TimeOfDay("22:00:00"), + duration: 50 * time.Hour, + wantTime: "00:00:00", + wantDayOffset: 3, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {