From 9ace7a6286eb4b49bf2fb2132f2831feba7f593d Mon Sep 17 00:00:00 2001
From: Prashanth Dintyala <vdintyala@nvidia.com>
Date: Thu, 26 Aug 2021 23:46:33 -0700
Subject: [PATCH] operator: allow seamless upgrade from 0.6 to 0.8

Signed-off-by: Prashanth Dintyala <vdintyala@nvidia.com>
---
 operator/pkg/client/api.go                    |  7 +++
 .../pkg/controllers/cluster_controller.go     | 50 ++++++++++++++++++-
 operator/pkg/resources/cmn/rbac.go            |  4 +-
 3 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/operator/pkg/client/api.go b/operator/pkg/client/api.go
index 7e2596d8..f3dd22b2 100644
--- a/operator/pkg/client/api.go
+++ b/operator/pkg/client/api.go
@@ -11,6 +11,7 @@ import (
 
 	apiv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -87,6 +88,12 @@ func (c *K8sClient) GetPodByName(ctx context.Context, name types.NamespacedName)
 	return pod, err
 }
 
+func (c *K8sClient) GetRoleByName(ctx context.Context, name types.NamespacedName) (*rbacv1.Role, error) {
+	role := &rbacv1.Role{}
+	err := c.Get(ctx, name, role)
+	return role, err
+}
+
 ////////////////////////////////////////
 //      create/update resources      //
 //////////////////////////////////////
diff --git a/operator/pkg/controllers/cluster_controller.go b/operator/pkg/controllers/cluster_controller.go
index d3545c34..c4b9bf7c 100644
--- a/operator/pkg/controllers/cluster_controller.go
+++ b/operator/pkg/controllers/cluster_controller.go
@@ -12,7 +12,9 @@ import (
 
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -314,6 +316,12 @@ func (r *AIStoreReconciler) bootstrapNew(ctx context.Context, ais *aisv1.AIStore
 // 2. Similarly, check the resource state for targets and ensure the state matches the reconciler request.
 // 3. If both proxy and target daemons have expected state, keep requeuing the event until all the pods are ready.
 func (r *AIStoreReconciler) handleCREvents(ctx context.Context, ais *aisv1.AIStore) (result ctrl.Result, err error) {
+	// Ensure correct RBAC resources exists
+	err = r.createRBACResources(ctx, ais)
+	if err != nil {
+		return r.manageError(ctx, ais, aisv1.RBACManagementError, err)
+	}
+
 	var proxyReady, targetReady bool
 	if proxyReady, err = r.handleProxyState(ctx, ais); err != nil {
 		return
@@ -341,6 +349,32 @@ requeue:
 	return
 }
 
+func (r *AIStoreReconciler) patchRole(ctx context.Context, ais *aisv1.AIStore, role *rbacv1.Role) error {
+	sliceContains := func(keys []string, e string) bool {
+		for _, v := range keys {
+			if v == e {
+				return true
+			}
+		}
+		return false
+	}
+	existingRole, err := r.client.GetRoleByName(ctx, types.NamespacedName{Namespace: role.Namespace, Name: role.Name})
+	if err != nil {
+		r.recordError(ais, err, "Failed to fetch Role")
+		return err
+	}
+
+	for _, rule := range existingRole.Rules {
+		if sliceContains(rule.Resources, cmn.ResourceTypePodsExec) {
+			return nil
+		}
+	}
+	if err = r.client.UpdateIfExists(ctx, role); err != nil {
+		r.recordError(ais, err, "Failed updating Role")
+	}
+	return err
+}
+
 func (r *AIStoreReconciler) createRBACResources(ctx context.Context, ais *aisv1.AIStore) (err error) {
 	// 1. Create service account if not exists
 	sa := cmn.NewAISServiceAccount(ais)
@@ -350,12 +384,24 @@ func (r *AIStoreReconciler) createRBACResources(ctx context.Context, ais *aisv1.
 	}
 
 	// 2. Create AIS Role
-	role := cmn.NewAISRBACRole(ais)
-	if _, err = r.client.CreateResourceIfNotExists(ctx, nil, role); err != nil {
+	var (
+		role   = cmn.NewAISRBACRole(ais)
+		exists bool
+	)
+
+	if exists, err = r.client.CreateResourceIfNotExists(ctx, nil, role); err != nil {
 		r.recordError(ais, err, "Failed to create Role")
 		return
 	}
 
+	// If the role already exists, ensure it has `pods/exec`.
+	if exists {
+		err = r.patchRole(ctx, ais, role)
+		if err != nil {
+			return
+		}
+	}
+
 	// 3. Create binding for the Role
 	rb := cmn.NewAISRBACRoleBinding(ais)
 	if _, err = r.client.CreateResourceIfNotExists(ctx, nil, rb); err != nil {
diff --git a/operator/pkg/resources/cmn/rbac.go b/operator/pkg/resources/cmn/rbac.go
index 48eeb81d..2d4038bd 100644
--- a/operator/pkg/resources/cmn/rbac.go
+++ b/operator/pkg/resources/cmn/rbac.go
@@ -13,11 +13,11 @@ import (
 )
 
 const (
+	ResourceTypePodsExec    = "pods/exec"
 	resourceTypeStatfulSets = "statefulsets"
 	resourceTypeDaemonSets  = "daemonsets"
 	resourceTypeNodes       = "nodes"
 	resourceTypePodLogs     = "pods/log"
-	resourceTypePodsExec    = "pods/exec"
 
 	verbAll = "*"
 
@@ -59,7 +59,7 @@ func NewAISRBACRole(ais *aisv1.AIStore) *rbacv1.Role {
 				Resources: []string{
 					string(corev1.ResourceSecrets), string(corev1.ResourcePods),
 					string(corev1.ResourceConfigMaps), string(corev1.ResourceServices),
-					resourceTypeStatfulSets, resourceTypeDaemonSets, resourceTypePodsExec,
+					resourceTypeStatfulSets, resourceTypeDaemonSets, ResourceTypePodsExec,
 				},
 				Verbs: []string{verbAll}, // TODO: set only required permissions
 			},