Move the GetSpaceCountFromSpaceProvisionerConfigs helper function into

a test package and simplify how the SpaceCountGetter is obtained.
codeready-toolchain · Dec 6, 2024 · 5972be2 · 5972be2
1 parent 087b3bd
commit 5972be2
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 61 deletions.
diff --git a/controllers/usersignup/approval_test.go b/controllers/usersignup/approval_test.go
@@ -40,7 +40,7 @@ func TestGetClusterIfApproved(t *testing.T) {
 		fakeClient := commontest.NewFakeClient(t, toolchainConfig, spc1, spc2)
 
 		// when
-		approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, capacity.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
+		approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, hspc.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
 
 		// then
 		require.NoError(t, err)
@@ -60,7 +60,7 @@ func TestGetClusterIfApproved(t *testing.T) {
 		fakeClient := commontest.NewFakeClient(t, toolchainConfig, spc1, spc2)
 
 		// when
-		approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, capacity.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
+		approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, hspc.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
 
 		// then
 		require.NoError(t, err)
@@ -124,7 +124,7 @@ func TestGetClusterIfApproved(t *testing.T) {
 			fakeClient := commontest.NewFakeClient(t, toolchainConfig, spc1)
 
 			// when
-			approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, capacity.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
+			approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, hspc.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
 
 			// then
 			if testFields.ErrorExpected {
@@ -151,7 +151,7 @@ func TestGetClusterIfApproved(t *testing.T) {
 		fakeClient := commontest.NewFakeClient(t, toolchainConfig, spc1, spc2)
 
 		// when
-		approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, capacity.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
+		approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, hspc.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
 
 		// then
 		require.NoError(t, err)
@@ -167,7 +167,7 @@ func TestGetClusterIfApproved(t *testing.T) {
 		fakeClient := commontest.NewFakeClient(t, toolchainConfig)
 
 		// when
-		approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, capacity.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
+		approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, hspc.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
 
 		// then
 		require.NoError(t, err)
@@ -182,7 +182,7 @@ func TestGetClusterIfApproved(t *testing.T) {
 		fakeClient := commontest.NewFakeClient(t, commonconfig.NewToolchainConfigObjWithReset(t), spc1, spc2)
 
 		// when
-		approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, capacity.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
+		approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, hspc.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
 
 		// then
 		require.NoError(t, err)
@@ -197,7 +197,7 @@ func TestGetClusterIfApproved(t *testing.T) {
 		fakeClient := commontest.NewFakeClient(t, spc1, spc2)
 
 		// when
-		approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, capacity.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
+		approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, hspc.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
 
 		// then
 		require.NoError(t, err)
@@ -213,7 +213,7 @@ func TestGetClusterIfApproved(t *testing.T) {
 		signup := commonsignup.NewUserSignup(commonsignup.ApprovedManually())
 
 		// when
-		approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, capacity.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
+		approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, hspc.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
 
 		// then
 		require.NoError(t, err)
@@ -229,7 +229,7 @@ func TestGetClusterIfApproved(t *testing.T) {
 		signup := commonsignup.NewUserSignup(commonsignup.ApprovedManually())
 
 		// when
-		approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, capacity.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
+		approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, hspc.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
 
 		// then
 		require.NoError(t, err)
@@ -245,7 +245,7 @@ func TestGetClusterIfApproved(t *testing.T) {
 		signup := commonsignup.NewUserSignup(commonsignup.ApprovedManually())
 
 		// when
-		approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, capacity.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
+		approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, hspc.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
 
 		// then
 		require.NoError(t, err)
@@ -261,7 +261,7 @@ func TestGetClusterIfApproved(t *testing.T) {
 		signup := commonsignup.NewUserSignup(commonsignup.ApprovedManually(), commonsignup.WithTargetCluster("member1"))
 
 		// when
-		approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, capacity.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
+		approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, hspc.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
 
 		// then
 		require.NoError(t, err)
@@ -278,7 +278,7 @@ func TestGetClusterIfApproved(t *testing.T) {
 			}
 
 			// when
-			approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, capacity.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
+			approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, hspc.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
 
 			// then
 			require.EqualError(t, err, "unable to get ToolchainConfig: some error")
@@ -297,7 +297,7 @@ func TestGetClusterIfApproved(t *testing.T) {
 			}
 
 			// when
-			approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, capacity.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
+			approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient, hspc.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)))
 
 			// then
 			require.EqualError(t, err, "unable to get the optimal target cluster: failed to find the optimal space provisioner config: some error")

diff --git a/controllers/usersignup/usersignup_controller_test.go b/controllers/usersignup/usersignup_controller_test.go
@@ -4081,7 +4081,7 @@ func prepareReconcile(t *testing.T, name string, initObjs ...runtimeclient.Objec
 			Client: fakeClient,
 		},
 		Scheme:         s,
-		ClusterManager: capacity.NewClusterManager(test.HostOperatorNs, fakeClient, capacity.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)),
+		ClusterManager: capacity.NewClusterManager(test.HostOperatorNs, fakeClient, hspc.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, test.HostOperatorNs)),
 		SegmentClient:  segment.NewClient(segmenttest.NewClient()),
 	}
 	return r, newReconcileRequest(name), fakeClient

diff --git a/pkg/capacity/manager.go b/pkg/capacity/manager.go
@@ -9,7 +9,6 @@ import (
 	"github.com/codeready-toolchain/host-operator/pkg/counter"
 	"github.com/codeready-toolchain/toolchain-common/pkg/cluster"
 	"github.com/codeready-toolchain/toolchain-common/pkg/condition"
-	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -75,6 +74,13 @@ func DefaultClusterManager(namespace string, cl runtimeclient.Client) *ClusterMa
 	return NewClusterManager(namespace, cl, nil)
 }
 
+// NewClusterManager constructs a new cluster manager for the namespace using the provided client.
+// If the `getSpaceCount` is nil, the default behavior is to find the space count in the counts cache.
+//
+// In tests this can be replaced with
+// github.com/codeready-toolchain/host-operator/test/spaceprovisionerconfig.GetSpaceCountFromSpaceProvisionerConfigs
+// function that reads the counts solely from the configured SPCs so that the counts cache and ToolchainStatus
+// doesn't have to be constructed and initialized making the setup of the tests slightly easier.
 func NewClusterManager(namespace string, cl runtimeclient.Client, getSpaceCount SpaceCountGetter) *ClusterManager {
 	return &ClusterManager{
 		namespace:     namespace,
@@ -90,45 +96,6 @@ type ClusterManager struct {
 	lastUsed      string
 }
 
-// GetSpaceCountFromSpaceProvisionerConfigs is a function that returns the space count as it is stored in the SpaceProvisionerConfig
-// objects in the provided namespace.
-//
-// NOTE: THIS FUNCTION SHOULD ONLY BE USED IN UNIT TESTS TO SIMPLIFY THEIR SETUP.
-func GetSpaceCountFromSpaceProvisionerConfigs(cl runtimeclient.Client, namespace string) SpaceCountGetter {
-	return func(ctx context.Context, clusterName string) (int, bool) {
-		l := &toolchainv1alpha1.SpaceProvisionerConfigList{}
-		if err := cl.List(context.TODO(), l, runtimeclient.InNamespace(namespace)); err != nil {
-			log.FromContext(ctx).Error(err, "failed to list the SpaceProvisionerConfig objects while computing figuring out the space count stored in them")
-			return 0, false
-		}
-
-		for _, spc := range l.Items {
-			if spc.Spec.ToolchainCluster == clusterName {
-				if spc.Status.ConsumedCapacity == nil {
-					return 0, false
-				}
-				return spc.Status.ConsumedCapacity.SpaceCount, true
-			}
-		}
-		return 0, false
-	}
-}
-
-// GetSpaceCountFromCountsCache is the default function used by the ClusterManager to obtain
-// the space count from the counter cache. When no specific function is supplied to
-// constructor of the ClusterManager, this function is used instead.
-func GetSpaceCountFromCountsCache() (SpaceCountGetter, error) {
-	counts, err := counter.GetCounts()
-	if err != nil {
-		return nil, err
-	}
-
-	return func(ctx context.Context, clusterName string) (int, bool) {
-		count, ok := counts.SpacesPerClusterCounts[clusterName]
-		return count, ok
-	}, nil
-}
-
 // OptimalTargetClusterFilter is used by GetOptimalTargetCluster
 // in order to retrieve an "optimal" cluster for the Space to be provisioned into.
 type OptimalTargetClusterFilter struct {
@@ -215,7 +182,15 @@ func (b *ClusterManager) getSpaceCountGetter() (SpaceCountGetter, error) {
 		return b.getSpaceCount, nil
 	}
 
-	return GetSpaceCountFromCountsCache()
+	counts, err := counter.GetCounts()
+	if err != nil {
+		return nil, err
+	}
+
+	return func(_ context.Context, clusterName string) (int, bool) {
+		count, ok := counts.SpacesPerClusterCounts[clusterName]
+		return count, ok
+	}, nil
 }
 
 func matches(spc *toolchainv1alpha1.SpaceProvisionerConfig, predicates []spaceProvisionerConfigPredicate) bool {

diff --git a/pkg/capacity/manager_test.go b/pkg/capacity/manager_test.go
@@ -88,7 +88,7 @@ func TestGetOptimalTargetCluster(t *testing.T) {
 				spc1 := buildSpaceProvisionerConfig("member1", m1_valid, m1_has_role)
 				spc2 := buildSpaceProvisionerConfig("member2", m2_valid, m2_has_role)
 				fakeClient := commontest.NewFakeClient(t, spc1, spc2)
-				cm := capacity.NewClusterManager(commontest.HostOperatorNs, fakeClient, capacity.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, commontest.HostOperatorNs))
+				cm := capacity.NewClusterManager(commontest.HostOperatorNs, fakeClient, hspc.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, commontest.HostOperatorNs))
 
 				// when
 				clusterName, err := cm.GetOptimalTargetCluster(ctx, capacity.OptimalTargetClusterFilter{
@@ -107,7 +107,7 @@ func TestGetOptimalTargetCluster(t *testing.T) {
 		// given
 		spaceProvisionerConfig := hspc.NewEnabledValidTenantSPC("member1")
 		fakeClient := commontest.NewFakeClient(t, spaceProvisionerConfig)
-		cm := capacity.NewClusterManager(commontest.HostOperatorNs, fakeClient, capacity.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, commontest.HostOperatorNs))
+		cm := capacity.NewClusterManager(commontest.HostOperatorNs, fakeClient, hspc.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, commontest.HostOperatorNs))
 
 		// when
 		clusterName, err := cm.GetOptimalTargetCluster(ctx, capacity.OptimalTargetClusterFilter{})
@@ -124,7 +124,7 @@ func TestGetOptimalTargetCluster(t *testing.T) {
 		spc3 := hspc.NewEnabledValidTenantSPC("member3", spc.MaxNumberOfSpaces(1000), spc.WithConsumedSpaceCount(200))
 
 		fakeClient := commontest.NewFakeClient(t, spc1, spc2, spc3)
-		cm := capacity.NewClusterManager(commontest.HostOperatorNs, fakeClient, capacity.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, commontest.HostOperatorNs))
+		cm := capacity.NewClusterManager(commontest.HostOperatorNs, fakeClient, hspc.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, commontest.HostOperatorNs))
 
 		// when
 		clusterName, err := cm.GetOptimalTargetCluster(ctx, capacity.OptimalTargetClusterFilter{})
@@ -140,7 +140,7 @@ func TestGetOptimalTargetCluster(t *testing.T) {
 		spc2 := hspc.NewEnabledValidTenantSPC("member2", spc.MaxNumberOfSpaces(1000), spc.WithConsumedSpaceCount(700))
 		spc3 := hspc.NewEnabledValidTenantSPC("member3", spc.MaxNumberOfSpaces(1000), spc.WithConsumedSpaceCount(200))
 		fakeClient := commontest.NewFakeClient(t, spc1, spc2, spc3)
-		cm := capacity.NewClusterManager(commontest.HostOperatorNs, fakeClient, capacity.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, commontest.HostOperatorNs))
+		cm := capacity.NewClusterManager(commontest.HostOperatorNs, fakeClient, hspc.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, commontest.HostOperatorNs))
 
 		// when
 		clusterName, err := cm.GetOptimalTargetCluster(ctx, capacity.OptimalTargetClusterFilter{})
@@ -155,7 +155,7 @@ func TestGetOptimalTargetCluster(t *testing.T) {
 		spc1 := hspc.NewEnabledValidTenantSPC("member1", spc.MaxNumberOfSpaces(1000), spc.WithConsumedSpaceCount(700))
 		spc2 := hspc.NewEnabledValidTenantSPC("member2", spc.MaxNumberOfSpaces(1000), spc.WithConsumedSpaceCount(500))
 		fakeClient := commontest.NewFakeClient(t, spc1, spc2)
-		cm := capacity.NewClusterManager(commontest.HostOperatorNs, fakeClient, capacity.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, commontest.HostOperatorNs))
+		cm := capacity.NewClusterManager(commontest.HostOperatorNs, fakeClient, hspc.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, commontest.HostOperatorNs))
 
 		// when
 		clusterName, err := cm.GetOptimalTargetCluster(ctx, capacity.OptimalTargetClusterFilter{
@@ -253,7 +253,7 @@ func TestGetOptimalTargetClusterInBatchesBy50WhenTwoClusterHaveTheSameUsage(t *t
 						spc.WithConsumedMemoryUsagePercentInNode("master", 50))
 
 					fakeClient := commontest.NewFakeClient(t, spc1, spc2, spc3)
-					clusterBalancer := capacity.NewClusterManager(commontest.HostOperatorNs, fakeClient, capacity.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, commontest.HostOperatorNs))
+					clusterBalancer := capacity.NewClusterManager(commontest.HostOperatorNs, fakeClient, hspc.GetSpaceCountFromSpaceProvisionerConfigs(fakeClient, commontest.HostOperatorNs))
 
 					// now run in 4 cycles and expect that the users will be provisioned in batches of 50
 					member2CurrentCount := numberOfSpaces

diff --git a/test/spaceprovisionerconfig/util.go b/test/spaceprovisionerconfig/util.go
@@ -1,11 +1,38 @@
 package spaceprovisionerconfig
 
 import (
+	"context"
+
+	"github.com/charmbracelet/log"
 	toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
+	"github.com/codeready-toolchain/host-operator/pkg/capacity"
 	"github.com/codeready-toolchain/toolchain-common/pkg/test"
 	. "github.com/codeready-toolchain/toolchain-common/pkg/test/spaceprovisionerconfig"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+// GetSpaceCountFromSpaceProvisionerConfigs is a function that returns the space count as it is stored in the SpaceProvisionerConfig
+// objects in the provided namespace. The returned function can be used as an argument to capacity.NewClusterManager().
+func GetSpaceCountFromSpaceProvisionerConfigs(cl client.Client, namespace string) capacity.SpaceCountGetter {
+	return func(ctx context.Context, clusterName string) (int, bool) {
+		l := &toolchainv1alpha1.SpaceProvisionerConfigList{}
+		if err := cl.List(ctx, l, client.InNamespace(namespace)); err != nil {
+			log.FromContext(ctx).Error(err, "failed to list the SpaceProvisionerConfig objects while computing figuring out the space count stored in them")
+			return 0, false
+		}
+
+		for _, spc := range l.Items {
+			if spc.Spec.ToolchainCluster == clusterName {
+				if spc.Status.ConsumedCapacity == nil {
+					return 0, false
+				}
+				return spc.Status.ConsumedCapacity.SpaceCount, true
+			}
+		}
+		return 0, false
+	}
+}
+
 func NewEnabledValidTenantSPC(referencedToolchainCluster string, opts ...CreateOption) *toolchainv1alpha1.SpaceProvisionerConfig {
 	return NewSpaceProvisionerConfig(referencedToolchainCluster+"Spc", test.HostOperatorNs,
 		append(opts,