Skip to content

Commit

Permalink
KSPACE-46: Use SpaceProvisionerConfig for space placement decisions (#…
Browse files Browse the repository at this point in the history 
…991)
  • Loading branch information
@metlos
metlos authored Mar 22, 2024
1 parent 547d334 commit de95481
Show file tree
Hide file tree
Showing 14 changed files with 571 additions and 470 deletions.
11 changes: 6 additions & 5 deletions config/crd/bases/toolchain.dev.openshift.com_toolchainconfigs.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,19 +53,20 @@ spec:
type: boolean
type: object
capacityThresholds:
description: Keeps parameters necessary for configuring capacity
limits
description: 'Keeps parameters necessary for configuring capacity
limits Deprecated: This is no longer used for anything.'
properties:
maxNumberOfSpacesPerMemberCluster:
additionalProperties:
type: integer
description: Contains a map of maximal number of spaces that
description: 'Contains a map of maximal number of spaces that
can be provisioned per member cluster mapped by the cluster
name
name Deprecated: This is no longer used for anything.'
type: object
x-kubernetes-map-type: atomic
resourceCapacityThreshold:
description: Contains capacity threshold configuration
description: 'Contains capacity threshold configuration Deprecated:
This is no longer used for anything.'
properties:
defaultThreshold:
description: It is the default capacity threshold (in
Expand Down
15 changes: 2 additions & 13 deletions controllers/deactivation/deactivation_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,7 @@ const (
)

func TestReconcile(t *testing.T) {
config := commonconfig.NewToolchainConfigObjWithReset(t, testconfig.CapacityThresholds().MaxNumberOfSpaces(
testconfig.PerMemberCluster("member1", 321)),
testconfig.Deactivation().DeactivatingNotificationDays(3))
config := commonconfig.NewToolchainConfigObjWithReset(t, testconfig.Deactivation().DeactivatingNotificationDays(3))

// given
logf.SetLogger(zap.New(zap.UseDevMode(true)))
Expand All @@ -65,7 +63,6 @@ func TestReconcile(t *testing.T) {
states.SetDeactivating(userSignupFoobar, true)

t.Run("controller should not deactivate user", func(t *testing.T) {

// the time since the mur was provisioned is within the deactivation timeout period for the 'deactivate30' tier
t.Run("usersignup should not be deactivated - deactivate30 (30 days)", func(t *testing.T) {
// given
Expand Down Expand Up @@ -137,10 +134,7 @@ func TestReconcile(t *testing.T) {
// a user that belongs to the deactivation domain excluded list
t.Run("user deactivation excluded", func(t *testing.T) {
// given
config := commonconfig.NewToolchainConfigObjWithReset(t, testconfig.CapacityThresholds().MaxNumberOfSpaces(
testconfig.PerMemberCluster("member1", 321)),
testconfig.Deactivation().DeactivatingNotificationDays(3),
)
config := commonconfig.NewToolchainConfigObjWithReset(t, testconfig.Deactivation().DeactivatingNotificationDays(3))
restore := commontest.SetEnvVarAndRestore(t, "HOST_OPERATOR_DEACTIVATION_DOMAINS_EXCLUDED", "@redhat.com")
defer restore()
murProvisionedTime := &metav1.Time{Time: time.Now().Add(-time.Duration(expectedDeactivationTimeoutDeactivate30Tier*24) * time.Hour)}
Expand All @@ -155,11 +149,9 @@ func TestReconcile(t *testing.T) {
require.Equal(t, time.Duration(0), res.RequeueAfter, "requeueAfter should not be set")
assertThatUserSignupDeactivated(t, cl, username, false)
})

})
// in these tests, the controller should (eventually) deactivate the user
t.Run("controller should deactivate user", func(t *testing.T) {

userSignupFoobar := userSignupWithEmail(username, "[email protected]")
t.Run("usersignup should be marked as deactivating - deactivate30 (30 days)", func(t *testing.T) {
// given
Expand Down Expand Up @@ -288,7 +280,6 @@ func TestReconcile(t *testing.T) {
assertThatUserSignupDeactivated(t, cl, username, true)
AssertMetricsCounterEquals(t, 1, metrics.UserSignupAutoDeactivatedTotal)
})

})

t.Run("test usersignup deactivating state reset to false", func(t *testing.T) {
Expand Down Expand Up @@ -355,7 +346,6 @@ func TestReconcile(t *testing.T) {
})

t.Run("failures", func(t *testing.T) {

// cannot find UserTier
t.Run("unable to get UserTier", func(t *testing.T) {
// given
Expand Down Expand Up @@ -433,7 +423,6 @@ func TestReconcile(t *testing.T) {
assertThatUserSignupDeactivated(t, cl, username, false)
})
})

}

func prepareReconcile(t *testing.T, name string, initObjs ...runtime.Object) (reconcile.Reconciler, reconcile.Request, *commontest.FakeClient) {
Expand Down
51 changes: 26 additions & 25 deletions controllers/spacecompletion/space_completion_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package spacecompletion_test

import (
"context"
"errors"
"fmt"
"os"
"testing"
Expand All @@ -12,28 +13,26 @@ import (
"github.com/codeready-toolchain/host-operator/pkg/capacity"
"github.com/codeready-toolchain/host-operator/pkg/counter"
. "github.com/codeready-toolchain/host-operator/test"
"github.com/codeready-toolchain/toolchain-common/pkg/cluster"
"github.com/codeready-toolchain/toolchain-common/pkg/configuration"
hspc "github.com/codeready-toolchain/host-operator/test/spaceprovisionerconfig"
"github.com/codeready-toolchain/toolchain-common/pkg/test"
spacetest "github.com/codeready-toolchain/toolchain-common/pkg/test/space"

"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes/scheme"
runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

func TestCreateSpace(t *testing.T) {
member1 := NewMemberClusterWithTenantRole(t, "member1", corev1.ConditionTrue)
getMemberClusters := NewGetMemberClusters(member1)
spc := hspc.NewEnabledValidTenantSPC("member1")

t.Run("without any field set - then it sets only tierName", func(t *testing.T) {
// given
space := spacetest.NewSpace(test.HostOperatorNs, "without-fields",
spacetest.WithTierName(""))
r, req, cl := prepareReconcile(t, space, getMemberClusters)
r, req, cl := prepareReconcile(t, space, spc)

// when
_, err := r.Reconcile(context.TODO(), req)
Expand All @@ -49,7 +48,7 @@ func TestCreateSpace(t *testing.T) {
// given
space := spacetest.NewSpace(test.HostOperatorNs, "without-targetCluster",
spacetest.WithTierName("advanced"))
r, req, cl := prepareReconcile(t, space, getMemberClusters)
r, req, cl := prepareReconcile(t, space, spc)

// when
_, err := r.Reconcile(context.TODO(), req)
Expand All @@ -66,7 +65,7 @@ func TestCreateSpace(t *testing.T) {
space := spacetest.NewSpace(test.HostOperatorNs, "without-tierName",
spacetest.WithTierName(""),
spacetest.WithSpecTargetCluster("member2"))
r, req, cl := prepareReconcile(t, space, getMemberClusters)
r, req, cl := prepareReconcile(t, space, spc)

// when
_, err := r.Reconcile(context.TODO(), req)
Expand All @@ -84,7 +83,7 @@ func TestCreateSpace(t *testing.T) {
space := spacetest.NewSpace(test.HostOperatorNs, "with-fields",
spacetest.WithTierName("advanced"),
spacetest.WithSpecTargetCluster("member2"))
r, req, cl := prepareReconcile(t, space, getMemberClusters)
r, req, cl := prepareReconcile(t, space, spc)

// when
_, err := r.Reconcile(context.TODO(), req)
Expand All @@ -101,7 +100,7 @@ func TestCreateSpace(t *testing.T) {
space := spacetest.NewSpace(test.HostOperatorNs, "without-fields",
spacetest.WithTierName(""),
spacetest.WithDeletionTimestamp())
r, req, cl := prepareReconcile(t, space, getMemberClusters)
r, req, cl := prepareReconcile(t, space, spc)

// when
_, err := r.Reconcile(context.TODO(), req)
Expand All @@ -117,7 +116,7 @@ func TestCreateSpace(t *testing.T) {
// given
space := spacetest.NewSpace(test.HostOperatorNs, "without-members",
spacetest.WithTierName("advanced"))
r, req, cl := prepareReconcile(t, space, NewGetMemberClusters())
r, req, cl := prepareReconcile(t, space, nil)

// when
_, err := r.Reconcile(context.TODO(), req)
Expand All @@ -133,7 +132,7 @@ func TestCreateSpace(t *testing.T) {
// given
space := spacetest.NewSpace(test.HostOperatorNs, "not-found",
spacetest.WithTierName("advanced"))
r, req, _ := prepareReconcile(t, space, NewGetMemberClusters())
r, req, _ := prepareReconcile(t, space, nil)
empty := test.NewFakeClient(t)
empty.MockUpdate = func(ctx context.Context, obj runtimeclient.Object, opts ...runtimeclient.UpdateOption) error {
return fmt.Errorf("shouldn't be called")
Expand All @@ -151,7 +150,7 @@ func TestCreateSpace(t *testing.T) {
// given
space := spacetest.NewSpace(test.HostOperatorNs, "get-fails",
spacetest.WithTierName("advanced"))
r, req, cl := prepareReconcile(t, space, NewGetMemberClusters())
r, req, cl := prepareReconcile(t, space, nil)
cl.MockGet = func(ctx context.Context, key runtimeclient.ObjectKey, obj runtimeclient.Object, opts ...runtimeclient.GetOption) error {
return fmt.Errorf("some error")
}
Expand All @@ -171,7 +170,7 @@ func TestCreateSpace(t *testing.T) {
// given
space := spacetest.NewSpace(test.HostOperatorNs, "oddity",
spacetest.WithTierName(""))
r, req, cl := prepareReconcile(t, space, NewGetMemberClusters())
r, req, cl := prepareReconcile(t, space, nil)
cl.MockGet = func(ctx context.Context, key runtimeclient.ObjectKey, obj runtimeclient.Object, opts ...runtimeclient.GetOption) error {
if key.Name == "config" {
return fmt.Errorf("some error")
Expand All @@ -189,16 +188,16 @@ func TestCreateSpace(t *testing.T) {
HasSpecTargetCluster("")
})

t.Run("when Get ToolchainConfig fails and only targetCluster is missing", func(t *testing.T) {
t.Run("when listing SpaceProvisionerConfig fails and only targetCluster is missing", func(t *testing.T) {
// given
space := spacetest.NewSpace(test.HostOperatorNs, "oddity",
spacetest.WithTierName("advanced"))
r, req, cl := prepareReconcile(t, space, NewGetMemberClusters())
cl.MockGet = func(ctx context.Context, key runtimeclient.ObjectKey, obj runtimeclient.Object, opts ...runtimeclient.GetOption) error {
if key.Name == "config" {
return fmt.Errorf("some error")
r, req, cl := prepareReconcile(t, space, nil)
cl.MockList = func(ctx context.Context, list runtimeclient.ObjectList, opts ...runtimeclient.ListOption) error {
if _, ok := list.(*toolchainv1alpha1.SpaceProvisionerConfigList); ok {
return errors.New("some error")
}
return cl.Client.Get(ctx, key, obj)
return cl.Client.List(ctx, list, opts...)
}

// when
Expand All @@ -213,7 +212,7 @@ func TestCreateSpace(t *testing.T) {
})
}

func prepareReconcile(t *testing.T, space *toolchainv1alpha1.Space, getMemberClusters cluster.GetMemberClustersFunc) (*spacecompletion.Reconciler, reconcile.Request, *test.FakeClient) {
func prepareReconcile(t *testing.T, space *toolchainv1alpha1.Space, member1SpaceProvisionerConfig *toolchainv1alpha1.SpaceProvisionerConfig) (*spacecompletion.Reconciler, reconcile.Request, *test.FakeClient) {
require.NoError(t, os.Setenv("WATCH_NAMESPACE", test.HostOperatorNs))
s := scheme.Scheme
err := apis.AddToScheme(s)
Expand All @@ -224,14 +223,16 @@ func prepareReconcile(t *testing.T, space *toolchainv1alpha1.Space, getMemberClu
t.Cleanup(counter.Reset)
InitializeCounters(t, toolchainStatus)

conf := configuration.NewToolchainConfigObjWithReset(t)

fakeClient := test.NewFakeClient(t, toolchainStatus, space, conf)
objs := []runtime.Object{toolchainStatus, space}
if member1SpaceProvisionerConfig != nil {
objs = append(objs, member1SpaceProvisionerConfig)
}
fakeClient := test.NewFakeClient(t, objs...)

r := &spacecompletion.Reconciler{
Client: fakeClient,
Namespace: test.HostOperatorNs,
ClusterManager: capacity.NewClusterManager(getMemberClusters, fakeClient),
ClusterManager: capacity.NewClusterManager(test.HostOperatorNs, fakeClient),
}
req := reconcile.Request{
NamespacedName: types.NamespacedName{
Expand Down
8 changes: 7 additions & 1 deletion controllers/toolchainconfig/configuration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ func (c *ToolchainConfig) Environment() string {
}

func (c *ToolchainConfig) GitHubSecret() GitHubSecret {
return GitHubSecret{s: c.cfg.Host.ToolchainStatus.GitHubSecret,
return GitHubSecret{
s: c.cfg.Host.ToolchainStatus.GitHubSecret,
secrets: c.secrets,
}
}
Expand All @@ -91,6 +92,7 @@ func (c *ToolchainConfig) AutomaticApproval() AutoApprovalConfig {
return AutoApprovalConfig{c.cfg.Host.AutomaticApproval}
}

// Deprecated: This is no longer used for anything.
func (c *ToolchainConfig) CapacityThresholds() CapacityThresholdsConfig {
return CapacityThresholdsConfig{c.cfg.Host.CapacityThresholds}
}
Expand Down Expand Up @@ -150,18 +152,22 @@ func (s SpaceConfig) SpaceBindingRequestIsEnabled() bool {
return commonconfig.GetBool(s.spaceConfig.SpaceBindingRequestEnabled, false)
}

// Deprecated: This is no longer used for anything.
type CapacityThresholdsConfig struct {
capacityThresholds toolchainv1alpha1.CapacityThresholds
}

// Deprecated: This is no longer used for anything.
func (c CapacityThresholdsConfig) MaxNumberOfSpacesSpecificPerMemberCluster() map[string]int {
return c.capacityThresholds.MaxNumberOfSpacesPerMemberCluster
}

// Deprecated: This is no longer used for anything.
func (c CapacityThresholdsConfig) ResourceCapacityThresholdDefault() int {
return commonconfig.GetInt(c.capacityThresholds.ResourceCapacityThreshold.DefaultThreshold, 80)
}

// Deprecated: This is no longer used for anything.
func (c CapacityThresholdsConfig) ResourceCapacityThresholdSpecificPerMemberCluster() map[string]int {
return c.capacityThresholds.ResourceCapacityThreshold.SpecificPerMemberCluster
}
Expand Down
5 changes: 2 additions & 3 deletions controllers/toolchainconfig/configuration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ func TestForceLoadToolchainConfig(t *testing.T) {
assert.Equal(t, "unit-tests", toolchainCfg.Environment()) // returns actual value
assert.Equal(t, "def456", toolchainCfg.Notifications().MailgunAPIKey()) // returns actual value
})

})

t.Run("error retrieving config object", func(t *testing.T) {
Expand Down Expand Up @@ -260,7 +259,7 @@ func TestCapacityThresholdsConfig(t *testing.T) {
assert.Empty(t, toolchainCfg.CapacityThresholds().ResourceCapacityThresholdSpecificPerMemberCluster())
})
t.Run("non-default", func(t *testing.T) {
cfg := commonconfig.NewToolchainConfigObjWithReset(t, testconfig.CapacityThresholds().MaxNumberOfSpaces(testconfig.PerMemberCluster("member1", 321)).ResourceCapacityThreshold(456, testconfig.PerMemberCluster("member1", 654)))
cfg := commonconfig.NewToolchainConfigObjWithReset(t, testconfig.CapacityThresholds().MaxNumberOfSpaces(testconfig.PerMemberCluster("member1", 321)).ResourceCapacityThreshold(456, testconfig.PerMemberCluster("member1", 654))) //nolint:staticcheck // this will be removed once we also remove the deprecated method
toolchainCfg := newToolchainConfig(cfg, map[string]map[string]string{})

assert.Equal(t, cfg.Spec.Host.CapacityThresholds.MaxNumberOfSpacesPerMemberCluster, toolchainCfg.CapacityThresholds().MaxNumberOfSpacesSpecificPerMemberCluster())
Expand Down Expand Up @@ -336,7 +335,7 @@ func TestNotifications(t *testing.T) {
assert.Empty(t, toolchainCfg.Notifications().MailgunReplyToEmail())
assert.Equal(t, "mailgun", toolchainCfg.Notifications().NotificationDeliveryService())
assert.Equal(t, 24*time.Hour, toolchainCfg.Notifications().DurationBeforeNotificationDeletion())
assert.Equal(t, "sandbox", toolchainCfg.Notifications().TemplateSetName()) //default notificationEnv
assert.Equal(t, "sandbox", toolchainCfg.Notifications().TemplateSetName()) // default notificationEnv
})
t.Run("non-default", func(t *testing.T) {
cfg := commonconfig.NewToolchainConfigObjWithReset(t,
Expand Down
Loading

0 comments on commit de95481

Please sign in to comment.