diff --git a/.golangci.yml b/.golangci.yml index 08ff38825a3..aa2d6e06baa 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -59,6 +59,8 @@ linters-settings: # also allow generics - generic - EventHandler # for ToOwner + - discovery.DiscoveryInterface + - dynamic.Interface - predicate.Predicate - client.Object revive: diff --git a/controllers/components/dashboard/dashboard_controller.go b/controllers/components/dashboard/dashboard_controller.go index d3ee018045b..8f640611bec 100644 --- a/controllers/components/dashboard/dashboard_controller.go +++ b/controllers/components/dashboard/dashboard_controller.go @@ -31,7 +31,7 @@ import ( componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" "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/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/security" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" @@ -90,7 +90,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(configureDependencies). WithAction(security.NewUpdatePodSecurityRoleBindingAction(serviceAccounts)). WithAction(kustomize.NewAction( - kustomize.WithCache(render.DefaultCachingKeyFn), + kustomize.WithCache(), // Those are the default labels added by the legacy deploy method // and should be preserved as the original plugin were affecting // deployment selectors that are immutable once created, so it won't @@ -112,6 +112,11 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. updatestatus.WithSelectorLabel(labels.ComponentPartOf, componentsv1.DashboardInstanceName), )). WithAction(updateStatus). + // must be the final action + WithAction(gc.NewAction( + gc.WithLabel(labels.ComponentPartOf, componentsv1.DashboardInstanceName), + gc.WithUnremovables(gvk.OdhDashboardConfig), + )). Build(ctx) if err != nil { diff --git a/controllers/components/datasciencepipelines/datasciencepipelines_controller.go b/controllers/components/datasciencepipelines/datasciencepipelines_controller.go index 16f5f57cc64..492ac54a75a 100644 --- a/controllers/components/datasciencepipelines/datasciencepipelines_controller.go +++ b/controllers/components/datasciencepipelines/datasciencepipelines_controller.go @@ -29,7 +29,7 @@ import ( componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" "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/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources" @@ -71,7 +71,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(initialize). WithAction(devFlags). WithAction(kustomize.NewAction( - kustomize.WithCache(render.DefaultCachingKeyFn), + kustomize.WithCache(), kustomize.WithLabel(labels.ODH.Component(componentsv1.DataSciencePipelinesComponentName), "true"), kustomize.WithLabel(labels.K8SCommon.PartOf, componentsv1.DataSciencePipelinesComponentName), )). @@ -83,6 +83,10 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(updatestatus.NewAction( updatestatus.WithSelectorLabel(labels.ComponentPartOf, componentsv1.DataSciencePipelinesInstanceName), )). + // must be the final action + WithAction(gc.NewAction( + gc.WithLabel(labels.ComponentPartOf, componentsv1.DataSciencePipelinesInstanceName), + )). Build(ctx) if err != nil { diff --git a/controllers/components/kueue/kueue_controller.go b/controllers/components/kueue/kueue_controller.go index f548acd889d..53d2254c816 100644 --- a/controllers/components/kueue/kueue_controller.go +++ b/controllers/components/kueue/kueue_controller.go @@ -30,7 +30,7 @@ import ( componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" "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/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources" @@ -64,7 +64,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(initialize). WithAction(devFlags). WithAction(kustomize.NewAction( - kustomize.WithCache(render.DefaultCachingKeyFn), + kustomize.WithCache(), kustomize.WithLabel(labels.ODH.Component(ComponentName), "true"), kustomize.WithLabel(labels.K8SCommon.PartOf, ComponentName), )). @@ -76,6 +76,10 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(updatestatus.NewAction( updatestatus.WithSelectorLabel(labels.ComponentPartOf, componentsv1.KueueInstanceName), )). + // must be the final action + WithAction(gc.NewAction( + gc.WithLabel(labels.ComponentPartOf, componentsv1.KueueInstanceName), + )). Build(ctx) if err != nil { diff --git a/controllers/components/modelregistry/modelregistry_controller.go b/controllers/components/modelregistry/modelregistry_controller.go index 30224345bad..c59a01df0a7 100644 --- a/controllers/components/modelregistry/modelregistry_controller.go +++ b/controllers/components/modelregistry/modelregistry_controller.go @@ -28,12 +28,15 @@ import ( ctrl "sigs.k8s.io/controller-runtime" componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" + dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" "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/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/template" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/generation" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/reconciler" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels" @@ -56,6 +59,13 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. Owns(&appsv1.Deployment{}, reconciler.WithPredicates(resources.NewDeploymentPredicate())). Owns(&admissionregistrationv1.MutatingWebhookConfiguration{}). Owns(&admissionregistrationv1.ValidatingWebhookConfiguration{}). + // MR also depends on DSCInitialization to properly configure the SMM + // resource + Watches( + &dsciv1.DSCInitialization{}, + reconciler.WithEventHandler(handlers.ToNamed(componentsv1.ModelRegistryInstanceName)), + reconciler.WithPredicates(generation.New()), + ). Watches(&corev1.Namespace{}). Watches(&extv1.CustomResourceDefinition{}). // Some ClusterRoles are part of the component deployment, but not owned by @@ -71,10 +81,10 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(initialize). WithAction(configureDependencies). WithAction(template.NewAction( - template.WithCache(render.DefaultCachingKeyFn), + template.WithCache(), )). WithAction(kustomize.NewAction( - kustomize.WithCache(render.DefaultCachingKeyFn), + kustomize.WithCache(), kustomize.WithLabel(labels.ODH.Component(ComponentName), "true"), kustomize.WithLabel(labels.K8SCommon.PartOf, ComponentName), )). @@ -88,6 +98,11 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. updatestatus.WithSelectorLabel(labels.ComponentPartOf, componentsv1.ModelRegistryInstanceName), )). WithAction(updateStatus). + // must be the final action + WithAction(gc.NewAction( + gc.WithLabel(labels.ComponentPartOf, componentsv1.ModelRegistryInstanceName), + gc.WithUnremovables(gvk.ServiceMeshMember), + )). Build(ctx) if err != nil { diff --git a/controllers/components/modelregistry/modelregistry_controller_actions.go b/controllers/components/modelregistry/modelregistry_controller_actions.go index 072957f190b..fdce8cfcca3 100644 --- a/controllers/components/modelregistry/modelregistry_controller_actions.go +++ b/controllers/components/modelregistry/modelregistry_controller_actions.go @@ -57,6 +57,7 @@ func initialize(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { baseManifestInfo(BaseManifestsSourcePath), extraManifestInfo(BaseManifestsSourcePath), } + rr.Templates = []odhtypes.TemplateInfo{{ FS: resourcesFS, Path: ServiceMeshMemberTemplate, diff --git a/controllers/components/ray/ray_controller.go b/controllers/components/ray/ray_controller.go index 33fdb1f57ae..fac23edcd3d 100644 --- a/controllers/components/ray/ray_controller.go +++ b/controllers/components/ray/ray_controller.go @@ -28,7 +28,7 @@ import ( componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" "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/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources" @@ -57,7 +57,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(initialize). WithAction(devFlags). WithAction(kustomize.NewAction( - kustomize.WithCache(render.DefaultCachingKeyFn), + kustomize.WithCache(), kustomize.WithLabel(labels.ODH.Component(ComponentName), "true"), kustomize.WithLabel(labels.K8SCommon.PartOf, ComponentName), )). @@ -69,6 +69,10 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(updatestatus.NewAction( updatestatus.WithSelectorLabel(labels.ComponentPartOf, componentsv1.RayInstanceName), )). + // must be the final action + WithAction(gc.NewAction( + gc.WithLabel(labels.ComponentPartOf, componentsv1.RayInstanceName), + )). Build(ctx) if err != nil { diff --git a/controllers/components/trainingoperator/trainingoperator_controller.go b/controllers/components/trainingoperator/trainingoperator_controller.go index 4a80485670d..3da6cb466de 100644 --- a/controllers/components/trainingoperator/trainingoperator_controller.go +++ b/controllers/components/trainingoperator/trainingoperator_controller.go @@ -28,7 +28,7 @@ import ( componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" "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/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources" @@ -54,7 +54,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(initialize). WithAction(devFlags). WithAction(kustomize.NewAction( - kustomize.WithCache(render.DefaultCachingKeyFn), + kustomize.WithCache(), kustomize.WithLabel(labels.ODH.Component(ComponentName), "true"), kustomize.WithLabel(labels.K8SCommon.PartOf, ComponentName), )). @@ -66,6 +66,10 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(updatestatus.NewAction( updatestatus.WithSelectorLabel(labels.ComponentPartOf, componentsv1.TrainingOperatorInstanceName), )). + // must be the final action + WithAction(gc.NewAction( + gc.WithLabel(labels.ComponentPartOf, componentsv1.TrainingOperatorInstanceName), + )). Build(ctx) if err != nil { diff --git a/controllers/components/trustyai/trustyai_controller.go b/controllers/components/trustyai/trustyai_controller.go index ea1e6b82c1a..947c1c933b8 100644 --- a/controllers/components/trustyai/trustyai_controller.go +++ b/controllers/components/trustyai/trustyai_controller.go @@ -27,7 +27,7 @@ import ( componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" "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/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources" @@ -55,7 +55,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(initialize). WithAction(devFlags). WithAction(kustomize.NewAction( - kustomize.WithCache(render.DefaultCachingKeyFn), + kustomize.WithCache(), kustomize.WithLabel(labels.ODH.Component(ComponentName), "true"), kustomize.WithLabel(labels.K8SCommon.PartOf, ComponentName), )). @@ -67,6 +67,10 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(updatestatus.NewAction( updatestatus.WithSelectorLabel(labels.ComponentPartOf, componentsv1.TrustyAIInstanceName), )). + // must be the final action + WithAction(gc.NewAction( + gc.WithLabel(labels.ComponentPartOf, componentsv1.TrustyAIInstanceName), + )). Build(ctx) if err != nil { diff --git a/controllers/dscinitialization/suite_test.go b/controllers/dscinitialization/suite_test.go index 618b9a62e91..4d014dae376 100644 --- a/controllers/dscinitialization/suite_test.go +++ b/controllers/dscinitialization/suite_test.go @@ -123,7 +123,7 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) Expect(k8sClient).NotTo(BeNil()) - odhClient, err := odhClient.New(gCtx, cfg, k8sClient) + odhClient, err := odhClient.NewFromConfig(cfg, k8sClient) Expect(err).NotTo(HaveOccurred()) Expect(odhClient).NotTo(BeNil()) diff --git a/main.go b/main.go index ed08486071a..3ffd829e571 100644 --- a/main.go +++ b/main.go @@ -36,6 +36,7 @@ import ( promv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" appsv1 "k8s.io/api/apps/v1" + authorizationv1 "k8s.io/api/authorization/v1" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -73,6 +74,7 @@ import ( odhClient "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/client" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/logger" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/services/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/upgrade" _ "github.com/opendatahub-io/opendatahub-operator/v2/controllers/components/dashboard" @@ -245,6 +247,7 @@ func main() { //nolint:funlen,maintidx Cache: &client.CacheOptions{ DisableFor: []client.Object{ resources.GvkToUnstructured(gvk.OpenshiftIngress), + &authorizationv1.SelfSubjectRulesReview{}, }, // Set it to true so the cache-backed client reads unstructured objects // or lists from the cache instead of a live lookup. @@ -259,7 +262,7 @@ func main() { //nolint:funlen,maintidx webhook.Init(mgr) - oc, err := odhClient.NewFromManager(ctx, mgr) + oc, err := odhClient.NewFromManager(mgr) if err != nil { setupLog.Error(err, "unable to create client") os.Exit(1) @@ -309,6 +312,24 @@ func main() { //nolint:funlen,maintidx os.Exit(1) } + ons, err := cluster.GetOperatorNamespace() + if err != nil { + setupLog.Error(err, "unable to determine Operator Namespace") + os.Exit(1) + } + + gc.Instance = gc.New( + oc, + ons, + gc.WithUnremovables(gvk.CustomResourceDefinition, gvk.Lease), + ) + + err = mgr.Add(gc.Instance) + if err != nil { + setupLog.Error(err, "unable to register GC service") + os.Exit(1) + } + // Initialize component reconcilers if err = CreateComponentReconcilers(ctx, mgr); err != nil { os.Exit(1) diff --git a/pkg/cluster/gvk/gvk.go b/pkg/cluster/gvk/gvk.go index 5ec6884b219..3222d5b20f3 100644 --- a/pkg/cluster/gvk/gvk.go +++ b/pkg/cluster/gvk/gvk.go @@ -2,6 +2,7 @@ package gvk import ( appsv1 "k8s.io/api/apps/v1" + coordinationv1 "k8s.io/api/coordination/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -174,4 +175,10 @@ var ( Version: "v1", Kind: "ServiceMeshMember", } + + Lease = schema.GroupVersionKind{ + Group: coordinationv1.SchemeGroupVersion.Group, + Version: coordinationv1.SchemeGroupVersion.Version, + Kind: "Lease", + } ) diff --git a/pkg/controller/actions/deleteresource/action_delete_resources_test.go b/pkg/controller/actions/deleteresource/action_delete_resources_test.go index 76a48109abe..c8fd022d22e 100644 --- a/pkg/controller/actions/deleteresource/action_delete_resources_test.go +++ b/pkg/controller/actions/deleteresource/action_delete_resources_test.go @@ -28,7 +28,6 @@ func TestDeleteResourcesAction(t *testing.T) { ns := xid.New().String() cl, err := fakeclient.New( - ctx, &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ APIVersion: gvk.Deployment.GroupVersion().String(), diff --git a/pkg/controller/actions/deploy/action_deploy.go b/pkg/controller/actions/deploy/action_deploy.go index 8ec877b7bad..dcd3a7eb2ba 100644 --- a/pkg/controller/actions/deploy/action_deploy.go +++ b/pkg/controller/actions/deploy/action_deploy.go @@ -113,8 +113,20 @@ func (a *Action) run(ctx context.Context, rr *odhTypes.ReconciliationRequest) er controllerName := strings.ToLower(rr.Instance.GetObjectKind().GroupVersionKind().Kind) + deployHash, err := odhTypes.HashStr(rr) + if err != nil { + return fmt.Errorf("unable to compute reauest hash: %w", err) + } + + deployAnnotations := map[string]string{ + annotations.ComponentGeneration: strconv.FormatInt(rr.Instance.GetGeneration(), 10), + annotations.ComponentHash: deployHash, + annotations.PlatformType: string(rr.Release.Name), + annotations.PlatformVersion: rr.Release.Version.String(), + } + for i := range rr.Resources { - ok, err := a.deploy(ctx, rr, rr.Resources[i]) + ok, err := a.deploy(ctx, rr, rr.Resources[i], deployAnnotations) if err != nil { return fmt.Errorf("failure deploying %s: %w", rr.Resources[i], err) } @@ -131,6 +143,7 @@ func (a *Action) deploy( ctx context.Context, rr *odhTypes.ReconciliationRequest, obj unstructured.Unstructured, + deployAnnotations map[string]string, ) (bool, error) { current, lookupErr := a.lookup(ctx, rr.Client, obj) if lookupErr != nil { @@ -145,10 +158,7 @@ func (a *Action) deploy( resources.SetLabels(&obj, a.labels) resources.SetAnnotations(&obj, a.annotations) - - resources.SetAnnotation(&obj, annotations.ComponentGeneration, strconv.FormatInt(rr.Instance.GetGeneration(), 10)) - resources.SetAnnotation(&obj, annotations.PlatformType, string(rr.Release.Name)) - resources.SetAnnotation(&obj, annotations.PlatformVersion, rr.Release.Version.String()) + resources.SetAnnotations(&obj, deployAnnotations) // backup copy for caching origObj := obj.DeepCopy() diff --git a/pkg/controller/actions/deploy/action_deploy_cache_test.go b/pkg/controller/actions/deploy/action_deploy_cache_test.go index 193e8861a7b..75a04eabddd 100644 --- a/pkg/controller/actions/deploy/action_deploy_cache_test.go +++ b/pkg/controller/actions/deploy/action_deploy_cache_test.go @@ -37,8 +37,6 @@ func TestDeployWithCacheAction(t *testing.T) { g := NewWithT(t) s := runtime.NewScheme() - ctx := context.Background() - utilruntime.Must(corev1.AddToScheme(s)) utilruntime.Must(appsv1.AddToScheme(s)) utilruntime.Must(apiextensionsv1.AddToScheme(s)) @@ -67,7 +65,7 @@ func TestDeployWithCacheAction(t *testing.T) { envTestClient, err := ctrlCli.New(cfg, ctrlCli.Options{Scheme: s}) g.Expect(err).NotTo(HaveOccurred()) - cli, err := client.New(ctx, cfg, envTestClient) + cli, err := client.NewFromConfig(cfg, envTestClient) g.Expect(err).NotTo(HaveOccurred()) t.Run("ExistingResource", func(t *testing.T) { diff --git a/pkg/controller/actions/deploy/action_deploy_test.go b/pkg/controller/actions/deploy/action_deploy_test.go index c06ce2f4393..11c18f4f54c 100644 --- a/pkg/controller/actions/deploy/action_deploy_test.go +++ b/pkg/controller/actions/deploy/action_deploy_test.go @@ -32,7 +32,7 @@ func TestDeployAction(t *testing.T) { ctx := context.Background() ns := xid.New().String() - cl, err := fakeclient.New(ctx) + cl, err := fakeclient.New() g.Expect(err).ShouldNot(HaveOccurred()) action := deploy.NewAction( @@ -78,8 +78,12 @@ func TestDeployAction(t *testing.T) { err = cl.Get(ctx, client.ObjectKeyFromObject(obj1), obj1) g.Expect(err).ShouldNot(HaveOccurred()) + dh, err := types.HashStr(&rr) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(obj1).Should(And( jq.Match(`.metadata.annotations."%s" == "%s"`, annotations.ComponentGeneration, strconv.FormatInt(rr.Instance.GetGeneration(), 10)), + jq.Match(`.metadata.annotations."%s" == "%s"`, annotations.ComponentHash, dh), jq.Match(`.metadata.annotations."%s" == "%s"`, annotations.PlatformVersion, "1.2.3"), jq.Match(`.metadata.annotations."%s" == "%s"`, annotations.PlatformType, string(cluster.OpenDataHub)), )) @@ -138,7 +142,7 @@ func TestDeployNotOwnedSkip(t *testing.T) { g.Expect(err).ShouldNot(HaveOccurred()) - cl, err := fakeclient.New(ctx, oldObj) + cl, err := fakeclient.New(oldObj) g.Expect(err).ShouldNot(HaveOccurred()) rr := types.ReconciliationRequest{ @@ -205,7 +209,7 @@ func TestDeployNotOwnedCreate(t *testing.T) { g.Expect(err).ShouldNot(HaveOccurred()) - cl, err := fakeclient.New(ctx) + cl, err := fakeclient.New() g.Expect(err).ShouldNot(HaveOccurred()) rr := types.ReconciliationRequest{ diff --git a/pkg/controller/actions/gc/action_gc.go b/pkg/controller/actions/gc/action_gc.go new file mode 100644 index 00000000000..3e51bdac613 --- /dev/null +++ b/pkg/controller/actions/gc/action_gc.go @@ -0,0 +1,139 @@ +package gc + +import ( + "context" + "fmt" + "slices" + "strings" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime/schema" + + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions" + odhTypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/services/gc" +) + +type PredicateFn func(*odhTypes.ReconciliationRequest, unstructured.Unstructured) (bool, error) +type ActionOpts func(*Action) + +type Action struct { + labels map[string]string + selector labels.Selector + unremovables []schema.GroupVersionKind + gc *gc.GC + predicateFn PredicateFn +} + +func WithLabel(name string, value string) ActionOpts { + return func(action *Action) { + if action.labels == nil { + action.labels = map[string]string{} + } + + action.labels[name] = value + } +} + +func WithLabels(values map[string]string) ActionOpts { + return func(action *Action) { + if action.labels == nil { + action.labels = map[string]string{} + } + + for k, v := range values { + action.labels[k] = v + } + } +} + +func WithUnremovables(items ...schema.GroupVersionKind) ActionOpts { + return func(action *Action) { + action.unremovables = append(action.unremovables, items...) + } +} + +func WithPredicate(value PredicateFn) ActionOpts { + return func(action *Action) { + if value == nil { + return + } + + action.predicateFn = value + } +} + +func WithGC(value *gc.GC) ActionOpts { + return func(action *Action) { + if value == nil { + return + } + + action.gc = value + } +} + +func (a *Action) run(ctx context.Context, rr *odhTypes.ReconciliationRequest) error { + // To avoid the expensive GC, run it only when resources have + // been generated + if !rr.Generated { + return nil + } + + controllerName := strings.ToLower(rr.Instance.GetObjectKind().GroupVersionKind().Kind) + + CyclesTotal.WithLabelValues(controllerName).Inc() + + ch, err := odhTypes.HashStr(rr) + if err != nil { + return err + } + + deleted, err := a.gc.Run( + ctx, + a.selector, + func(ctx context.Context, obj unstructured.Unstructured) (bool, error) { + if slices.Contains(a.unremovables, obj.GroupVersionKind()) { + return false, nil + } + + objectHash := obj.GetAnnotations()[annotations.ComponentHash] + if objectHash != "" { + return objectHash != ch, nil + } + + return a.predicateFn(rr, obj) + }, + ) + + if err != nil { + return fmt.Errorf("cannot run gc: %w", err) + } + + if deleted > 0 { + DeletedTotal.WithLabelValues(controllerName).Add(float64(deleted)) + } + + return nil +} + +func NewAction(opts ...ActionOpts) actions.Fn { + action := Action{} + action.predicateFn = DefaultPredicate + action.unremovables = make([]schema.GroupVersionKind, 0) + + for _, opt := range opts { + opt(&action) + } + + action.selector = labels.SelectorFromSet(action.labels) + + // TODO: refactor + if action.gc == nil { + action.gc = gc.Instance + } + + return action.run +} diff --git a/pkg/controller/actions/gc/action_gc_metrics.go b/pkg/controller/actions/gc/action_gc_metrics.go new file mode 100644 index 00000000000..d345fe6536f --- /dev/null +++ b/pkg/controller/actions/gc/action_gc_metrics.go @@ -0,0 +1,43 @@ +package gc + +import ( + "github.com/prometheus/client_golang/prometheus" + "sigs.k8s.io/controller-runtime/pkg/metrics" +) + +var ( + // DeletedTotal is a prometheus counter metrics which holds the total number + // of resource deleted by the action per controller. It has one label. + // controller label refers to the controller name. + DeletedTotal = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Name: "action_gc_deleted_total", + Help: "Number of GCed resources", + }, + []string{ + "controller", + }, + ) + + // CyclesTotal is a prometheus counter metrics which holds the total number + // gc cycles per controller. It has one label. + // controller label refers to the controller name. + CyclesTotal = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Name: "action_gc_cycles_total", + Help: "Number of GC cycles", + }, + []string{ + "controller", + }, + ) +) + +// init register metrics to the global registry from controller-runtime/pkg/metrics. +// see https://book.kubebuilder.io/reference/metrics#publishing-additional-metrics +// +//nolint:gochecknoinits +func init() { + metrics.Registry.MustRegister(DeletedTotal) + metrics.Registry.MustRegister(CyclesTotal) +} diff --git a/pkg/controller/actions/gc/action_gc_support.go b/pkg/controller/actions/gc/action_gc_support.go new file mode 100644 index 00000000000..ea2124e1edd --- /dev/null +++ b/pkg/controller/actions/gc/action_gc_support.go @@ -0,0 +1,41 @@ +package gc + +import ( + "fmt" + "strconv" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + odhTypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" + odhAnnotations "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources" +) + +func DefaultPredicate(rr *odhTypes.ReconciliationRequest, obj unstructured.Unstructured) (bool, error) { + if obj.GetAnnotations() == nil { + return false, nil + } + + pv := resources.GetAnnotation(&obj, odhAnnotations.PlatformVersion) + pt := resources.GetAnnotation(&obj, odhAnnotations.PlatformType) + cg := resources.GetAnnotation(&obj, odhAnnotations.ComponentGeneration) + + if pv == "" || pt == "" || cg == "" { + return false, nil + } + + if pv != rr.Release.Version.String() { + return true, nil + } + + if pt != string(rr.Release.Name) { + return true, nil + } + + g, err := strconv.Atoi(cg) + if err != nil { + return false, fmt.Errorf("cannot determine generation: %w", err) + } + + return rr.Instance.GetGeneration() != int64(g), nil +} diff --git a/pkg/controller/actions/gc/action_gc_test.go b/pkg/controller/actions/gc/action_gc_test.go new file mode 100644 index 00000000000..6b9abd9d72b --- /dev/null +++ b/pkg/controller/actions/gc/action_gc_test.go @@ -0,0 +1,244 @@ +package gc_test + +import ( + "context" + "testing" + + "github.com/blang/semver/v4" + gTypes "github.com/onsi/gomega/types" + "github.com/operator-framework/api/pkg/lib/version" + "github.com/prometheus/client_golang/prometheus/testutil" + "github.com/rs/xid" + appsv1 "k8s.io/api/apps/v1" + authorizationv1 "k8s.io/api/authorization/v1" + corev1 "k8s.io/api/core/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + k8serr "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + ctrlCli "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + + componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/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/gc" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/client" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" + gcSvc "github.com/opendatahub-io/opendatahub-operator/v2/pkg/services/gc" + + . "github.com/onsi/gomega" +) + +func TestGcAction(t *testing.T) { + g := NewWithT(t) + + s := runtime.NewScheme() + ctx := context.Background() + + utilruntime.Must(corev1.AddToScheme(s)) + utilruntime.Must(appsv1.AddToScheme(s)) + utilruntime.Must(apiextensionsv1.AddToScheme(s)) + utilruntime.Must(authorizationv1.AddToScheme(s)) + + envTest := &envtest.Environment{ + CRDInstallOptions: envtest.CRDInstallOptions{ + Scheme: s, + CleanUpAfterUse: true, + }, + } + + t.Cleanup(func() { + _ = envTest.Stop() + }) + + cfg, err := envTest.Start() + g.Expect(err).NotTo(HaveOccurred()) + + envTestClient, err := ctrlCli.New(cfg, ctrlCli.Options{Scheme: s}) + g.Expect(err).NotTo(HaveOccurred()) + + cli, err := client.NewFromConfig(cfg, envTestClient) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(cli).NotTo(BeNil()) + + tests := []struct { + name string + version semver.Version + generated bool + matcher gTypes.GomegaMatcher + metricsMatcher gTypes.GomegaMatcher + labels map[string]string + options []gc.ActionOpts + hashFn func(*types.ReconciliationRequest) (string, error) + }{ + { + name: "should delete leftovers", + version: semver.Version{Major: 0, Minor: 0, Patch: 1}, + generated: true, + matcher: Satisfy(k8serr.IsNotFound), + metricsMatcher: BeNumerically("==", 1), + }, + { + name: "should not delete resources because same annotations", + version: semver.Version{Major: 0, Minor: 1, Patch: 0}, + generated: true, + matcher: Not(HaveOccurred()), + metricsMatcher: BeNumerically("==", 1), + }, + { + name: "should not delete resources because of no generated resources have been detected", + version: semver.Version{Major: 0, Minor: 0, Patch: 1}, + generated: false, + matcher: Not(HaveOccurred()), + metricsMatcher: BeNumerically("==", 0), + }, + { + name: "should not delete resources because of selector", + version: semver.Version{Major: 0, Minor: 0, Patch: 1}, + generated: true, + matcher: Not(HaveOccurred()), + metricsMatcher: BeNumerically("==", 1), + labels: map[string]string{"foo": "bar"}, + options: []gc.ActionOpts{gc.WithLabel("foo", "baz")}, + }, + { + name: "should not delete resources because of unremovable type", + version: semver.Version{Major: 0, Minor: 0, Patch: 1}, + generated: true, + matcher: Not(HaveOccurred()), + metricsMatcher: BeNumerically("==", 1), + options: []gc.ActionOpts{gc.WithUnremovables(gvk.ConfigMap)}, + }, + { + name: "should not delete resources because of predicate", + version: semver.Version{Major: 0, Minor: 0, Patch: 1}, + generated: true, + matcher: Not(HaveOccurred()), + metricsMatcher: BeNumerically("==", 1), + options: []gc.ActionOpts{gc.WithPredicate( + func(request *types.ReconciliationRequest, unstructured unstructured.Unstructured) (bool, error) { + return unstructured.GroupVersionKind() != gvk.ConfigMap, nil + }, + )}, + }, + { + name: "should delete leftovers because of hash", + version: semver.Version{Major: 0, Minor: 1, Patch: 0}, + generated: true, + matcher: Satisfy(k8serr.IsNotFound), + metricsMatcher: BeNumerically("==", 1), + hashFn: func(rr *types.ReconciliationRequest) (string, error) { + return xid.New().String(), nil + }, + }, + { + name: "should not delete leftovers because of hash", + version: semver.Version{Major: 0, Minor: 1, Patch: 0}, + generated: true, + matcher: Not(HaveOccurred()), + metricsMatcher: BeNumerically("==", 1), + hashFn: types.HashStr, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gc.CyclesTotal.Reset() + gc.CyclesTotal.WithLabelValues("dashboard").Add(0) + + g := NewWithT(t) + nsn := xid.New().String() + + gci := gcSvc.New( + cli, + nsn, + // This is required as there are no kubernetes controller running + // with the envtest, hence we can't use the foreground deletion + // policy (default) + gcSvc.WithPropagationPolicy(metav1.DeletePropagationBackground), + ) + + ns := corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: nsn, + }, + } + + g.Expect(cli.Create(ctx, &ns)). + NotTo(HaveOccurred()) + g.Expect(gci.Start(ctx)). + NotTo(HaveOccurred()) + + rr := types.ReconciliationRequest{ + DSCI: &dsciv1.DSCInitialization{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 1, + }, + }, + Instance: &componentsv1.Dashboard{ + TypeMeta: metav1.TypeMeta{ + APIVersion: componentsv1.GroupVersion.String(), + Kind: componentsv1.DashboardKind, + }, + ObjectMeta: metav1.ObjectMeta{ + Generation: 1, + }, + }, + Release: cluster.Release{ + Name: cluster.OpenDataHub, + Version: version.OperatorVersion{ + Version: tt.version, + }, + }, + Generated: tt.generated, + } + + ch := "" + if tt.hashFn != nil { + ch, err = tt.hashFn(&rr) + g.Expect(err).NotTo(HaveOccurred()) + } + + cm := corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gc-cm", + Namespace: nsn, + Annotations: map[string]string{ + annotations.ComponentGeneration: "1", + annotations.ComponentHash: ch, + annotations.PlatformVersion: "0.1.0", + annotations.PlatformType: string(cluster.OpenDataHub), + }, + Labels: tt.labels, + }, + } + + g.Expect(cli.Create(ctx, &cm)). + NotTo(HaveOccurred()) + + opts := make([]gc.ActionOpts, 0, len(tt.options)+1) + opts = append(opts, gc.WithGC(gci)) + opts = append(opts, tt.options...) + + a := gc.NewAction(opts...) + + err = a(ctx, &rr) + g.Expect(err).NotTo(HaveOccurred()) + + if tt.matcher != nil { + err = cli.Get(ctx, ctrlCli.ObjectKeyFromObject(&cm), &corev1.ConfigMap{}) + g.Expect(err).To(tt.matcher) + } + + if tt.metricsMatcher != nil { + ct := testutil.ToFloat64(gc.CyclesTotal) + g.Expect(ct).Should(tt.metricsMatcher) + } + }) + } +} diff --git a/pkg/controller/actions/render/kustomize/action_render_manifests.go b/pkg/controller/actions/render/kustomize/action_render_manifests.go index b548b54874e..157e1d0b50d 100644 --- a/pkg/controller/actions/render/kustomize/action_render_manifests.go +++ b/pkg/controller/actions/render/kustomize/action_render_manifests.go @@ -68,9 +68,9 @@ func WithManifestsOptions(values ...kustomize.EngineOptsFn) ActionOpts { } } -func WithCache(value render.CachingKeyFn) ActionOpts { +func WithCache() ActionOpts { return func(action *Action) { - action.cachingKeyFn = value + action.cachingKeyFn = types.Hash } } @@ -79,7 +79,7 @@ func (a *Action) run(ctx context.Context, rr *types.ReconciliationRequest) error var cachingKey []byte if rr.Instance.GetDevFlags() == nil { - cachingKey, err = a.cachingKeyFn(ctx, rr) + cachingKey, err = a.cachingKeyFn(rr) if err != nil { return fmt.Errorf("unable to calculate checksum of reconciliation object: %w", err) } @@ -108,6 +108,8 @@ func (a *Action) run(ctx context.Context, rr *types.ReconciliationRequest) error controllerName := strings.ToLower(rr.Instance.GetObjectKind().GroupVersionKind().Kind) render.RenderedResourcesTotal.WithLabelValues(controllerName, RendererEngine).Add(float64(len(result))) + + rr.Generated = true } // deep copy object so changes done in the pipelines won't @@ -138,7 +140,7 @@ func (a *Action) render(rr *types.ReconciliationRequest) ([]unstructured.Unstruc func NewAction(opts ...ActionOpts) actions.Fn { action := Action{ - cachingKeyFn: func(_ context.Context, rr *types.ReconciliationRequest) ([]byte, error) { + cachingKeyFn: func(rr *types.ReconciliationRequest) ([]byte, error) { return nil, nil }, } diff --git a/pkg/controller/actions/render/kustomize/action_render_manifests_test.go b/pkg/controller/actions/render/kustomize/action_render_manifests_test.go index e8809515780..6043ab1764c 100644 --- a/pkg/controller/actions/render/kustomize/action_render_manifests_test.go +++ b/pkg/controller/actions/render/kustomize/action_render_manifests_test.go @@ -112,7 +112,7 @@ 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)) - cl, err := fakeclient.New(ctx) + cl, err := fakeclient.New() g.Expect(err).ShouldNot(HaveOccurred()) action := kustomize.NewAction( @@ -189,11 +189,11 @@ func TestRenderResourcesWithCacheAction(t *testing.T) { _ = fs.WriteFile(path.Join(id, mk.DefaultKustomizationFileName), []byte(testRenderResourcesWithCacheKustomization)) _ = fs.WriteFile(path.Join(id, "test-resources-deployment.yaml"), []byte(testRenderResourcesWithCacheDeployment)) - cl, err := fakeclient.New(ctx) + cl, err := fakeclient.New() g.Expect(err).ShouldNot(HaveOccurred()) action := kustomize.NewAction( - kustomize.WithCache(render.DefaultCachingKeyFn), + kustomize.WithCache(), kustomize.WithLabel(labels.ComponentPartOf, "foo"), kustomize.WithLabel("platform.opendatahub.io/namespace", ns), kustomize.WithAnnotation("platform.opendatahub.io/release", "1.2.3"), diff --git a/pkg/controller/actions/render/render_support.go b/pkg/controller/actions/render/render_support.go index 9cf3c1105ae..259ea6823d4 100644 --- a/pkg/controller/actions/render/render_support.go +++ b/pkg/controller/actions/render/render_support.go @@ -1,48 +1,7 @@ package render import ( - "context" - "crypto/sha256" - "encoding/binary" - "fmt" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" ) -type CachingKeyFn func(_ context.Context, rr *types.ReconciliationRequest) ([]byte, error) - -func DefaultCachingKeyFn(_ context.Context, rr *types.ReconciliationRequest) ([]byte, error) { - hash := sha256.New() - - dsciGeneration := make([]byte, binary.MaxVarintLen64) - binary.PutVarint(dsciGeneration, rr.DSCI.GetGeneration()) - - instanceGeneration := make([]byte, binary.MaxVarintLen64) - binary.PutVarint(instanceGeneration, rr.Instance.GetGeneration()) - - if _, err := hash.Write(dsciGeneration); err != nil { - return nil, fmt.Errorf("unable to calculate checksum of reconciliation object: %w", err) - } - if _, err := hash.Write(instanceGeneration); err != nil { - return nil, fmt.Errorf("unable to calculate checksum of reconciliation object: %w", err) - } - if _, err := hash.Write([]byte(rr.Release.Name)); err != nil { - return nil, fmt.Errorf("unable to calculate checksum of reconciliation object: %w", err) - } - if _, err := hash.Write([]byte(rr.Release.Version.String())); err != nil { - return nil, fmt.Errorf("unable to calculate checksum of reconciliation object: %w", err) - } - - for i := range rr.Manifests { - if _, err := hash.Write([]byte(rr.Manifests[i].String())); err != nil { - return nil, fmt.Errorf("unable to calculate checksum of reconciliation object: %w", err) - } - } - for i := range rr.Templates { - if _, err := hash.Write([]byte(rr.Templates[i].Path)); err != nil { - return nil, fmt.Errorf("unable to calculate checksum of reconciliation object: %w", err) - } - } - - return hash.Sum(nil), nil -} +type CachingKeyFn func(rr *types.ReconciliationRequest) ([]byte, error) diff --git a/pkg/controller/actions/render/template/action_render_templates.go b/pkg/controller/actions/render/template/action_render_templates.go index 520f83b7d46..306dc6a6093 100644 --- a/pkg/controller/actions/render/template/action_render_templates.go +++ b/pkg/controller/actions/render/template/action_render_templates.go @@ -36,9 +36,9 @@ type Action struct { type ActionOpts func(*Action) -func WithCache(value render.CachingKeyFn) ActionOpts { +func WithCache() ActionOpts { return func(action *Action) { - action.cachingKeyFn = value + action.cachingKeyFn = types.Hash } } @@ -55,7 +55,7 @@ func (a *Action) run(ctx context.Context, rr *types.ReconciliationRequest) error var cachingKey []byte if rr.Instance.GetDevFlags() == nil { - cachingKey, err = a.cachingKeyFn(ctx, rr) + cachingKey, err = a.cachingKeyFn(rr) if err != nil { return fmt.Errorf("unable to calculate checksum of reconciliation object: %w", err) } @@ -84,6 +84,8 @@ func (a *Action) run(ctx context.Context, rr *types.ReconciliationRequest) error controllerName := strings.ToLower(rr.Instance.GetObjectKind().GroupVersionKind().Kind) render.RenderedResourcesTotal.WithLabelValues(controllerName, RendererEngine).Add(float64(len(result))) + + rr.Generated = true } // deep copy object so changes done in the pipelines won't @@ -137,7 +139,7 @@ func (a *Action) render(rr *types.ReconciliationRequest) ([]unstructured.Unstruc func NewAction(opts ...ActionOpts) actions.Fn { action := Action{ - cachingKeyFn: func(_ context.Context, rr *types.ReconciliationRequest) ([]byte, error) { + cachingKeyFn: func(rr *types.ReconciliationRequest) ([]byte, error) { return nil, nil }, data: make(map[string]any), diff --git a/pkg/controller/actions/render/template/action_render_templates_test.go b/pkg/controller/actions/render/template/action_render_templates_test.go index 15477dd1ca2..0d679a7e94a 100644 --- a/pkg/controller/actions/render/template/action_render_templates_test.go +++ b/pkg/controller/actions/render/template/action_render_templates_test.go @@ -32,7 +32,7 @@ func TestRenderTemplate(t *testing.T) { ctx := context.Background() ns := xid.New().String() - cl, err := fakeclient.New(ctx) + cl, err := fakeclient.New() g.Expect(err).ShouldNot(HaveOccurred()) action := template.NewAction() @@ -82,7 +82,7 @@ func TestRenderTemplateWithData(t *testing.T) { id := xid.New().String() name := xid.New().String() - cl, err := fakeclient.New(ctx) + cl, err := fakeclient.New() g.Expect(err).ShouldNot(HaveOccurred()) action := template.NewAction( @@ -139,11 +139,11 @@ func TestRenderTemplateWithCache(t *testing.T) { ctx := context.Background() ns := xid.New().String() - cl, err := fakeclient.New(ctx) + cl, err := fakeclient.New() g.Expect(err).ShouldNot(HaveOccurred()) action := template.NewAction( - template.WithCache(render.DefaultCachingKeyFn), + template.WithCache(), ) render.RenderedResourcesTotal.Reset() diff --git a/pkg/controller/actions/security/actions_test.go b/pkg/controller/actions/security/actions_test.go index fd1fe77cbc0..b03a0ff1418 100644 --- a/pkg/controller/actions/security/actions_test.go +++ b/pkg/controller/actions/security/actions_test.go @@ -44,7 +44,6 @@ func TestUpdatePodSecurityRoleBindingAction(t *testing.T) { ns := xid.New().String() cl, err := fakeclient.New( - ctx, &rbacv1.RoleBinding{ TypeMeta: metav1.TypeMeta{ APIVersion: gvk.RoleBinding.GroupVersion().String(), diff --git a/pkg/controller/actions/updatestatus/action_update_status_test.go b/pkg/controller/actions/updatestatus/action_update_status_test.go index 3e3a52411cc..fdde2ffc52c 100644 --- a/pkg/controller/actions/updatestatus/action_update_status_test.go +++ b/pkg/controller/actions/updatestatus/action_update_status_test.go @@ -33,7 +33,6 @@ func TestUpdateStatusActionNotReady(t *testing.T) { ns := xid.New().String() cl, err := fakeclient.New( - ctx, &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ APIVersion: gvk.Deployment.GroupVersion().String(), @@ -105,7 +104,6 @@ func TestUpdateStatusActionReady(t *testing.T) { ns := xid.New().String() cl, err := fakeclient.New( - ctx, &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ APIVersion: gvk.Deployment.GroupVersion().String(), diff --git a/pkg/controller/client/client.go b/pkg/controller/client/client.go index dad42d5c904..b38d2028411 100644 --- a/pkg/controller/client/client.go +++ b/pkg/controller/client/client.go @@ -6,6 +6,9 @@ import ( "github.com/pkg/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/client-go/discovery" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" ctrl "sigs.k8s.io/controller-runtime" ctrlCli "sigs.k8s.io/controller-runtime/pkg/client" @@ -13,18 +16,44 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources" ) -func NewFromManager(ctx context.Context, mgr ctrl.Manager) (*Client, error) { - return New(ctx, mgr.GetConfig(), mgr.GetClient()) +func NewFromManager(mgr ctrl.Manager) (*Client, error) { + return NewFromConfig(mgr.GetConfig(), mgr.GetClient()) } -func New(_ context.Context, _ *rest.Config, client ctrlCli.Client) (*Client, error) { +func NewFromConfig(cfg *rest.Config, client ctrlCli.Client) (*Client, error) { + kubernetesCl, err := kubernetes.NewForConfig(cfg) + if err != nil { + return nil, fmt.Errorf("unable to construct a Kubernetes client: %w", err) + } + + dynamicCl, err := dynamic.NewForConfig(cfg) + if err != nil { + return nil, fmt.Errorf("unable to construct a Discovery client: %w", err) + } + + return New(client, kubernetesCl, dynamicCl), nil +} + +func New(client ctrlCli.Client, kubernetes kubernetes.Interface, dynamic dynamic.Interface) *Client { return &Client{ - Client: client, - }, nil + Client: client, + kubernetes: kubernetes, + dynamic: dynamic, + } } type Client struct { ctrlCli.Client + kubernetes kubernetes.Interface + dynamic dynamic.Interface +} + +func (c *Client) Discovery() discovery.DiscoveryInterface { + return c.kubernetes.Discovery() +} + +func (c *Client) Dynamic() dynamic.Interface { + return c.dynamic } func (c *Client) Apply(ctx context.Context, in ctrlCli.Object, opts ...ctrlCli.PatchOption) error { diff --git a/pkg/controller/reconciler/component_reconciler.go b/pkg/controller/reconciler/component_reconciler.go index 3804900f2b1..57cd973b744 100644 --- a/pkg/controller/reconciler/component_reconciler.go +++ b/pkg/controller/reconciler/component_reconciler.go @@ -42,8 +42,8 @@ type ComponentReconciler struct { instanceFactory func() (components.ComponentObject, error) } -func NewComponentReconciler(ctx context.Context, mgr manager.Manager, name string, object components.ComponentObject) (*ComponentReconciler, error) { - oc, err := odhClient.NewFromManager(ctx, mgr) +func NewComponentReconciler(mgr manager.Manager, name string, object components.ComponentObject) (*ComponentReconciler, error) { + oc, err := odhClient.NewFromManager(mgr) if err != nil { return nil, err } @@ -96,6 +96,7 @@ func (r *ComponentReconciler) AddFinalizer(action actions.Fn) { func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { l := log.FromContext(ctx) + l.Info("reconcile") res, err := r.instanceFactory() if err != nil { diff --git a/pkg/controller/reconciler/component_reconciler_support.go b/pkg/controller/reconciler/component_reconciler_support.go index d8a629a3e70..586dc25cc2e 100644 --- a/pkg/controller/reconciler/component_reconciler_support.go +++ b/pkg/controller/reconciler/component_reconciler_support.go @@ -183,7 +183,7 @@ func (b *ComponentReconcilerBuilder) WithEventFilter(p predicate.Predicate) *Com return b } -func (b *ComponentReconcilerBuilder) Build(ctx context.Context) (*ComponentReconciler, error) { +func (b *ComponentReconcilerBuilder) Build(_ context.Context) (*ComponentReconciler, error) { name := b.componentName if name == "" { kinds, _, err := b.mgr.GetScheme().ObjectKinds(b.input.object) @@ -198,7 +198,7 @@ func (b *ComponentReconcilerBuilder) Build(ctx context.Context) (*ComponentRecon name = strings.ToLower(name) } - r, err := NewComponentReconciler(ctx, b.mgr, name, b.input.object) + r, err := NewComponentReconciler(b.mgr, name, b.input.object) if err != nil { return nil, fmt.Errorf("failed to create reconciler for component %s: %w", name, err) } diff --git a/pkg/controller/types/types.go b/pkg/controller/types/types.go index f890b0f3896..bcabd3e4fab 100644 --- a/pkg/controller/types/types.go +++ b/pkg/controller/types/types.go @@ -1,6 +1,8 @@ package types import ( + "crypto/sha256" + "encoding/binary" "fmt" "io/fs" "path" @@ -16,6 +18,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" odhClient "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/client" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/manager" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources" ) type ResourceObject interface { @@ -84,6 +87,12 @@ type ReconciliationRequest struct { // Templates []TemplateInfo Resources []unstructured.Unstructured + + // TODO: this has been added to reduce GC work and only run when + // resources have been generated. It should be removed and + // replaced with a better way of describing resources and + // their origin + Generated bool } func (rr *ReconciliationRequest) AddResource(in interface{}) error { @@ -122,3 +131,51 @@ func (rr *ReconciliationRequest) normalize(obj client.Object) error { return nil } + +func Hash(rr *ReconciliationRequest) ([]byte, error) { + hash := sha256.New() + + dsciGeneration := make([]byte, binary.MaxVarintLen64) + binary.PutVarint(dsciGeneration, rr.DSCI.GetGeneration()) + + instanceGeneration := make([]byte, binary.MaxVarintLen64) + binary.PutVarint(instanceGeneration, rr.Instance.GetGeneration()) + + if _, err := hash.Write([]byte(rr.Instance.GetUID())); err != nil { + return nil, fmt.Errorf("failed to hash instance: %w", err) + } + if _, err := hash.Write(dsciGeneration); err != nil { + return nil, fmt.Errorf("failed to hash dsci generation: %w", err) + } + if _, err := hash.Write(instanceGeneration); err != nil { + return nil, fmt.Errorf("failed to hash instance generation: %w", err) + } + if _, err := hash.Write([]byte(rr.Release.Name)); err != nil { + return nil, fmt.Errorf("failed to hash release: %w", err) + } + if _, err := hash.Write([]byte(rr.Release.Version.String())); err != nil { + return nil, fmt.Errorf("failed to hash release: %w", err) + } + + for i := range rr.Manifests { + if _, err := hash.Write([]byte(rr.Manifests[i].String())); err != nil { + return nil, fmt.Errorf("failed to hash manifest: %w", err) + } + } + for i := range rr.Templates { + if _, err := hash.Write([]byte(rr.Templates[i].Path)); err != nil { + return nil, fmt.Errorf("failed to hash template: %w", err) + } + } + + return hash.Sum(nil), nil +} + +func HashStr(rr *ReconciliationRequest) (string, error) { + h, err := Hash(rr) + if err != nil { + return "", err + } + + return resources.EncodeToString(h), nil +} diff --git a/pkg/metadata/annotations/annotations.go b/pkg/metadata/annotations/annotations.go index 1ef2dc24ac2..a39de39bb9e 100644 --- a/pkg/metadata/annotations/annotations.go +++ b/pkg/metadata/annotations/annotations.go @@ -18,7 +18,8 @@ const ( const ManagementStateAnnotation = "component.opendatahub.io/management-state" const ( - ComponentGeneration = "components.opendatahub.io/generation" + ComponentGeneration = "components.opendatahub.io/component-generation" + ComponentHash = "components.opendatahub.io/component-hash" PlatformVersion = "platform.opendatahub.io/version" PlatformType = "platform.opendatahub.io/type" ) diff --git a/pkg/resources/resources.go b/pkg/resources/resources.go index b1144deb30a..a9df8414d13 100644 --- a/pkg/resources/resources.go +++ b/pkg/resources/resources.go @@ -3,6 +3,7 @@ package resources import ( "bytes" "crypto/sha256" + "encoding/base64" "errors" "fmt" "io" @@ -223,3 +224,7 @@ func Hash(in *unstructured.Unstructured) ([]byte, error) { return hasher.Sum(nil), nil } + +func EncodeToString(in []byte) string { + return "v" + base64.RawURLEncoding.EncodeToString(in) +} diff --git a/pkg/services/gc/gc.go b/pkg/services/gc/gc.go new file mode 100644 index 00000000000..fa85f709c95 --- /dev/null +++ b/pkg/services/gc/gc.go @@ -0,0 +1,341 @@ +package gc + +import ( + "context" + "fmt" + "slices" + "strings" + + "github.com/go-logr/logr" + "golang.org/x/exp/maps" + authorizationv1 "k8s.io/api/authorization/v1" + k8serr "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/discovery" + ctrlCli "sigs.k8s.io/controller-runtime/pkg/client" + logf "sigs.k8s.io/controller-runtime/pkg/log" + + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/client" +) + +// Instance a global instance of the GC service. +// +// TODO: since the GC service is quite heavy, as it has to discover +// +// resources that can be subject to GC, we share single global +// instance, however as long term, we should find a better way +// to let consumer of the service to access it. +var Instance *GC + +const ( + DeleteVerb = "delete" + AnyVerb = "*" + AnyResource = "*" +) + +type options struct { + propagationPolicy ctrlCli.PropagationPolicy + unremovables []schema.GroupVersionKind +} + +type OptsFn func(*options) + +func WithUnremovables(items ...schema.GroupVersionKind) OptsFn { + return func(o *options) { + o.unremovables = append(o.unremovables, items...) + } +} + +func WithPropagationPolicy(policy metav1.DeletionPropagation) OptsFn { + return func(o *options) { + o.propagationPolicy = ctrlCli.PropagationPolicy(policy) + } +} + +func New(cli *client.Client, ns string, opts ...OptsFn) *GC { + res := GC{ + client: cli, + ns: ns, + options: options{ + propagationPolicy: ctrlCli.PropagationPolicy(metav1.DeletePropagationForeground), + unremovables: make([]schema.GroupVersionKind, 0), + }, + + resources: Resources{ + items: make([]Resource, 0), + }, + } + + for _, o := range opts { + o(&res.options) + } + + return &res +} + +type GC struct { + client *client.Client + ns string + resources Resources + options options +} + +func (gc *GC) Start(ctx context.Context) error { + l := gc.log(ctx) + l.Info("Start computing deletable types") + + res, err := gc.computeDeletableTypes(ctx) + if err != nil { + return fmt.Errorf("cannot discover deletable types: %w", err) + } + + gc.resources.Set(res) + + l.Info("Deletable types computed", "count", gc.resources.Len()) + + return nil +} + +func (gc *GC) Run( + ctx context.Context, + selector labels.Selector, + predicate func(context.Context, unstructured.Unstructured) (bool, error), +) (int, error) { + l := gc.log(ctx) + + deleted := 0 + resources := gc.resources.Get() + + dc := gc.client.Dynamic() + lo := metav1.ListOptions{LabelSelector: selector.String()} + + for r := range resources { + items, err := dc.Resource(resources[r].GroupVersionResource()).Namespace("").List(ctx, lo) + if err != nil { + if k8serr.IsForbidden(err) { + l.Info( + "cannot list resource", + "reason", err.Error(), + "gvk", resources[r].GroupVersionKind(), + ) + + continue + } + + if k8serr.IsNotFound(err) { + continue + } + + return 0, fmt.Errorf("cannot list child resources %s: %w", resources[r].String(), err) + } + + for i := range items.Items { + ok, err := gc.delete(ctx, items.Items[i], predicate) + if err != nil { + return 0, err + } + + if ok { + deleted++ + } + } + } + + return deleted, nil +} + +func (gc *GC) delete( + ctx context.Context, + resource unstructured.Unstructured, + predicate func(context.Context, unstructured.Unstructured) (bool, error), +) (bool, error) { + if slices.Contains(gc.options.unremovables, resource.GroupVersionKind()) { + return false, nil + } + + canBeDeleted, err := predicate(ctx, resource) + if err != nil { + return false, err + } + + if !canBeDeleted { + return false, err + } + + gc.log(ctx).Info( + "delete", + "gvk", resource.GroupVersionKind(), + "ns", resource.GetNamespace(), + "name", resource.GetName(), + ) + + err = gc.client.Delete(ctx, &resource, gc.options.propagationPolicy) + if err != nil { + // The resource may have already been deleted + if k8serr.IsNotFound(err) { + return true, nil + } + + return false, fmt.Errorf( + "cannot delete resources gvk:%s, namespace: %s, name: %s, reason: %w", + resource.GroupVersionKind().String(), + resource.GetNamespace(), + resource.GetName(), + err, + ) + } + + return true, nil +} + +func (gc *GC) computeDeletableTypes( + ctx context.Context, +) ([]Resource, error) { + // We rely on the discovery API to retrieve all the resources GVK, + // that results in an unbounded set that can impact garbage collection + // latency when scaling up. + items, err := gc.client.Discovery().ServerPreferredNamespacedResources() + + // Swallow group discovery errors, e.g., Knative serving exposes + // an aggregated API for custom.metrics.k8s.io that requires special + // authentication scheme while discovering preferred resources. + if err != nil && !discovery.IsGroupDiscoveryFailedError(err) { + return nil, fmt.Errorf("failure retrieving supported namespaced resources: %w", err) + } + + // We only take types that support the "delete" verb, + // to prevents from performing queries that we know are going to + // return "MethodNotAllowed". + apiResourceLists := discovery.FilteredBy( + discovery.SupportsAllVerbs{ + Verbs: []string{DeleteVerb}, + }, + items, + ) + + // Get the permissions of the service account in the specified namespace. + rules, err := gc.retrieveResourceRules(ctx) + if err != nil { + return nil, fmt.Errorf("failure retiring resource rules: %w", err) + } + + // Collect deletable resources. + resources, err := gc.collectDeletableResources(apiResourceLists, rules) + if err != nil { + return nil, fmt.Errorf("failure retiring deletable resources: %w", err) + } + + return resources, nil +} + +func (gc *GC) retrieveResourceRules( + ctx context.Context, +) ([]authorizationv1.ResourceRule, error) { + // Retrieve the permissions granted to the operator service account. + // We assume the operator has only to garbage collect the resources + // it has created. + rulesReview := authorizationv1.SelfSubjectRulesReview{ + Spec: authorizationv1.SelfSubjectRulesReviewSpec{ + Namespace: gc.ns, + }, + } + + err := gc.client.Create(ctx, &rulesReview) + if err != nil { + return nil, fmt.Errorf("unable to create SelfSubjectRulesReviews: %w", err) + } + + return rulesReview.Status.ResourceRules, nil +} + +func (gc *GC) isResourceDeletable( + group string, + apiRes metav1.APIResource, + rules []authorizationv1.ResourceRule, +) bool { + for _, rule := range rules { + if !slices.Contains(rule.Verbs, DeleteVerb) && !slices.Contains(rule.Verbs, AnyVerb) { + continue + } + if !MatchRule(group, apiRes, rule) { + continue + } + + return true + } + + return false +} + +func (gc *GC) collectDeletableResources( + apiResourceLists []*metav1.APIResourceList, + rules []authorizationv1.ResourceRule, +) ([]Resource, error) { + resp := make(map[Resource]struct{}) + + for i := range apiResourceLists { + res := apiResourceLists[i] + + for _, apiRes := range res.APIResources { + resourceGroup := apiRes.Group + if resourceGroup == "" { + gv, err := schema.ParseGroupVersion(res.GroupVersion) + if err != nil { + return nil, fmt.Errorf("unable to parse group version: %w", err) + } + + resourceGroup = gv.Group + } + + if !gc.isResourceDeletable(resourceGroup, apiRes, rules) { + continue + } + + gv, err := schema.ParseGroupVersion(res.GroupVersion) + if err != nil { + return nil, err + } + + gvr := Resource{ + RESTMapping: meta.RESTMapping{ + Resource: schema.GroupVersionResource{ + Group: gv.Group, + Version: gv.Version, + Resource: apiRes.Name, + }, + GroupVersionKind: schema.GroupVersionKind{ + Group: gv.Group, + Version: gv.Version, + Kind: apiRes.Kind, + }, + Scope: meta.RESTScopeNamespace, + }, + } + + if !apiRes.Namespaced { + gvr.Scope = meta.RESTScopeRoot + } + + if slices.Contains(gc.options.unremovables, gvr.GroupVersionKind()) { + continue + } + + resp[gvr] = struct{}{} + } + } + + resources := maps.Keys(resp) + slices.SortFunc(resources, func(a, b Resource) int { + return strings.Compare(a.String(), b.String()) + }) + + return resources, nil +} + +func (gc *GC) log(ctx context.Context) logr.Logger { + return logf.FromContext(ctx).WithName("service").WithName("gc") +} diff --git a/pkg/services/gc/gc_support.go b/pkg/services/gc/gc_support.go new file mode 100644 index 00000000000..5b412dc9b88 --- /dev/null +++ b/pkg/services/gc/gc_support.go @@ -0,0 +1,78 @@ +package gc + +import ( + "sync" + + "golang.org/x/exp/slices" + authorizationv1 "k8s.io/api/authorization/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +type Resource struct { + meta.RESTMapping +} + +func (r Resource) GroupVersionResource() schema.GroupVersionResource { + return r.RESTMapping.Resource +} + +func (r Resource) GroupVersionKind() schema.GroupVersionKind { + return r.RESTMapping.GroupVersionKind +} + +func (r Resource) String() string { + return r.RESTMapping.Resource.String() +} + +func (r Resource) IsNamespaced() bool { + if r.Scope == nil { + return false + } + + return r.Scope.Name() == meta.RESTScopeNameNamespace +} + +// We may want to introduce iterators (https://pkg.go.dev/iter) once moved to go 1.23 + +type Resources struct { + lock sync.RWMutex + items []Resource +} + +func (r *Resources) Set(resources []Resource) { + r.lock.Lock() + defer r.lock.Unlock() + + r.items = resources +} +func (r *Resources) Get() []Resource { + r.lock.RLock() + defer r.lock.RUnlock() + + return slices.Clone(r.items) +} + +func (r *Resources) Len() int { + return len(r.items) +} + +func MatchRule(resourceGroup string, apiRes metav1.APIResource, rule authorizationv1.ResourceRule) bool { + for rgi := range rule.APIGroups { + // Check if the resource group matches the rule group or is a wildcard, if not + // discard it + if resourceGroup != rule.APIGroups[rgi] && rule.APIGroups[rgi] != AnyResource { + continue + } + + for ri := range rule.Resources { + // Check if the API resource name matches the rule resource or is a wildcard + if apiRes.Name == rule.Resources[ri] || rule.Resources[ri] == AnyResource { + return true + } + } + } + + return false +} diff --git a/pkg/services/gc/gc_test.go b/pkg/services/gc/gc_test.go new file mode 100644 index 00000000000..2419c2157ea --- /dev/null +++ b/pkg/services/gc/gc_test.go @@ -0,0 +1,98 @@ +package gc_test + +import ( + "testing" + + gTypes "github.com/onsi/gomega/types" + authorizationv1 "k8s.io/api/authorization/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/services/gc" + + . "github.com/onsi/gomega" +) + +func allVerb() []string { + return []string{"delete", "create", "get", "list", "patch"} +} + +func anyRule() authorizationv1.ResourceRule { + return authorizationv1.ResourceRule{ + Verbs: []string{gc.AnyVerb}, + APIGroups: []string{gc.AnyVerb}, + Resources: []string{gc.AnyVerb}, + } +} + +func TestMatchRules(t *testing.T) { + tests := []struct { + name string + resourceGroup string + apiResource metav1.APIResource + rule authorizationv1.ResourceRule + matcher gTypes.GomegaMatcher + }{ + // + // Positive Match + // + + { + name: "Should match", + resourceGroup: "", + apiResource: metav1.APIResource{ + Verbs: allVerb(), + }, + rule: anyRule(), + matcher: BeTrue(), + }, + { + name: "Should match as resource is explicitly listed", + resourceGroup: "unknown", + apiResource: metav1.APIResource{ + Name: "baz", + }, + rule: authorizationv1.ResourceRule{ + APIGroups: []string{gc.AnyResource}, + Resources: []string{"baz"}, + }, + matcher: BeTrue(), + }, + + // + // Negative Match + // + + { + name: "Should not match as API group is not listed", + resourceGroup: "unknown", + apiResource: metav1.APIResource{}, + rule: authorizationv1.ResourceRule{ + APIGroups: []string{"baz"}, + }, + matcher: BeFalse(), + }, + { + name: "Should not match as resource is not listed", + resourceGroup: "unknown", + apiResource: metav1.APIResource{}, + rule: authorizationv1.ResourceRule{ + Resources: []string{"baz"}, + }, + matcher: BeFalse(), + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + + g.Expect( + gc.MatchRule( + test.resourceGroup, + test.apiResource, + test.rule, + ), + ).To(test.matcher) + }) + } +} diff --git a/pkg/utils/test/fakeclient/fakeclient.go b/pkg/utils/test/fakeclient/fakeclient.go index d2e8115e8b6..8cbef38a411 100644 --- a/pkg/utils/test/fakeclient/fakeclient.go +++ b/pkg/utils/test/fakeclient/fakeclient.go @@ -1,21 +1,22 @@ package fakeclient import ( - "context" - appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + dynamicFake "k8s.io/client-go/dynamic/fake" + k8sFake "k8s.io/client-go/kubernetes/fake" ctrlClient "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/fake" + clientFake "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/resources" ) -func New(ctx context.Context, objs ...ctrlClient.Object) (*client.Client, error) { +func New(objs ...ctrlClient.Object) (*client.Client, error) { scheme := runtime.NewScheme() utilruntime.Must(corev1.AddToScheme(scheme)) utilruntime.Must(appsv1.AddToScheme(scheme)) @@ -26,13 +27,25 @@ func New(ctx context.Context, objs ...ctrlClient.Object) (*client.Client, error) fakeMapper.Add(gvk, meta.RESTScopeNamespace) } - return client.New( - ctx, - nil, - fake.NewClientBuilder(). + ro := make([]runtime.Object, len(objs)) + for i := range objs { + u, err := resources.ToUnstructured(objs[i]) + if err != nil { + return nil, err + } + + ro[i] = u + } + + c := client.New( + clientFake.NewClientBuilder(). WithScheme(scheme). WithRESTMapper(fakeMapper). WithObjects(objs...). Build(), + k8sFake.NewSimpleClientset(ro...), + dynamicFake.NewSimpleDynamicClient(scheme, ro...), ) + + return c, nil }