Skip to content

Commit

Permalink
Implement garbage collection action
Browse files Browse the repository at this point in the history
Signed-off-by: Luca Burgazzoli <lburgazzoli@gmail.com>
  • Loading branch information
lburgazzoli committed Nov 20, 2024
1 parent 74b3e85 commit dcfd87a
Show file tree
Hide file tree
Showing 37 changed files with 1,239 additions and 107 deletions.
2 changes: 2 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ linters-settings:
# also allow generics
- generic
- EventHandler # for ToOwner
- discovery.DiscoveryInterface
- dynamic.Interface
- predicate.Predicate
- client.Object
revive:
Expand Down
9 changes: 7 additions & 2 deletions controllers/components/dashboard/dashboard_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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),
)).
Expand All @@ -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 {
Expand Down
8 changes: 6 additions & 2 deletions controllers/components/kueue/kueue_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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),
)).
Expand All @@ -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 {
Expand Down
21 changes: 18 additions & 3 deletions controllers/components/modelregistry/modelregistry_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand All @@ -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),
)).
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 6 additions & 2 deletions controllers/components/ray/ray_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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),
)).
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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),
)).
Expand All @@ -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 {
Expand Down
8 changes: 6 additions & 2 deletions controllers/components/trustyai/trustyai_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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),
)).
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion controllers/dscinitialization/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand Down
23 changes: 22 additions & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 7 additions & 0 deletions pkg/cluster/gvk/gvk.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -174,4 +175,10 @@ var (
Version: "v1",
Kind: "ServiceMeshMember",
}

Lease = schema.GroupVersionKind{
Group: coordinationv1.SchemeGroupVersion.Group,
Version: coordinationv1.SchemeGroupVersion.Version,
Kind: "Lease",
}
)
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
20 changes: 15 additions & 5 deletions pkg/controller/actions/deploy/action_deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Check warning on line 119 in pkg/controller/actions/deploy/action_deploy.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/actions/deploy/action_deploy.go#L118-L119

Added lines #L118 - L119 were not covered by tests

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)
}
Expand All @@ -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 {
Expand All @@ -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()
Expand Down
Loading

0 comments on commit dcfd87a

Please sign in to comment.