Skip to content

Commit

Permalink
fix(configcache): return on error in ForceUpdateCluster (#4193)
Browse files Browse the repository at this point in the history
Otherwise, we panic inside updateSingle method.
This commit also contains a small test for
testing this behavior.
  • Loading branch information
Michal-Leszczynski authored Jan 10, 2025
1 parent ad1cb39 commit de6fefe
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 0 deletions.
1 change: 1 addition & 0 deletions pkg/service/configcache/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ func (svc *Service) ForceUpdateCluster(ctx context.Context, clusterID uuid.UUID)
c, err := svc.clusterSvc.GetCluster(ctx, clusterID.String())
if err != nil {
logger.Error(ctx, "Update failed", "error", err)
return false
}

return svc.updateSingle(ctx, c)
Expand Down
29 changes: 29 additions & 0 deletions pkg/service/configcache/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"testing"
"time"

"github.com/pkg/errors"
"github.com/scylladb/go-log"
"github.com/scylladb/scylla-manager/v3/pkg/scyllaclient"
"github.com/scylladb/scylla-manager/v3/pkg/service/cluster"
"github.com/scylladb/scylla-manager/v3/pkg/store"
Expand Down Expand Up @@ -203,6 +205,23 @@ func TestService_Run(t *testing.T) {
})
}

func TestServiceForceUpdateCluster(t *testing.T) {
t.Run("validate no panic when updating non-existing cluster", func(t *testing.T) {
svc := Service{
svcConfig: DefaultConfig(),
clusterSvc: &mockErrorClusterSvc{},
scyllaClient: mockProviderFunc,
secretsStore: &mockStore{},
configs: &sync.Map{},
logger: log.NewDevelopment(),
}

if svc.ForceUpdateCluster(context.Background(), uuid.MustRandom()) {
t.Fatalf("Expected updating non-existing cluster config to fail")
}
})
}

func (nc NodeConfig) sha256hash() (hash [32]byte, err error) {
data, err := json.Marshal(nc)
if err != nil {
Expand Down Expand Up @@ -299,3 +318,13 @@ var (
return nil, nil
}
)

// mockErrorClusterSvc works like mockClusterServicer with custom overrides for error responses.
type mockErrorClusterSvc struct {
mockClusterServicer
}

// GetCluster mocks the GetCluster method of Servicer with error response.
func (s *mockErrorClusterSvc) GetCluster(_ context.Context, _ string) (*cluster.Cluster, error) {
return nil, errors.New("not found")
}

0 comments on commit de6fefe

Please sign in to comment.