From a717aa955b74aa9d6b6af54808ebe5ccbb104635 Mon Sep 17 00:00:00 2001 From: Luca Burgazzoli Date: Fri, 18 Oct 2024 10:28:05 +0200 Subject: [PATCH] feat: component fluent builder --- .../components/dashboard/dashboard_actions.go | 96 ++++++ .../dashboard/dashboard_controller.go | 282 +++++------------- .../components/dashboard/dashboard_support.go | 62 ++++ pkg/controller/actions/action_fn.go | 35 --- pkg/controller/actions/actions.go | 6 - .../action_delete_resources.go | 31 +- .../action_delete_resources_test.go | 16 +- .../actions/{ => deploy}/action_deploy.go | 122 ++++---- .../action_deploy_merge_deployment.go | 2 +- .../action_deploy_merge_deployment_test.go | 8 +- ...tion_deploy_remove_deployment_resources.go | 2 +- ...deploy_remove_deployment_resources_test.go | 8 +- pkg/controller/actions/fn/action_fn.go | 25 ++ .../{ => render}/action_render_manifests.go | 26 +- .../action_render_manifests_test.go | 80 +---- .../action_update_status.go | 27 +- .../action_update_status_test.go | 28 +- .../reconciler/component_reconciler.go | 2 +- .../component_reconciler_support.go | 133 +++++++++ .../test/fakeclient/fakeclient.go} | 17 +- pkg/utils/test/matchers/matechers.go | 19 ++ 21 files changed, 544 insertions(+), 483 deletions(-) create mode 100644 controllers/components/dashboard/dashboard_actions.go create mode 100644 controllers/components/dashboard/dashboard_support.go delete mode 100644 pkg/controller/actions/action_fn.go rename pkg/controller/actions/{ => deleteresource}/action_delete_resources.go (50%) rename pkg/controller/actions/{ => deleteresource}/action_delete_resources_test.go (83%) rename pkg/controller/actions/{ => deploy}/action_deploy.go (60%) rename pkg/controller/actions/{ => deploy}/action_deploy_merge_deployment.go (99%) rename pkg/controller/actions/{ => deploy}/action_deploy_merge_deployment_test.go (96%) rename pkg/controller/actions/{ => deploy}/action_deploy_remove_deployment_resources.go (98%) rename pkg/controller/actions/{ => deploy}/action_deploy_remove_deployment_resources_test.go (90%) create mode 100644 pkg/controller/actions/fn/action_fn.go rename pkg/controller/actions/{ => render}/action_render_manifests.go (54%) rename pkg/controller/actions/{ => render}/action_render_manifests_test.go (68%) rename pkg/controller/actions/{ => updatestatus}/action_update_status.go (70%) rename pkg/controller/actions/{ => updatestatus}/action_update_status_test.go (85%) create mode 100644 pkg/controller/reconciler/component_reconciler_support.go rename pkg/{controller/actions/action_test.go => utils/test/fakeclient/fakeclient.go} (61%) create mode 100644 pkg/utils/test/matchers/matechers.go diff --git a/controllers/components/dashboard/dashboard_actions.go b/controllers/components/dashboard/dashboard_actions.go new file mode 100644 index 00000000000..8d9f1416e09 --- /dev/null +++ b/controllers/components/dashboard/dashboard_actions.go @@ -0,0 +1,96 @@ +package dashboard + +import ( + "context" + "errors" + "fmt" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + logf "sigs.k8s.io/controller-runtime/pkg/log" + + componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" + odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" + odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" +) + +func initialize(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { + // Implement initialization logic + log := logf.FromContext(ctx).WithName(ComponentNameUpstream) + + rr.Manifests = []odhtypes.ManifestInfo{defaultManifestInfo(rr.Platform)} + + if err := odhdeploy.ApplyParams(rr.Manifests[0].ManifestsPath(), imagesMap); err != nil { + log.Error(err, "failed to update image", "path", rr.Manifests[0].ManifestsPath()) + } + + extraParamsMap, err := updateKustomizeVariable(ctx, rr.Client, rr.Platform, &rr.DSCI.Spec) + if err != nil { + return errors.New("failed to set variable for extraParamsMap") + } + + if err := odhdeploy.ApplyParams(rr.Manifests[0].ManifestsPath(), nil, extraParamsMap); err != nil { + return fmt.Errorf("failed to update params.env from %s : %w", rr.Manifests[0].ManifestsPath(), err) + } + + return nil +} + +func devFlags(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { + dashboard, ok := rr.Instance.(*componentsv1.Dashboard) + if !ok { + return fmt.Errorf("resource instance %v is not a componentsv1.Dashboard)", rr.Instance) + } + + if dashboard.Spec.DevFlags == nil { + return nil + } + // Implement devflags support logic + // If dev flags are set, update default manifests path + if len(dashboard.Spec.DevFlags.Manifests) != 0 { + manifestConfig := dashboard.Spec.DevFlags.Manifests[0] + if err := odhdeploy.DownloadManifests(ctx, ComponentNameUpstream, manifestConfig); err != nil { + return err + } + if manifestConfig.SourcePath != "" { + rr.Manifests[0].Path = odhdeploy.DefaultManifestPath + rr.Manifests[0].ContextDir = ComponentNameUpstream + rr.Manifests[0].SourcePath = manifestConfig.SourcePath + } + } + + return nil +} + +func customizeResources(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { + switch rr.Platform { + case cluster.SelfManagedRhods, cluster.ManagedRhods: + if err := cluster.UpdatePodSecurityRolebinding(ctx, rr.Client, rr.DSCI.Spec.ApplicationsNamespace, "rhods-dashboard"); err != nil { + return fmt.Errorf("failed to update PodSecurityRolebinding for rhods-dashboard: %w", err) + } + + err := rr.AddResource(&corev1.Secret{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "Secret", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "anaconda-ce-access", + Namespace: rr.DSCI.Spec.ApplicationsNamespace, + }, + Type: corev1.SecretTypeOpaque, + }) + + if err != nil { + return fmt.Errorf("failed to create access-secret for anaconda: %w", err) + } + + default: + if err := cluster.UpdatePodSecurityRolebinding(ctx, rr.Client, rr.DSCI.Spec.ApplicationsNamespace, "odh-dashboard"); err != nil { + return fmt.Errorf("failed to update PodSecurityRolebinding for odh-dashboard: %w", err) + } + } + + return nil +} diff --git a/controllers/components/dashboard/dashboard_controller.go b/controllers/components/dashboard/dashboard_controller.go index cb7a28cd713..e9feee7d063 100644 --- a/controllers/components/dashboard/dashboard_controller.go +++ b/controllers/components/dashboard/dashboard_controller.go @@ -18,31 +18,27 @@ package dashboard import ( "context" - "errors" "fmt" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" - logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" - dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" - dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" odhrec "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/reconciler" - odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" + odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/manifests/kustomize" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels" ) @@ -53,15 +49,17 @@ const ( var ( ComponentNameUpstream = ComponentName - PathUpstream = deploy.DefaultManifestPath + "/" + ComponentNameUpstream + "/odh" + PathUpstream = odhdeploy.DefaultManifestPath + "/" + ComponentNameUpstream + "/odh" ComponentNameDownstream = "rhods-dashboard" - PathDownstream = deploy.DefaultManifestPath + "/" + ComponentNameUpstream + "/rhoai" + PathDownstream = odhdeploy.DefaultManifestPath + "/" + ComponentNameUpstream + "/rhoai" PathSelfDownstream = PathDownstream + "/onprem" PathManagedDownstream = PathDownstream + "/addon" OverridePath = "" DefaultPath = "" + dashboardInstanceID = types.NamespacedName{Name: componentsv1.DashboardInstanceName} + adminGroups = map[cluster.Platform]string{ cluster.SelfManagedRhods: "rhods-admins", cluster.ManagedRhods: "dedicated-admins", @@ -95,90 +93,6 @@ var ( } ) -func NewDashboardReconciler(ctx context.Context, mgr ctrl.Manager) error { - r, err := odhrec.NewComponentReconciler[*componentsv1.Dashboard](ctx, mgr, ComponentName) - if err != nil { - return err - } - - actionCtx := logf.IntoContext(ctx, r.Log) - d := Dashboard{} - - // Add Dashboard-specific actions - r.AddAction(actions.NewActionFn(actionCtx, d.initialize)) - r.AddAction(actions.NewActionFn(actionCtx, d.devFlags)) - - r.AddAction(actions.NewRenderManifestsAction( - actionCtx, - actions.WithRenderManifestsOptions( - kustomize.WithEngineRenderOpts( - kustomize.WithLabel(labels.ComponentName, ComponentName), - ), - ), - )) - - r.AddAction(actions.NewActionFn(actionCtx, d.customizeResources)) - - r.AddAction(actions.NewDeployAction( - actionCtx, - actions.WithDeployedMode(actions.DeployModeSSA), - actions.WithDeployedResourceFieldOwner(ComponentName), - )) - - r.AddAction(actions.NewUpdateStatusAction( - actionCtx, - actions.WithUpdateStatusLabel(labels.ComponentName, ComponentName), - )) - - predicates := make([]predicate.Predicate, 0) - switch r.Platform { - case cluster.SelfManagedRhods, cluster.ManagedRhods: - predicates = append(predicates, dashboardWatchPredicate(ComponentNameUpstream)) - default: - predicates = append(predicates, dashboardWatchPredicate(ComponentNameDownstream)) - } - - eh := handler.EnqueueRequestsFromMapFunc(watchDashboardResources) - ef := builder.WithPredicates(predicates...) - - err = ctrl.NewControllerManagedBy(mgr). - For(&componentsv1.Dashboard{}). - // dependants - Watches(&appsv1.Deployment{}, eh, ef). - Watches(&appsv1.ReplicaSet{}, eh, ef). - Watches(&corev1.Namespace{}, eh, ef). - Watches(&corev1.ConfigMap{}, eh, ef). - Watches(&corev1.PersistentVolumeClaim{}, eh, ef). - Watches(&rbacv1.ClusterRoleBinding{}, eh, ef). - Watches(&rbacv1.ClusterRole{}, eh, ef). - Watches(&rbacv1.Role{}, eh, ef). - Watches(&rbacv1.RoleBinding{}, eh, ef). - Watches(&corev1.ServiceAccount{}, eh, ef). - // done - Complete(r) - - if err != nil { - return fmt.Errorf("could not create the dashboard controller: %w", err) - } - - return nil -} - -func CreateDashboardInstance(dsc *dscv1.DataScienceCluster) *componentsv1.Dashboard { - return &componentsv1.Dashboard{ - TypeMeta: metav1.TypeMeta{ - Kind: "Dashboard", - APIVersion: "components.opendatahub.io/v1", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: componentsv1.DashboardInstanceName, - }, - Spec: componentsv1.DashboardSpec{ - DSCDashboard: dsc.Spec.Components.Dashboard, - }, - } -} - // +kubebuilder:rbac:groups=components.opendatahub.io,resources=dashboards,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=components.opendatahub.io,resources=dashboards/status,verbs=get;update;patch // +kubebuilder:rbac:groups=components.opendatahub.io,resources=dashboards/finalizers,verbs=update @@ -219,11 +133,73 @@ func CreateDashboardInstance(dsc *dscv1.DataScienceCluster) *componentsv1.Dashbo // +kubebuilder:rbac:groups="*",resources=replicasets,verbs=* +func NewDashboardReconciler(ctx context.Context, mgr ctrl.Manager) error { + predicates := make([]predicate.Predicate, 0) + switch cluster.GetRelease().Name { + case cluster.SelfManagedRhods, cluster.ManagedRhods: + predicates = append(predicates, dashboardWatchPredicate(ComponentNameUpstream)) + default: + predicates = append(predicates, dashboardWatchPredicate(ComponentNameDownstream)) + } + + eh := handler.EnqueueRequestsFromMapFunc(watchDashboardResources) + ef := builder.WithPredicates(predicates...) + + forOpts := builder.WithPredicates(predicate.Or( + predicate.GenerationChangedPredicate{}, + predicate.LabelChangedPredicate{}, + predicate.AnnotationChangedPredicate{}, + )) + + _, err := odhrec.ComponentReconcilerFor[*componentsv1.Dashboard](mgr, &componentsv1.Dashboard{}, forOpts). + // operands + Watches(&appsv1.Deployment{}, eh, ef). + Watches(&appsv1.ReplicaSet{}, eh, ef). + Watches(&corev1.Namespace{}, eh, ef). + Watches(&corev1.ConfigMap{}, eh, ef). + Watches(&corev1.PersistentVolumeClaim{}, eh, ef). + Watches(&rbacv1.ClusterRoleBinding{}, eh, ef). + Watches(&rbacv1.ClusterRole{}, eh, ef). + Watches(&rbacv1.Role{}, eh, ef). + Watches(&rbacv1.RoleBinding{}, eh, ef). + Watches(&corev1.ServiceAccount{}, eh, ef). + // misc + WithComponentName(ComponentName). + // actions + WithActionFn(initialize). + WithActionFn(devFlags). + WithAction(render.New( + render.WithManifestsOptions( + kustomize.WithEngineRenderOpts( + kustomize.WithLabel(labels.ComponentName, ComponentName), + ), + ), + )). + WithActionFn(customizeResources). + WithAction(deploy.New( + deploy.WithMode(deploy.ModeSSA), + deploy.WithFieldOwner(ComponentName), + )). + WithAction(updatestatus.New( + updatestatus.WithSelectorLabel(labels.ComponentName, ComponentName), + )). + Build(ctx) + + if err != nil { + return fmt.Errorf("could not create the dashboard controller: %w", err) + } + + return nil +} + func watchDashboardResources(_ context.Context, a client.Object) []reconcile.Request { - if a.GetLabels()[labels.ODH.Component(ComponentNameUpstream)] == "true" || a.GetLabels()[labels.ODH.Component(ComponentNameDownstream)] == "true" { - return []reconcile.Request{{ - NamespacedName: types.NamespacedName{Name: componentsv1.DashboardInstanceName}, - }} + switch { + case a.GetLabels()[labels.ODH.Component(ComponentNameUpstream)] == "true": + return []reconcile.Request{{NamespacedName: dashboardInstanceID}} + case a.GetLabels()[labels.ODH.Component(ComponentNameDownstream)] == "true": + return []reconcile.Request{{NamespacedName: dashboardInstanceID}} + case a.GetLabels()[labels.ComponentName] == ComponentName: + return []reconcile.Request{{NamespacedName: dashboardInstanceID}} } return nil @@ -270,113 +246,3 @@ func dashboardWatchPredicate(componentName string) predicate.Funcs { }, } } - -// TODO added only to avoid name collision - -type Dashboard struct{} - -func (d Dashboard) updateKustomizeVariable(ctx context.Context, cli client.Client, platform cluster.Platform, dscispec *dsciv1.DSCInitializationSpec) (map[string]string, error) { - consoleLinkDomain, err := cluster.GetDomain(ctx, cli) - if err != nil { - return nil, fmt.Errorf("error getting console route URL %s : %w", consoleLinkDomain, err) - } - - return map[string]string{ - "admin_groups": adminGroups[platform], - "dashboard-url": baseConsoleURL[platform] + dscispec.ApplicationsNamespace + "." + consoleLinkDomain, - "section-title": sectionTitle[platform], - }, nil -} - -func (d Dashboard) initialize(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { - // Implement initialization logic - log := logf.FromContext(ctx).WithName(ComponentNameUpstream) - - componentName := ComponentNameUpstream - if rr.Platform == cluster.SelfManagedRhods || rr.Platform == cluster.ManagedRhods { - componentName = ComponentNameDownstream - } - - rr.Manifests = []odhtypes.ManifestInfo{{ - Path: manifestPaths[rr.Platform], - ContextDir: "", - SourcePath: "", - RenderOpts: []kustomize.RenderOptsFn{ - kustomize.WithLabel(labels.ODH.Component(componentName), "true"), - kustomize.WithLabel(labels.K8SCommon.PartOf, componentName), - }, - }} - - if err := deploy.ApplyParams(rr.Manifests[0].ManifestsPath(), imagesMap); err != nil { - log.Error(err, "failed to update image", "path", rr.Manifests[0].ManifestsPath()) - } - - extraParamsMap, err := d.updateKustomizeVariable(ctx, rr.Client, rr.Platform, &rr.DSCI.Spec) - if err != nil { - return errors.New("failed to set variable for extraParamsMap") - } - - if err := deploy.ApplyParams(rr.Manifests[0].ManifestsPath(), nil, extraParamsMap); err != nil { - return fmt.Errorf("failed to update params.env from %s : %w", rr.Manifests[0].ManifestsPath(), err) - } - - return nil -} - -func (d Dashboard) devFlags(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { - dashboard, ok := rr.Instance.(*componentsv1.Dashboard) - if !ok { - return fmt.Errorf("resource instance %v is not a componentsv1.Dashboard)", rr.Instance) - } - - if dashboard.Spec.DevFlags == nil { - return nil - } - // Implement devflags support logic - // If dev flags are set, update default manifests path - if len(dashboard.Spec.DevFlags.Manifests) != 0 { - manifestConfig := dashboard.Spec.DevFlags.Manifests[0] - if err := deploy.DownloadManifests(ctx, ComponentNameUpstream, manifestConfig); err != nil { - return err - } - if manifestConfig.SourcePath != "" { - rr.Manifests[0].Path = deploy.DefaultManifestPath - rr.Manifests[0].ContextDir = ComponentNameUpstream - rr.Manifests[0].SourcePath = manifestConfig.SourcePath - } - } - - return nil -} - -func (d Dashboard) customizeResources(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { - switch rr.Platform { - case cluster.SelfManagedRhods, cluster.ManagedRhods: - if err := cluster.UpdatePodSecurityRolebinding(ctx, rr.Client, rr.DSCI.Spec.ApplicationsNamespace, "rhods-dashboard"); err != nil { - return fmt.Errorf("failed to update PodSecurityRolebinding for rhods-dashboard: %w", err) - } - - err := rr.AddResource(&corev1.Secret{ - TypeMeta: metav1.TypeMeta{ - APIVersion: corev1.SchemeGroupVersion.String(), - Kind: "Secret", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "anaconda-ce-access", - Namespace: rr.DSCI.Spec.ApplicationsNamespace, - }, - Type: corev1.SecretTypeOpaque, - }) - - if err != nil { - return fmt.Errorf("failed to create access-secret for anaconda: %w", err) - } - - default: - if err := cluster.UpdatePodSecurityRolebinding(ctx, rr.Client, rr.DSCI.Spec.ApplicationsNamespace, "odh-dashboard"); err != nil { - return fmt.Errorf("failed to update PodSecurityRolebinding for odh-dashboard: %w", err) - } - } - - return nil -} diff --git a/controllers/components/dashboard/dashboard_support.go b/controllers/components/dashboard/dashboard_support.go new file mode 100644 index 00000000000..772cecbac03 --- /dev/null +++ b/controllers/components/dashboard/dashboard_support.go @@ -0,0 +1,62 @@ +package dashboard + +import ( + "context" + "fmt" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" + dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" + dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" + odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/manifests/kustomize" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels" +) + +func CreateDashboardInstance(dsc *dscv1.DataScienceCluster) *componentsv1.Dashboard { + return &componentsv1.Dashboard{ + TypeMeta: metav1.TypeMeta{ + Kind: "Dashboard", + APIVersion: "components.opendatahub.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: componentsv1.DashboardInstanceName, + }, + Spec: componentsv1.DashboardSpec{ + DSCDashboard: dsc.Spec.Components.Dashboard, + }, + } +} + +func defaultManifestInfo(p cluster.Platform) odhtypes.ManifestInfo { + componentName := ComponentNameUpstream + if p == cluster.SelfManagedRhods || p == cluster.ManagedRhods { + componentName = ComponentNameDownstream + } + + return odhtypes.ManifestInfo{ + Path: manifestPaths[p], + ContextDir: "", + SourcePath: "", + RenderOpts: []kustomize.RenderOptsFn{ + kustomize.WithLabel(labels.ODH.Component(componentName), "true"), + kustomize.WithLabel(labels.K8SCommon.PartOf, componentName), + }, + } +} + +func updateKustomizeVariable(ctx context.Context, cli client.Client, platform cluster.Platform, dscispec *dsciv1.DSCInitializationSpec) (map[string]string, error) { + consoleLinkDomain, err := cluster.GetDomain(ctx, cli) + if err != nil { + return nil, fmt.Errorf("error getting console route URL %s : %w", consoleLinkDomain, err) + } + + return map[string]string{ + "admin_groups": adminGroups[platform], + "dashboard-url": baseConsoleURL[platform] + dscispec.ApplicationsNamespace + "." + consoleLinkDomain, + "section-title": sectionTitle[platform], + }, nil +} diff --git a/pkg/controller/actions/action_fn.go b/pkg/controller/actions/action_fn.go deleted file mode 100644 index 8ea99c1a91f..00000000000 --- a/pkg/controller/actions/action_fn.go +++ /dev/null @@ -1,35 +0,0 @@ -package actions - -import ( - "context" - "reflect" - "runtime" - - "sigs.k8s.io/controller-runtime/pkg/log" - - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" -) - -type ActionFn func(ctx context.Context, rr *types.ReconciliationRequest) error - -type WrapperAction struct { - BaseAction - fn ActionFn -} - -func (r *WrapperAction) Execute(ctx context.Context, rr *types.ReconciliationRequest) error { - return r.fn(ctx, rr) -} - -func NewActionFn(ctx context.Context, fn ActionFn) *WrapperAction { - name := runtime.FuncForPC(reflect.ValueOf(fn).Pointer()).Name() - - action := WrapperAction{ - BaseAction: BaseAction{ - Log: log.FromContext(ctx).WithName(ActionGroup).WithName(name), - }, - fn: fn, - } - - return &action -} diff --git a/pkg/controller/actions/actions.go b/pkg/controller/actions/actions.go index bf1739e9f0b..f80e96bf9a5 100644 --- a/pkg/controller/actions/actions.go +++ b/pkg/controller/actions/actions.go @@ -3,8 +3,6 @@ package actions import ( "context" - "github.com/go-logr/logr" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" ) @@ -19,7 +17,3 @@ const ( type Action interface { Execute(ctx context.Context, rr *types.ReconciliationRequest) error } - -type BaseAction struct { - Log logr.Logger -} diff --git a/pkg/controller/actions/action_delete_resources.go b/pkg/controller/actions/deleteresource/action_delete_resources.go similarity index 50% rename from pkg/controller/actions/action_delete_resources.go rename to pkg/controller/actions/deleteresource/action_delete_resources.go index aa21da284e9..8e8d80e0bf3 100644 --- a/pkg/controller/actions/action_delete_resources.go +++ b/pkg/controller/actions/deleteresource/action_delete_resources.go @@ -1,47 +1,45 @@ -package actions +package deleteresource import ( "context" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/log" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" ) const ( - DeleteResourcesActionName = "delete-resources" + ActionName = "delete-resources" ) -type DeleteResourcesAction struct { - BaseAction +type Action struct { types []client.Object labels map[string]string } -type DeleteResourcesActionOpts func(*DeleteResourcesAction) +type ActionOpts func(*Action) -func WithDeleteResourcesTypes(values ...client.Object) DeleteResourcesActionOpts { - return func(action *DeleteResourcesAction) { +func WithDeleteResourcesTypes(values ...client.Object) ActionOpts { + return func(action *Action) { action.types = append(action.types, values...) } } -func WithDeleteResourcesLabel(k string, v string) DeleteResourcesActionOpts { - return func(action *DeleteResourcesAction) { +func WithDeleteResourcesLabel(k string, v string) ActionOpts { + return func(action *Action) { action.labels[k] = v } } -func WithDeleteResourcesLabels(values map[string]string) DeleteResourcesActionOpts { - return func(action *DeleteResourcesAction) { +func WithDeleteResourcesLabels(values map[string]string) ActionOpts { + return func(action *Action) { for k, v := range values { action.labels[k] = v } } } -func (r *DeleteResourcesAction) Execute(ctx context.Context, rr *types.ReconciliationRequest) error { +func (r *Action) Execute(ctx context.Context, rr *types.ReconciliationRequest) error { for i := range r.types { opts := make([]client.DeleteAllOfOption, 0) @@ -67,11 +65,8 @@ func (r *DeleteResourcesAction) Execute(ctx context.Context, rr *types.Reconcili return nil } -func NewDeleteResourcesAction(ctx context.Context, opts ...DeleteResourcesActionOpts) *DeleteResourcesAction { - action := DeleteResourcesAction{ - BaseAction: BaseAction{ - Log: log.FromContext(ctx).WithName(ActionGroup).WithName(DeleteResourcesActionName), - }, +func New(opts ...ActionOpts) *Action { + action := Action{ types: make([]client.Object, 0), labels: map[string]string{}, } diff --git a/pkg/controller/actions/action_delete_resources_test.go b/pkg/controller/actions/deleteresource/action_delete_resources_test.go similarity index 83% rename from pkg/controller/actions/action_delete_resources_test.go rename to pkg/controller/actions/deleteresource/action_delete_resources_test.go index 7804f2c4ade..864b5e3e830 100644 --- a/pkg/controller/actions/action_delete_resources_test.go +++ b/pkg/controller/actions/deleteresource/action_delete_resources_test.go @@ -1,9 +1,11 @@ -package actions_test +package deleteresource_test import ( "context" "testing" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deleteresource" + "github.com/onsi/gomega/gstruct" "github.com/rs/xid" appsv1 "k8s.io/api/apps/v1" @@ -13,9 +15,10 @@ import ( dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deleteresource" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/fakeclient" . "github.com/onsi/gomega" ) @@ -26,7 +29,7 @@ func TestDeleteResourcesAction(t *testing.T) { ctx := context.Background() ns := xid.New().String() - client, err := NewFakeClient( + client, err := fakeclient.New( ctx, &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ @@ -58,10 +61,9 @@ func TestDeleteResourcesAction(t *testing.T) { g.Expect(err).ShouldNot(HaveOccurred()) - action := actions.NewDeleteResourcesAction( - ctx, - actions.WithDeleteResourcesTypes(&appsv1.Deployment{}), - actions.WithDeleteResourcesLabel(labels.K8SCommon.PartOf, "foo")) + action := deleteresource.New( + deleteresource.WithDeleteResourcesTypes(&appsv1.Deployment{}), + deleteresource.WithDeleteResourcesLabel(labels.K8SCommon.PartOf, "foo")) err = action.Execute(ctx, &types.ReconciliationRequest{ Client: client, diff --git a/pkg/controller/actions/action_deploy.go b/pkg/controller/actions/deploy/action_deploy.go similarity index 60% rename from pkg/controller/actions/action_deploy.go rename to pkg/controller/actions/deploy/action_deploy.go index 7b8eaad825d..b4efd77f872 100644 --- a/pkg/controller/actions/action_deploy.go +++ b/pkg/controller/actions/deploy/action_deploy.go @@ -1,4 +1,4 @@ -package actions +package deploy import ( "context" @@ -10,7 +10,6 @@ import ( "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/log" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" odhClient "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/client" @@ -18,41 +17,38 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" ) -type DeployMode int +type Mode int const ( - DeployModePatch DeployMode = iota - DeployModeSSA + ModePatch Mode = iota + ModeSSA ) const ( - DeployActionName = "deploy" + ActionName = "deploy" ) -// DeployAction deploys the resources that are included in the ReconciliationRequest using -// the same create or patch machinery implemented as part of deploy.DeployManifestsFromPath -// -// TODO: we should support full Server-Side Apply. -type DeployAction struct { - BaseAction +// Action deploys the resources that are included in the ReconciliationRequest using +// the same create or patch machinery implemented as part of deploy.DeployManifestsFromPath. +type Action struct { fieldOwner string - deployMode DeployMode + deployMode Mode } -type DeployActionOpts func(*DeployAction) +type ActionOpts func(*Action) -func WithDeployedResourceFieldOwner(value string) DeployActionOpts { - return func(action *DeployAction) { +func WithFieldOwner(value string) ActionOpts { + return func(action *Action) { action.fieldOwner = value } } -func WithDeployedMode(value DeployMode) DeployActionOpts { - return func(action *DeployAction) { +func WithMode(value Mode) ActionOpts { + return func(action *Action) { action.deployMode = value } } -func (r *DeployAction) Execute(ctx context.Context, rr *odhTypes.ReconciliationRequest) error { +func (r *Action) Execute(ctx context.Context, rr *odhTypes.ReconciliationRequest) error { for i := range rr.Resources { obj := rr.Resources[i] @@ -72,6 +68,17 @@ func (r *DeployAction) Execute(ctx context.Context, rr *odhTypes.ReconciliationR } } + old, err := r.lookup(ctx, rr.Client, obj) + if err != nil { + return fmt.Errorf("failed to lookup object %s/%s: %w", obj.GetNamespace(), obj.GetName(), err) + } + + // if the resource is found and not managed by the operator, remove it from + // the deployed resources + if old != nil && old.GetAnnotations()[annotations.ManagedByODHOperator] == "false" { + continue + } + switch obj.GroupVersionKind() { case gvk.OdhDashboardConfig: // The OdhDashboardConfig should only be created once, or @@ -84,10 +91,10 @@ func (r *DeployAction) Execute(ctx context.Context, rr *odhTypes.ReconciliationR var err error switch r.deployMode { - case DeployModePatch: - err = r.apply(ctx, rr.Client, obj) - case DeployModeSSA: - err = r.patch(ctx, rr.Client, obj) + case ModePatch: + err = r.apply(ctx, rr.Client, obj, old) + case ModeSSA: + err = r.patch(ctx, rr.Client, obj, old) default: err = fmt.Errorf("unsupported deploy mode %d", r.deployMode) } @@ -101,35 +108,34 @@ func (r *DeployAction) Execute(ctx context.Context, rr *odhTypes.ReconciliationR return nil } -func (r *DeployAction) patch(ctx context.Context, c *odhClient.Client, obj unstructured.Unstructured) error { +func (r *Action) lookup(ctx context.Context, c *odhClient.Client, obj unstructured.Unstructured) (*unstructured.Unstructured, error) { found := unstructured.Unstructured{} found.SetGroupVersionKind(obj.GroupVersionKind()) - foundErr := c.Get(ctx, client.ObjectKeyFromObject(&obj), &found) - if foundErr != nil && !k8serr.IsNotFound(foundErr) { - return fmt.Errorf("failed to retrieve object %s/%s: %w", obj.GetNamespace(), obj.GetName(), foundErr) + // TODO: use PartialObjectMetadata for resources where it make sense + err := c.Get(ctx, client.ObjectKeyFromObject(&obj), &found) + switch { + case err != nil && !k8serr.IsNotFound(err): + return nil, err + case err != nil && k8serr.IsNotFound(err): + return nil, nil + default: + return &found, nil } +} +func (r *Action) patch(ctx context.Context, c *odhClient.Client, obj unstructured.Unstructured, old *unstructured.Unstructured) error { switch obj.GroupVersionKind() { case gvk.Deployment: - found := unstructured.Unstructured{} - found.SetGroupVersionKind(obj.GroupVersionKind()) - - err := c.Get(ctx, client.ObjectKeyFromObject(&obj), &found) - if err != nil && !k8serr.IsNotFound(err) { - return fmt.Errorf("failed to retrieve Deployment %s/%s: %w", obj.GetNamespace(), obj.GetName(), err) - } - - // If the resource does not exist, then don't apply the allow list - // as the resource must be created - if k8serr.IsNotFound(err) { + // For deployments, we allow the user to change some parameters, such as + // container resources and replicas except: + // - If the resource does not exist (the resource must be created) + // - If the resource is forcefully marked as managed by the operator via + // annotations (i.e. to bring it back to the default values) + if old == nil { break } - - // If the resource is forcefully marked as managed by the operator, don't apply - // the allow list as the user is explicitly forcing the resource to be managed - // i.e. to restore default values - if found.GetAnnotations()[annotations.ManagedByODHOperator] == "true" { + if old.GetAnnotations()[annotations.ManagedByODHOperator] == "true" { break } @@ -147,7 +153,7 @@ func (r *DeployAction) patch(ctx context.Context, c *odhClient.Client, obj unstr break } - if k8serr.IsNotFound(foundErr) { + if old == nil { err := c.Create(ctx, &obj) if err != nil { return fmt.Errorf("failed to create object %s/%s: %w", obj.GetNamespace(), obj.GetName(), err) @@ -160,7 +166,7 @@ func (r *DeployAction) patch(ctx context.Context, c *odhClient.Client, obj unstr err = c.Patch( ctx, - &found, + old, client.RawPatch(types.ApplyPatchType, data), client.ForceOwnership, client.FieldOwner(r.fieldOwner), @@ -174,23 +180,18 @@ func (r *DeployAction) patch(ctx context.Context, c *odhClient.Client, obj unstr return nil } -func (r *DeployAction) apply(ctx context.Context, c *odhClient.Client, obj unstructured.Unstructured) error { +func (r *Action) apply(ctx context.Context, c *odhClient.Client, obj unstructured.Unstructured, old *unstructured.Unstructured) error { switch obj.GroupVersionKind() { case gvk.Deployment: - found := unstructured.Unstructured{} - found.SetGroupVersionKind(obj.GroupVersionKind()) - - err := c.Get(ctx, client.ObjectKeyFromObject(&obj), &found) - if err != nil && !k8serr.IsNotFound(err) { - return fmt.Errorf("failed to retrieve Deployment %s/%s: %w", obj.GetNamespace(), obj.GetName(), err) - } - // For deployments, we allow the user to change some parameters, such as // container resources and replicas except: // - If the resource does not exist (the resource must be created) // - If the resource is forcefully marked as managed by the operator via // annotations (i.e. to bring it back to the default values) - if k8serr.IsNotFound(err) || found.GetAnnotations()[annotations.ManagedByODHOperator] == "true" { + if old == nil { + break + } + if old.GetAnnotations()[annotations.ManagedByODHOperator] == "true" { break } @@ -201,7 +202,7 @@ func (r *DeployAction) apply(ctx context.Context, c *odhClient.Client, obj unstr // Ideally deployed resources should be configured only via the platform API // // [1] https://kubernetes.io/docs/reference/using-api/server-side-apply/#conflicts - if err := MergeDeployments(&found, &obj); err != nil { + if err := MergeDeployments(old, &obj); err != nil { return fmt.Errorf("failed to merge Deployment %s/%s: %w", obj.GetNamespace(), obj.GetName(), err) } default: @@ -223,12 +224,9 @@ func (r *DeployAction) apply(ctx context.Context, c *odhClient.Client, obj unstr return nil } -func NewDeployAction(ctx context.Context, opts ...DeployActionOpts) *DeployAction { - action := DeployAction{ - BaseAction: BaseAction{ - Log: log.FromContext(ctx).WithName(ActionGroup).WithName(DeployActionName), - }, - deployMode: DeployModeSSA, +func New(opts ...ActionOpts) *Action { + action := Action{ + deployMode: ModeSSA, } for _, opt := range opts { diff --git a/pkg/controller/actions/action_deploy_merge_deployment.go b/pkg/controller/actions/deploy/action_deploy_merge_deployment.go similarity index 99% rename from pkg/controller/actions/action_deploy_merge_deployment.go rename to pkg/controller/actions/deploy/action_deploy_merge_deployment.go index b1a5d5d0e18..79ba467ff4f 100644 --- a/pkg/controller/actions/action_deploy_merge_deployment.go +++ b/pkg/controller/actions/deploy/action_deploy_merge_deployment.go @@ -1,4 +1,4 @@ -package actions +package deploy import ( "errors" diff --git a/pkg/controller/actions/action_deploy_merge_deployment_test.go b/pkg/controller/actions/deploy/action_deploy_merge_deployment_test.go similarity index 96% rename from pkg/controller/actions/action_deploy_merge_deployment_test.go rename to pkg/controller/actions/deploy/action_deploy_merge_deployment_test.go index ee58978d173..dbd68768ebe 100644 --- a/pkg/controller/actions/action_deploy_merge_deployment_test.go +++ b/pkg/controller/actions/deploy/action_deploy_merge_deployment_test.go @@ -1,4 +1,4 @@ -package actions_test +package deploy_test import ( "testing" @@ -10,7 +10,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/utils/ptr" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/matchers/jq" . "github.com/onsi/gomega" @@ -75,7 +75,7 @@ func TestMergeDeploymentsOverride(t *testing.T) { src := unstructured.Unstructured{Object: source} trg := unstructured.Unstructured{Object: target} - err = actions.MergeDeployments(&src, &trg) + err = deploy.MergeDeployments(&src, &trg) g.Expect(err).ShouldNot(HaveOccurred()) g.Expect(trg).Should(And( @@ -135,7 +135,7 @@ func TestMergeDeploymentsRemove(t *testing.T) { src := unstructured.Unstructured{Object: source} trg := unstructured.Unstructured{Object: target} - err = actions.MergeDeployments(&src, &trg) + err = deploy.MergeDeployments(&src, &trg) g.Expect(err).ShouldNot(HaveOccurred()) g.Expect(trg).Should(And( diff --git a/pkg/controller/actions/action_deploy_remove_deployment_resources.go b/pkg/controller/actions/deploy/action_deploy_remove_deployment_resources.go similarity index 98% rename from pkg/controller/actions/action_deploy_remove_deployment_resources.go rename to pkg/controller/actions/deploy/action_deploy_remove_deployment_resources.go index 41c0c070010..44ca5e8e5c5 100644 --- a/pkg/controller/actions/action_deploy_remove_deployment_resources.go +++ b/pkg/controller/actions/deploy/action_deploy_remove_deployment_resources.go @@ -1,4 +1,4 @@ -package actions +package deploy import ( "errors" diff --git a/pkg/controller/actions/action_deploy_remove_deployment_resources_test.go b/pkg/controller/actions/deploy/action_deploy_remove_deployment_resources_test.go similarity index 90% rename from pkg/controller/actions/action_deploy_remove_deployment_resources_test.go rename to pkg/controller/actions/deploy/action_deploy_remove_deployment_resources_test.go index a9ff9ead177..170bf850a7b 100644 --- a/pkg/controller/actions/action_deploy_remove_deployment_resources_test.go +++ b/pkg/controller/actions/deploy/action_deploy_remove_deployment_resources_test.go @@ -1,10 +1,8 @@ -package actions_test +package deploy_test import ( "testing" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions" - appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -12,7 +10,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/utils/ptr" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/matchers/jq" . "github.com/onsi/gomega" @@ -49,7 +47,7 @@ func TestMRemoveDeploymentsResources(t *testing.T) { src := unstructured.Unstructured{Object: source} - err = actions.RemoveDeploymentsResources(&src) + err = deploy.RemoveDeploymentsResources(&src) g.Expect(err).ShouldNot(HaveOccurred()) g.Expect(src).Should(And( diff --git a/pkg/controller/actions/fn/action_fn.go b/pkg/controller/actions/fn/action_fn.go new file mode 100644 index 00000000000..02fde7be3e9 --- /dev/null +++ b/pkg/controller/actions/fn/action_fn.go @@ -0,0 +1,25 @@ +package fn + +import ( + "context" + + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" +) + +type Fn func(ctx context.Context, rr *types.ReconciliationRequest) error + +type WrapperAction struct { + fn Fn +} + +func (r *WrapperAction) Execute(ctx context.Context, rr *types.ReconciliationRequest) error { + return r.fn(ctx, rr) +} + +func New(fn Fn) *WrapperAction { + action := WrapperAction{ + fn: fn, + } + + return &action +} diff --git a/pkg/controller/actions/action_render_manifests.go b/pkg/controller/actions/render/action_render_manifests.go similarity index 54% rename from pkg/controller/actions/action_render_manifests.go rename to pkg/controller/actions/render/action_render_manifests.go index 42c9a2224e0..b66d1de74ca 100644 --- a/pkg/controller/actions/action_render_manifests.go +++ b/pkg/controller/actions/render/action_render_manifests.go @@ -1,34 +1,30 @@ -package actions +package render import ( "context" - "sigs.k8s.io/controller-runtime/pkg/log" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/manifests/kustomize" ) const ( - RenderManifestsActionName = "render-manifests" + ActionName = "render-manifests" ) -type RenderManifestsAction struct { - BaseAction - +type Action struct { keOpts []kustomize.EngineOptsFn ke *kustomize.Engine } -type RenderManifestsActionOpts func(*RenderManifestsAction) +type ActionOpts func(*Action) -func WithRenderManifestsOptions(values ...kustomize.EngineOptsFn) RenderManifestsActionOpts { - return func(action *RenderManifestsAction) { +func WithManifestsOptions(values ...kustomize.EngineOptsFn) ActionOpts { + return func(action *Action) { action.keOpts = append(action.keOpts, values...) } } -func (r *RenderManifestsAction) Execute(ctx context.Context, rr *types.ReconciliationRequest) error { +func (r *Action) Execute(_ context.Context, rr *types.ReconciliationRequest) error { for i := range rr.Manifests { opts := make([]kustomize.RenderOptsFn, 0, len(rr.Manifests[i].RenderOpts)+3) opts = append(opts, kustomize.WithNamespace(rr.DSCI.Spec.ApplicationsNamespace)) @@ -49,12 +45,8 @@ func (r *RenderManifestsAction) Execute(ctx context.Context, rr *types.Reconcili return nil } -func NewRenderManifestsAction(ctx context.Context, opts ...RenderManifestsActionOpts) *RenderManifestsAction { - action := RenderManifestsAction{ - BaseAction: BaseAction{ - Log: log.FromContext(ctx).WithName(ActionGroup).WithName(RenderManifestsActionName), - }, - } +func New(opts ...ActionOpts) *Action { + action := Action{} for _, opt := range opts { opt(&action) diff --git a/pkg/controller/actions/action_render_manifests_test.go b/pkg/controller/actions/render/action_render_manifests_test.go similarity index 68% rename from pkg/controller/actions/action_render_manifests_test.go rename to pkg/controller/actions/render/action_render_manifests_test.go index 09217b454b8..d689baa33c6 100644 --- a/pkg/controller/actions/action_render_manifests_test.go +++ b/pkg/controller/actions/render/action_render_manifests_test.go @@ -1,4 +1,4 @@ -package actions_test +package render_test import ( "context" @@ -6,18 +6,15 @@ import ( "testing" "github.com/rs/xid" - appsv1 "k8s.io/api/apps/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/ptr" "sigs.k8s.io/kustomize/kyaml/filesys" dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/manifests/kustomize" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/fakeclient" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/matchers/jq" . "github.com/onsi/gomega" @@ -111,55 +108,11 @@ func TestRenderResourcesAction(t *testing.T) { _ = fs.WriteFile(path.Join(id, "test-resources-deployment-unmanaged.yaml"), []byte(testRenderResourcesUnmanaged)) _ = fs.WriteFile(path.Join(id, "test-resources-deployment-forced.yaml"), []byte(testRenderResourcesForced)) - client, err := NewFakeClient( - ctx, - &appsv1.Deployment{ - TypeMeta: metav1.TypeMeta{ - APIVersion: gvk.Deployment.GroupVersion().String(), - Kind: gvk.Deployment.Kind, - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-deployment-managed", - Namespace: ns, - Labels: map[string]string{}, - }, - }, - &appsv1.Deployment{ - TypeMeta: metav1.TypeMeta{ - APIVersion: gvk.Deployment.GroupVersion().String(), - Kind: gvk.Deployment.Kind, - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-deployment-unmanaged", - Namespace: ns, - Annotations: map[string]string{ - "opendatahub.io/managed": "false", - }, - }, - }, - &appsv1.Deployment{ - TypeMeta: metav1.TypeMeta{ - APIVersion: gvk.Deployment.GroupVersion().String(), - Kind: gvk.Deployment.Kind, - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-deployment-forced", - Namespace: ns, - Annotations: map[string]string{ - "opendatahub.io/managed": "true", - }, - }, - Spec: appsv1.DeploymentSpec{ - Replicas: ptr.To[int32](1), - }, - }, - ) - + client, err := fakeclient.New(ctx) g.Expect(err).ShouldNot(HaveOccurred()) - action := actions.NewRenderManifestsAction( - ctx, - actions.WithRenderManifestsOptions( + action := render.New( + render.WithManifestsOptions( kustomize.WithEngineFS(fs), kustomize.WithEngineRenderOpts( kustomize.WithAnnotations(map[string]string{ @@ -190,10 +143,8 @@ func TestRenderResourcesAction(t *testing.T) { err = action.Execute(ctx, &rr) g.Expect(err).ShouldNot(HaveOccurred()) - - // common customizations g.Expect(rr.Resources).Should(And( - HaveLen(3), + HaveLen(4), HaveEach(And( jq.Match(`.metadata.namespace == "%s"`, ns), jq.Match(`.metadata.labels."component.opendatahub.io/name" == "%s"`, "foo"), @@ -202,21 +153,4 @@ func TestRenderResourcesAction(t *testing.T) { jq.Match(`.metadata.annotations."platform.opendatahub.io/type" == "%s"`, "managed"), )), )) - - // config map - g.Expect(rr.Resources[0]).Should(And( - jq.Match(`.metadata.name == "%s"`, "test-cm"), - )) - - // deployment managed - g.Expect(rr.Resources[1]).Should(And( - jq.Match(`.metadata.name == "%s"`, "test-deployment-managed"), - jq.Match(`.spec.template.spec.containers[0] | has("resources") | not`), - )) - - // deployment forced - g.Expect(rr.Resources[2]).Should(And( - jq.Match(`.metadata.name == "%s"`, "test-deployment-forced"), - jq.Match(`.spec.replicas == 3`), - )) } diff --git a/pkg/controller/actions/action_update_status.go b/pkg/controller/actions/updatestatus/action_update_status.go similarity index 70% rename from pkg/controller/actions/action_update_status.go rename to pkg/controller/actions/updatestatus/action_update_status.go index c1805eff74d..07e93cdcf59 100644 --- a/pkg/controller/actions/action_update_status.go +++ b/pkg/controller/actions/updatestatus/action_update_status.go @@ -1,4 +1,4 @@ -package actions +package updatestatus import ( "context" @@ -8,40 +8,38 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/log" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" ) const ( - UpdateStatusActionName = "update-status" + ActionName = "update-status" DeploymentsNotReadyReason = "DeploymentsNotReady" ReadyReason = "Ready" ) -type UpdateStatusAction struct { - BaseAction +type Action struct { labels map[string]string } -type UpdateStatusActionOpts func(*UpdateStatusAction) +type ActionOpts func(*Action) -func WithUpdateStatusLabel(k string, v string) UpdateStatusActionOpts { - return func(action *UpdateStatusAction) { +func WithSelectorLabel(k string, v string) ActionOpts { + return func(action *Action) { action.labels[k] = v } } -func WithUpdateStatusLabels(values map[string]string) UpdateStatusActionOpts { - return func(action *UpdateStatusAction) { +func WithSelectorLabels(values map[string]string) ActionOpts { + return func(action *Action) { for k, v := range values { action.labels[k] = v } } } -func (a *UpdateStatusAction) Execute(ctx context.Context, rr *types.ReconciliationRequest) error { +func (a *Action) Execute(ctx context.Context, rr *types.ReconciliationRequest) error { if len(a.labels) == 0 { return nil } @@ -95,11 +93,8 @@ func (a *UpdateStatusAction) Execute(ctx context.Context, rr *types.Reconciliati return nil } -func NewUpdateStatusAction(ctx context.Context, opts ...UpdateStatusActionOpts) *UpdateStatusAction { - action := UpdateStatusAction{ - BaseAction: BaseAction{ - Log: log.FromContext(ctx).WithName(ActionGroup).WithName(UpdateStatusActionName), - }, +func New(opts ...ActionOpts) *Action { + action := Action{ labels: map[string]string{}, } diff --git a/pkg/controller/actions/action_update_status_test.go b/pkg/controller/actions/updatestatus/action_update_status_test.go similarity index 85% rename from pkg/controller/actions/action_update_status_test.go rename to pkg/controller/actions/updatestatus/action_update_status_test.go index 7627415ba47..40680e5d1bf 100644 --- a/pkg/controller/actions/action_update_status_test.go +++ b/pkg/controller/actions/updatestatus/action_update_status_test.go @@ -1,5 +1,5 @@ //nolint:dupl -package actions_test +package updatestatus_test import ( "context" @@ -16,9 +16,11 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/fakeclient" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/matchers" . "github.com/onsi/gomega" ) @@ -30,7 +32,7 @@ func TestUpdateStatusActionNotReady(t *testing.T) { ctx := context.Background() ns := xid.New().String() - client, err := NewFakeClient( + client, err := fakeclient.New( ctx, &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ @@ -70,9 +72,8 @@ func TestUpdateStatusActionNotReady(t *testing.T) { g.Expect(err).ShouldNot(HaveOccurred()) - action := actions.NewUpdateStatusAction( - ctx, - actions.WithUpdateStatusLabel(labels.K8SCommon.PartOf, "foo")) + action := updatestatus.New( + updatestatus.WithSelectorLabel(labels.K8SCommon.PartOf, "foo")) rr := types.ReconciliationRequest{ Client: client, @@ -88,10 +89,10 @@ func TestUpdateStatusActionNotReady(t *testing.T) { g.Expect(err).ShouldNot(HaveOccurred()) g.Expect(rr.Instance).Should( WithTransform( - ExtractStatusCondition(status.ConditionTypeReady), + matchers.ExtractStatusCondition(status.ConditionTypeReady), gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{ "Status": Equal(metav1.ConditionFalse), - "Reason": Equal(actions.DeploymentsNotReadyReason), + "Reason": Equal(updatestatus.DeploymentsNotReadyReason), }), ), ) @@ -103,7 +104,7 @@ func TestUpdateStatusActionReady(t *testing.T) { ctx := context.Background() ns := xid.New().String() - client, err := NewFakeClient( + client, err := fakeclient.New( ctx, &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ @@ -143,9 +144,8 @@ func TestUpdateStatusActionReady(t *testing.T) { g.Expect(err).ShouldNot(HaveOccurred()) - action := actions.NewUpdateStatusAction( - ctx, - actions.WithUpdateStatusLabel(labels.K8SCommon.PartOf, "foo")) + action := updatestatus.New( + updatestatus.WithSelectorLabel(labels.K8SCommon.PartOf, "foo")) rr := types.ReconciliationRequest{ Client: client, @@ -161,10 +161,10 @@ func TestUpdateStatusActionReady(t *testing.T) { g.Expect(err).ShouldNot(HaveOccurred()) g.Expect(rr.Instance).Should( WithTransform( - ExtractStatusCondition(status.ConditionTypeReady), + matchers.ExtractStatusCondition(status.ConditionTypeReady), gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{ "Status": Equal(metav1.ConditionTrue), - "Reason": Equal(actions.ReadyReason), + "Reason": Equal(updatestatus.ReadyReason), }), ), ) diff --git a/pkg/controller/reconciler/component_reconciler.go b/pkg/controller/reconciler/component_reconciler.go index d7663b980c5..154891f689e 100644 --- a/pkg/controller/reconciler/component_reconciler.go +++ b/pkg/controller/reconciler/component_reconciler.go @@ -1,4 +1,4 @@ -package types +package reconciler import ( "context" diff --git a/pkg/controller/reconciler/component_reconciler_support.go b/pkg/controller/reconciler/component_reconciler_support.go new file mode 100644 index 00000000000..44eeb285714 --- /dev/null +++ b/pkg/controller/reconciler/component_reconciler_support.go @@ -0,0 +1,133 @@ +package reconciler + +import ( + "context" + "fmt" + "strings" + + "golang.org/x/exp/slices" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/predicate" + + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/fn" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" +) + +type forInput struct { + object client.Object + options []builder.ForOption +} + +type watchInput struct { + object client.Object + eventHandler handler.EventHandler + options []builder.WatchesOption +} + +type ComponentReconcilerBuilder[T types.ResourceObject] struct { + mgr ctrl.Manager + input forInput + watches []watchInput + predicates []predicate.Predicate + componentName string + actions []actions.Action + finalizers []actions.Action +} + +func ComponentReconcilerFor[T types.ResourceObject](mgr ctrl.Manager, object client.Object, opts ...builder.ForOption) *ComponentReconcilerBuilder[T] { + crb := ComponentReconcilerBuilder[T]{ + mgr: mgr, + input: forInput{ + object: object, + options: slices.Clone(opts), + }, + } + + return &crb +} + +func (b *ComponentReconcilerBuilder[T]) WithComponentName(componentName string) *ComponentReconcilerBuilder[T] { + b.componentName = componentName + return b +} + +func (b *ComponentReconcilerBuilder[T]) WithAction(value actions.Action) *ComponentReconcilerBuilder[T] { + b.actions = append(b.actions, value) + return b +} + +func (b *ComponentReconcilerBuilder[T]) WithActionFn(value fn.Fn) *ComponentReconcilerBuilder[T] { + b.actions = append(b.actions, fn.New(value)) + return b +} + +func (b *ComponentReconcilerBuilder[T]) WithFinalizer(value actions.Action) *ComponentReconcilerBuilder[T] { + b.finalizers = append(b.finalizers, value) + return b +} + +func (b *ComponentReconcilerBuilder[T]) WithFinalizerf(value fn.Fn) *ComponentReconcilerBuilder[T] { + b.finalizers = append(b.finalizers, fn.New(value)) + return b +} + +func (b *ComponentReconcilerBuilder[T]) Watches(object client.Object, eventHandler handler.EventHandler, opts ...builder.WatchesOption) *ComponentReconcilerBuilder[T] { + b.watches = append(b.watches, watchInput{ + object: object, + eventHandler: eventHandler, + options: slices.Clone(opts), + }) + + return b +} + +func (b *ComponentReconcilerBuilder[T]) WatchesM(object client.Object, fn handler.MapFunc, opts ...builder.WatchesOption) *ComponentReconcilerBuilder[T] { + b.watches = append(b.watches, watchInput{ + object: object, + eventHandler: handler.EnqueueRequestsFromMapFunc(fn), + options: slices.Clone(opts), + }) + + return b +} + +func (b *ComponentReconcilerBuilder[T]) WithEventFilter(p predicate.Predicate) *ComponentReconcilerBuilder[T] { + b.predicates = append(b.predicates, p) + return b +} + +func (b *ComponentReconcilerBuilder[T]) Build(ctx context.Context) (*ComponentReconciler[T], error) { + name := b.componentName + if name == "" { + name = b.input.object.GetObjectKind().GroupVersionKind().Kind + name = strings.ToLower(name) + } + + r, err := NewComponentReconciler[T](ctx, b.mgr, name) + if err != nil { + return nil, fmt.Errorf("failed to create reconciler for component %s: %w", b.componentName, err) + } + + c := ctrl.NewControllerManagedBy(b.mgr) + c = c.For(b.input.object, b.input.options...) + + for i := range b.watches { + c = c.Watches(b.watches[i].object, b.watches[i].eventHandler, b.watches[i].options...) + } + for i := range b.predicates { + c = c.WithEventFilter(b.predicates[i]) + } + + for i := range b.actions { + r.AddAction(b.finalizers[i]) + } + for i := range b.finalizers { + r.AddFinalizer(b.finalizers[i]) + } + + return r, nil +} diff --git a/pkg/controller/actions/action_test.go b/pkg/utils/test/fakeclient/fakeclient.go similarity index 61% rename from pkg/controller/actions/action_test.go rename to pkg/utils/test/fakeclient/fakeclient.go index a32733e5672..f617ade607c 100644 --- a/pkg/controller/actions/action_test.go +++ b/pkg/utils/test/fakeclient/fakeclient.go @@ -1,4 +1,4 @@ -package actions_test +package fakeclient import ( "context" @@ -6,17 +6,15 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" ctrlClient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/client" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" ) -func NewFakeClient(ctx context.Context, objs ...ctrlClient.Object) (*client.Client, error) { +func New(ctx context.Context, objs ...ctrlClient.Object) (*client.Client, error) { scheme := runtime.NewScheme() utilruntime.Must(corev1.AddToScheme(scheme)) utilruntime.Must(appsv1.AddToScheme(scheme)) @@ -36,14 +34,3 @@ func NewFakeClient(ctx context.Context, objs ...ctrlClient.Object) (*client.Clie Build(), ) } - -func ExtractStatusCondition(conditionType string) func(in types.ResourceObject) metav1.Condition { - return func(in types.ResourceObject) metav1.Condition { - c := meta.FindStatusCondition(in.GetStatus().Conditions, conditionType) - if c == nil { - return metav1.Condition{} - } - - return *c - } -} diff --git a/pkg/utils/test/matchers/matechers.go b/pkg/utils/test/matchers/matechers.go new file mode 100644 index 00000000000..59f2001b7db --- /dev/null +++ b/pkg/utils/test/matchers/matechers.go @@ -0,0 +1,19 @@ +package matchers + +import ( + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" +) + +func ExtractStatusCondition(conditionType string) func(in types.ResourceObject) metav1.Condition { + return func(in types.ResourceObject) metav1.Condition { + c := meta.FindStatusCondition(in.GetStatus().Conditions, conditionType) + if c == nil { + return metav1.Condition{} + } + + return *c + } +}