Address review comments

Makefile

@@ -283,13 +283,17 @@ install-acm-crds:
 	kubectl apply -f deploy/acm/crds/policy.open-cluster-management.io_policies_crd.yaml
 	kubectl apply -f deploy/acm/crds/policy.open-cluster-management.io_policyautomations_crd.yaml
 	kubectl apply -f deploy/acm/crds/cluster.open-cluster-management.io_managedclusters.yaml
+	kubectl apply -f deploy/acm/crds/view.open-cluster-management.io_managedclusterviews.yaml
+	kubectl apply -f deploy/acm/crds/action.open-cluster-management.io_managedclusteractions.yaml
 
 uninstall-acm-crds:
 	kubectl delete -f deploy/acm/crds/apps.open-cluster-management.io_placementrules_crd.yaml
 	kubectl delete -f deploy/acm/crds/policy.open-cluster-management.io_placementbindings_crd.yaml
 	kubectl delete -f deploy/acm/crds/policy.open-cluster-management.io_policies_crd.yaml
 	kubectl delete -f deploy/acm/crds/policy.open-cluster-management.io_policyautomations_crd.yaml
 	kubectl delete -f deploy/acm/crds/cluster.open-cluster-management.io_managedclusters.yaml
+	kubectl delete -f deploy/acm/crds/view.open-cluster-management.io_managedclusterviews.yaml
+	kubectl delete -f deploy/acm/crds/action.open-cluster-management.io_managedclusteractions.yaml
 
 kuttl-test: ## Run KUTTL tests
 	@echo "Running KUTTL tests"

api/v1alpha1/clustergroupupgrade_types.go

@@ -96,7 +96,7 @@ type ClusterGroupUpgradeSpec struct {
 	// Once set to true, the clusters start being upgraded, one batch at a time.
 	//+kubebuilder:default=true
 	Enable   *bool    `json:"enable,omitempty"`
-	Clusters []string `json:"clusters"`
+	Clusters []string `json:"clusters,omitempty"`
 	// This field holds a label common to multiple clusters that will be updated.
 	// The expected format is as follows:
 	// clusterSelector:
@@ -108,7 +108,7 @@ type ClusterGroupUpgradeSpec struct {
 	// All the clusters matching the labels specified in clusterSelector will be included
 	// in the update plan.
 	ClusterSelector     []string                 `json:"clusterSelector,omitempty"`
-	RemediationStrategy *RemediationStrategySpec `json:"remediationStrategy,omitempty"`
+	RemediationStrategy *RemediationStrategySpec `json:"remediationStrategy"`
 	ManagedPolicies     []string                 `json:"managedPolicies,omitempty"`
 	BlockingCRs         []BlockingCR             `json:"blockingCRs,omitempty"`
 	Actions             Actions                  `json:"actions,omitempty"`
@@ -167,6 +167,7 @@ type ClusterGroupUpgradeStatus struct {
 	ManagedPoliciesContent                map[string]string         `json:"managedPoliciesContent,omitempty"`
 	Status                                UpgradeStatus             `json:"status,omitempty"`
 	Precaching                            PrecachingStatus          `json:"precaching,omitempty"`
+	ComputedMaxConcurrency                int                       `json:"computedMaxConcurrency,omitempty"`
 }
 
 //+kubebuilder:object:root=true

config/crd/bases/ran.openshift.io_clustergroupupgrades.yaml

@@ -153,11 +153,13 @@ spec:
                 - maxConcurrency
                 type: object
             required:
-            - clusters
+            - remediationStrategy
             type: object
           status:
             description: ClusterGroupUpgradeStatus defines the observed state of ClusterGroupUpgrade
             properties:
+              computedMaxConcurrency:
+                type: integer
               conditions:
                 items:
                   description: "Condition contains details for one aspect of the current

controllers/clustergroupupgrade_controller.go

@@ -456,11 +456,12 @@ func (r *ClusterGroupUpgradeReconciler) remediateCurrentBatch(
 func (r *ClusterGroupUpgradeReconciler) approveInstallPlan(
 	ctx context.Context, clusterGroupUpgrade *ranv1alpha1.ClusterGroupUpgrade,
 	managedPolicyIndex int, clusterName string) (bool, error) {
-	managedPolicyName := clusterGroupUpgrade.Spec.ManagedPolicies[managedPolicyIndex]
+	managedPolicyName := clusterGroupUpgrade.Status.ManagedPoliciesForUpgrade[managedPolicyIndex].Name
 
 	// If there is no content saved for the current managed policy, return.
 	_, ok := clusterGroupUpgrade.Status.ManagedPoliciesContent[managedPolicyName]
 	if ok == false {
+		r.Log.Info("[approveInstallPlan] No content for policy", "managedPolicyName", managedPolicyName)
 		return false, nil
 	}
 
@@ -1487,7 +1488,7 @@ func (r *ClusterGroupUpgradeReconciler) buildRemediationPlan(
 			clusterCount++
 		}
 
-		if clusterCount == clusterGroupUpgrade.Spec.RemediationStrategy.MaxConcurrency || i == len(allClustersForUpgrade)-1 {
+		if clusterCount == clusterGroupUpgrade.Status.ComputedMaxConcurrency || i == len(allClustersForUpgrade)-1 {
 			if len(batch) > 0 {
 				remediationPlan = append(remediationPlan, batch)
 				clusterCount = 0
@@ -1675,15 +1676,24 @@ func (r *ClusterGroupUpgradeReconciler) validateCR(ctx context.Context, clusterG
 		}
 	}
 
-	// Automatically adjust maxConcurrency to the min of maxConcurrency, number of clusters, 100.
-	newMaxConcurrency := utils.GetMinOf3(
-		clusterGroupUpgrade.Spec.RemediationStrategy.MaxConcurrency,
-		len(clusters),
-		utils.MaxNumberOfClustersForUpgrade)
+	var newMaxConcurrency int
+	// Automatically adjust maxConcurrency to the min of maxConcurrency, number of clusters, 100 or
+	// to the min of number of clusters, 100 if maxConcurrency is set to 0.
+	if clusterGroupUpgrade.Spec.RemediationStrategy.MaxConcurrency > 0 {
+		newMaxConcurrency = utils.GetMinOf3(
+			clusterGroupUpgrade.Spec.RemediationStrategy.MaxConcurrency,
+			len(clusters),
+			utils.MaxNumberOfClustersForUpgrade)
+	} else {
+		newMaxConcurrency = len(clusters)
+		if utils.MaxNumberOfClustersForUpgrade < len(clusters) {
+			newMaxConcurrency = utils.MaxNumberOfClustersForUpgrade
+		}
+	}
 
-	if newMaxConcurrency != clusterGroupUpgrade.Spec.RemediationStrategy.MaxConcurrency {
-		clusterGroupUpgrade.Spec.RemediationStrategy.MaxConcurrency = newMaxConcurrency
-		err = r.Client.Update(ctx, clusterGroupUpgrade)
+	if newMaxConcurrency != clusterGroupUpgrade.Status.ComputedMaxConcurrency {
+		clusterGroupUpgrade.Status.ComputedMaxConcurrency = newMaxConcurrency
+		err = r.Status().Update(ctx, clusterGroupUpgrade)
 		if err != nil {
 			r.Log.Info("Error updating Cluster Group Upgrade")
 			return reconcile, err

controllers/utils/multicloud_util_test.go

@@ -21,28 +21,21 @@ import (
 	"fmt"
 	"testing"
 
-	//"github.com/go-logr/logr"
 	policiesv1 "github.com/open-cluster-management/governance-policy-propagator/api/v1"
 	ranv1alpha1 "github.com/openshift-kni/cluster-group-upgrades-operator/api/v1alpha1"
 	"github.com/stretchr/testify/assert"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 
-	//"k8s.io/apimachinery/pkg/api/errors"
 	corev1 "k8s.io/api/core/v1"
-	//"k8s.io/apimachinery/pkg/api/errors"
 
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
-	//"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/scheme"
-	//clusterv1 "open-cluster-management.io/api/cluster/v1"
-	//ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
-	//"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	actionv1beta1 "github.com/open-cluster-management/multicloud-operators-foundation/pkg/apis/action/v1beta1"
 	viewv1beta1 "github.com/open-cluster-management/multicloud-operators-foundation/pkg/apis/view/v1beta1"
 	operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"

deploy/upgrades/cluster-selector/cgu-cluster-selector.yaml

@@ -18,7 +18,6 @@ spec:
   - spoke2
   - spoke5
   - spoke6
-  remediationAction: inform
   remediationStrategy:
     maxConcurrency: 2
 

deploy/upgrades/skip-compliant-policies/skip-compliant-policies.yaml

@@ -15,7 +15,6 @@ spec:
   - spoke2
   - spoke5
   - spoke6
-  remediationAction: inform
   remediationStrategy:
     maxConcurrency: 4
 

kuttl-test.yaml

@@ -12,7 +12,7 @@ testDirs:
 crdDir: ./config/crd/bases/
 # The operator has a reconcile loop of 5 minutes, so we need to wait at least that amount of time
 # before deciding to error.
-timeout: 350
+timeout: 375
 parallel: 1
 namespace: default
 

tests/kuttl/tests/adjust-max-concurrency/00-assert.yaml

@@ -11,9 +11,10 @@ spec:
   - policy1-common-cluster-version-policy
   - policy2-common-pao-sub-policy
   remediationStrategy:
-    maxConcurrency: 1
+    maxConcurrency: 3
     timeout: 240
 status:
+  computedMaxConcurrency: 1
   conditions:
   - message: The ClusterGroupUpgrade CR is not enabled
     reason: UpgradeNotStarted

tests/kuttl/tests/skip-compliant-policies/00-assert.yaml

@@ -19,6 +19,7 @@ spec:
     maxConcurrency: 4
     timeout: 240
 status:
+  computedMaxConcurrency: 4
   conditions:
   - message: The ClusterGroupUpgrade CR has upgrade policies that are still non compliant
     reason: UpgradeNotCompleted
@@ -30,13 +31,21 @@ status:
   - cgu-policy4-common-sriov-sub-policy
   managedPoliciesCompliantBeforeUpgrade:
   - policy2-common-pao-sub-policy
+  managedPoliciesContent:
+    policy1-common-cluster-version-policy: "null"
+    policy3-common-ptp-sub-policy: '[{"kind":"Subscription","name":"ptp-operator-subscription","namespace":"openshift-ptp"}]'
+    policy4-common-sriov-sub-policy: '[{"kind":"Subscription","name":"sriov-network-operator-subscription","namespace":"openshift-sriov-network-operator"}]'
   managedPoliciesForUpgrade:
   - name: policy1-common-cluster-version-policy
     namespace: default
   - name: policy3-common-ptp-sub-policy
     namespace: default
   - name: policy4-common-sriov-sub-policy
     namespace: default
+  managedPoliciesNs:
+    policy1-common-cluster-version-policy: default
+    policy3-common-ptp-sub-policy: default
+    policy4-common-sriov-sub-policy: default
   placementBindings:
   - cgu-policy1-common-cluster-version-policy
   - cgu-policy3-common-ptp-sub-policy