From 902693d8787daab2cc5c3554a3c7fda1f67c1a7f Mon Sep 17 00:00:00 2001 From: Alexey Kazakov Date: Sat, 28 Oct 2023 17:01:10 -0700 Subject: [PATCH 1/2] Allow for disable inheritace when creating spaces --- .../spacerequest_controller_test.go | 37 ++++++++++++++++++- controllers/usersignup/spacebinding_test.go | 2 +- .../usersignup/usersignup_controller.go | 2 +- .../usersignup/usersignup_controller_test.go | 4 +- .../advanced/based_on_tier.yaml | 4 +- .../nstemplatetiers/base/cluster.yaml | 12 +++--- .../nstemplatetiers/base/ns_dev.yaml | 2 +- .../nstemplatetiers/base/ns_stage.yaml | 2 +- .../baselarge/based_on_tier.yaml | 4 +- pkg/space/space.go | 4 +- pkg/space/space_test.go | 2 +- test/spacerequest/spacerequest.go | 6 +++ test/spacerequest/spacerequest_assertions.go | 7 ++++ 13 files changed, 68 insertions(+), 20 deletions(-) diff --git a/controllers/spacerequest/spacerequest_controller_test.go b/controllers/spacerequest/spacerequest_controller_test.go index eabbf654d..47619957e 100644 --- a/controllers/spacerequest/spacerequest_controller_test.go +++ b/controllers/spacerequest/spacerequest_controller_test.go @@ -42,7 +42,8 @@ func TestCreateSpaceRequest(t *testing.T) { t.Run("success", func(t *testing.T) { sr := spacerequesttest.NewSpaceRequest("jane", srNamespace.GetName(), spacerequesttest.WithTierName("appstudio"), - spacerequesttest.WithTargetClusterRoles(srClusterRoles)) + spacerequesttest.WithTargetClusterRoles(srClusterRoles), + spacerequesttest.WithDisableInheritance(false)) t.Run("subSpace doesn't exists it should be created", func(t *testing.T) { // given @@ -59,12 +60,44 @@ func TestCreateSpaceRequest(t *testing.T) { spacerequesttest.AssertThatSpaceRequest(t, srNamespace.Name, sr.GetName(), member1.Client). HasSpecTargetClusterRoles(srClusterRoles). HasSpecTierName("appstudio"). + HasDisableInheritance(false). HasConditions(spacetest.Provisioning()). HasFinalizer() // there should be 1 subSpace that was created from the spacerequest spacetest.AssertThatSubSpace(t, hostClient, sr, parentSpace). HasTier("appstudio"). - HasSpecTargetClusterRoles(srClusterRoles) + HasSpecTargetClusterRoles(srClusterRoles). + HasDisableInheritance(false) + }) + + t.Run("subSpace created with disableInheritance", func(t *testing.T) { + sr := spacerequesttest.NewSpaceRequest("jane", srNamespace.GetName(), + spacerequesttest.WithTierName("appstudio"), + spacerequesttest.WithTargetClusterRoles(srClusterRoles), + spacerequesttest.WithDisableInheritance(true)) + + // given + member1 := NewMemberClusterWithClient(test.NewFakeClient(t, sr, srNamespace), "member-1", corev1.ConditionTrue) + hostClient := test.NewFakeClient(t, appstudioTier, parentSpace) + ctrl := newReconciler(t, hostClient, member1) + + // when + _, err = ctrl.Reconcile(context.TODO(), requestFor(sr)) + + // then + require.NoError(t, err) + // spacerequest exists with expected cluster roles and finalizer + spacerequesttest.AssertThatSpaceRequest(t, srNamespace.Name, sr.GetName(), member1.Client). + HasSpecTargetClusterRoles(srClusterRoles). + HasSpecTierName("appstudio"). + HasDisableInheritance(true). + HasConditions(spacetest.Provisioning()). + HasFinalizer() + // there should be 1 subSpace that was created from the spacerequest + spacetest.AssertThatSubSpace(t, hostClient, sr, parentSpace). + HasTier("appstudio"). + HasSpecTargetClusterRoles(srClusterRoles). + HasDisableInheritance(true) }) t.Run("subSpace exists but is not ready yet", func(t *testing.T) { diff --git a/controllers/usersignup/spacebinding_test.go b/controllers/usersignup/spacebinding_test.go index ca9ebbc89..8fdcdfaa2 100644 --- a/controllers/usersignup/spacebinding_test.go +++ b/controllers/usersignup/spacebinding_test.go @@ -19,7 +19,7 @@ func TestNewSpaceBinding(t *testing.T) { // given userSignup := commonsignup.NewUserSignup() nsTemplateTier := tiertest.NewNSTemplateTier("advanced", "dev", "stage", "extra") - space := NewSpace(userSignup, test.MemberClusterName, "smith", nsTemplateTier.Name) + space := NewSpace(userSignup, test.MemberClusterName, "smith", nsTemplateTier.Name, false) mur := newMasterUserRecord(userSignup, test.MemberClusterName, "deactivate90", "johny") // when diff --git a/controllers/usersignup/usersignup_controller.go b/controllers/usersignup/usersignup_controller.go index 21a5e0c59..79666d36b 100644 --- a/controllers/usersignup/usersignup_controller.go +++ b/controllers/usersignup/usersignup_controller.go @@ -693,7 +693,7 @@ func (r *Reconciler) ensureSpace( } tCluster := targetCluster(mur.Spec.UserAccounts[0].TargetCluster) - space = spaceutil.NewSpace(userSignup, tCluster.getClusterName(), mur.Name, spaceTier.Name) + space = spaceutil.NewSpace(userSignup, tCluster.getClusterName(), mur.Name, spaceTier.Name, false) err = r.Client.Create(ctx, space) if err != nil { diff --git a/controllers/usersignup/usersignup_controller_test.go b/controllers/usersignup/usersignup_controller_test.go index a6c6fa6fd..87d26f238 100644 --- a/controllers/usersignup/usersignup_controller_test.go +++ b/controllers/usersignup/usersignup_controller_test.go @@ -1584,7 +1584,7 @@ func TestUserSignupMUROrSpaceOrSpaceBindingCreateFails(t *testing.T) { mur := newMasterUserRecord(userSignup, "member1", deactivate30Tier.Name, "foo") mur.Labels = map[string]string{toolchainv1alpha1.MasterUserRecordOwnerLabelKey: userSignup.Name} - space := NewSpace(userSignup, "member1", "foo", "base") + space := NewSpace(userSignup, "member1", "foo", "base", false) ready := NewGetMemberClusters(NewMemberClusterWithTenantRole(t, "member1", corev1.ConditionTrue)) initObjs := []runtime.Object{userSignup, baseNSTemplateTier, deactivate30Tier} @@ -1868,7 +1868,7 @@ func TestUserSignupWithExistingMUROK(t *testing.T) { }, } - space := NewSpace(userSignup, "member1", "foo", "base") + space := NewSpace(userSignup, "member1", "foo", "base", false) spacebinding := spacebindingtest.NewSpaceBinding("foo", "foo", "admin", userSignup.Name) diff --git a/deploy/templates/nstemplatetiers/advanced/based_on_tier.yaml b/deploy/templates/nstemplatetiers/advanced/based_on_tier.yaml index 5f1ff948c..40c789f34 100644 --- a/deploy/templates/nstemplatetiers/advanced/based_on_tier.yaml +++ b/deploy/templates/nstemplatetiers/advanced/based_on_tier.yaml @@ -1,8 +1,8 @@ from: base parameters: - name: MEMORY_LIMIT - value: "16Gi" + value: "32Gi" - name: MEMORY_REQUEST - value: "16Gi" + value: "32Gi" - name: IDLER_TIMEOUT_SECONDS value: 0 diff --git a/deploy/templates/nstemplatetiers/base/cluster.yaml b/deploy/templates/nstemplatetiers/base/cluster.yaml index bee23b21d..2fcdf9724 100644 --- a/deploy/templates/nstemplatetiers/base/cluster.yaml +++ b/deploy/templates/nstemplatetiers/base/cluster.yaml @@ -10,13 +10,13 @@ objects: spec: quota: hard: - limits.cpu: 20000m + limits.cpu: 40000m limits.memory: ${MEMORY_LIMIT} - limits.ephemeral-storage: 7Gi - requests.cpu: 1750m + limits.ephemeral-storage: 15Gi + requests.cpu: 6000m requests.memory: ${MEMORY_REQUEST} requests.storage: 40Gi - requests.ephemeral-storage: 7Gi + requests.ephemeral-storage: 15Gi count/persistentvolumeclaims: "5" selector: annotations: null @@ -153,6 +153,6 @@ parameters: # 12 hours value: "43200" - name: MEMORY_LIMIT - value: "7Gi" + value: "28Gi" - name: MEMORY_REQUEST - value: "7Gi" + value: "28Gi" diff --git a/deploy/templates/nstemplatetiers/base/ns_dev.yaml b/deploy/templates/nstemplatetiers/base/ns_dev.yaml index 8ac67bee5..ddaafeffc 100644 --- a/deploy/templates/nstemplatetiers/base/ns_dev.yaml +++ b/deploy/templates/nstemplatetiers/base/ns_dev.yaml @@ -72,7 +72,7 @@ objects: - type: "Container" default: cpu: 1000m - memory: 750Mi + memory: 1024Mi defaultRequest: cpu: 10m memory: 64Mi diff --git a/deploy/templates/nstemplatetiers/base/ns_stage.yaml b/deploy/templates/nstemplatetiers/base/ns_stage.yaml index bc1dda573..0f2e43a54 100644 --- a/deploy/templates/nstemplatetiers/base/ns_stage.yaml +++ b/deploy/templates/nstemplatetiers/base/ns_stage.yaml @@ -72,7 +72,7 @@ objects: - type: "Container" default: cpu: 1000m - memory: 750Mi + memory: 1024Mi defaultRequest: cpu: 10m memory: 64Mi diff --git a/deploy/templates/nstemplatetiers/baselarge/based_on_tier.yaml b/deploy/templates/nstemplatetiers/baselarge/based_on_tier.yaml index d228dc380..e30b2c8a7 100644 --- a/deploy/templates/nstemplatetiers/baselarge/based_on_tier.yaml +++ b/deploy/templates/nstemplatetiers/baselarge/based_on_tier.yaml @@ -1,6 +1,6 @@ from: base parameters: - name: MEMORY_LIMIT - value: "16Gi" + value: "32Gi" - name: MEMORY_REQUEST - value: "16Gi" + value: "32Gi" diff --git a/pkg/space/space.go b/pkg/space/space.go index 78f403126..8903c4d3f 100644 --- a/pkg/space/space.go +++ b/pkg/space/space.go @@ -7,7 +7,7 @@ import ( ) // NewSpace creates a space CR for a UserSignup object. -func NewSpace(userSignup *toolchainv1alpha1.UserSignup, targetClusterName string, compliantUserName, tier string) *toolchainv1alpha1.Space { +func NewSpace(userSignup *toolchainv1alpha1.UserSignup, targetClusterName string, compliantUserName, tier string, disableInheritance bool) *toolchainv1alpha1.Space { labels := map[string]string{ toolchainv1alpha1.SpaceCreatorLabelKey: userSignup.Name, } @@ -22,6 +22,7 @@ func NewSpace(userSignup *toolchainv1alpha1.UserSignup, targetClusterName string TargetCluster: targetClusterName, TargetClusterRoles: []string{cluster.RoleLabel(cluster.Tenant)}, // by default usersignups should be provisioned to tenant clusters TierName: tier, + DisableInheritance: disableInheritance, }, } return space @@ -45,6 +46,7 @@ func NewSubSpace(spaceRequest *toolchainv1alpha1.SpaceRequest, parentSpace *tool TargetClusterRoles: spaceRequest.Spec.TargetClusterRoles, TierName: spaceRequest.Spec.TierName, ParentSpace: parentSpace.GetName(), + DisableInheritance: spaceRequest.Spec.DisableInheritance, }, } diff --git a/pkg/space/space_test.go b/pkg/space/space_test.go index 54e0e2b8a..d831aa161 100644 --- a/pkg/space/space_test.go +++ b/pkg/space/space_test.go @@ -20,7 +20,7 @@ func TestNewSpace(t *testing.T) { userSignup := commonsignup.NewUserSignup() // when - space := NewSpace(userSignup, test.MemberClusterName, "johny", "advanced") + space := NewSpace(userSignup, test.MemberClusterName, "johny", "advanced", false) // then expectedSpace := spacetest.NewSpace(test.HostOperatorNs, "johny", diff --git a/test/spacerequest/spacerequest.go b/test/spacerequest/spacerequest.go index 7aad24cec..d5267e1a4 100644 --- a/test/spacerequest/spacerequest.go +++ b/test/spacerequest/spacerequest.go @@ -39,6 +39,12 @@ func WithTargetClusterRoles(targetClusterRoles []string) Option { } } +func WithDisableInheritance(disableInheritance bool) Option { + return func(spaceRequest *toolchainv1alpha1.SpaceRequest) { + spaceRequest.Spec.DisableInheritance = disableInheritance + } +} + func WithDeletionTimestamp() Option { return func(spaceRequest *toolchainv1alpha1.SpaceRequest) { now := metav1.NewTime(time.Now()) diff --git a/test/spacerequest/spacerequest_assertions.go b/test/spacerequest/spacerequest_assertions.go index 5e4a201cd..865603f25 100644 --- a/test/spacerequest/spacerequest_assertions.go +++ b/test/spacerequest/spacerequest_assertions.go @@ -119,6 +119,13 @@ func (a *Assertion) HasSpecTierName(tierName string) *Assertion { return a } +func (a *Assertion) HasDisableInheritance(disableInheritance bool) *Assertion { + err := a.loadResource() + require.NoError(a.t, err) + assert.Equal(a.t, disableInheritance, a.spaceRequest.Spec.DisableInheritance) + return a +} + func (a *Assertion) HasTargetClusterURL(targetCluster string) *Assertion { err := a.loadResource() require.NoError(a.t, err) From f392d92ba526479b232f15d257c62fa0b89d426e Mon Sep 17 00:00:00 2001 From: Martin Mulholland Date: Mon, 13 Nov 2023 10:32:00 -0500 Subject: [PATCH 2/2] add yaml files --- ...chain.dev.openshift.com_spacerequests.yaml | 6 +++++ .../toolchain.dev.openshift.com_spaces.yaml | 6 +++++ ...in.dev.openshift.com_toolchainconfigs.yaml | 25 +++++++++++++++++++ 3 files changed, 37 insertions(+) diff --git a/config/crd/bases/toolchain.dev.openshift.com_spacerequests.yaml b/config/crd/bases/toolchain.dev.openshift.com_spacerequests.yaml index 6cd7ca8ad..b187c6ef1 100644 --- a/config/crd/bases/toolchain.dev.openshift.com_spacerequests.yaml +++ b/config/crd/bases/toolchain.dev.openshift.com_spacerequests.yaml @@ -48,6 +48,12 @@ spec: spec: description: SpaceRequestSpec defines the desired state of Space properties: + disableInheritance: + description: "DisableInheritance indicates whether or not SpaceBindings + from the parent-spaces are automatically inherited to all sub-spaces + in the tree. \n Set to True to disable SpaceBinding inheritance + from the parent-spaces. Default is False." + type: boolean targetClusterRoles: description: TargetClusterRoles one or more label keys that define a set of clusters where the Space can be provisioned. The target diff --git a/config/crd/bases/toolchain.dev.openshift.com_spaces.yaml b/config/crd/bases/toolchain.dev.openshift.com_spaces.yaml index 41b994bc1..8f8139a52 100644 --- a/config/crd/bases/toolchain.dev.openshift.com_spaces.yaml +++ b/config/crd/bases/toolchain.dev.openshift.com_spaces.yaml @@ -48,6 +48,12 @@ spec: spec: description: SpaceSpec defines the desired state of Space properties: + disableInheritance: + description: "DisableInheritance indicates whether or not SpaceBindings + from the parent-spaces are automatically inherited to all sub-spaces + in the tree. \n Set to True to disable SpaceBinding inheritance + from the parent-spaces. Default is False." + type: boolean parentSpace: description: "ParentSpace holds the name of the context (Space) from which this space was created (requested), enabling hierarchy relationships diff --git a/config/crd/bases/toolchain.dev.openshift.com_toolchainconfigs.yaml b/config/crd/bases/toolchain.dev.openshift.com_toolchainconfigs.yaml index 29cca14c6..c2a3f40df 100644 --- a/config/crd/bases/toolchain.dev.openshift.com_toolchainconfigs.yaml +++ b/config/crd/bases/toolchain.dev.openshift.com_toolchainconfigs.yaml @@ -619,6 +619,18 @@ spec: description: Defines the flag that determines whether to deploy the Webhook type: boolean + secret: + description: Defines all secrets related to webhook configuration + properties: + ref: + description: Reference is the name of the secret resource + to look up + type: string + virtualMachineAccessKey: + description: The key in the secret values map that + contains a comma-separated list of SSH keys + type: string + type: object type: object type: object specificPerMemberCluster: @@ -759,6 +771,19 @@ spec: description: Defines the flag that determines whether to deploy the Webhook type: boolean + secret: + description: Defines all secrets related to webhook + configuration + properties: + ref: + description: Reference is the name of the secret + resource to look up + type: string + virtualMachineAccessKey: + description: The key in the secret values map that + contains a comma-separated list of SSH keys + type: string + type: object type: object type: object description: A map of cluster-specific member operator configurations