Merge pull request #51 from caas-team/feat/target-manager-shutdown

feat/target-manager-shutdown

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

@@ -35,9 +35,14 @@ import (
 // Gitlab handles interaction with a gitlab repository containing
 // the global targets for the Sparrow instance
 type Gitlab interface {
+	// FetchFiles fetches the files from the global targets repository
 	FetchFiles(ctx context.Context) ([]checks.GlobalTarget, error)
+	// PutFile updates the file in the repository
 	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 from the repository
+	DeleteFile(ctx context.Context, file File) error
 }
 
 // Client implements Gitlab
@@ -51,6 +56,67 @@ type Client struct {
 	client *http.Client
 }
 
+// DeleteFile deletes the file matching the filename from the configured
+// gitlab repository
+func (g *Client) DeleteFile(ctx context.Context, file File) error { //nolint:gocritic // no performance concerns yet
+	log := logger.FromContext(ctx).With("file", file)
+
+	if file.fileName == "" {
+		return fmt.Errorf("filename is empty")
+	}
+
+	log.Debug("Deleting file from gitlab")
+	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),
+		bytes.NewBuffer(b),
+	)
+	if err != nil {
+		log.Error("Failed to create request", "error", err)
+		return err
+	}
+
+	req.Header.Add("PRIVATE-TOKEN", g.token)
+	req.Header.Add("Content-Type", "application/json")
+
+	resp, err := g.client.Do(req) //nolint:bodyclose // closed in defer
+	if err != nil {
+		log.Error("Failed to delete file", "error", err)
+		return err
+	}
+
+	defer func(Body io.ReadCloser) {
+		err = Body.Close()
+		if err != nil {
+			log.Error("Failed to close response body", "error", err)
+		}
+	}(resp.Body)
+
+	if resp.StatusCode != http.StatusNoContent {
+		log.Error("Failed to delete file", "status", resp.Status)
+		return fmt.Errorf("request failed, status is %s", resp.Status)
+	}
+
+	return nil
+}
+
+// File represents a File manipulation operation via the Gitlab API
+type File struct {
+	Branch        string              `json:"branch"`
+	AuthorEmail   string              `json:"author_email"`
+	AuthorName    string              `json:"author_name"`
+	Content       checks.GlobalTarget `json:"content"`
+	CommitMessage string              `json:"commit_message"`
+	fileName      string
+}
+
 func New(baseURL, token string, pid int) Gitlab {
 	return &Client{
 		baseUrl:   baseURL,
@@ -278,16 +344,6 @@ func (g *Client) PostFile(ctx context.Context, body File) error { //nolint:dupl,
 	return nil
 }
 
-// File represents a File manipulation operation via the Gitlab API
-type File struct {
-	Branch        string              `json:"branch"`
-	AuthorEmail   string              `json:"author_email"`
-	AuthorName    string              `json:"author_name"`
-	Content       checks.GlobalTarget `json:"content"`
-	CommitMessage string              `json:"commit_message"`
-	fileName      string
-}
-
 // Bytes returns the File as a byte array. The Content
 // is base64 encoded for Gitlab API compatibility.
 func (g *File) Bytes() ([]byte, error) {

pkg/sparrow/gitlab/gitlab_test.go

@@ -440,3 +440,57 @@ func TestClient_PostFile(t *testing.T) { //nolint:dupl // no need to refactor ye
 		})
 	}
 }
+
+func TestClient_DeleteFile(t *testing.T) {
+	tests := []struct {
+		name     string
+		fileName string
+		mockCode int
+		wantErr  bool
+	}{
+		{
+			name:     "success",
+			fileName: "test.de.json",
+			mockCode: http.StatusNoContent,
+		},
+		{
+			name:     "failure - API error",
+			fileName: "test.de.json",
+			mockCode: http.StatusInternalServerError,
+			wantErr:  true,
+		},
+		{
+			name:     "failure - empty file",
+			wantErr:  true,
+			fileName: "",
+		},
+	}
+
+	httpmock.Activate()
+	defer httpmock.DeactivateAndReset()
+	projID := 1
+	g := &Client{
+		baseUrl:   "http://test",
+		projectID: projID,
+		token:     "test",
+		client:    http.DefaultClient,
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(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)
+
+			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

@@ -31,6 +31,7 @@ type MockClient struct {
 	fetchFilesErr error
 	putFileErr    error
 	postFileErr   error
+	deleteFileErr error
 }
 
 func (m *MockClient) PutFile(ctx context.Context, _ gitlab.File) error { //nolint: gocritic // irrelevant
@@ -51,6 +52,12 @@ func (m *MockClient) FetchFiles(ctx context.Context) ([]checks.GlobalTarget, err
 	return m.targets, m.fetchFilesErr
 }
 
+func (m *MockClient) DeleteFile(ctx context.Context, file gitlab.File) error { //nolint: gocritic // irrelevant
+	log := logger.FromContext(ctx)
+	log.Info("MockDeleteFile called", "filename", file, "err", m.deleteFileErr)
+	return m.deleteFileErr
+}
+
 // SetFetchFilesErr sets the error returned by FetchFiles
 func (m *MockClient) SetFetchFilesErr(err error) {
 	m.fetchFilesErr = err
@@ -66,6 +73,11 @@ func (m *MockClient) SetPostFileErr(err error) {
 	m.postFileErr = err
 }
 
+// SetDeleteFileErr sets the error returned by DeleteFile
+func (m *MockClient) SetDeleteFileErr(err error) {
+	m.deleteFileErr = err
+}
+
 // New creates a new MockClient to mock Gitlab interaction
 func New(targets []checks.GlobalTarget) *MockClient {
 	return &MockClient{

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,17 +94,14 @@ 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 {
-				return err
-			}
-			return nil
+			return s.shutdown(ctx)
 		case result := <-s.cResult:
 			go s.db.Save(result)
 		case configChecks := <-s.cCfgChecks:
@@ -278,3 +280,17 @@ func fanInResults(checkChan chan checks.Result, cResult chan checks.ResultDTO, n
 		}
 	}
 }
+
+// shutdown shuts down the sparrow and all managed components gracefully.
+// It returns an error if one is present in the context or if any of the
+// components fail to shut down.
+func (s *Sparrow) shutdown(ctx context.Context) error {
+	errC := ctx.Err()
+	ctx, cancel := context.WithTimeout(context.Background(), shutdownTimeout)
+	defer cancel()
+	errS := s.targets.Shutdown(ctx)
+	if errS != nil {
+		return fmt.Errorf("failed to shutdown sparrow: %w", errors.Join(errC, errS))
+	}
+	return errC
+}

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,9 +88,11 @@ 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", "error", err)
+					log.Error("Failed to shutdown gracefully, stopping routine", "error", err)
 					return
 				}
 			}
@@ -124,12 +128,27 @@ func (t *gitlabTargetManager) Shutdown(ctx context.Context) error {
 	t.mu.Lock()
 	defer t.mu.Unlock()
 	log := logger.FromContext(ctx)
-	log.Info("Shutting down global gitlabTargetManager")
-	t.registered = false
+	log.Debug("Shut down signal received")
+
+	if t.Registered() {
+		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
+		}
+		t.registered = false
+	}
 
 	select {
 	case t.done <- struct{}{}:
-		log.Debug("Stopping reconcile routine")
+		log.Info("Stopping reconcile routine")
 	default:
 	}