Restructure cloudmigration service (#83211)

* Restructure cloudmigation service

* Adjust codewoners and wire

* Comment out unused metrics

.github/CODEOWNERS

@@ -605,7 +605,7 @@ playwright.config.ts @grafana/plugins-platform-frontend
 /pkg/infra/httpclient/httpclientprovider/sigv4_middleware_test.go @grafana/grafana-operator-experience-squad
 /pkg/services/caching/ @grafana/grafana-operator-experience-squad
 /pkg/services/featuremgmt/ @grafana/grafana-operator-experience-squad
-/pkg/services/cloudmigrations/ @grafana/grafana-operator-experience-squad
+/pkg/services/cloudmigration/ @grafana/grafana-operator-experience-squad
 
 # Kind definitions
 /kinds/dashboard @grafana/dashboards-squad

pkg/server/wire.go

@@ -50,7 +50,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/auth/jwt"
 	"github.com/grafana/grafana/pkg/services/authn/authnimpl"
 	"github.com/grafana/grafana/pkg/services/cleanup"
-	cloudmigrations "github.com/grafana/grafana/pkg/services/cloudmigrations/service"
+	"github.com/grafana/grafana/pkg/services/cloudmigration/cloudmigrationimpl"
 	"github.com/grafana/grafana/pkg/services/contexthandler"
 	"github.com/grafana/grafana/pkg/services/correlations"
 	"github.com/grafana/grafana/pkg/services/dashboardimport"
@@ -381,7 +381,7 @@ var wireBasicSet = wire.NewSet(
 	wire.Bind(new(ssosettings.Service), new(*ssoSettingsImpl.Service)),
 	idimpl.ProvideService,
 	wire.Bind(new(auth.IDService), new(*idimpl.Service)),
-	cloudmigrations.ProvideService,
+	cloudmigrationimpl.ProvideService,
 	// Kubernetes API server
 	grafanaapiserver.WireSet,
 	apiregistry.WireSet,

pkg/services/cloudmigration/api/api.go

@@ -7,21 +7,20 @@ import (
 	"github.com/grafana/grafana/pkg/api/routing"
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/middleware"
-	"github.com/grafana/grafana/pkg/services/cloudmigrations"
+	"github.com/grafana/grafana/pkg/services/cloudmigration"
-	"github.com/grafana/grafana/pkg/services/cloudmigrations/models"
 	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
 	"github.com/grafana/grafana/pkg/web"
 )
 
 type MigrationAPI struct {
-	cloudMigrationsService cloudmigrations.CloudMigrationService
+	cloudMigrationsService cloudmigration.Service
 	routeRegister          routing.RouteRegister
 	log                    log.Logger
 }
 
 func RegisterApi(
 	rr routing.RouteRegister,
-	cms cloudmigrations.CloudMigrationService,
+	cms cloudmigration.Service,
 ) *MigrationAPI {
 	api := &MigrationAPI{
 		log:                    log.New("cloudmigrations.api"),
@@ -43,15 +42,15 @@ func (api *MigrationAPI) registerEndpoints() {
 }
 
 func (api *MigrationAPI) MigrateDatasources(c *contextmodel.ReqContext) response.Response {
-	var req migrateDatasourcesRequestDTO
+	var req cloudmigration.MigrateDatasourcesRequestDTO
 	if err := web.Bind(c.Req, &req); err != nil {
 		return response.Error(http.StatusBadRequest, "bad request data", err)
 	}
 
-	resp, err := api.cloudMigrationsService.MigrateDatasources(c.Req.Context(), &models.MigrateDatasourcesRequest{MigrateToPDC: req.MigrateToPDC, MigrateCredentials: req.MigrateCredentials})
+	resp, err := api.cloudMigrationsService.MigrateDatasources(c.Req.Context(), &cloudmigration.MigrateDatasourcesRequest{MigrateToPDC: req.MigrateToPDC, MigrateCredentials: req.MigrateCredentials})
 	if err != nil {
 		return response.Error(http.StatusInternalServerError, "data source migrations error", err)
 	}
 
-	return response.JSON(http.StatusOK, migrateDatasourcesResponseDTO{DatasourcesMigrated: resp.DatasourcesMigrated})
+	return response.JSON(http.StatusOK, cloudmigration.MigrateDatasourcesResponseDTO{DatasourcesMigrated: resp.DatasourcesMigrated})
 }

pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration.go

@@ -1,4 +1,4 @@
-package service
+package cloudmigrationimpl
 
 import (
 	"context"
@@ -6,10 +6,8 @@ import (
 	"github.com/grafana/grafana/pkg/api/routing"
 	"github.com/grafana/grafana/pkg/infra/db"
 	"github.com/grafana/grafana/pkg/infra/log"
-	"github.com/grafana/grafana/pkg/services/cloudmigrations"
+	"github.com/grafana/grafana/pkg/services/cloudmigration"
-	"github.com/grafana/grafana/pkg/services/cloudmigrations/api"
+	"github.com/grafana/grafana/pkg/services/cloudmigration/api"
-	"github.com/grafana/grafana/pkg/services/cloudmigrations/metrics"
-	"github.com/grafana/grafana/pkg/services/cloudmigrations/models"
 	"github.com/grafana/grafana/pkg/services/datasources"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/setting"
@@ -17,54 +15,53 @@ import (
 )
 
 // CloudMigrationsServiceImpl Define the Service Implementation.
-type CloudMigrationsServiceImpl struct {
+type Service struct {
+	store store
+
 	log log.Logger
 	cfg *setting.Cfg
 
-	sqlStore  db.DB
 	features  featuremgmt.FeatureToggles
 	dsService datasources.DataSourceService
 
-	api     *api.MigrationAPI
+	api *api.MigrationAPI
-	metrics *metrics.Metrics
+	// metrics *Metrics
 }
 
-var LogPrefix = "cloudmigrations.service"
+var LogPrefix = "cloudmigration.service"
 
-var _ cloudmigrations.CloudMigrationService = (*CloudMigrationsServiceImpl)(nil)
+var _ cloudmigration.Service = (*Service)(nil)
 
 // ProvideService Factory for method used by wire to inject dependencies.
 // builds the service, and api, and configures routes
 func ProvideService(
 	cfg *setting.Cfg,
 	features featuremgmt.FeatureToggles,
-	sqlStore db.DB,
+	db db.DB,
 	dsService datasources.DataSourceService,
 	routeRegister routing.RouteRegister,
 	prom prometheus.Registerer,
-) cloudmigrations.CloudMigrationService {
+) cloudmigration.Service {
 	if !features.IsEnabledGlobally(featuremgmt.FlagOnPremToCloudMigrations) {
 		return &NoopServiceImpl{}
 	}
 
-	s := &CloudMigrationsServiceImpl{
+	s := &Service{
+		store:     &sqlStore{db: db},
 		log:       log.New(LogPrefix),
 		cfg:       cfg,
-		sqlStore:  sqlStore,
 		features:  features,
 		dsService: dsService,
 	}
 	s.api = api.RegisterApi(routeRegister, s)
 
-	if m, err := metrics.RegisterMetrics(prom); err != nil {
+	if err := s.registerMetrics(prom); err != nil {
 		s.log.Warn("error registering prom metrics", "error", err.Error())
-	} else {
-		s.metrics = m
 	}
 
 	return s
 }
 
-func (cm *CloudMigrationsServiceImpl) MigrateDatasources(ctx context.Context, request *models.MigrateDatasourcesRequest) (*models.MigrateDatasourcesResponse, error) {
+func (s *Service) MigrateDatasources(ctx context.Context, request *cloudmigration.MigrateDatasourcesRequest) (*cloudmigration.MigrateDatasourcesResponse, error) {
-	return nil, models.ErrInternalNotImplementedError
+	return s.store.MigrateDatasources(ctx, request)
 }

pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration_noop.go

@@ -0,0 +1,16 @@
+package cloudmigrationimpl
+
+import (
+	"context"
+
+	"github.com/grafana/grafana/pkg/services/cloudmigration"
+)
+
+// CloudMigrationsServiceImpl Define the Service Implementation.
+type NoopServiceImpl struct{}
+
+var _ cloudmigration.Service = (*NoopServiceImpl)(nil)
+
+func (s *NoopServiceImpl) MigrateDatasources(ctx context.Context, request *cloudmigration.MigrateDatasourcesRequest) (*cloudmigration.MigrateDatasourcesResponse, error) {
+	return nil, cloudmigration.ErrFeatureDisabledError
+}

pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration_test.go

@@ -0,0 +1,15 @@
+package cloudmigrationimpl
+
+import (
+	"context"
+	"testing"
+
+	"github.com/grafana/grafana/pkg/services/cloudmigration"
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_NoopServiceDoesNothing(t *testing.T) {
+	s := &NoopServiceImpl{}
+	_, e := s.MigrateDatasources(context.Background(), &cloudmigration.MigrateDatasourcesRequest{})
+	assert.ErrorIs(t, e, cloudmigration.ErrFeatureDisabledError)
+}

pkg/services/cloudmigration/cloudmigrationimpl/metric.go

@@ -0,0 +1,26 @@
+package cloudmigrationimpl
+
+import (
+	"errors"
+
+	"github.com/grafana/grafana/pkg/services/cloudmigration"
+	"github.com/prometheus/client_golang/prometheus"
+)
+
+// type Metrics struct {
+// 	log log.Logger
+// }
+
+func (s *Service) registerMetrics(prom prometheus.Registerer) error {
+	for _, m := range cloudmigration.PromMetrics {
+		if err := prom.Register(m); err != nil {
+			var alreadyRegisterErr prometheus.AlreadyRegisteredError
+			if errors.As(err, &alreadyRegisterErr) {
+				s.log.Warn("metric already registered", "metric", m)
+				continue
+			}
+			return err
+		}
+	}
+	return nil
+}

pkg/services/cloudmigration/cloudmigrationimpl/store.go

@@ -0,0 +1,11 @@
+package cloudmigrationimpl
+
+import (
+	"context"
+
+	"github.com/grafana/grafana/pkg/services/cloudmigration"
+)
+
+type store interface {
+	MigrateDatasources(context.Context, *cloudmigration.MigrateDatasourcesRequest) (*cloudmigration.MigrateDatasourcesResponse, error)
+}

pkg/services/cloudmigration/cloudmigrationimpl/xorm_store.go

@@ -0,0 +1,16 @@
+package cloudmigrationimpl
+
+import (
+	"context"
+
+	"github.com/grafana/grafana/pkg/infra/db"
+	"github.com/grafana/grafana/pkg/services/cloudmigration"
+)
+
+type sqlStore struct {
+	db db.DB
+}
+
+func (ss *sqlStore) MigrateDatasources(ctx context.Context, request *cloudmigration.MigrateDatasourcesRequest) (*cloudmigration.MigrateDatasourcesResponse, error) {
+	return nil, cloudmigration.ErrInternalNotImplementedError
+}

pkg/services/cloudmigration/cloudmigrationimpl/xorm_store_test.go

@@ -0,0 +1,9 @@
+package cloudmigrationimpl
+
+import (
+	"testing"
+)
+
+func TestMigrateDatasources(t *testing.T) {
+	// TODO: Write this test
+}

pkg/services/cloudmigration/cloudmigrations.go

@@ -0,0 +1,9 @@
+package cloudmigration
+
+import (
+	"context"
+)
+
+type Service interface {
+	MigrateDatasources(context.Context, *MigrateDatasourcesRequest) (*MigrateDatasourcesResponse, error)
+}

pkg/services/cloudmigration/cloudmigrationtest/fake.go

@@ -0,0 +1,15 @@
+package cloudmigrationtest
+
+import (
+	"context"
+
+	"github.com/grafana/grafana/pkg/services/cloudmigration"
+)
+
+type Service struct {
+	ExpectedError error
+}
+
+func (s *Service) MigrateDatasources(ctx context.Context, request *cloudmigration.MigrateDatasourcesRequest) (*cloudmigration.MigrateDatasourcesResponse, error) {
+	return nil, cloudmigration.ErrInternalNotImplementedError
+}

pkg/services/cloudmigration/model.go

@@ -0,0 +1,43 @@
+package cloudmigration
+
+import (
+	"github.com/grafana/grafana/pkg/util/errutil"
+	"github.com/prometheus/client_golang/prometheus"
+)
+
+var (
+	ErrInternalNotImplementedError = errutil.Internal("cloudmigrations.notImplemented", errutil.WithPublicMessage("Internal server error"))
+	ErrFeatureDisabledError        = errutil.Internal("cloudmigrations.disabled", errutil.WithPublicMessage("Cloud migrations are disabled on this instance"))
+)
+
+type MigrateDatasourcesRequest struct {
+	MigrateToPDC       bool
+	MigrateCredentials bool
+}
+
+type MigrateDatasourcesResponse struct {
+	DatasourcesMigrated int
+}
+
+type MigrateDatasourcesRequestDTO struct {
+	MigrateToPDC       bool `json:"migrateToPDC"`
+	MigrateCredentials bool `json:"migrateCredentials"`
+}
+
+type MigrateDatasourcesResponseDTO struct {
+	DatasourcesMigrated int `json:"datasourcesMigrated"`
+}
+
+const (
+	namespace = "grafana"
+	subsystem = "cloudmigrations"
+)
+
+var PromMetrics = []prometheus.Collector{
+	prometheus.NewCounterVec(prometheus.CounterOpts{
+		Namespace: namespace,
+		Subsystem: subsystem,
+		Name:      "datasources_migrated",
+		Help:      "Total amount of data sources migrated",
+	}, []string{"pdc_converted"}),
+}

pkg/services/cloudmigrations/api/models.go

@@ -1,10 +0,0 @@
-package api
-
-type migrateDatasourcesRequestDTO struct {
-	MigrateToPDC       bool `json:"migrateToPDC"`
-	MigrateCredentials bool `json:"migrateCredentials"`
-}
-
-type migrateDatasourcesResponseDTO struct {
-	DatasourcesMigrated int `json:"datasourcesMigrated"`
-}

pkg/services/cloudmigrations/cloudmigrations.go

@@ -1,11 +0,0 @@
-package cloudmigrations
-
-import (
-	"context"
-
-	"github.com/grafana/grafana/pkg/services/cloudmigrations/models"
-)
-
-type CloudMigrationService interface {
-	MigrateDatasources(ctx context.Context, request *models.MigrateDatasourcesRequest) (*models.MigrateDatasourcesResponse, error)
-}

pkg/services/cloudmigrations/metrics/metric.go

@@ -1,40 +0,0 @@
-package metrics
-
-import (
-	"errors"
-
-	"github.com/grafana/grafana/pkg/infra/log"
-	"github.com/prometheus/client_golang/prometheus"
-)
-
-type Metrics struct {
-	log log.Logger
-}
-
-func RegisterMetrics(
-	prom prometheus.Registerer,
-) (*Metrics, error) {
-	s := &Metrics{
-		log: log.New("cloudmigrations.metrics"),
-	}
-
-	if err := s.registerMetrics(prom); err != nil {
-		return nil, err
-	}
-
-	return s, nil
-}
-
-func (s *Metrics) registerMetrics(prom prometheus.Registerer) error {
-	for _, m := range promMetrics {
-		if err := prom.Register(m); err != nil {
-			var alreadyRegisterErr prometheus.AlreadyRegisteredError
-			if errors.As(err, &alreadyRegisterErr) {
-				s.log.Warn("metric already registered", "metric", m)
-				continue
-			}
-			return err
-		}
-	}
-	return nil
-}

pkg/services/cloudmigrations/metrics/metrics.go

@@ -1,19 +0,0 @@
-package metrics
-
-import (
-	"github.com/prometheus/client_golang/prometheus"
-)
-
-const (
-	namespace = "grafana"
-	subsystem = "cloudmigrations"
-)
-
-var promMetrics = []prometheus.Collector{
-	prometheus.NewCounterVec(prometheus.CounterOpts{
-		Namespace: namespace,
-		Subsystem: subsystem,
-		Name:      "datasources_migrated",
-		Help:      "Total amount of data sources migrated",
-	}, []string{"pdc_converted"}),
-}

pkg/services/cloudmigrations/models/errors.go

@@ -1,8 +0,0 @@
-package models
-
-import "github.com/grafana/grafana/pkg/util/errutil"
-
-var (
-	ErrInternalNotImplementedError = errutil.Internal("cloudmigrations.notImplemented", errutil.WithPublicMessage("Internal server error"))
-	ErrFeatureDisabledError        = errutil.Internal("cloudmigrations.disabled", errutil.WithPublicMessage("Cloud migrations are disabled on this instance"))
-)

pkg/services/cloudmigrations/models/models.go

@@ -1,10 +0,0 @@
-package models
-
-type MigrateDatasourcesRequest struct {
-	MigrateToPDC       bool
-	MigrateCredentials bool
-}
-
-type MigrateDatasourcesResponse struct {
-	DatasourcesMigrated int
-}

pkg/services/cloudmigrations/service/noop_service.go

@@ -1,17 +0,0 @@
-package service
-
-import (
-	"context"
-
-	"github.com/grafana/grafana/pkg/services/cloudmigrations"
-	"github.com/grafana/grafana/pkg/services/cloudmigrations/models"
-)
-
-// CloudMigrationsServiceImpl Define the Service Implementation.
-type NoopServiceImpl struct{}
-
-var _ cloudmigrations.CloudMigrationService = (*NoopServiceImpl)(nil)
-
-func (cm *NoopServiceImpl) MigrateDatasources(ctx context.Context, request *models.MigrateDatasourcesRequest) (*models.MigrateDatasourcesResponse, error) {
-	return nil, models.ErrFeatureDisabledError
-}

pkg/services/cloudmigrations/service/service_test.go

@@ -1,15 +0,0 @@
-package service
-
-import (
-	"context"
-	"testing"
-
-	"github.com/grafana/grafana/pkg/services/cloudmigrations/models"
-	"github.com/stretchr/testify/assert"
-)
-
-func Test_NoopServiceDoesNothing(t *testing.T) {
-	s := &NoopServiceImpl{}
-	_, e := s.MigrateDatasources(context.Background(), &models.MigrateDatasourcesRequest{})
-	assert.ErrorIs(t, e, models.ErrFeatureDisabledError)
-}