chore/remove-controller-errors (#130)

* chore: removed unused errors from controller

The ChecksController had mutliple functions which should return errors
but no errors were returned. This commit fixes that and cleans up the tests
that expected errors to be returned (but were never also used, as no error could
ever be returned).

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

* chore: removed unused fields & errors

* chore: removed unused fields and errors

Removed the unsued error field from the error struct and the error check
that is no longer needed.

---------

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

pkg/sparrow/controller.go

@@ -78,7 +78,7 @@ func (cc *ChecksController) Run(ctx context.Context) error {
 }
 
 // Shutdown shuts down the ChecksController.
-func (cc *ChecksController) Shutdown(ctx context.Context) (err error) {
+func (cc *ChecksController) Shutdown(ctx context.Context) {
 	log := logger.FromContext(ctx)
 	log.Info("Shutting down checks controller")
 
@@ -88,7 +88,6 @@ func (cc *ChecksController) Shutdown(ctx context.Context) (err error) {
 	cc.done <- struct{}{}
 	close(cc.done)
 	close(cc.cResult)
-	return err
 }
 
 // Reconcile reconciles the checks.
@@ -124,15 +123,12 @@ func (cc *ChecksController) Reconcile(ctx context.Context, cfg runtime.Config) {
 
 	// Register new checks
 	for _, c := range newChecks {
-		err = cc.RegisterCheck(ctx, c)
-		if err != nil {
-			log.ErrorContext(ctx, "Failed to register check", "check", c.Name(), "error", err)
-		}
+		cc.RegisterCheck(ctx, c)
 	}
 }
 
 // RegisterCheck registers a new check.
-func (cc *ChecksController) RegisterCheck(ctx context.Context, check checks.Check) error {
+func (cc *ChecksController) RegisterCheck(ctx context.Context, check checks.Check) {
 	log := logger.FromContext(ctx).With("check", check.Name())
 
 	// Add prometheus collectors of check to registry
@@ -154,7 +150,6 @@ func (cc *ChecksController) RegisterCheck(ctx context.Context, check checks.Chec
 	}()
 
 	cc.checks.Add(check)
-	return nil
 }
 
 // UnregisterCheck unregisters a check.

pkg/sparrow/controller_test.go

@@ -53,10 +53,7 @@ func TestRun_CheckRunError(t *testing.T) {
 		ShutdownFunc: func() {},
 	}
 
-	err := cc.RegisterCheck(ctx, mockCheck)
-	if err != nil {
-		t.Fatalf("RegisterCheck() error = %v", err)
-	}
+	cc.RegisterCheck(ctx, mockCheck)
 
 	go func() {
 		err := cc.Run(ctx)
@@ -78,9 +75,7 @@ func TestRun_CheckRunError(t *testing.T) {
 	if found {
 		t.Errorf("Expected check to be unregistered after encountering a run error")
 	}
-	if err := cc.Shutdown(ctx); err != nil {
-		t.Fatalf("Shutdown() error = %v", err)
-	}
+	cc.Shutdown(ctx)
 }
 
 func TestRun_ContextCancellation(t *testing.T) {
@@ -236,79 +231,60 @@ func TestChecksController_Reconcile(t *testing.T) {
 
 func TestChecksController_RegisterCheck(t *testing.T) {
 	tests := []struct {
-		name    string
-		setup   func(t *testing.T) *ChecksController
-		check   checks.Check
-		wantErr bool
+		name  string
+		setup func() *ChecksController
+		check checks.Check
 	}{
 		{
 			name: "valid check",
-			setup: func(_ *testing.T) *ChecksController {
+			setup: func() *ChecksController {
 				return NewChecksController(db.NewInMemory(), NewMetrics())
 			},
-			check:   health.NewCheck(),
-			wantErr: false,
+			check: health.NewCheck(),
 		},
 		{
 			name: "duplicate check registration",
-			setup: func(t *testing.T) *ChecksController {
+			setup: func() *ChecksController {
 				cc := NewChecksController(db.NewInMemory(), NewMetrics())
 				check := health.NewCheck()
-				err := cc.RegisterCheck(context.Background(), check)
-				if err != nil {
-					t.Fatalf("RegisterCheck() error = %v", err)
-				}
+				cc.RegisterCheck(context.Background(), check)
 				return cc
 			},
-			check:   health.NewCheck(),
-			wantErr: false,
+			check: health.NewCheck(),
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			cc := tt.setup(t)
+			cc := tt.setup()
 
-			err := cc.RegisterCheck(context.Background(), tt.check)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("RegisterCheck() error = %v, wantErr %v", err, tt.wantErr)
-			}
+			cc.RegisterCheck(context.Background(), tt.check)
 		})
 	}
 }
 
 func TestChecksController_UnregisterCheck(t *testing.T) {
 	tests := []struct {
-		name    string
-		setup   func(t *testing.T) *ChecksController
-		check   checks.Check
-		wantErr bool
+		name  string
+		check checks.Check
 	}{
 		{
-			name: "valid check",
-			setup: func(_ *testing.T) *ChecksController {
-				return NewChecksController(db.NewInMemory(), NewMetrics())
-			},
-			check:   health.NewCheck(),
-			wantErr: false,
+			name:  "valid check",
+			check: health.NewCheck(),
 		},
 		{
-			name: "unregister non-existent check",
-			setup: func(_ *testing.T) *ChecksController {
-				return NewChecksController(db.NewInMemory(), NewMetrics())
-			},
-			check:   health.NewCheck(),
-			wantErr: false,
+			name:  "unregister non-existent check",
+			check: health.NewCheck(),
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			cc := tt.setup(t)
+			cc := NewChecksController(db.NewInMemory(), NewMetrics())
 
 			cc.UnregisterCheck(context.Background(), tt.check)
 
-			if !tt.wantErr && len(cc.checks.Iter()) != 0 {
+			if len(cc.checks.Iter()) != 0 {
 				t.Errorf("Expected check to be unregistered")
 			}
 		})

pkg/sparrow/run.go

@@ -172,7 +172,7 @@ func (s *Sparrow) shutdown(ctx context.Context) {
 		}
 		sErrs.errAPI = s.api.Shutdown(ctx)
 		s.loader.Shutdown(ctx)
-		sErrs.errCheckCont = s.controller.Shutdown(ctx)
+		s.controller.Shutdown(ctx)
 
 		if sErrs.HasError() {
 			log.Error("Failed to shutdown gracefully", "contextError", errC, "errors", sErrs)

pkg/sparrow/run_errors.go

@@ -19,11 +19,10 @@
 package sparrow
 
 type ErrShutdown struct {
-	errAPI       error
-	errTarMan    error
-	errCheckCont error
+	errAPI    error
+	errTarMan error
 }
 
 func (e ErrShutdown) HasError() bool {
-	return e.errAPI != nil || e.errTarMan != nil || e.errCheckCont != nil
+	return e.errAPI != nil || e.errTarMan != nil
 }