Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add feature toggle annotations to Space & NSTemplateSet #1048

Merged
merged 20 commits into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion controllers/space/space_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ func (r *Reconciler) ensureNSTemplateSet(ctx context.Context, space *toolchainv1
// look-up and delete the NSTemplateSet on the current member cluster
if isBeingDeleted, err := r.deleteNSTemplateSetFromCluster(ctx, space, space.Status.TargetCluster); err != nil {
return norequeue, r.setStatusRetargetFailed(ctx, space, err)

} else if isBeingDeleted {
logger.Info("wait while NSTemplateSet is being deleted", "member_cluster", space.Status.TargetCluster)
return norequeue, r.setStatusRetargeting(ctx, space)
Expand Down Expand Up @@ -387,6 +386,15 @@ func NewNSTemplateSet(namespace string, space *toolchainv1alpha1.Space, bindings
},
}
nsTmplSet.Spec = NewNSTemplateSetSpec(space, bindings, tmplTier)

// propagate feature annotation from the space
winners, found := space.Annotations[toolchainv1alpha1.FeatureToggleNameAnnotationKey]
if found {
if nsTmplSet.Annotations == nil {
nsTmplSet.Annotations = make(map[string]string)
}
nsTmplSet.Annotations[toolchainv1alpha1.FeatureToggleNameAnnotationKey] = winners
}
alexeykazakov marked this conversation as resolved.
Show resolved Hide resolved
return nsTmplSet
}

Expand Down
393 changes: 212 additions & 181 deletions controllers/space/space_controller_test.go

Large diffs are not rendered by default.

24 changes: 24 additions & 0 deletions controllers/toolchainconfig/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@
return newToolchainConfig(config, secrets), nil
}

func NewToolchainConfig(config *toolchainv1alpha1.ToolchainConfigSpec, secrets map[string]map[string]string) ToolchainConfig {
alexeykazakov marked this conversation as resolved.
Show resolved Hide resolved
return ToolchainConfig{cfg: config, secrets: secrets}

Check warning on line 62 in controllers/toolchainconfig/configuration.go

View check run for this annotation

Codecov / codecov/patch

controllers/toolchainconfig/configuration.go#L61-L62

Added lines #L61 - L62 were not covered by tests
}

func newToolchainConfig(config runtime.Object, secrets map[string]map[string]string) ToolchainConfig {
if config == nil {
// return default config if there's no config resource
Expand Down Expand Up @@ -304,6 +308,26 @@
return duration
}

func (d TiersConfig) FeatureToggles() []FeatureToggle {
toggles := make([]FeatureToggle, 0, len(d.tiers.FeatureToggles))
for _, t := range d.tiers.FeatureToggles {
toggles = append(toggles, FeatureToggle{t})
}
return toggles
}

type FeatureToggle struct {
alexeykazakov marked this conversation as resolved.
Show resolved Hide resolved
toggle toolchainv1alpha1.FeatureToggle
}

func (t FeatureToggle) Name() string {
return t.toggle.Name
}

func (t FeatureToggle) Weight() uint {
return commonconfig.GetUint(t.toggle.Weight, 100)
}

type ToolchainStatusConfig struct {
t toolchainv1alpha1.ToolchainStatusConfig
}
Expand Down
11 changes: 10 additions & 1 deletion controllers/toolchainconfig/configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,7 @@ func TestTiers(t *testing.T) {
assert.Equal(t, "deactivate30", toolchainCfg.Tiers().DefaultUserTier())
assert.Equal(t, "base", toolchainCfg.Tiers().DefaultSpaceTier())
assert.Equal(t, 24*time.Hour, toolchainCfg.Tiers().DurationBeforeChangeTierRequestDeletion())
assert.Empty(t, toolchainCfg.Tiers().FeatureToggles())
})
t.Run("invalid", func(t *testing.T) {
cfg := commonconfig.NewToolchainConfigObjWithReset(t, testconfig.Tiers().DurationBeforeChangeTierRequestDeletion("rapid"))
Expand All @@ -438,15 +439,23 @@ func TestTiers(t *testing.T) {
assert.Equal(t, 24*time.Hour, toolchainCfg.Tiers().DurationBeforeChangeTierRequestDeletion())
})
t.Run("non-default", func(t *testing.T) {
weight10 := uint(10)
cfg := commonconfig.NewToolchainConfigObjWithReset(t, testconfig.Tiers().
DefaultUserTier("deactivate90").
DefaultSpaceTier("advanced").
DurationBeforeChangeTierRequestDeletion("48h"))
DurationBeforeChangeTierRequestDeletion("48h").
FeatureToggle("feature-1", nil). // With default weight
FeatureToggle("feature-2", &weight10))
toolchainCfg := newToolchainConfig(cfg, map[string]map[string]string{})

assert.Equal(t, "deactivate90", toolchainCfg.Tiers().DefaultUserTier())
assert.Equal(t, "advanced", toolchainCfg.Tiers().DefaultSpaceTier())
assert.Equal(t, 48*time.Hour, toolchainCfg.Tiers().DurationBeforeChangeTierRequestDeletion())
assert.Len(t, toolchainCfg.Tiers().FeatureToggles(), 2)
assert.Equal(t, "feature-1", toolchainCfg.Tiers().FeatureToggles()[0].Name())
assert.Equal(t, uint(100), toolchainCfg.Tiers().FeatureToggles()[0].Weight()) // default weight
assert.Equal(t, "feature-2", toolchainCfg.Tiers().FeatureToggles()[1].Name())
assert.Equal(t, weight10, toolchainCfg.Tiers().FeatureToggles()[1].Weight())
})
}

Expand Down
5 changes: 3 additions & 2 deletions controllers/usersignup/usersignup_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ func (r *Reconciler) checkIfMurAlreadyExists(
logger.Info("MasterUserRecord exists", "Name", mur.Name)

if shouldManageSpace(userSignup) {
space, created, err := r.ensureSpace(ctx, userSignup, mur, spaceTier)
space, created, err := r.ensureSpace(ctx, userSignup, mur, spaceTier, config)
if err != nil {
return true, r.wrapErrorWithStatusUpdate(ctx, userSignup, r.setStatusFailedToCreateSpace, err, "error creating Space")
}
Expand Down Expand Up @@ -677,6 +677,7 @@ func (r *Reconciler) ensureSpace(
userSignup *toolchainv1alpha1.UserSignup,
mur *toolchainv1alpha1.MasterUserRecord,
spaceTier *toolchainv1alpha1.NSTemplateTier,
config toolchainconfig.ToolchainConfig,
) (*toolchainv1alpha1.Space, bool, error) {
logger := log.FromContext(ctx)
logger.Info("Ensuring Space", "UserSignup", userSignup.Name, "MUR", mur.Name, "NSTemplateTier", spaceTier.Name)
Expand All @@ -703,7 +704,7 @@ func (r *Reconciler) ensureSpace(
}
tCluster := targetCluster(mur.Spec.UserAccounts[0].TargetCluster)

space = spaceutil.NewSpace(userSignup, tCluster.getClusterName(), mur.Name, spaceTier.Name)
space = spaceutil.NewSpaceWithFeatureToggles(userSignup, tCluster.getClusterName(), mur.Name, spaceTier.Name, config)

err = r.Client.Create(ctx, space)
if err != nil {
Expand Down
32 changes: 28 additions & 4 deletions controllers/usersignup/usersignup_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,14 +206,27 @@ func TestUserSignupCreateSpaceAndSpaceBindingOk(t *testing.T) {
commonsignup.WithTargetCluster("member1"),
commonsignup.WithStateLabel(toolchainv1alpha1.UserSignupStateLabelValueNotReady),
commonsignup.WithAnnotation(toolchainv1alpha1.SkipAutoCreateSpaceAnnotationKey, "true")),
"with feature toggles": commonsignup.NewUserSignup(
commonsignup.ApprovedManually(),
commonsignup.WithTargetCluster("member1"),
commonsignup.WithStateLabel(toolchainv1alpha1.UserSignupStateLabelValueNotReady),
commonsignup.WithoutAnnotation(toolchainv1alpha1.SkipAutoCreateSpaceAnnotationKey)),
} {
t.Run(testname, func(t *testing.T) {
// given
defer counter.Reset()

mur := newMasterUserRecord(userSignup, "member1", deactivate30Tier.Name, "foo")
mur.Labels = map[string]string{toolchainv1alpha1.MasterUserRecordOwnerLabelKey: userSignup.Name}
r, req, _ := prepareReconcile(t, userSignup.Name, spaceProvisionerConfig, userSignup, mur, baseNSTemplateTier, base2NSTemplateTier, deactivate30Tier, deactivate80Tier, event)

config := &toolchainv1alpha1.ToolchainConfig{}
switch testname {
alexeykazakov marked this conversation as resolved.
Show resolved Hide resolved
case "with feature toggles":
weight100 := uint(100)
weight0 := uint(0)
config = commonconfig.NewToolchainConfigObjWithReset(t, testconfig.Tiers().FeatureToggle("feature-on", &weight100).FeatureToggle("feature-off", &weight0))
}
r, req, _ := prepareReconcile(t, userSignup.Name, spaceProvisionerConfig, config, userSignup, mur, baseNSTemplateTier, base2NSTemplateTier, deactivate30Tier, deactivate80Tier, event)

// when
res, err := r.Reconcile(context.TODO(), req)
Expand All @@ -230,15 +243,26 @@ func TestUserSignupCreateSpaceAndSpaceBindingOk(t *testing.T) {
HasLabelWithValue(toolchainv1alpha1.SpaceCreatorLabelKey, userSignup.Name).
HasSpecTargetCluster("member1").
HasSpecTargetClusterRoles([]string{cluster.RoleLabel(cluster.Tenant)}).
HasTier("base")
HasTier("base").
DoesNotHaveAnnotation(toolchainv1alpha1.FeatureToggleNameAnnotationKey)
AssertThatUserSignup(t, req.Namespace, userSignup.Name, r.Client).HasHomeSpace("foo")
case "with feature toggles":
spacetest.AssertThatSpace(t, test.HostOperatorNs, "foo", r.Client).
Exists().
HasLabelWithValue(toolchainv1alpha1.SpaceCreatorLabelKey, userSignup.Name).
HasSpecTargetCluster("member1").
HasSpecTargetClusterRoles([]string{cluster.RoleLabel(cluster.Tenant)}).
HasTier("base").
HasAnnotationWithValue(toolchainv1alpha1.FeatureToggleNameAnnotationKey, "feature-on")
AssertThatUserSignup(t, req.Namespace, userSignup.Name, r.Client).HasHomeSpace("foo")
case "with social event":
spacetest.AssertThatSpace(t, test.HostOperatorNs, "foo", r.Client).
Exists().
HasLabelWithValue(toolchainv1alpha1.SpaceCreatorLabelKey, userSignup.Name).
HasSpecTargetCluster("member1").
HasSpecTargetClusterRoles([]string{cluster.RoleLabel(cluster.Tenant)}).
HasTier("base2")
HasTier("base2").
DoesNotHaveAnnotation(toolchainv1alpha1.FeatureToggleNameAnnotationKey)
AssertThatUserSignup(t, req.Namespace, userSignup.Name, r.Client).HasHomeSpace("foo")
case "with skip space creation annotation set to true":
spacetest.AssertThatSpace(t, test.HostOperatorNs, "foo", r.Client).
Expand All @@ -255,7 +279,7 @@ func TestUserSignupCreateSpaceAndSpaceBindingOk(t *testing.T) {
// then
require.Equal(t, reconcile.Result{}, res)
switch testname {
case "without skip space creation annotation", "with skip space creation annotation set to false", "with social event":
case "without skip space creation annotation", "with skip space creation annotation set to false", "with social event", "with feature toggles":
alexeykazakov marked this conversation as resolved.
Show resolved Hide resolved
spacebindingtest.AssertThatSpaceBinding(t, test.HostOperatorNs, "foo", "foo", r.Client).
Exists().
HasLabelWithValue(toolchainv1alpha1.SpaceCreatorLabelKey, userSignup.Name).
Expand Down
6 changes: 4 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
module github.com/codeready-toolchain/host-operator

require (
github.com/codeready-toolchain/api v0.0.0-20240530120602-c11598ccffb7
github.com/codeready-toolchain/toolchain-common v0.0.0-20240613121043-7e6ef858cdff
github.com/codeready-toolchain/api v0.0.0-20240620205422-a29b6c658d03
github.com/codeready-toolchain/toolchain-common v0.0.0-20240621204413-d3f78a78634d
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/go-bindata/go-bindata v3.1.2+incompatible
github.com/go-logr/logr v1.2.3
Expand Down Expand Up @@ -112,4 +112,6 @@ require (
sigs.k8s.io/yaml v1.3.0 // indirect
)

replace github.com/codeready-toolchain/toolchain-common v0.0.0-20240621204413-d3f78a78634d => github.com/alexeykazakov/toolchain-common v0.0.0-20240627023345-48673930b141

go 1.20
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuy
github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk541a8SKzHPHnH3zbiI+7dagKZ0cgpgrD7Fyho=
github.com/alexeykazakov/toolchain-common v0.0.0-20240627023345-48673930b141 h1:mJMV4ITwfH/7XsnyoJIGq97w+6KQ/HGZVaMfEZLuWaA=
github.com/alexeykazakov/toolchain-common v0.0.0-20240627023345-48673930b141/go.mod h1:ZwfGkrTArynbvIPOJwQQCwFVW7Dj0XOgi5XzkNG1ov8=
github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY=
github.com/armon/circbuf v0.0.0-20150827004946-bbbad097214e/go.mod h1:3U/XgcO3hCbHZ8TKRvWD2dDTCfh9M9ya+I9JpbB7O8o=
github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
Expand Down Expand Up @@ -136,10 +138,8 @@ github.com/cncf/xds/go v0.0.0-20210312221358-fbca930ec8ed/go.mod h1:eXthEFrGJvWH
github.com/cockroachdb/datadriven v0.0.0-20200714090401-bf6692d28da5/go.mod h1:h6jFvWxBdQXxjopDMZyH2UVceIRfR84bdzbkoKrsWNo=
github.com/cockroachdb/errors v1.2.4/go.mod h1:rQD95gz6FARkaKkQXUksEje/d9a6wBJoCr5oaCLELYA=
github.com/cockroachdb/logtags v0.0.0-20190617123548-eb05cc24525f/go.mod h1:i/u985jwjWRlyHXQbwatDASoW0RMlZ/3i9yJHE2xLkI=
github.com/codeready-toolchain/api v0.0.0-20240530120602-c11598ccffb7 h1:o5JLcHCVS1BlZevw2mh1mH+iKwn9fLUrT1Ek8NFjvPY=
github.com/codeready-toolchain/api v0.0.0-20240530120602-c11598ccffb7/go.mod h1:ie9p4LenCCS0LsnbWp6/xwpFDdCWYE0KWzUO6Sk1g0E=
github.com/codeready-toolchain/toolchain-common v0.0.0-20240613121043-7e6ef858cdff h1:bVWL+2eayFKUnEzdEAwltPs+pzbGlGDSmrM3oOV2Ams=
github.com/codeready-toolchain/toolchain-common v0.0.0-20240613121043-7e6ef858cdff/go.mod h1:cyHrUfvBYEtsf+FbqQYmR9y0AQi9QAVtM3SUWLA5bd4=
github.com/codeready-toolchain/api v0.0.0-20240620205422-a29b6c658d03 h1:uqApdceOkNQI1wy2gwA6pEUzhLlqqbbc+ChTvEshIW0=
github.com/codeready-toolchain/api v0.0.0-20240620205422-a29b6c658d03/go.mod h1:ie9p4LenCCS0LsnbWp6/xwpFDdCWYE0KWzUO6Sk1g0E=
github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=
github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
github.com/coreos/etcd v3.3.13+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
Expand Down
37 changes: 37 additions & 0 deletions pkg/space/space.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@ package space

import (
"fmt"
"math/rand"

toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
"github.com/codeready-toolchain/host-operator/controllers/toolchainconfig"
"github.com/codeready-toolchain/toolchain-common/pkg/cluster"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand All @@ -29,6 +32,40 @@ func NewSpace(userSignup *toolchainv1alpha1.UserSignup, targetClusterName string
return space
}

// NewSpaceWithFeatureToggles is the same as NewSpace() but also does a feature toggle lottery drawing
// and adds the corresponding feature annotations for features which "won" and should be enabled for the space.
func NewSpaceWithFeatureToggles(userSignup *toolchainv1alpha1.UserSignup, targetClusterName string, compliantUserName, tier string, config toolchainconfig.ToolchainConfig) *toolchainv1alpha1.Space {
s := NewSpace(userSignup, targetClusterName, compliantUserName, tier)
addFeatureToggles(s, config)
return s
}

// addFeatureToggles does "lottery" drawing for all feature toggles defined in the configuration according to their weights.
// And it adds the corresponding feature annotation to the space for features that won and should be enabled for the space.
func addFeatureToggles(space *toolchainv1alpha1.Space, config toolchainconfig.ToolchainConfig) {
toggles := config.Tiers().FeatureToggles()
alexeykazakov marked this conversation as resolved.
Show resolved Hide resolved
var winners string
for _, t := range toggles {
weight := int(t.Weight())
alexeykazakov marked this conversation as resolved.
Show resolved Hide resolved
// We generate a random number between 0 and 100. If the number is equal to or lower than the weight
// then the feature wins.
if weight == 100 || (weight > 0 && rand.Intn(100) <= weight) {
// Winner!
if winners == "" {
winners = t.Name() // First winner so far
} else {
winners = fmt.Sprintf("%s,%s", winners, t.Name()) // Add to the comma separated list of existing winners
alexeykazakov marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
if winners != "" {
if space.Annotations == nil {
space.Annotations = make(map[string]string)
}
space.Annotations[toolchainv1alpha1.FeatureToggleNameAnnotationKey] = winners
}
}

// NewSubSpace creates a space CR for a SpaceRequest object.
func NewSubSpace(spaceRequest *toolchainv1alpha1.SpaceRequest, parentSpace *toolchainv1alpha1.Space) *toolchainv1alpha1.Space {
labels := map[string]string{
Expand Down
66 changes: 66 additions & 0 deletions pkg/space/space_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package space

import (
"fmt"
"github.com/codeready-toolchain/host-operator/controllers/toolchainconfig"
"strings"
"testing"

toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
Expand Down Expand Up @@ -30,6 +32,70 @@ func TestNewSpace(t *testing.T) {
assert.Equal(t, expectedSpace, space)
}

func TestNewSpaceWithFeatureToggles(t *testing.T) {
// given
userSignup := commonsignup.NewUserSignup()
weight100 := uint(100)
weight0 := uint(0)
weight50 := uint(50)
configSpec := &toolchainv1alpha1.ToolchainConfigSpec{
Host: toolchainv1alpha1.HostConfig{
Tiers: toolchainv1alpha1.TiersConfig{
FeatureToggles: []toolchainv1alpha1.FeatureToggle{
{
Name: "my-cool-feature",
Weight: &weight100,
},
{
Name: "my-not-so-cool-feature",
Weight: &weight0,
},
{
Name: "my-so-so-feature",
Weight: &weight50,
},
},
},
},
}
config := toolchainconfig.NewToolchainConfig(configSpec, nil)

// when
// create space 100 times and verify that the feature with weight 100 is always added
// the feature with weight 0 never added
// and the feature with weight 50 is added at least once and not added at least once too
var myCoolFeatureCount, myNotSoCoolFeatureCount, mySoSoFeatureCount int
for i := 0; i < 100; i++ {
space := NewSpaceWithFeatureToggles(userSignup, test.MemberClusterName, "johny", "advanced", config)

// then
expectedSpace := spacetest.NewSpace(test.HostOperatorNs, "johny",
spacetest.WithTierName("advanced"),
spacetest.WithSpecTargetCluster("member-cluster"),
spacetest.WithSpecTargetClusterRoles([]string{commoncluster.RoleLabel(commoncluster.Tenant)}),
spacetest.WithCreatorLabel(userSignup.Name))
assert.Equal(t, expectedSpace.Spec, space.Spec)
features := space.GetAnnotations()[toolchainv1alpha1.FeatureToggleNameAnnotationKey]
for _, feature := range strings.Split(features, ",") {
switch feature {
case "my-cool-feature":
myCoolFeatureCount++
case "my-not-so-cool-feature":
myNotSoCoolFeatureCount++
case "my-so-so-feature":
mySoSoFeatureCount++
default:
assert.Fail(t, "unknown feature", feature)
}
}
}

// then
assert.Equal(t, 100, myCoolFeatureCount)
assert.Equal(t, 0, myNotSoCoolFeatureCount)
assert.True(t, mySoSoFeatureCount > 0 && mySoSoFeatureCount < 100, "Feature with weight 50 should be present at least once but less than 100 times", mySoSoFeatureCount)
alexeykazakov marked this conversation as resolved.
Show resolved Hide resolved
}

func TestNewSubSpace(t *testing.T) {
// given
srClusterRoles := []string{commoncluster.RoleLabel(commoncluster.Tenant)}
Expand Down
Loading