From d6707cf25de4141851e10270e59bc2f056a5669b Mon Sep 17 00:00:00 2001
From: Lukasz Raczylo <lukasz@raczylo.com>
Date: Fri, 10 Jan 2025 15:15:20 +0000
Subject: [PATCH] fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup!
 fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! General improvements

---
 docker-image-worker/s3_utils.py               |  2 +-
 .../clusterimageexport_controller.go          | 21 ++++++++++++-------
 internal/shared/jobs.go                       |  2 +-
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/docker-image-worker/s3_utils.py b/docker-image-worker/s3_utils.py
index cb037f9..0da8f78 100644
--- a/docker-image-worker/s3_utils.py
+++ b/docker-image-worker/s3_utils.py
@@ -20,7 +20,7 @@ def get_s3_client(use_role=False, role_name=None, use_current_role=False, aws_ac
     
     if endpoint_url:
         client_kwargs['endpoint_url'] = endpoint_url
-    elif region:
+    if region:
         client_kwargs['region_name'] = region
 
     # Check for AWS Web Identity token
diff --git a/internal/controller/raczylo.com/clusterimageexport_controller.go b/internal/controller/raczylo.com/clusterimageexport_controller.go
index cf3b8de..457d51e 100644
--- a/internal/controller/raczylo.com/clusterimageexport_controller.go
+++ b/internal/controller/raczylo.com/clusterimageexport_controller.go
@@ -44,7 +44,7 @@ func (r *ClusterImageExportReconciler) InjectPodAnnotations(annotations map[stri
 // +kubebuilder:rbac:groups=batch,resources=cronjobs,verbs=get;list;watch
 // +kubebuilder:rbac:groups="",resources=pods,verbs=get;list;watch;create;update;patch;delete
 
-const clusterImageExportFinalizer = "finalizer.clusterimageexport.raczylo.com"
+const clusterImageExportFinalizer = "raczylo.com/clusterimageexport-finalizer"
 
 func (r *ClusterImageExportReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	l := log.FromContext(ctx)
@@ -308,7 +308,7 @@ func (r *ClusterImageExportReconciler) handleDeletion(ctx context.Context, clust
 			// Continue with deletion even if cleanup fails
 		}
 
-		// Check if cleanup job already exists
+		// Delete existing cleanup job if it exists
 		jobName := "cleanup-" + shared.NormalizeImageName(clusterImageExport.Name)
 		existingJob := &batchv1.Job{}
 		err := r.Get(ctx, client.ObjectKey{
@@ -317,16 +317,21 @@ func (r *ClusterImageExportReconciler) handleDeletion(ctx context.Context, clust
 		}, existingJob)
 
 		if err == nil {
-			// Job exists, don't create a new one
-			l.Info("Cleanup job already exists, skipping creation", "job", jobName)
-		} else if errors.IsNotFound(err) {
-			// Job doesn't exist, create it
-			r.runCleanupJob(ctx, clusterImageExport)
-		} else {
+			// Job exists, delete it
+			if err := r.Delete(ctx, existingJob); err != nil && !errors.IsNotFound(err) {
+				l.Error(err, "Failed to delete existing cleanup job")
+				// Continue with deletion even if cleanup job deletion fails
+			} else {
+				l.Info("Successfully deleted existing cleanup job", "job", jobName)
+			}
+		} else if !errors.IsNotFound(err) {
 			// Unexpected error, log but continue
 			l.Error(err, "Error checking for existing cleanup job")
 		}
 
+		// Create new cleanup job
+		r.runCleanupJob(ctx, clusterImageExport)
+
 		// Remove the finalizer regardless of cleanup job status
 		controllerutil.RemoveFinalizer(clusterImageExport, clusterImageExportFinalizer)
 		if err := r.Update(ctx, clusterImageExport); err != nil {
diff --git a/internal/shared/jobs.go b/internal/shared/jobs.go
index 8cd4307..45cfdb2 100644
--- a/internal/shared/jobs.go
+++ b/internal/shared/jobs.go
@@ -118,7 +118,7 @@ func SetupS3Params(s3Config raczylocomv1.ClusterImageStorageS3) []string {
 		params = append(params, fmt.Sprintf("--endpoint_url='%s'", s3Config.Endpoint))
 	}
 	if s3Config.Region != "" {
-		params = append(params, fmt.Sprintf("--region=%s", s3Config.Region))
+		params = append(params, fmt.Sprintf("--region='%s'", s3Config.Region))
 	}
 	return params
 }