From b993b5459a03e0a1f22e40b3eb9cbe75d4dd1971 Mon Sep 17 00:00:00 2001 From: Mauro Stettler Date: Wed, 8 May 2024 10:27:29 +0200 Subject: [PATCH 1/4] make the unavailability limit configurable Signed-off-by: Mauro Stettler --- pkg/icassigner/action.go | 8 +++---- pkg/icassigner/calendar/google.go | 6 +++-- pkg/icassigner/calendar/ical.go | 35 ++++++++++++++++++++-------- pkg/icassigner/calendar/ical_test.go | 5 ++-- pkg/icassigner/config.go | 11 +++++++-- 5 files changed, 45 insertions(+), 20 deletions(-) diff --git a/pkg/icassigner/action.go b/pkg/icassigner/action.go index 985143a..4ecffe2 100644 --- a/pkg/icassigner/action.go +++ b/pkg/icassigner/action.go @@ -96,7 +96,7 @@ func (a *Action) Run(ctx context.Context, event *github.IssuesEvent, dryRun bool continue } - isAvailable, err := checkAvailability(member) + isAvailable, err := checkAvailability(member, a.Config.UnavailabilityLimit) if err != nil { log.Printf("Unable to fetch availability of %q, due %v", name, err) } @@ -161,16 +161,16 @@ func (a *Action) calculateIssueBusynessPerTeamMember(ctx context.Context, now ti return busyness.CalculateBusynessForTeam(ctx, now, a.Client, a.Config.IgnoredLabels, team) } -func checkAvailability(m MemberConfig) (bool, error) { +func checkAvailability(m MemberConfig, unavailabilityLimit time.Duration) (bool, error) { if m.GoogleCalendar != "" { cfg, err := GetGoogleConfig() if err != nil { return true, err } - return calendar.CheckGoogleAvailability(cfg, m.GoogleCalendar, m.Name, time.Now()) + return calendar.CheckGoogleAvailability(cfg, m.GoogleCalendar, m.Name, time.Now(), unavailabilityLimit) } - return calendar.CheckAvailability(m.IcalURL, m.Name, time.Now()) + return calendar.CheckAvailability(m.IcalURL, m.Name, time.Now(), unavailabilityLimit) } func GetGoogleConfig() (calendar.GoogleConfigJSON, error) { diff --git a/pkg/icassigner/calendar/google.go b/pkg/icassigner/calendar/google.go index 052c6bf..ea6c869 100644 --- a/pkg/icassigner/calendar/google.go +++ b/pkg/icassigner/calendar/google.go @@ -27,7 +27,7 @@ import ( type GoogleConfigJSON string -func CheckGoogleAvailability(cfg GoogleConfigJSON, calendarName string, name string, now time.Time) (bool, error) { +func CheckGoogleAvailability(cfg GoogleConfigJSON, calendarName string, name string, now time.Time, unavailabilityLimit time.Duration) (bool, error) { opt := option.WithCredentialsJSON([]byte(cfg)) calService, err := calendar.NewService(context.Background(), opt) if err != nil { @@ -54,6 +54,8 @@ func CheckGoogleAvailability(cfg GoogleConfigJSON, calendarName string, name str return true, fmt.Errorf("unable to access calendar from %v, please ensure they shared their calendar with the service account. Internal error %q", name, calendar.Errors[0].Reason) } + icalAvailabilityChecker := newIcalAvailabilityChecker(now, unavailabilityLimit, time.UTC) + // check all events for _, e := range calendar.Busy { start, err := time.Parse(time.RFC3339, e.Start) @@ -66,7 +68,7 @@ func CheckGoogleAvailability(cfg GoogleConfigJSON, calendarName string, name str continue } - if isEventBlockingAvailability(now, start, end, time.UTC) { + if icalAvailabilityChecker.isEventBlockingAvailability(start, end) { return false, nil } } diff --git a/pkg/icassigner/calendar/ical.go b/pkg/icassigner/calendar/ical.go index fbe490f..af6aee5 100644 --- a/pkg/icassigner/calendar/ical.go +++ b/pkg/icassigner/calendar/ical.go @@ -25,16 +25,28 @@ import ( "github.com/emersion/go-ical" ) -const UnavailabilityLimit = 6 * time.Hour // 6hr +type icalAvailabilityChecker struct { + now time.Time + unavailabilityLimit time.Duration + location *time.Location +} + +func newIcalAvailabilityChecker(now time.Time, unavailabilityLimit time.Duration, location *time.Location) icalAvailabilityChecker { + return icalAvailabilityChecker{ + now: now, + unavailabilityLimit: unavailabilityLimit, + location: location, + } +} -func isEventBlockingAvailability(now time.Time, start, end time.Time, loc *time.Location) bool { +func (i *icalAvailabilityChecker) isEventBlockingAvailability(start, end time.Time) bool { // if event is shorter than unavailabilityLimit, skip it - if end.Sub(start) < UnavailabilityLimit { + if end.Sub(start) < i.unavailabilityLimit { return false } // if the end of this date is already before the current date, skip it - if end.Before(now) { + if end.Before(i.now) { return false } @@ -44,7 +56,7 @@ func isEventBlockingAvailability(now time.Time, start, end time.Time, loc *time. // // Now we need to check if that event starts in the next 12 business hours lookAheadTime := 12 * time.Hour - localDate := now.In(loc) + localDate := i.now.In(i.location) switch localDate.Weekday() { case time.Friday: @@ -76,7 +88,10 @@ func parseStartEnd(e ical.Event, loc *time.Location) (time.Time, time.Time, erro return start, end, nil } -func checkEvents(events []ical.Event, name string, now time.Time, loc *time.Location) (bool, error) { +func checkEvents(events []ical.Event, name string, now time.Time, loc *time.Location, unavailabilityLimit time.Duration) (bool, error) { + + icalAvailabilityChecker := newIcalAvailabilityChecker(now, unavailabilityLimit, loc) + for _, event := range events { if prop := event.Props.Get(ical.PropTransparency); prop != nil && prop.Value == "TRANSPARENT" { continue @@ -89,7 +104,7 @@ func checkEvents(events []ical.Event, name string, now time.Time, loc *time.Loca } // check original occurence - if isEventBlockingAvailability(now, start, end, loc) { + if icalAvailabilityChecker.isEventBlockingAvailability(start, end) { log.Printf("calendar.isAvailableOn: person %q in %q is unavailable due to event from %q to %q\n", name, loc.String(), start, end) return false, nil } @@ -109,7 +124,7 @@ func checkEvents(events []ical.Event, name string, now time.Time, loc *time.Loca start := o end := o.Add(completeDuration) - if isEventBlockingAvailability(now, start, end, loc) { + if icalAvailabilityChecker.isEventBlockingAvailability(start, end) { log.Printf(`calendar.isAvailableOn: person %q is unavailable due to event from %q to %q`, name, start, end) return false, nil } @@ -119,7 +134,7 @@ func checkEvents(events []ical.Event, name string, now time.Time, loc *time.Loca return true, nil } -func CheckAvailability(icalUrl string, name string, now time.Time) (bool, error) { +func CheckAvailability(icalUrl string, name string, now time.Time, unavailabilityLimit time.Duration) (bool, error) { resp, err := http.Get(icalUrl) if err != nil { return true, fmt.Errorf("unable to download ical file, due %w", err) @@ -143,5 +158,5 @@ func CheckAvailability(icalUrl string, name string, now time.Time) (bool, error) } } - return checkEvents(cal.Events(), name, now, loc) + return checkEvents(cal.Events(), name, now, loc, unavailabilityLimit) } diff --git a/pkg/icassigner/calendar/ical_test.go b/pkg/icassigner/calendar/ical_test.go index 25ae453..be63c45 100644 --- a/pkg/icassigner/calendar/ical_test.go +++ b/pkg/icassigner/calendar/ical_test.go @@ -191,7 +191,8 @@ func TestIsEventBlockingAvailability(t *testing.T) { for _, testcase := range testCases { t.Run(testcase.name, func(t *testing.T) { - res := isEventBlockingAvailability(testcase.now, testcase.start, testcase.end, testcase.location) + availabilityChecker := newIcalAvailabilityChecker(testcase.now, 6*time.Hour, testcase.location) + res := availabilityChecker.isEventBlockingAvailability(testcase.start, testcase.end) if res != testcase.expectedResult { t.Errorf("Expected isEventBlockingAvailability to be %v, but got %v for event between %q and %q (tz=%v)", testcase.expectedResult, res, testcase.start, testcase.end, testcase.location.String()) } @@ -230,7 +231,7 @@ END:VCALENDAR` loc, _ := time.LoadLocation("UTC") now := time.Date(2023, time.December, 07, 16, 0, 0, 0, loc) - r, err := CheckAvailability(ts.URL, "tester", now) + r, err := CheckAvailability(ts.URL, "tester", now, 6*time.Hour) if err != nil { t.Errorf("No error expected during basic ical check, but got %v", err) diff --git a/pkg/icassigner/config.go b/pkg/icassigner/config.go index cdc8e37..a5a47bc 100644 --- a/pkg/icassigner/config.go +++ b/pkg/icassigner/config.go @@ -21,14 +21,16 @@ import ( "context" "fmt" "io" + "time" "github.com/google/go-github/github" "gopkg.in/yaml.v2" ) type Config struct { - Teams map[string]TeamConfig `yaml:"teams,omitempty"` - IgnoredLabels []string `yaml:"ignoreLabels,omitempty"` + UnavailabilityLimit time.Duration `yaml:"unavailabilityLimit,omitempty"` + Teams map[string]TeamConfig `yaml:"teams,omitempty"` + IgnoredLabels []string `yaml:"ignoreLabels,omitempty"` } type TeamConfig struct { @@ -51,6 +53,11 @@ func ParseConfig(r io.Reader) (Config, error) { return cfg, fmt.Errorf("unable to parse config, due: %w", err) } + if cfg.UnavailabilityLimit == 0 { + // If unset, set default value + cfg.UnavailabilityLimit = 6 * time.Hour + } + return cfg, nil } From fef8dd547eb1fbff4d8c620e4e33e8a86d20a1ac Mon Sep 17 00:00:00 2001 From: Mauro Stettler Date: Wed, 8 May 2024 10:29:44 +0200 Subject: [PATCH 2/4] consistent naming Signed-off-by: Mauro Stettler --- pkg/icassigner/calendar/google.go | 4 ++-- pkg/icassigner/calendar/ical.go | 7 +++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/pkg/icassigner/calendar/google.go b/pkg/icassigner/calendar/google.go index ea6c869..dbd899d 100644 --- a/pkg/icassigner/calendar/google.go +++ b/pkg/icassigner/calendar/google.go @@ -54,7 +54,7 @@ func CheckGoogleAvailability(cfg GoogleConfigJSON, calendarName string, name str return true, fmt.Errorf("unable to access calendar from %v, please ensure they shared their calendar with the service account. Internal error %q", name, calendar.Errors[0].Reason) } - icalAvailabilityChecker := newIcalAvailabilityChecker(now, unavailabilityLimit, time.UTC) + availabilityChecker := newIcalAvailabilityChecker(now, unavailabilityLimit, time.UTC) // check all events for _, e := range calendar.Busy { @@ -68,7 +68,7 @@ func CheckGoogleAvailability(cfg GoogleConfigJSON, calendarName string, name str continue } - if icalAvailabilityChecker.isEventBlockingAvailability(start, end) { + if availabilityChecker.isEventBlockingAvailability(start, end) { return false, nil } } diff --git a/pkg/icassigner/calendar/ical.go b/pkg/icassigner/calendar/ical.go index af6aee5..fea0775 100644 --- a/pkg/icassigner/calendar/ical.go +++ b/pkg/icassigner/calendar/ical.go @@ -89,8 +89,7 @@ func parseStartEnd(e ical.Event, loc *time.Location) (time.Time, time.Time, erro } func checkEvents(events []ical.Event, name string, now time.Time, loc *time.Location, unavailabilityLimit time.Duration) (bool, error) { - - icalAvailabilityChecker := newIcalAvailabilityChecker(now, unavailabilityLimit, loc) + availabilityChecker := newIcalAvailabilityChecker(now, unavailabilityLimit, loc) for _, event := range events { if prop := event.Props.Get(ical.PropTransparency); prop != nil && prop.Value == "TRANSPARENT" { @@ -104,7 +103,7 @@ func checkEvents(events []ical.Event, name string, now time.Time, loc *time.Loca } // check original occurence - if icalAvailabilityChecker.isEventBlockingAvailability(start, end) { + if availabilityChecker.isEventBlockingAvailability(start, end) { log.Printf("calendar.isAvailableOn: person %q in %q is unavailable due to event from %q to %q\n", name, loc.String(), start, end) return false, nil } @@ -124,7 +123,7 @@ func checkEvents(events []ical.Event, name string, now time.Time, loc *time.Loca start := o end := o.Add(completeDuration) - if icalAvailabilityChecker.isEventBlockingAvailability(start, end) { + if availabilityChecker.isEventBlockingAvailability(start, end) { log.Printf(`calendar.isAvailableOn: person %q is unavailable due to event from %q to %q`, name, start, end) return false, nil } From 23e372158057fc43c7a7eebd887e08dfc4b13140 Mon Sep 17 00:00:00 2001 From: Mauro Stettler Date: Wed, 8 May 2024 10:32:26 +0200 Subject: [PATCH 3/4] define default setting in constant Signed-off-by: Mauro Stettler --- pkg/icassigner/calendar/ical.go | 2 ++ pkg/icassigner/calendar/ical_test.go | 2 +- pkg/icassigner/config.go | 3 ++- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/icassigner/calendar/ical.go b/pkg/icassigner/calendar/ical.go index fea0775..6ab3393 100644 --- a/pkg/icassigner/calendar/ical.go +++ b/pkg/icassigner/calendar/ical.go @@ -25,6 +25,8 @@ import ( "github.com/emersion/go-ical" ) +const DefaultUnavailabilityLimit = 6 * time.Hour + type icalAvailabilityChecker struct { now time.Time unavailabilityLimit time.Duration diff --git a/pkg/icassigner/calendar/ical_test.go b/pkg/icassigner/calendar/ical_test.go index be63c45..7de2ec3 100644 --- a/pkg/icassigner/calendar/ical_test.go +++ b/pkg/icassigner/calendar/ical_test.go @@ -231,7 +231,7 @@ END:VCALENDAR` loc, _ := time.LoadLocation("UTC") now := time.Date(2023, time.December, 07, 16, 0, 0, 0, loc) - r, err := CheckAvailability(ts.URL, "tester", now, 6*time.Hour) + r, err := CheckAvailability(ts.URL, "tester", now, DefaultUnavailabilityLimit) if err != nil { t.Errorf("No error expected during basic ical check, but got %v", err) diff --git a/pkg/icassigner/config.go b/pkg/icassigner/config.go index a5a47bc..d09fc95 100644 --- a/pkg/icassigner/config.go +++ b/pkg/icassigner/config.go @@ -24,6 +24,7 @@ import ( "time" "github.com/google/go-github/github" + "github.com/grafana/escalation-scheduler/pkg/icassigner/calendar" "gopkg.in/yaml.v2" ) @@ -55,7 +56,7 @@ func ParseConfig(r io.Reader) (Config, error) { if cfg.UnavailabilityLimit == 0 { // If unset, set default value - cfg.UnavailabilityLimit = 6 * time.Hour + cfg.UnavailabilityLimit = calendar.DefaultUnavailabilityLimit } return cfg, nil From 41f0c063b25474badda2978f5930e978b940e4c1 Mon Sep 17 00:00:00 2001 From: Mauro Stettler Date: Wed, 8 May 2024 10:37:20 +0200 Subject: [PATCH 4/4] docs and test Signed-off-by: Mauro Stettler --- README.md | 4 +++- pkg/icassigner/config_test.go | 8 +++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 1f57245..4242c43 100644 --- a/README.md +++ b/README.md @@ -117,6 +117,7 @@ teams: output: "slack-handle" ignoreLabels: - stale +unavailabilityLimit: 6h ``` #### Root configuration struct @@ -125,6 +126,7 @@ ignoreLabels: | -------------- | -------------------------- | -------- | ------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `ignoreLabels` | List of Strings | false | `[]` | List of labels which mark this issue to be ignored. If triggered on an issue which has at least **one** of the labels to be ignored, the action exits without doing something | | `teams` | Map of Team configurations | true | `nil` | Definition of the teams this issue is distributed between. | +| `unavailabilityLimit` | Duration | false | `6h` | Duration for which a calendar event must block someone's availability for them to be considered unavailable. | #### Team configuration struct @@ -176,4 +178,4 @@ Next members can share their availability with this service account via: 2. Open Settings by opening the hamburger menu next to your personal calendar and clicking `Settings and sharing` 3. In the `Share with specific people or groups` section you click `+ Add people and groups` 4. Enter the email address of the service account you created and select `See only free/busy (hide details)` under Permissions. -5. Click `Send` and you are done \ No newline at end of file +5. Click `Send` and you are done diff --git a/pkg/icassigner/config_test.go b/pkg/icassigner/config_test.go index a662987..1e71e01 100644 --- a/pkg/icassigner/config_test.go +++ b/pkg/icassigner/config_test.go @@ -19,6 +19,7 @@ package icassigner import ( "bytes" "testing" + "time" ) func TestMimirConfigCanBeParsed(t *testing.T) { @@ -38,7 +39,8 @@ func TestMimirConfigCanBeParsed(t *testing.T) { ical-url: https://tester2/basic.ics output: slack2 ignoreLabels: -- stale` // redacted excerpt from a real world config +- stale +unavailabilityLimit: 6h` // redacted excerpt from a real world config r := bytes.NewBuffer([]byte(rawConfig)) @@ -57,6 +59,10 @@ ignoreLabels: t.Fatal("Expected to find team \"mimir\", but got none") } + if cfg.UnavailabilityLimit != 6*time.Hour { + t.Error("Expected unavailability limit to be 6h, but got", cfg.UnavailabilityLimit) + } + expectedRequiredLabels := []string{"cloud-prometheus", "enterprise-metrics"} for i, e := range expectedRequiredLabels { if i >= len(team.RequireLabel) {