From ecc99a2e95ee0acec4b12b3a84fdb117db3d4662 Mon Sep 17 00:00:00 2001
From: Mulham Raee <mraee@redhat.com>
Date: Mon, 25 Sep 2023 17:42:19 +0200
Subject: [PATCH] adressed code review

---
 .../rosacontrolplane_controller.go            | 20 ++++++++++++++-----
 feature/feature.go                            | 12 +++++------
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/controlplane/rosa/controllers/rosacontrolplane_controller.go b/controlplane/rosa/controllers/rosacontrolplane_controller.go
index 2df6546948..ed80c1dcfa 100644
--- a/controlplane/rosa/controllers/rosacontrolplane_controller.go
+++ b/controlplane/rosa/controllers/rosacontrolplane_controller.go
@@ -18,6 +18,7 @@ package controllers
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"strings"
@@ -140,8 +141,8 @@ func (r *ROSAControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.Req
 
 	// Always close the scope
 	defer func() {
-		if err := rosaScope.Close(); err != nil && reterr == nil {
-			reterr = err
+		if err := rosaScope.Close(); err != nil {
+			reterr = errors.Join(reterr, err)
 		}
 	}()
 
@@ -298,7 +299,11 @@ func (r *ROSAControlPlaneReconciler) reconcileNormal(ctx context.Context, rosaSc
 	if err != nil {
 		return ctrl.Result{}, fmt.Errorf("failed to build connection: %w", err)
 	}
-	defer connection.Close()
+	defer func() {
+		if err := connection.Close(); err != nil {
+			reterr = errors.Join(reterr, err)
+		}
+	}()
 
 	log := logger.FromContext(ctx)
 	cluster, err := connection.ClustersMgmt().V1().Clusters().
@@ -317,7 +322,7 @@ func (r *ROSAControlPlaneReconciler) reconcileNormal(ctx context.Context, rosaSc
 	return ctrl.Result{}, nil
 }
 
-func (r *ROSAControlPlaneReconciler) reconcileDelete(_ context.Context, rosaScope *scope.ROSAControlPlaneScope) (ctrl.Result, error) {
+func (r *ROSAControlPlaneReconciler) reconcileDelete(_ context.Context, rosaScope *scope.ROSAControlPlaneScope) (res ctrl.Result, reterr error) {
 	// log := logger.FromContext(ctx)
 
 	rosaScope.Info("Reconciling ROSAControlPlane delete")
@@ -331,6 +336,7 @@ func (r *ROSAControlPlaneReconciler) reconcileDelete(_ context.Context, rosaScop
 	}
 
 	// Create the connection, and remember to close it:
+	// TODO: token should be read from a secret: https://github.com/kubernetes-sigs/cluster-api-provider-aws/issues/4460
 	token := os.Getenv("OCM_TOKEN")
 	connection, err := sdk.NewConnectionBuilder().
 		Logger(ocmLogger).
@@ -340,7 +346,11 @@ func (r *ROSAControlPlaneReconciler) reconcileDelete(_ context.Context, rosaScop
 	if err != nil {
 		return ctrl.Result{}, fmt.Errorf("failed to build connection: %w", err)
 	}
-	defer connection.Close()
+	defer func() {
+		if err := connection.Close(); err != nil {
+			reterr = errors.Join(reterr, err)
+		}
+	}()
 
 	cluster, err := r.getOcmCluster(rosaScope, connection)
 	if err != nil {
diff --git a/feature/feature.go b/feature/feature.go
index 176ccbbcf1..8180138e2b 100644
--- a/feature/feature.go
+++ b/feature/feature.go
@@ -28,11 +28,6 @@ const (
 	// // alpha: v1.X
 	// MyFeature featuregate.Feature = "MyFeature".
 
-	// ROSA is used to enable ROSA support
-	// owner: @enxebre
-	// alpha: v2.2
-	ROSA featuregate.Feature = "ROSA"
-
 	// EKS is used to enable EKS support
 	// owner: @richardcase
 	// alpha: v0.4
@@ -85,6 +80,11 @@ const (
 	// owner: @skarlso
 	// alpha: v2.0
 	TagUnmanagedNetworkResources featuregate.Feature = "TagUnmanagedNetworkResources"
+
+	// ROSA is used to enable ROSA support
+	// owner: @enxebre
+	// alpha: v2.2
+	ROSA featuregate.Feature = "ROSA"
 )
 
 func init() {
@@ -95,7 +95,6 @@ func init() {
 // To add a new feature, define a key for it above and add it here.
 var defaultCAPAFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
 	// Every feature should be initiated here:
-	ROSA:                          {Default: false, PreRelease: featuregate.Alpha},
 	EKS:                           {Default: true, PreRelease: featuregate.Beta},
 	EKSEnableIAM:                  {Default: false, PreRelease: featuregate.Beta},
 	EKSAllowAddRoles:              {Default: false, PreRelease: featuregate.Beta},
@@ -107,4 +106,5 @@ var defaultCAPAFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
 	ExternalResourceGC:            {Default: false, PreRelease: featuregate.Alpha},
 	AlternativeGCStrategy:         {Default: false, PreRelease: featuregate.Alpha},
 	TagUnmanagedNetworkResources:  {Default: true, PreRelease: featuregate.Alpha},
+	ROSA:                          {Default: false, PreRelease: featuregate.Alpha},
 }