Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement garbage collection action #1374

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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),
lburgazzoli marked this conversation as resolved.
Show resolved Hide resolved
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),
zdtsw marked this conversation as resolved.
Show resolved Hide resolved
)

err = mgr.Add(gc.Instance)
if err != nil {
setupLog.Error(err, "unable to register GC service")
lburgazzoli marked this conversation as resolved.
Show resolved Hide resolved
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 @@

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 @@
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 @@

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