feat: no more panics and proper shutdown

Signed-off-by: Bruno Bressi <[email protected]>

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)
 		}
 	}
 }

pkg/sparrow/api.go

@@ -40,7 +40,6 @@ type encoder interface {
 const (
 	urlParamCheckName = "checkName"
 	readHeaderTimeout = time.Second * 5
-	shutdownTimeout   = time.Second * 5
 )
 
 var (

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)

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)
 			}
 		})

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
 }
 

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,14 +94,20 @@ 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)
 		}
 	}()
 
 	for {
 		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

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