From 24bf337c562dc9b9d8684cc9acb7ea171ea83414 Mon Sep 17 00:00:00 2001 From: Charandas <542168+charandas@users.noreply.github.com> Date: Thu, 19 Dec 2024 19:09:31 -0800 Subject: [PATCH] Playlists: convert to use reconcilers instead (#98075) --- .bra.toml | 1 + apps/playlist/go.mod | 2 +- apps/playlist/pkg/app/app.go | 34 ++++++--- .../pkg/reconcilers/reconciler_playlist.go | 46 ++++++++++++ .../playlist/pkg/watchers/watcher_playlist.go | 75 ------------------- .../feature-toggles/index.md | 2 +- .../src/types/featureToggles.gen.ts | 2 +- pkg/registry/apps/playlist/register.go | 2 +- pkg/services/featuremgmt/registry.go | 4 +- pkg/services/featuremgmt/toggles_gen.csv | 2 +- pkg/services/featuremgmt/toggles_gen.go | 6 +- pkg/services/featuremgmt/toggles_gen.json | 12 ++- 12 files changed, 89 insertions(+), 99 deletions(-) create mode 100644 apps/playlist/pkg/reconcilers/reconciler_playlist.go delete mode 100644 apps/playlist/pkg/watchers/watcher_playlist.go diff --git a/.bra.toml b/.bra.toml index 6dc5907da11..e3b7510804f 100644 --- a/.bra.toml +++ b/.bra.toml @@ -7,6 +7,7 @@ init_cmds = [ watch_all = true follow_symlinks = true watch_dirs = [ + "$WORKDIR/apps", "$WORKDIR/pkg", "$WORKDIR/public/views", "$WORKDIR/conf", diff --git a/apps/playlist/go.mod b/apps/playlist/go.mod index b1a2f9f18ef..775b31ec7a8 100644 --- a/apps/playlist/go.mod +++ b/apps/playlist/go.mod @@ -5,6 +5,7 @@ go 1.23.1 require ( github.com/grafana/grafana-app-sdk v0.23.1 k8s.io/apimachinery v0.31.3 + k8s.io/client-go v0.31.3 k8s.io/klog/v2 v2.130.1 k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 ) @@ -75,7 +76,6 @@ require ( gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/api v0.31.3 // indirect k8s.io/apiextensions-apiserver v0.31.3 // indirect - k8s.io/client-go v0.31.3 // indirect k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect diff --git a/apps/playlist/pkg/app/app.go b/apps/playlist/pkg/app/app.go index 6627fc774b5..184372310f5 100644 --- a/apps/playlist/pkg/app/app.go +++ b/apps/playlist/pkg/app/app.go @@ -2,34 +2,48 @@ package app import ( "context" - "fmt" "github.com/grafana/grafana-app-sdk/app" + "github.com/grafana/grafana-app-sdk/k8s" "github.com/grafana/grafana-app-sdk/operator" "github.com/grafana/grafana-app-sdk/resource" "github.com/grafana/grafana-app-sdk/simple" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/rest" "k8s.io/klog/v2" + "github.com/grafana/grafana/apps/playlist/pkg/reconcilers" + playlistv0alpha1 "github.com/grafana/grafana/apps/playlist/pkg/apis/playlist/v0alpha1" - "github.com/grafana/grafana/apps/playlist/pkg/watchers" ) type PlaylistConfig struct { - EnableWatchers bool + EnableReconcilers bool +} + +func getPatchClient(restConfig rest.Config, playlistKind resource.Kind) (operator.PatchClient, error) { + clientGenerator := k8s.NewClientRegistry(restConfig, k8s.ClientConfig{}) + return clientGenerator.ClientFor(playlistKind) } func New(cfg app.Config) (app.App, error) { var ( - playlistWatcher operator.ResourceWatcher - err error + playlistReconciler operator.Reconciler + err error ) playlistConfig, ok := cfg.SpecificConfig.(*PlaylistConfig) - if ok && playlistConfig.EnableWatchers { - playlistWatcher, err = watchers.NewPlaylistWatcher() + if ok && playlistConfig.EnableReconcilers { + patchClient, err := getPatchClient(cfg.KubeConfig, playlistv0alpha1.PlaylistKind()) + if err != nil { + klog.ErrorS(err, "Error getting patch client for use with opinionated reconciler") + return nil, err + } + + playlistReconciler, err = reconcilers.NewPlaylistReconciler(patchClient) if err != nil { - return nil, fmt.Errorf("unable to create PlaylistWatcher: %w", err) + klog.ErrorS(err, "Error creating playlist reconciler") + return nil, err } } @@ -43,8 +57,8 @@ func New(cfg app.Config) (app.App, error) { }, ManagedKinds: []simple.AppManagedKind{ { - Kind: playlistv0alpha1.PlaylistKind(), - Watcher: playlistWatcher, + Kind: playlistv0alpha1.PlaylistKind(), + Reconciler: playlistReconciler, Mutator: &simple.Mutator{ MutateFunc: func(ctx context.Context, req *app.AdmissionRequest) (*app.MutatingResponse, error) { // modify req.Object if needed diff --git a/apps/playlist/pkg/reconcilers/reconciler_playlist.go b/apps/playlist/pkg/reconcilers/reconciler_playlist.go new file mode 100644 index 00000000000..e1d0c4f0ad5 --- /dev/null +++ b/apps/playlist/pkg/reconcilers/reconciler_playlist.go @@ -0,0 +1,46 @@ +package reconcilers + +import ( + "context" + + "k8s.io/klog/v2" + + "github.com/grafana/grafana-app-sdk/operator" + playlist "github.com/grafana/grafana/apps/playlist/pkg/apis/playlist/v0alpha1" +) + +func NewPlaylistReconciler(patchClient operator.PatchClient) (operator.Reconciler, error) { + inner := operator.TypedReconciler[*playlist.Playlist]{} + + inner.ReconcileFunc = func(ctx context.Context, request operator.TypedReconcileRequest[*playlist.Playlist]) (operator.ReconcileResult, error) { + switch request.Action { + case operator.ReconcileActionCreated: + klog.InfoS("Added resource", "name", request.Object.GetStaticMetadata().Identifier().Name) + return operator.ReconcileResult{}, nil + case operator.ReconcileActionUpdated: + klog.InfoS("Updated resource", "name", request.Object.GetStaticMetadata().Identifier().Name) + return operator.ReconcileResult{}, nil + case operator.ReconcileActionDeleted: + klog.InfoS("Deleted resource", "name", request.Object.GetStaticMetadata().Identifier().Name) + return operator.ReconcileResult{}, nil + case operator.ReconcileActionResynced: + klog.InfoS("Possibly updated resource", "name", request.Object.GetStaticMetadata().Identifier().Name) + return operator.ReconcileResult{}, nil + case operator.ReconcileActionUnknown: + klog.InfoS("error reconciling unknown action for Playlist", "action", request.Action, "object", request.Object) + return operator.ReconcileResult{}, nil + } + + klog.InfoS("error reconciling invalid action for Playlist", "action", request.Action, "object", request.Object) + return operator.ReconcileResult{}, nil + } + + // prefixing the finalizer with - similar to how OpinionatedWatcher does + reconciler, err := operator.NewOpinionatedReconciler(patchClient, "playlist-playlists-finalizer") + if err != nil { + klog.ErrorS(err, "Error creating opinionated reconciler for playlists") + return nil, err + } + reconciler.Reconciler = &inner + return reconciler, nil +} diff --git a/apps/playlist/pkg/watchers/watcher_playlist.go b/apps/playlist/pkg/watchers/watcher_playlist.go deleted file mode 100644 index 44be08c25ae..00000000000 --- a/apps/playlist/pkg/watchers/watcher_playlist.go +++ /dev/null @@ -1,75 +0,0 @@ -package watchers - -import ( - "context" - "fmt" - - "github.com/grafana/grafana-app-sdk/operator" - "github.com/grafana/grafana-app-sdk/resource" - "k8s.io/klog/v2" - - playlist "github.com/grafana/grafana/apps/playlist/pkg/apis/playlist/v0alpha1" -) - -var _ operator.ResourceWatcher = &PlaylistWatcher{} - -type PlaylistWatcher struct{} - -func NewPlaylistWatcher() (*PlaylistWatcher, error) { - return &PlaylistWatcher{}, nil -} - -// Add handles add events for playlist.Playlist resources. -func (s *PlaylistWatcher) Add(ctx context.Context, rObj resource.Object) error { - object, ok := rObj.(*playlist.Playlist) - if !ok { - return fmt.Errorf("provided object is not of type *playlist.Playlist (name=%s, namespace=%s, kind=%s)", - rObj.GetStaticMetadata().Name, rObj.GetStaticMetadata().Namespace, rObj.GetStaticMetadata().Kind) - } - - klog.InfoS("Added resource", "name", object.GetStaticMetadata().Identifier().Name) - return nil -} - -// Update handles update events for playlist.Playlist resources. -func (s *PlaylistWatcher) Update(ctx context.Context, rOld resource.Object, rNew resource.Object) error { - oldObject, ok := rOld.(*playlist.Playlist) - if !ok { - return fmt.Errorf("provided object is not of type *playlist.Playlist (name=%s, namespace=%s, kind=%s)", - rOld.GetStaticMetadata().Name, rOld.GetStaticMetadata().Namespace, rOld.GetStaticMetadata().Kind) - } - - _, ok = rNew.(*playlist.Playlist) - if !ok { - return fmt.Errorf("provided object is not of type *playlist.Playlist (name=%s, namespace=%s, kind=%s)", - rNew.GetStaticMetadata().Name, rNew.GetStaticMetadata().Namespace, rNew.GetStaticMetadata().Kind) - } - - klog.InfoS("Updated resource", "name", oldObject.GetStaticMetadata().Identifier().Name) - return nil -} - -// Delete handles delete events for playlist.Playlist resources. -func (s *PlaylistWatcher) Delete(ctx context.Context, rObj resource.Object) error { - object, ok := rObj.(*playlist.Playlist) - if !ok { - return fmt.Errorf("provided object is not of type *playlist.Playlist (name=%s, namespace=%s, kind=%s)", - rObj.GetStaticMetadata().Name, rObj.GetStaticMetadata().Namespace, rObj.GetStaticMetadata().Kind) - } - - klog.InfoS("Deleted resource", "name", object.GetStaticMetadata().Identifier().Name) - return nil -} - -// Sync is not a standard resource.Watcher function, but is used when wrapping this watcher in an operator.OpinionatedWatcher. -// It handles resources which MAY have been updated during an outage period where the watcher was not able to consume events. -func (s *PlaylistWatcher) Sync(ctx context.Context, rObj resource.Object) error { - object, ok := rObj.(*playlist.Playlist) - if !ok { - return fmt.Errorf("provided object is not of type *playlist.Playlist (name=%s, namespace=%s, kind=%s)", - rObj.GetStaticMetadata().Name, rObj.GetStaticMetadata().Namespace, rObj.GetStaticMetadata().Kind) - } - - klog.InfoS("Possible resource update", "name", object.GetStaticMetadata().Identifier().Name) - return nil -} diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index 0ae6ead7728..2402516a246 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -220,7 +220,7 @@ Experimental features might be changed or removed without prior notice. | `timeRangeProvider` | Enables time pickers sync | | `prometheusUsesCombobox` | Use new combobox component for Prometheus query editor | | `userStorageAPI` | Enables the user storage API | -| `playlistsWatcher` | Enables experimental watcher for playlists | +| `playlistsReconciler` | Enables experimental reconciler for playlists | | `prometheusSpecialCharsInLabelValues` | Adds support for quotes and special characters in label values for Prometheus queries | | `enableExtensionsAdminPage` | Enables the extension admin page regardless of development mode | | `enableSCIM` | Enables SCIM support for user and group management | diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index c2306f613bd..ef6049346c7 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -228,7 +228,7 @@ export interface FeatureToggles { userStorageAPI?: boolean; azureMonitorDisableLogLimit?: boolean; preinstallAutoUpdate?: boolean; - playlistsWatcher?: boolean; + playlistsReconciler?: boolean; passwordlessMagicLinkAuthentication?: boolean; exploreMetricsRelatedLogs?: boolean; prometheusSpecialCharsInLabelValues?: boolean; diff --git a/pkg/registry/apps/playlist/register.go b/pkg/registry/apps/playlist/register.go index 9bba77fee16..9f79bb22f1d 100644 --- a/pkg/registry/apps/playlist/register.go +++ b/pkg/registry/apps/playlist/register.go @@ -41,7 +41,7 @@ func RegisterApp( LegacyStorageGetter: provider.legacyStorageGetter, ManagedKinds: playlistapp.GetKinds(), CustomConfig: any(&playlistapp.PlaylistConfig{ - EnableWatchers: features.IsEnabledGlobally(featuremgmt.FlagPlaylistsWatcher), + EnableReconcilers: features.IsEnabledGlobally(featuremgmt.FlagPlaylistsReconciler), }), } provider.Provider = simple.NewAppProvider(apis.LocalManifest(), appCfg, playlistapp.New) diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 0ec12c6a4b6..c3ecbab4883 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1578,8 +1578,8 @@ var ( Expression: "true", // enabled by default }, { - Name: "playlistsWatcher", - Description: "Enables experimental watcher for playlists", + Name: "playlistsReconciler", + Description: "Enables experimental reconciler for playlists", Stage: FeatureStageExperimental, Owner: grafanaAppPlatformSquad, RequiresRestart: true, diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 2a3a9b101bf..ff8687d9ad5 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -209,7 +209,7 @@ prometheusUsesCombobox,experimental,@grafana/observability-metrics,false,false,f userStorageAPI,experimental,@grafana/plugins-platform-backend,false,false,false azureMonitorDisableLogLimit,GA,@grafana/partner-datasources,false,false,false preinstallAutoUpdate,GA,@grafana/plugins-platform-backend,false,false,false -playlistsWatcher,experimental,@grafana/grafana-app-platform-squad,false,true,false +playlistsReconciler,experimental,@grafana/grafana-app-platform-squad,false,true,false passwordlessMagicLinkAuthentication,experimental,@grafana/identity-access-team,false,false,false exploreMetricsRelatedLogs,experimental,@grafana/observability-metrics,false,false,true prometheusSpecialCharsInLabelValues,experimental,@grafana/observability-metrics,false,false,true diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 12d255f7eaf..4cd67bed405 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -847,9 +847,9 @@ const ( // Enables automatic updates for pre-installed plugins FlagPreinstallAutoUpdate = "preinstallAutoUpdate" - // FlagPlaylistsWatcher - // Enables experimental watcher for playlists - FlagPlaylistsWatcher = "playlistsWatcher" + // FlagPlaylistsReconciler + // Enables experimental reconciler for playlists + FlagPlaylistsReconciler = "playlistsReconciler" // FlagPasswordlessMagicLinkAuthentication // Enable passwordless login via magic link authentication diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 847f28a4be3..b764e3bc1c2 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -2670,12 +2670,16 @@ }, { "metadata": { - "name": "playlistsWatcher", - "resourceVersion": "1730462910506", - "creationTimestamp": "2024-11-01T12:08:30Z" + "name": "playlistsReconciler", + "resourceVersion": "1734463170112", + "creationTimestamp": "2024-11-01T12:08:30Z", + "deletionTimestamp": "2024-12-19T19:17:00Z", + "annotations": { + "grafana.app/updatedTimestamp": "2024-12-17 19:19:30.112629 +0000 UTC" + } }, "spec": { - "description": "Enables experimental watcher for playlists", + "description": "Enables experimental reconciler for playlists", "stage": "experimental", "codeowner": "@grafana/grafana-app-platform-squad", "requiresRestart": true