From e322b3118555a16fc6ba7bbec08b0549265479e3 Mon Sep 17 00:00:00 2001 From: Tomer Figenblat Date: Tue, 12 Nov 2024 19:42:30 -0500 Subject: [PATCH 1/6] chore: cleanup nim integration tech preview resources Signed-off-by: Tomer Figenblat --- pkg/upgrade/upgrade.go | 46 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/pkg/upgrade/upgrade.go b/pkg/upgrade/upgrade.go index e1dabb407a4..7a2078d7b63 100644 --- a/pkg/upgrade/upgrade.go +++ b/pkg/upgrade/upgrade.go @@ -8,11 +8,14 @@ import ( "fmt" "reflect" + batchv1 "k8s.io/api/batch/v1" + "github.com/hashicorp/go-multierror" operatorv1 "github.com/openshift/api/operator/v1" routev1 "github.com/openshift/api/route/v1" monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" appsv1 "k8s.io/api/apps/v1" + batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -287,6 +290,9 @@ func CleanupExistingResource(ctx context.Context, toDelete := getDashboardWatsonResources(dscApplicationsNamespace) multiErr = multierror.Append(multiErr, deleteResources(ctx, cli, &toDelete)) + // cleanup nvidia nim integration remove tech preview + multiErr = multierror.Append(multiErr, cleanupNimIntegrationTechPreview(ctx, cli, oldReleaseVersion)) + return multiErr.ErrorOrNil() } @@ -603,3 +609,43 @@ func GetDeployedRelease(ctx context.Context, cli client.Client) (cluster.Release // could be a clean installation or both CRs are deleted already return cluster.Release{}, nil } + +func cleanupNimIntegrationTechPreview(ctx context.Context, cli client.Client, oldRelease cluster.Release) error { + logger := logf.FromContext(ctx) + var errs *multierror.Error + + // TODO where can we get the system namespace, opendatahub | redhat-ods-applications ? + ns := "todo-get-namespace" + + if oldRelease.Version.Minor < 16 { + cm := &corev1.ConfigMap{} + if err := cli.Get(ctx, types.NamespacedName{Name: "nvidia-nim-validation-result", Namespace: ns}, cm); err != nil { + if !k8serr.IsNotFound(err) { + logger.V(1).Error(err, "failed to fetch tech preview validation result configmap") + } + } else { + if dErr := cli.Delete(ctx, cm); dErr != nil { + logger.Error(dErr, "failed to remove tech preview validation result configmap") + errs = multierror.Append(errs, dErr) + } else { + logger.V(1).Info("tech preview validation result configmap successfully removed") + } + } + + job := &batchv1.CronJob{} + if err := cli.Get(ctx, types.NamespacedName{Name: "nvidia-nim-periodic-validator", Namespace: ns}, job); err != nil { + if !k8serr.IsNotFound(err) { + logger.V(1).Error(err, "failed to fetch tech preview validation result configmap") + } + } else { + if dErr := cli.Delete(ctx, job); dErr != nil { + logger.Error(dErr, "failed to remove tech preview cron job") + errs = multierror.Append(errs, dErr) + } else { + logger.Info("tech preview cron job successfully removed") + } + } + } + + return errs.ErrorOrNil() +} From 85077d093a2207b7e43343a3104bfcf0fb4de349 Mon Sep 17 00:00:00 2001 From: Tomer Figenblat Date: Wed, 13 Nov 2024 08:30:18 -0500 Subject: [PATCH 2/6] chore: apply suggestions from code review Co-authored-by: Wen Zhou --- pkg/upgrade/upgrade.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/upgrade/upgrade.go b/pkg/upgrade/upgrade.go index 7a2078d7b63..71126e08554 100644 --- a/pkg/upgrade/upgrade.go +++ b/pkg/upgrade/upgrade.go @@ -291,7 +291,7 @@ func CleanupExistingResource(ctx context.Context, multiErr = multierror.Append(multiErr, deleteResources(ctx, cli, &toDelete)) // cleanup nvidia nim integration remove tech preview - multiErr = multierror.Append(multiErr, cleanupNimIntegrationTechPreview(ctx, cli, oldReleaseVersion)) + multiErr = multierror.Append(multiErr, cleanupNimIntegrationTechPreview(ctx, cli, oldReleaseVersion,dscApplicationsNamespace)) return multiErr.ErrorOrNil() } @@ -610,16 +610,16 @@ func GetDeployedRelease(ctx context.Context, cli client.Client) (cluster.Release return cluster.Release{}, nil } -func cleanupNimIntegrationTechPreview(ctx context.Context, cli client.Client, oldRelease cluster.Release) error { +func cleanupNimIntegrationTechPreview(ctx context.Context, cli client.Client, oldRelease cluster.Release, applicationNS string) error { logger := logf.FromContext(ctx) var errs *multierror.Error - // TODO where can we get the system namespace, opendatahub | redhat-ods-applications ? - ns := "todo-get-namespace" - if oldRelease.Version.Minor < 16 { + if oldRelease.Version.Minor == 15 { + nimConfigMap = "nvidia-nim-validation-result" + nimCronjob = "nvidia-nim-periodic-validator" cm := &corev1.ConfigMap{} - if err := cli.Get(ctx, types.NamespacedName{Name: "nvidia-nim-validation-result", Namespace: ns}, cm); err != nil { + if err := cli.Get(ctx, types.NamespacedName{Name: nimConfigMap, Namespace: applicationNS}, cm); err != nil { if !k8serr.IsNotFound(err) { logger.V(1).Error(err, "failed to fetch tech preview validation result configmap") } @@ -633,7 +633,7 @@ func cleanupNimIntegrationTechPreview(ctx context.Context, cli client.Client, ol } job := &batchv1.CronJob{} - if err := cli.Get(ctx, types.NamespacedName{Name: "nvidia-nim-periodic-validator", Namespace: ns}, job); err != nil { + if err := cli.Get(ctx, types.NamespacedName{Name: nimCronjob, Namespace: applicationNS}, job); err != nil { if !k8serr.IsNotFound(err) { logger.V(1).Error(err, "failed to fetch tech preview validation result configmap") } From 837fee033c89e21d2b7463d1aca7d2ba3813f902 Mon Sep 17 00:00:00 2001 From: Tomer Figenblat Date: Wed, 13 Nov 2024 08:37:07 -0500 Subject: [PATCH 3/6] chore: addressed pr reviews, better logging Signed-off-by: Tomer Figenblat --- pkg/upgrade/upgrade.go | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/pkg/upgrade/upgrade.go b/pkg/upgrade/upgrade.go index 71126e08554..ea0df9f2970 100644 --- a/pkg/upgrade/upgrade.go +++ b/pkg/upgrade/upgrade.go @@ -15,7 +15,6 @@ import ( routev1 "github.com/openshift/api/route/v1" monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" appsv1 "k8s.io/api/apps/v1" - batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -291,7 +290,7 @@ func CleanupExistingResource(ctx context.Context, multiErr = multierror.Append(multiErr, deleteResources(ctx, cli, &toDelete)) // cleanup nvidia nim integration remove tech preview - multiErr = multierror.Append(multiErr, cleanupNimIntegrationTechPreview(ctx, cli, oldReleaseVersion,dscApplicationsNamespace)) + multiErr = multierror.Append(multiErr, cleanupNimIntegrationTechPreview(ctx, cli, oldReleaseVersion, dscApplicationsNamespace)) return multiErr.ErrorOrNil() } @@ -614,35 +613,34 @@ func cleanupNimIntegrationTechPreview(ctx context.Context, cli client.Client, ol logger := logf.FromContext(ctx) var errs *multierror.Error - if oldRelease.Version.Minor == 15 { - nimConfigMap = "nvidia-nim-validation-result" - nimCronjob = "nvidia-nim-periodic-validator" + nimConfigMap := "nvidia-nim-validation-result" + nimCronjob := "nvidia-nim-periodic-validator" cm := &corev1.ConfigMap{} - if err := cli.Get(ctx, types.NamespacedName{Name: nimConfigMap, Namespace: applicationNS}, cm); err != nil { + if err := cli.Get(ctx, types.NamespacedName{Name: nimConfigMap, Namespace: applicationNS}, cm); err != nil { if !k8serr.IsNotFound(err) { - logger.V(1).Error(err, "failed to fetch tech preview validation result configmap") + logger.V(1).Error(err, "failed to get NIM configmap "+nimConfigMap) } } else { if dErr := cli.Delete(ctx, cm); dErr != nil { - logger.Error(dErr, "failed to remove tech preview validation result configmap") + logger.Error(dErr, "failed to remove NIM configmap "+nimConfigMap) errs = multierror.Append(errs, dErr) } else { - logger.V(1).Info("tech preview validation result configmap successfully removed") + logger.V(1).Info("removed NIM configmap successfully") } } job := &batchv1.CronJob{} if err := cli.Get(ctx, types.NamespacedName{Name: nimCronjob, Namespace: applicationNS}, job); err != nil { if !k8serr.IsNotFound(err) { - logger.V(1).Error(err, "failed to fetch tech preview validation result configmap") + logger.V(1).Error(err, "failed to get NIM cronjob "+nimCronjob) } } else { if dErr := cli.Delete(ctx, job); dErr != nil { - logger.Error(dErr, "failed to remove tech preview cron job") + logger.Error(dErr, "failed to remove NIM cronjob "+nimCronjob) errs = multierror.Append(errs, dErr) } else { - logger.Info("tech preview cron job successfully removed") + logger.Info("removed NIM cronjob successfully") } } } From e808e82e9cccb98a0ef3515e5b51427c562421cf Mon Sep 17 00:00:00 2001 From: Tomer Figenblat Date: Wed, 13 Nov 2024 08:49:53 -0500 Subject: [PATCH 4/6] chore: set nim cleanup for minors 14 and 15 Signed-off-by: Tomer Figenblat --- pkg/upgrade/upgrade.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/upgrade/upgrade.go b/pkg/upgrade/upgrade.go index ea0df9f2970..3bf9207cb93 100644 --- a/pkg/upgrade/upgrade.go +++ b/pkg/upgrade/upgrade.go @@ -8,13 +8,12 @@ import ( "fmt" "reflect" - batchv1 "k8s.io/api/batch/v1" - "github.com/hashicorp/go-multierror" operatorv1 "github.com/openshift/api/operator/v1" routev1 "github.com/openshift/api/route/v1" monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" appsv1 "k8s.io/api/apps/v1" + batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -610,12 +609,13 @@ func GetDeployedRelease(ctx context.Context, cli client.Client) (cluster.Release } func cleanupNimIntegrationTechPreview(ctx context.Context, cli client.Client, oldRelease cluster.Release, applicationNS string) error { - logger := logf.FromContext(ctx) var errs *multierror.Error - if oldRelease.Version.Minor == 15 { + if oldRelease.Version.Minor >= 14 && oldRelease.Version.Minor <= 15 { + logger := logf.FromContext(ctx) nimConfigMap := "nvidia-nim-validation-result" nimCronjob := "nvidia-nim-periodic-validator" + cm := &corev1.ConfigMap{} if err := cli.Get(ctx, types.NamespacedName{Name: nimConfigMap, Namespace: applicationNS}, cm); err != nil { if !k8serr.IsNotFound(err) { From 0483894b856ac71e7ff3154ecc37f35f46ae1364 Mon Sep 17 00:00:00 2001 From: Tomer Figenblat Date: Wed, 13 Nov 2024 09:32:21 -0500 Subject: [PATCH 5/6] chore: nim tp cleanup should remove api key secret Signed-off-by: Tomer Figenblat --- pkg/upgrade/upgrade.go | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/pkg/upgrade/upgrade.go b/pkg/upgrade/upgrade.go index 3bf9207cb93..4c1a817d500 100644 --- a/pkg/upgrade/upgrade.go +++ b/pkg/upgrade/upgrade.go @@ -613,8 +613,23 @@ func cleanupNimIntegrationTechPreview(ctx context.Context, cli client.Client, ol if oldRelease.Version.Minor >= 14 && oldRelease.Version.Minor <= 15 { logger := logf.FromContext(ctx) - nimConfigMap := "nvidia-nim-validation-result" nimCronjob := "nvidia-nim-periodic-validator" + nimConfigMap := "nvidia-nim-validation-result" + nimAPISec := "nvidia-nim-access" + + job := &batchv1.CronJob{} + if err := cli.Get(ctx, types.NamespacedName{Name: nimCronjob, Namespace: applicationNS}, job); err != nil { + if !k8serr.IsNotFound(err) { + logger.V(1).Error(err, "failed to get NIM cronjob "+nimCronjob) + } + } else { + if dErr := cli.Delete(ctx, job); dErr != nil { + logger.Error(dErr, "failed to remove NIM cronjob "+nimCronjob) + errs = multierror.Append(errs, dErr) + } else { + logger.Info("removed NIM cronjob successfully") + } + } cm := &corev1.ConfigMap{} if err := cli.Get(ctx, types.NamespacedName{Name: nimConfigMap, Namespace: applicationNS}, cm); err != nil { @@ -630,17 +645,17 @@ func cleanupNimIntegrationTechPreview(ctx context.Context, cli client.Client, ol } } - job := &batchv1.CronJob{} - if err := cli.Get(ctx, types.NamespacedName{Name: nimCronjob, Namespace: applicationNS}, job); err != nil { + sec := &corev1.Secret{} + if err := cli.Get(ctx, types.NamespacedName{Name: nimAPISec, Namespace: applicationNS}, sec); err != nil { if !k8serr.IsNotFound(err) { - logger.V(1).Error(err, "failed to get NIM cronjob "+nimCronjob) + logger.V(1).Error(err, "failed to get NIM API key secret "+nimAPISec) } } else { - if dErr := cli.Delete(ctx, job); dErr != nil { - logger.Error(dErr, "failed to remove NIM cronjob "+nimCronjob) + if dErr := cli.Delete(ctx, sec); dErr != nil { + logger.Error(dErr, "failed to remove NIM API key secret "+nimAPISec) errs = multierror.Append(errs, dErr) } else { - logger.Info("removed NIM cronjob successfully") + logger.V(1).Info("removed NIM API key secret successfully") } } } From 610ea2e16fe15e42a163648bec5a7529d06438bb Mon Sep 17 00:00:00 2001 From: Tomer Figenblat Date: Wed, 13 Nov 2024 10:43:09 -0500 Subject: [PATCH 6/6] chore: addresed pr reviews, rewrite cleanup func Signed-off-by: Tomer Figenblat --- pkg/upgrade/upgrade.go | 70 +++++++++++++++++++----------------------- 1 file changed, 32 insertions(+), 38 deletions(-) diff --git a/pkg/upgrade/upgrade.go b/pkg/upgrade/upgrade.go index 4c1a817d500..48c7fd17068 100644 --- a/pkg/upgrade/upgrade.go +++ b/pkg/upgrade/upgrade.go @@ -612,50 +612,44 @@ func cleanupNimIntegrationTechPreview(ctx context.Context, cli client.Client, ol var errs *multierror.Error if oldRelease.Version.Minor >= 14 && oldRelease.Version.Minor <= 15 { - logger := logf.FromContext(ctx) + log := logf.FromContext(ctx) nimCronjob := "nvidia-nim-periodic-validator" nimConfigMap := "nvidia-nim-validation-result" nimAPISec := "nvidia-nim-access" - job := &batchv1.CronJob{} - if err := cli.Get(ctx, types.NamespacedName{Name: nimCronjob, Namespace: applicationNS}, job); err != nil { - if !k8serr.IsNotFound(err) { - logger.V(1).Error(err, "failed to get NIM cronjob "+nimCronjob) - } - } else { - if dErr := cli.Delete(ctx, job); dErr != nil { - logger.Error(dErr, "failed to remove NIM cronjob "+nimCronjob) - errs = multierror.Append(errs, dErr) - } else { - logger.Info("removed NIM cronjob successfully") - } - } - - cm := &corev1.ConfigMap{} - if err := cli.Get(ctx, types.NamespacedName{Name: nimConfigMap, Namespace: applicationNS}, cm); err != nil { - if !k8serr.IsNotFound(err) { - logger.V(1).Error(err, "failed to get NIM configmap "+nimConfigMap) - } - } else { - if dErr := cli.Delete(ctx, cm); dErr != nil { - logger.Error(dErr, "failed to remove NIM configmap "+nimConfigMap) - errs = multierror.Append(errs, dErr) - } else { - logger.V(1).Info("removed NIM configmap successfully") - } + deleteObjs := []struct { + obj client.Object + name, desc string + }{ + { + obj: &batchv1.CronJob{}, + name: nimCronjob, + desc: "validator CronJob", + }, + { + obj: &corev1.ConfigMap{}, + name: nimConfigMap, + desc: "data ConfigMap", + }, + { + obj: &corev1.Secret{}, + name: nimAPISec, + desc: "API key Secret", + }, } - - sec := &corev1.Secret{} - if err := cli.Get(ctx, types.NamespacedName{Name: nimAPISec, Namespace: applicationNS}, sec); err != nil { - if !k8serr.IsNotFound(err) { - logger.V(1).Error(err, "failed to get NIM API key secret "+nimAPISec) - } - } else { - if dErr := cli.Delete(ctx, sec); dErr != nil { - logger.Error(dErr, "failed to remove NIM API key secret "+nimAPISec) - errs = multierror.Append(errs, dErr) + for _, delObj := range deleteObjs { + if gErr := cli.Get(ctx, types.NamespacedName{Name: delObj.name, Namespace: applicationNS}, delObj.obj); gErr != nil { + if !k8serr.IsNotFound(gErr) { + log.V(1).Error(gErr, fmt.Sprintf("failed to get NIM %s %s", delObj.desc, delObj.name)) + errs = multierror.Append(errs, gErr) + } } else { - logger.V(1).Info("removed NIM API key secret successfully") + if dErr := cli.Delete(ctx, delObj.obj); dErr != nil { + log.Error(dErr, fmt.Sprintf("failed to remove NIM %s %s", delObj.desc, delObj.name)) + errs = multierror.Append(errs, dErr) + } else { + log.Info(fmt.Sprintf("removed NIM %s successfully", delObj.desc)) + } } } }