diff --git a/cmd/run.go b/cmd/run.go index 026465c8..1e3f8af1 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -20,6 +20,7 @@ package cmd import ( "context" + "os" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -116,7 +117,10 @@ func run(fm *config.RunFlagsNameMapping) func(cmd *cobra.Command, args []string) s := sparrow.New(cfg) log.Info("Running sparrow") if err := s.Run(ctx); err != nil { - panic(err) + log.Error("Error while running sparrow", "error", err) + // by this time all shutdown routines should have been called + // so we can exit here + os.Exit(1) } } } diff --git a/pkg/sparrow/api.go b/pkg/sparrow/api.go index d3554264..e208b365 100644 --- a/pkg/sparrow/api.go +++ b/pkg/sparrow/api.go @@ -40,7 +40,6 @@ type encoder interface { const ( urlParamCheckName = "checkName" readHeaderTimeout = time.Second * 5 - shutdownTimeout = time.Second * 5 ) var ( diff --git a/pkg/sparrow/gitlab/gitlab.go b/pkg/sparrow/gitlab/gitlab.go index 95b47322..2d709766 100644 --- a/pkg/sparrow/gitlab/gitlab.go +++ b/pkg/sparrow/gitlab/gitlab.go @@ -41,8 +41,8 @@ type Gitlab interface { PutFile(ctx context.Context, file File) error // PostFile creates the file in the repository PostFile(ctx context.Context, file File) error - // DeleteFile deletes the file matching the filename - DeleteFile(ctx context.Context, filename string) error + // DeleteFile deletes the file from the repository + DeleteFile(ctx context.Context, file File) error } // Client implements Gitlab @@ -58,19 +58,25 @@ type Client struct { // DeleteFile deletes the file matching the filename from the configured // gitlab repository -func (g *Client) DeleteFile(ctx context.Context, filename string) error { - log := logger.FromContext(ctx).With("file", filename) +func (g *Client) DeleteFile(ctx context.Context, file File) error { //nolint:gocritic // no performance concerns yet + log := logger.FromContext(ctx).With("file", file) - if filename == "" { + if file.fileName == "" { return fmt.Errorf("filename is empty") } log.Debug("Deleting file from gitlab") - n := url.PathEscape(filename) + n := url.PathEscape(file.fileName) + b, err := file.Bytes() + if err != nil { + log.Error("Failed to create request", "error", err) + return err + } + req, err := http.NewRequestWithContext(ctx, http.MethodDelete, fmt.Sprintf("%s/api/v4/projects/%d/repository/files/%s", g.baseUrl, g.projectID, n), - http.NoBody, + bytes.NewBuffer(b), ) if err != nil { log.Error("Failed to create request", "error", err) diff --git a/pkg/sparrow/gitlab/gitlab_test.go b/pkg/sparrow/gitlab/gitlab_test.go index dcc15ba6..937ec298 100644 --- a/pkg/sparrow/gitlab/gitlab_test.go +++ b/pkg/sparrow/gitlab/gitlab_test.go @@ -481,7 +481,14 @@ func TestClient_DeleteFile(t *testing.T) { resp := httpmock.NewStringResponder(tt.mockCode, "") httpmock.RegisterResponder("DELETE", fmt.Sprintf("http://test/api/v4/projects/%d/repository/files/%s", projID, tt.fileName), resp) - if err := g.DeleteFile(context.Background(), tt.fileName); (err != nil) != tt.wantErr { + f := File{ + fileName: tt.fileName, + CommitMessage: "Deleted registration file", + AuthorName: "sparrow-test", + AuthorEmail: "sparrow-test@sparrow", + Branch: "main", + } + if err := g.DeleteFile(context.Background(), f); (err != nil) != tt.wantErr { t.Fatalf("DeleteFile() error = %v, wantErr %v", err, tt.wantErr) } }) diff --git a/pkg/sparrow/gitlab/test/mockclient.go b/pkg/sparrow/gitlab/test/mockclient.go index 1f31f29d..57ba94b6 100644 --- a/pkg/sparrow/gitlab/test/mockclient.go +++ b/pkg/sparrow/gitlab/test/mockclient.go @@ -52,9 +52,9 @@ func (m *MockClient) FetchFiles(ctx context.Context) ([]checks.GlobalTarget, err return m.targets, m.fetchFilesErr } -func (m *MockClient) DeleteFile(ctx context.Context, fn string) error { +func (m *MockClient) DeleteFile(ctx context.Context, file gitlab.File) error { //nolint: gocritic // irrelevant log := logger.FromContext(ctx) - log.Info("MockDeleteFile called", "filename", fn, "err", m.deleteFileErr) + log.Info("MockDeleteFile called", "filename", file, "err", m.deleteFileErr) return m.deleteFileErr } diff --git a/pkg/sparrow/run.go b/pkg/sparrow/run.go index f885f5c9..ba1a5f70 100644 --- a/pkg/sparrow/run.go +++ b/pkg/sparrow/run.go @@ -20,9 +20,11 @@ package sparrow import ( "context" + "errors" "fmt" "net/http" "slices" + "time" targets "github.com/caas-team/sparrow/pkg/sparrow/targets" @@ -34,6 +36,8 @@ import ( "github.com/go-chi/chi/v5" ) +const shutdownTimeout = time.Second * 90 + type Sparrow struct { db db.DB // the existing checks @@ -81,6 +85,7 @@ func New(cfg *config.Config) *Sparrow { // Run starts the sparrow func (s *Sparrow) Run(ctx context.Context) error { ctx, cancel := logger.NewContextWithLogger(ctx, "sparrow") + log := logger.FromContext(ctx) defer cancel() go s.loader.Run(ctx) @@ -89,7 +94,7 @@ func (s *Sparrow) Run(ctx context.Context) error { go func() { err := s.api(ctx) if err != nil { - panic(fmt.Sprintf("Failed to start api: %v", err)) + log.Error("Error running api server", "error", err) } }() @@ -97,6 +102,12 @@ func (s *Sparrow) Run(ctx context.Context) error { select { case <-ctx.Done(): if err := ctx.Err(); err != nil { + ctx, cancel = context.WithTimeout(context.Background(), shutdownTimeout) + defer cancel() //nolint: gocritic // how else can we defer a cancel? + errS := s.targets.Shutdown(ctx) + if errS != nil { + return fmt.Errorf("failed to shutdown targets: %w", errors.Join(err, errS)) + } return err } return nil diff --git a/pkg/sparrow/targets/gitlab.go b/pkg/sparrow/targets/gitlab.go index ae8b0d9a..3d5cd752 100644 --- a/pkg/sparrow/targets/gitlab.go +++ b/pkg/sparrow/targets/gitlab.go @@ -34,6 +34,8 @@ import ( var _ TargetManager = &gitlabTargetManager{} +const shutdownTimeout = 30 * time.Second + // gitlabTargetManager implements TargetManager type gitlabTargetManager struct { targets []checks.GlobalTarget @@ -86,7 +88,9 @@ func (t *gitlabTargetManager) Reconcile(ctx context.Context) { case <-ctx.Done(): if err := ctx.Err(); err != nil { log.Error("Context canceled", "error", err) - err = t.Shutdown(ctx) + ctxS, cancel := context.WithTimeout(context.Background(), shutdownTimeout) + defer cancel() //nolint: gocritic // how else can we defer a cancel? + err = t.Shutdown(ctxS) if err != nil { log.Error("Failed to shutdown gracefully, stopping routine", "error", err) return @@ -127,7 +131,14 @@ func (t *gitlabTargetManager) Shutdown(ctx context.Context) error { log.Debug("Shut down signal received") if t.Registered() { - err := t.gitlab.DeleteFile(ctx, fmt.Sprintf("%s.json", t.name)) + f := gitlab.File{ + Branch: "main", + AuthorEmail: fmt.Sprintf("%s@sparrow", t.name), + AuthorName: t.name, + CommitMessage: "Unregistering global target", + } + f.SetFileName(fmt.Sprintf("%s.json", t.name)) + err := t.gitlab.DeleteFile(ctx, f) if err != nil { log.Error("Failed to shutdown gracefully", "error", err) return err