From a782f5e55717ac0659d3e7db353c3757bef95fb6 Mon Sep 17 00:00:00 2001 From: Devtools Date: Fri, 24 Jan 2025 17:16:25 +0100 Subject: [PATCH 01/10] test tiertemlpate and nstelpatetier changes --- test/e2e/parallel/nstemplatetier_test.go | 120 +++++++++++++++++------ 1 file changed, 88 insertions(+), 32 deletions(-) diff --git a/test/e2e/parallel/nstemplatetier_test.go b/test/e2e/parallel/nstemplatetier_test.go index 3319efb76..998c11428 100644 --- a/test/e2e/parallel/nstemplatetier_test.go +++ b/test/e2e/parallel/nstemplatetier_test.go @@ -378,29 +378,7 @@ func TestTierTemplateRevision(t *testing.T) { require.NoError(t, err) // for the tiertemplaterevisions to be created the tiertemplates need to have template objects populated // we add the RawExtension objects to the TemplateObjects field - crq := unstructured.Unstructured{Object: map[string]interface{}{ - "kind": "ClusterResourceQuota", - "metadata": map[string]interface{}{ - "name": "for-{{.SPACE_NAME}}-deployments", - }, - "spec": map[string]interface{}{ - "quota": map[string]interface{}{ - "hard": map[string]string{ - "count/deploymentconfigs.apps": "{{.DEPLOYMENT_QUOTA}}", - "count/deployments.apps": "{{.DEPLOYMENT_QUOTA}}", - "count/pods": "600", - }, - }, - "selector": map[string]interface{}{ - "annotations": map[string]string{}, - "labels": map[string]interface{}{ - "matchLabels": map[string]string{ - "toolchain.dev.openshift.com/space": "{{.SPACE_NAME}}", - }, - }, - }, - }, - }} + crq := getTestCRQ("600") rawTemplateObjects := []runtime.RawExtension{{Object: &crq}} updateTierTemplateObjects := func(template *toolchainv1alpha1.TierTemplate) error { template.Spec.TemplateObjects = rawTemplateObjects @@ -409,7 +387,7 @@ func TestTierTemplateRevision(t *testing.T) { // for simplicity, we add the CRQ to all types of templates (both cluster scope and namespace scoped), // even if the CRQ is cluster scoped. // WARNING: thus THIS NSTemplateTier should NOT be sued to provision a user!!! - tiers.CreateCustomNSTemplateTier(t, hostAwait, "ttr", baseTier, + customTier := tiers.CreateCustomNSTemplateTier(t, hostAwait, "ttr", baseTier, tiers.WithNamespaceResources(t, baseTier, updateTierTemplateObjects), tiers.WithClusterResources(t, baseTier, updateTierTemplateObjects), tiers.WithSpaceRoles(t, baseTier, updateTierTemplateObjects), @@ -417,22 +395,73 @@ func TestTierTemplateRevision(t *testing.T) { // when // we verify the counters in the status.history for 'tierUsingTierTemplateRevisions' tier // and verify that TierTemplateRevision CRs were created, since all the tiertemplates now have templateObjects field populated - customTier, err := hostAwait.WaitForNSTemplateTierAndCheckTemplates(t, "ttr", + _, err = hostAwait.WaitForNSTemplateTierAndCheckTemplates(t, "ttr", wait.HasStatusTierTemplateRevisions(tiers.GetTemplateRefs(t, hostAwait, "ttr").Flatten())) require.NoError(t, err) // then // check the expected total number of ttr matches - err = apiwait.PollUntilContextTimeout(context.TODO(), hostAwait.RetryInterval, hostAwait.Timeout, true, func(ctx context.Context) (done bool, err error) { + // we IDEALLY expect one TTR per each tiertemplate to be created (clusterresource, namespace and spacerole), thus a total of 3 TTRs ideally. + // But since the creation of a TTR could be very quick and could trigger another reconcile of the NSTemplateTier before the status is actually updated with the reference, + // this might generate some copies of the TTRs. This is not a problem in production since the cleanup mechanism of TTRs will remove the extra ones but could cause some flakiness with the test, + // thus we assert the number of TTRs doesn't exceed the double of the expected number. + ttrCount, err := WaitForTTRsToMatch(t, hostAwait, customTier.NSTemplateTier, len(tiers.GetTemplateRefs(t, hostAwait, "ttr").Flatten())*2, func(actualCount, expectedCount int) { + assert.LessOrEqual(t, actualCount, expectedCount) + }) + require.NoError(t, err) + + t.Run("update of tiertemplate should trigger creation of new TTR", func(t *testing.T) { + // given + // that the tiertemplates and nstemlpatetier are provisioned from the parent test + + // when + // we update one tiertemplate + _, err = wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.TierTemplate{}). + Update(customTier.Spec.ClusterResources.TemplateRef, hostAwait.Namespace, func(tiertemplate *toolchainv1alpha1.TierTemplate) { + // let's increase the pod count + updatedCRQ := getTestCRQ("100") + tiertemplate.Spec.TemplateObjects = []runtime.RawExtension{{Object: &updatedCRQ}} + }) + require.NoError(t, err) + + // then + // a new TTR was created + updatedTTrCount, err := WaitForTTRsToMatch(t, hostAwait, customTier.NSTemplateTier, ttrCount+1, func(actualCount, expectedCount int) { + assert.GreaterOrEqual(t, actualCount, expectedCount) + }) + require.NoError(t, err) + + t.Run("update of the NSTemplateTier parameters should trigger creation of new TTR", func(t *testing.T) { + // given + // that the tiertemplates and nstemlpatetier are provisioned from the parent test + + // when + // we update a parameter in the NSTemplateTier + // by increasing the deployment quota + customTier = tiers.UpdateCustomNSTemplateTier(t, hostAwait, customTier, tiers.WithParameter("DEPLOYMENT_QUOTA", "100")) + require.NoError(t, err) + + // then + // an additional TTR was created + _, err := WaitForTTRsToMatch(t, hostAwait, customTier.NSTemplateTier, updatedTTrCount+1, func(actualCount, expectedCount int) { + assert.GreaterOrEqual(t, actualCount, expectedCount) + }) + require.NoError(t, err) + }) + + }) + +} + +func WaitForTTRsToMatch(t *testing.T, hostAwait *wait.HostAwaitility, customTier *toolchainv1alpha1.NSTemplateTier, expectedTTRsCount int, ttrCountComparisonFunc func(currentTTRsCount, expectedTTRsCount int)) (int, error) { + var ttrCount int + err := apiwait.PollUntilContextTimeout(context.TODO(), hostAwait.RetryInterval, hostAwait.Timeout, true, func(ctx context.Context) (done bool, err error) { objs := &toolchainv1alpha1.TierTemplateRevisionList{} if err := hostAwait.Client.List(ctx, objs, client.InNamespace(hostAwait.Namespace)); err != nil { return false, err } - // we IDEALLY expect one TTR per each tiertemplate to be created (clusterresource, namespace and spacerole), thus a total of 3 TTRs ideally. - // But since the creation of a TTR could be very quick and could trigger another reconcile of the NSTemplateTier before the status is actually updated with the reference, - // this might generate some copies of the TTRs. This is not a problem in production since the cleanup mechanism of TTRs will remove the extra ones but could cause some flakiness with the test, - // thus we assert the number of TTRs doesn't exceed the double of the expected number. - assert.LessOrEqual(t, len(objs.Items), len(tiers.GetTemplateRefs(t, hostAwait, "ttr").Flatten())*2) + ttrCount = len(objs.Items) + ttrCountComparisonFunc(ttrCount, expectedTTRsCount) // we check that the TTR content has the parameters replaced with values from the NSTemplateTier for _, obj := range objs.Items { // the object should have all the variables still there since this one will be replaced when provisioning the Space @@ -447,7 +476,34 @@ func TestTierTemplateRevision(t *testing.T) { } return true, nil }) - require.NoError(t, err) + return ttrCount, err +} + +func getTestCRQ(podsCount string) unstructured.Unstructured { + crq := unstructured.Unstructured{Object: map[string]interface{}{ + "kind": "ClusterResourceQuota", + "metadata": map[string]interface{}{ + "name": "for-{{.SPACE_NAME}}-deployments", + }, + "spec": map[string]interface{}{ + "quota": map[string]interface{}{ + "hard": map[string]string{ + "count/deploymentconfigs.apps": "{{.DEPLOYMENT_QUOTA}}", + "count/deployments.apps": "{{.DEPLOYMENT_QUOTA}}", + "count/pods": podsCount, + }, + }, + "selector": map[string]interface{}{ + "annotations": map[string]string{}, + "labels": map[string]interface{}{ + "matchLabels": map[string]string{ + "toolchain.dev.openshift.com/space": "{{.SPACE_NAME}}", + }, + }, + }, + }, + }} + return crq } func withClusterRoleBindings(t *testing.T, otherTier *toolchainv1alpha1.NSTemplateTier, feature string) tiers.CustomNSTemplateTierModifier { From 9f70a26a19031458a67de5bf2c2c4a278e8bb3ed Mon Sep 17 00:00:00 2001 From: Devtools Date: Sat, 25 Jan 2025 18:56:09 +0100 Subject: [PATCH 02/10] improve ttrs wait functions --- test/e2e/parallel/nstemplatetier_test.go | 42 ++---------- testsupport/wait/host.go | 87 ++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 38 deletions(-) diff --git a/test/e2e/parallel/nstemplatetier_test.go b/test/e2e/parallel/nstemplatetier_test.go index 998c11428..51bc4f5f4 100644 --- a/test/e2e/parallel/nstemplatetier_test.go +++ b/test/e2e/parallel/nstemplatetier_test.go @@ -21,8 +21,6 @@ import ( . "github.com/codeready-toolchain/toolchain-e2e/testsupport/space" "github.com/codeready-toolchain/toolchain-e2e/testsupport/tiers" "github.com/codeready-toolchain/toolchain-e2e/testsupport/wait" - apiwait "k8s.io/apimachinery/pkg/util/wait" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/labels" @@ -405,9 +403,7 @@ func TestTierTemplateRevision(t *testing.T) { // But since the creation of a TTR could be very quick and could trigger another reconcile of the NSTemplateTier before the status is actually updated with the reference, // this might generate some copies of the TTRs. This is not a problem in production since the cleanup mechanism of TTRs will remove the extra ones but could cause some flakiness with the test, // thus we assert the number of TTRs doesn't exceed the double of the expected number. - ttrCount, err := WaitForTTRsToMatch(t, hostAwait, customTier.NSTemplateTier, len(tiers.GetTemplateRefs(t, hostAwait, "ttr").Flatten())*2, func(actualCount, expectedCount int) { - assert.LessOrEqual(t, actualCount, expectedCount) - }) + ttrs, err := hostAwait.WaitForTTRs(t, customTier.Name, wait.LessOrEqual(len(tiers.GetTemplateRefs(t, hostAwait, "ttr").Flatten())*2)) require.NoError(t, err) t.Run("update of tiertemplate should trigger creation of new TTR", func(t *testing.T) { @@ -426,14 +422,12 @@ func TestTierTemplateRevision(t *testing.T) { // then // a new TTR was created - updatedTTrCount, err := WaitForTTRsToMatch(t, hostAwait, customTier.NSTemplateTier, ttrCount+1, func(actualCount, expectedCount int) { - assert.GreaterOrEqual(t, actualCount, expectedCount) - }) + updatedTTRs, err := hostAwait.WaitForTTRs(t, customTier.Name, wait.GreaterOrEqual(len(ttrs)+1)) require.NoError(t, err) t.Run("update of the NSTemplateTier parameters should trigger creation of new TTR", func(t *testing.T) { // given - // that the tiertemplates and nstemlpatetier are provisioned from the parent test + // that the TierTemplates and NSTemplateTier are provisioned from the parent test // when // we update a parameter in the NSTemplateTier @@ -443,9 +437,7 @@ func TestTierTemplateRevision(t *testing.T) { // then // an additional TTR was created - _, err := WaitForTTRsToMatch(t, hostAwait, customTier.NSTemplateTier, updatedTTrCount+1, func(actualCount, expectedCount int) { - assert.GreaterOrEqual(t, actualCount, expectedCount) - }) + _, err := hostAwait.WaitForTTRs(t, customTier.Name, wait.GreaterOrEqual(len(updatedTTRs)+1)) require.NoError(t, err) }) @@ -453,32 +445,6 @@ func TestTierTemplateRevision(t *testing.T) { } -func WaitForTTRsToMatch(t *testing.T, hostAwait *wait.HostAwaitility, customTier *toolchainv1alpha1.NSTemplateTier, expectedTTRsCount int, ttrCountComparisonFunc func(currentTTRsCount, expectedTTRsCount int)) (int, error) { - var ttrCount int - err := apiwait.PollUntilContextTimeout(context.TODO(), hostAwait.RetryInterval, hostAwait.Timeout, true, func(ctx context.Context) (done bool, err error) { - objs := &toolchainv1alpha1.TierTemplateRevisionList{} - if err := hostAwait.Client.List(ctx, objs, client.InNamespace(hostAwait.Namespace)); err != nil { - return false, err - } - ttrCount = len(objs.Items) - ttrCountComparisonFunc(ttrCount, expectedTTRsCount) - // we check that the TTR content has the parameters replaced with values from the NSTemplateTier - for _, obj := range objs.Items { - // the object should have all the variables still there since this one will be replaced when provisioning the Space - assert.Contains(t, string(obj.Spec.TemplateObjects[0].Raw), ".SPACE_NAME") - assert.Contains(t, string(obj.Spec.TemplateObjects[0].Raw), ".DEPLOYMENT_QUOTA") - // the parameter is copied from the NSTemplateTier - assert.NotNil(t, obj.Spec.Parameters) - assert.NotNil(t, customTier.Spec.Parameters) - // we only expect the static parameter DEPLOYMENT_QUOTA to be copied from the tier to the TTR. - // the SPACE_NAME is not a parameter, but a dynamic variable which will be evaluated when provisioning a namespace for the user. - assert.ElementsMatch(t, customTier.Spec.Parameters, obj.Spec.Parameters) - } - return true, nil - }) - return ttrCount, err -} - func getTestCRQ(podsCount string) unstructured.Unstructured { crq := unstructured.Unstructured{Object: map[string]interface{}{ "kind": "ClusterResourceQuota", diff --git a/testsupport/wait/host.go b/testsupport/wait/host.go index 83f125fef..a6b7d91cd 100644 --- a/testsupport/wait/host.go +++ b/testsupport/wait/host.go @@ -994,6 +994,93 @@ func (a *HostAwaitility) WaitForTierTemplate(t *testing.T, name string) (*toolch return tierTemplate, err } +// TierTemplateRevisionWaitCriterion a struct to compare with an expected TierTemplateRevision +type TierTemplateRevisionWaitCriterion struct { + Match func([]toolchainv1alpha1.TierTemplateRevision) bool + Diff func([]toolchainv1alpha1.TierTemplateRevision) string +} + +func matchTierTemplateRevisionWaitCriterion(actual []toolchainv1alpha1.TierTemplateRevision, criteria ...TierTemplateRevisionWaitCriterion) bool { + for _, c := range criteria { + // if at least one criteria does not match, keep waiting + if !c.Match(actual) { + return false + } + } + return true +} + +func (a *HostAwaitility) printTierTemplateRevisionWaitCriterionDiffs(t *testing.T, actual []toolchainv1alpha1.TierTemplateRevision, criteria ...TierTemplateRevisionWaitCriterion) { + buf := &strings.Builder{} + if len(actual) == 0 { + buf.WriteString("no ttrs found\n") + } else { + buf.WriteString("failed to find ttrs with matching criteria:\n") + buf.WriteString("actual:\n") + for _, obj := range actual { + y, _ := StringifyObject(&obj) // nolint:gosec + buf.Write(y) + } + buf.WriteString("\n----\n") + buf.WriteString("diffs:\n") + for _, c := range criteria { + if !c.Match(actual) { + buf.WriteString(c.Diff(actual)) + buf.WriteString("\n") + } + } + } + // include also all TierTemplateRevisions in the host namespace, to help troubleshooting + a.listAndPrint(t, "TierTemplateRevisions", a.Namespace, &toolchainv1alpha1.TierTemplateRevisionList{}) + + t.Log(buf.String()) +} + +// GreaterOrEqual checks if the number of TTRs is greater or equal than the expected one +func GreaterOrEqual(count int) TierTemplateRevisionWaitCriterion { + return TierTemplateRevisionWaitCriterion{ + Match: func(actual []toolchainv1alpha1.TierTemplateRevision) bool { + return len(actual) >= count + }, + Diff: func(actual []toolchainv1alpha1.TierTemplateRevision) string { + return fmt.Sprintf("number of ttrs %d is not greater or equal than %d \n", len(actual), count) + }, + } +} + +// LessOrEqual checks if the number of TTRs is less or equal than the expected one +func LessOrEqual(count int) TierTemplateRevisionWaitCriterion { + return TierTemplateRevisionWaitCriterion{ + Match: func(actual []toolchainv1alpha1.TierTemplateRevision) bool { + return len(actual) <= count + }, + Diff: func(actual []toolchainv1alpha1.TierTemplateRevision) string { + return fmt.Sprintf("number of ttrs %d is not less or equal than %d \n", len(actual), count) + }, + } +} + +func (a *HostAwaitility) WaitForTTRs(t *testing.T, tierName string, criteria ...TierTemplateRevisionWaitCriterion) ([]toolchainv1alpha1.TierTemplateRevision, error) { + t.Logf("waiting for ttrs to match criteria for tier '%s'", tierName) + var ttrs []toolchainv1alpha1.TierTemplateRevision + err := wait.PollUntilContextTimeout(context.TODO(), a.RetryInterval, a.Timeout, true, func(ctx context.Context) (done bool, err error) { + objs := &toolchainv1alpha1.TierTemplateRevisionList{} + if err := a.Client.List(ctx, objs, client.InNamespace(a.Namespace), client.MatchingLabels{toolchainv1alpha1.TierLabelKey: tierName}); err != nil { + return false, err + } + if len(objs.Items) == 0 { + return false, nil + } + ttrs = objs.Items + return matchTierTemplateRevisionWaitCriterion(ttrs, criteria...), nil + }) + // no match found, print the diffs + if err != nil { + a.printTierTemplateRevisionWaitCriterionDiffs(t, ttrs, criteria...) + } + return ttrs, err +} + // NSTemplateTierWaitCriterion a struct to compare with an expected NSTemplateTier type NSTemplateTierWaitCriterion struct { Match func(*toolchainv1alpha1.NSTemplateTier) bool From 033039b8a22791d66d39eb6843f0fdb412c11d4a Mon Sep 17 00:00:00 2001 From: Devtools Date: Sat, 25 Jan 2025 21:30:59 +0100 Subject: [PATCH 03/10] fix duplicate map value --- test/e2e/parallel/nstemplatetier_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/e2e/parallel/nstemplatetier_test.go b/test/e2e/parallel/nstemplatetier_test.go index 51bc4f5f4..6d6bdcef5 100644 --- a/test/e2e/parallel/nstemplatetier_test.go +++ b/test/e2e/parallel/nstemplatetier_test.go @@ -384,7 +384,7 @@ func TestTierTemplateRevision(t *testing.T) { } // for simplicity, we add the CRQ to all types of templates (both cluster scope and namespace scoped), // even if the CRQ is cluster scoped. - // WARNING: thus THIS NSTemplateTier should NOT be sued to provision a user!!! + // WARNING: thus THIS NSTemplateTier should NOT be used to provision a user!!! customTier := tiers.CreateCustomNSTemplateTier(t, hostAwait, "ttr", baseTier, tiers.WithNamespaceResources(t, baseTier, updateTierTemplateObjects), tiers.WithClusterResources(t, baseTier, updateTierTemplateObjects), @@ -432,7 +432,8 @@ func TestTierTemplateRevision(t *testing.T) { // when // we update a parameter in the NSTemplateTier // by increasing the deployment quota - customTier = tiers.UpdateCustomNSTemplateTier(t, hostAwait, customTier, tiers.WithParameter("DEPLOYMENT_QUOTA", "100")) + customTier.NSTemplateTier.Spec.Parameters = []toolchainv1alpha1.Parameter{{Name: "DEPLOYMENT_QUOTA", Value: "100"}} + customTier = tiers.UpdateCustomNSTemplateTier(t, hostAwait, customTier) require.NoError(t, err) // then From ab66034ead6201acf64e7ada857af5afaa7ab1b2 Mon Sep 17 00:00:00 2001 From: Devtools Date: Sun, 26 Jan 2025 09:20:45 +0100 Subject: [PATCH 04/10] fix with parameters to update values --- test/e2e/parallel/nstemplatetier_test.go | 3 +-- testsupport/tiers/tier_setup.go | 28 +++++++++++++++++++----- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/test/e2e/parallel/nstemplatetier_test.go b/test/e2e/parallel/nstemplatetier_test.go index 6d6bdcef5..644e8ba7b 100644 --- a/test/e2e/parallel/nstemplatetier_test.go +++ b/test/e2e/parallel/nstemplatetier_test.go @@ -432,8 +432,7 @@ func TestTierTemplateRevision(t *testing.T) { // when // we update a parameter in the NSTemplateTier // by increasing the deployment quota - customTier.NSTemplateTier.Spec.Parameters = []toolchainv1alpha1.Parameter{{Name: "DEPLOYMENT_QUOTA", Value: "100"}} - customTier = tiers.UpdateCustomNSTemplateTier(t, hostAwait, customTier) + customTier = tiers.UpdateCustomNSTemplateTier(t, hostAwait, customTier, tiers.WithParameter("DEPLOYMENT_QUOTA", "100")) require.NoError(t, err) // then diff --git a/testsupport/tiers/tier_setup.go b/testsupport/tiers/tier_setup.go index a1fe4a34a..02241063a 100644 --- a/testsupport/tiers/tier_setup.go +++ b/testsupport/tiers/tier_setup.go @@ -86,12 +86,28 @@ func WithParameter(name, value string) CustomNSTemplateTierModifier { if tier.Spec.Parameters == nil { tier.Spec.Parameters = []toolchainv1alpha1.Parameter{} } - tier.Spec.Parameters = append(tier.Spec.Parameters, - toolchainv1alpha1.Parameter{ - Name: name, - Value: value, - }, - ) + + newParams := make([]toolchainv1alpha1.Parameter, len(tier.Spec.Parameters)) + // search for the param, in case it already exists, we need only to change its value + found := false + for _, param := range tier.Spec.Parameters { + if param.Name == name { + // if the param already exists, let's set the new value + param.Value = value + found = true + } + newParams = append(newParams, param) + } + // if it's a new parameter, let's append it to the existing ones + if !found { + newParams = append(newParams, + toolchainv1alpha1.Parameter{ + Name: name, + Value: value, + }, + ) + } + tier.Spec.Parameters = newParams return nil } } From 7735de5a82282bffcd32342f8144d9faf97ba82c Mon Sep 17 00:00:00 2001 From: Devtools Date: Sun, 26 Jan 2025 11:33:15 +0100 Subject: [PATCH 05/10] fix linter --- testsupport/tiers/tier_setup.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testsupport/tiers/tier_setup.go b/testsupport/tiers/tier_setup.go index 02241063a..705af143b 100644 --- a/testsupport/tiers/tier_setup.go +++ b/testsupport/tiers/tier_setup.go @@ -87,7 +87,7 @@ func WithParameter(name, value string) CustomNSTemplateTierModifier { tier.Spec.Parameters = []toolchainv1alpha1.Parameter{} } - newParams := make([]toolchainv1alpha1.Parameter, len(tier.Spec.Parameters)) + newParams := make([]toolchainv1alpha1.Parameter, 0, len(tier.Spec.Parameters)) // search for the param, in case it already exists, we need only to change its value found := false for _, param := range tier.Spec.Parameters { From e1c758d51793b49325a7a9279ea8fff48f8f79f8 Mon Sep 17 00:00:00 2001 From: Devtools Date: Wed, 29 Jan 2025 18:07:28 +0100 Subject: [PATCH 06/10] improve debugging info --- test/e2e/parallel/nstemplatetier_test.go | 2 +- testsupport/wait/host.go | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/test/e2e/parallel/nstemplatetier_test.go b/test/e2e/parallel/nstemplatetier_test.go index 644e8ba7b..4028970b8 100644 --- a/test/e2e/parallel/nstemplatetier_test.go +++ b/test/e2e/parallel/nstemplatetier_test.go @@ -414,7 +414,7 @@ func TestTierTemplateRevision(t *testing.T) { // we update one tiertemplate _, err = wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.TierTemplate{}). Update(customTier.Spec.ClusterResources.TemplateRef, hostAwait.Namespace, func(tiertemplate *toolchainv1alpha1.TierTemplate) { - // let's increase the pod count + // let's reduce the pod count updatedCRQ := getTestCRQ("100") tiertemplate.Spec.TemplateObjects = []runtime.RawExtension{{Object: &updatedCRQ}} }) diff --git a/testsupport/wait/host.go b/testsupport/wait/host.go index a6b7d91cd..4baa4f6a5 100644 --- a/testsupport/wait/host.go +++ b/testsupport/wait/host.go @@ -1010,7 +1010,7 @@ func matchTierTemplateRevisionWaitCriterion(actual []toolchainv1alpha1.TierTempl return true } -func (a *HostAwaitility) printTierTemplateRevisionWaitCriterionDiffs(t *testing.T, actual []toolchainv1alpha1.TierTemplateRevision, criteria ...TierTemplateRevisionWaitCriterion) { +func (a *HostAwaitility) printTierTemplateRevisionWaitCriterionDiffs(t *testing.T, actual []toolchainv1alpha1.TierTemplateRevision, tierName string, criteria ...TierTemplateRevisionWaitCriterion) { buf := &strings.Builder{} if len(actual) == 0 { buf.WriteString("no ttrs found\n") @@ -1030,8 +1030,15 @@ func (a *HostAwaitility) printTierTemplateRevisionWaitCriterionDiffs(t *testing. } } } - // include also all TierTemplateRevisions in the host namespace, to help troubleshooting - a.listAndPrint(t, "TierTemplateRevisions", a.Namespace, &toolchainv1alpha1.TierTemplateRevisionList{}) + opts := client.MatchingLabels(map[string]string{ + toolchainv1alpha1.TierLabelKey: tierName, + }) + // include also all TierTemplateRevisions for the given tier, to help troubleshooting + a.listAndPrint(t, "TierTemplateRevisions", a.Namespace, &toolchainv1alpha1.TierTemplateRevisionList{}, opts) + // include also all TierTemplate for the given tiertemplate revisions, to help troubleshooting + for _, ttr := range actual { + a.GetAndPrint(t, "TierTemplate", a.Namespace, ttr.GetLabels()[toolchainv1alpha1.TemplateRefLabelKey], &toolchainv1alpha1.TierTemplate{}) + } t.Log(buf.String()) } @@ -1076,7 +1083,7 @@ func (a *HostAwaitility) WaitForTTRs(t *testing.T, tierName string, criteria ... }) // no match found, print the diffs if err != nil { - a.printTierTemplateRevisionWaitCriterionDiffs(t, ttrs, criteria...) + a.printTierTemplateRevisionWaitCriterionDiffs(t, ttrs, tierName, criteria...) } return ttrs, err } From e3a9bcc8e53085b5c5638d9c17ca2d05f679142a Mon Sep 17 00:00:00 2001 From: Francisc Munteanu Date: Wed, 5 Feb 2025 14:43:30 +0100 Subject: [PATCH 07/10] Update testsupport/tiers/tier_setup.go Co-authored-by: Matous Jobanek --- testsupport/tiers/tier_setup.go | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/testsupport/tiers/tier_setup.go b/testsupport/tiers/tier_setup.go index 705af143b..82ac46fcf 100644 --- a/testsupport/tiers/tier_setup.go +++ b/testsupport/tiers/tier_setup.go @@ -87,27 +87,20 @@ func WithParameter(name, value string) CustomNSTemplateTierModifier { tier.Spec.Parameters = []toolchainv1alpha1.Parameter{} } - newParams := make([]toolchainv1alpha1.Parameter, 0, len(tier.Spec.Parameters)) - // search for the param, in case it already exists, we need only to change its value - found := false - for _, param := range tier.Spec.Parameters { + for i, param := range tier.Spec.Parameters { if param.Name == name { // if the param already exists, let's set the new value - param.Value = value - found = true + tier.Spec.Parameters[i].Value = value + return nil } - newParams = append(newParams, param) } // if it's a new parameter, let's append it to the existing ones - if !found { - newParams = append(newParams, - toolchainv1alpha1.Parameter{ - Name: name, - Value: value, - }, - ) - } - tier.Spec.Parameters = newParams + tier.Spec.Parameters = append(tier.Spec.Parameters, + toolchainv1alpha1.Parameter{ + Name: name, + Value: value, + }, + ) return nil } } From 124a1c023dfb41a2b28dc2e2e1e9b77143bc4924 Mon Sep 17 00:00:00 2001 From: Devtools Date: Wed, 5 Feb 2025 14:42:26 +0100 Subject: [PATCH 08/10] check for ttr changes --- test/e2e/parallel/nstemplatetier_test.go | 40 ++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/test/e2e/parallel/nstemplatetier_test.go b/test/e2e/parallel/nstemplatetier_test.go index 4028970b8..9bea0d3af 100644 --- a/test/e2e/parallel/nstemplatetier_test.go +++ b/test/e2e/parallel/nstemplatetier_test.go @@ -409,13 +409,16 @@ func TestTierTemplateRevision(t *testing.T) { t.Run("update of tiertemplate should trigger creation of new TTR", func(t *testing.T) { // given // that the tiertemplates and nstemlpatetier are provisioned from the parent test + ttrToBeModified, found := customTier.Status.Revisions[customTier.Spec.ClusterResources.TemplateRef] + require.True(t, found) + checkThatTTRContainsCRQ(t, ttrToBeModified, ttrs, crq) // when // we update one tiertemplate + // let's reduce the pod count + updatedCRQ := getTestCRQ("100") _, err = wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.TierTemplate{}). Update(customTier.Spec.ClusterResources.TemplateRef, hostAwait.Namespace, func(tiertemplate *toolchainv1alpha1.TierTemplate) { - // let's reduce the pod count - updatedCRQ := getTestCRQ("100") tiertemplate.Spec.TemplateObjects = []runtime.RawExtension{{Object: &updatedCRQ}} }) require.NoError(t, err) @@ -424,6 +427,12 @@ func TestTierTemplateRevision(t *testing.T) { // a new TTR was created updatedTTRs, err := hostAwait.WaitForTTRs(t, customTier.Name, wait.GreaterOrEqual(len(ttrs)+1)) require.NoError(t, err) + // get the updated nstemplatetier + updatedCustomTier, err := hostAwait.WaitForNSTemplateTier(t, customTier.Name) + newTTR, found := updatedCustomTier.Status.Revisions[updatedCustomTier.Spec.ClusterResources.TemplateRef] + require.True(t, found) + // check that it has the updated crq + checkThatTTRContainsCRQ(t, newTTR, updatedTTRs, updatedCRQ) t.Run("update of the NSTemplateTier parameters should trigger creation of new TTR", func(t *testing.T) { // given @@ -437,8 +446,12 @@ func TestTierTemplateRevision(t *testing.T) { // then // an additional TTR was created - _, err := hostAwait.WaitForTTRs(t, customTier.Name, wait.GreaterOrEqual(len(updatedTTRs)+1)) + ttrsWithNewParams, err := hostAwait.WaitForTTRs(t, customTier.Name, wait.GreaterOrEqual(len(updatedTTRs)+1)) require.NoError(t, err) + checkThatTTRsHaveParameter(t, customTier, ttrsWithNewParams, toolchainv1alpha1.Parameter{ + Name: "DEPLOYMENT_QUOTA", + Value: "100", + }) }) }) @@ -520,3 +533,24 @@ const ( } ` ) + +// checkThatTTRContainsCRQ verifies if a given ttr from the list contains the CRQ in the templateObjects field +func checkThatTTRContainsCRQ(t *testing.T, ttrName string, ttrs []toolchainv1alpha1.TierTemplateRevision, crq unstructured.Unstructured) { + for _, ttr := range ttrs { + if ttr.Name == ttrName { + assert.NotEmpty(t, ttr.Spec.TemplateObjects) + assert.Equal(t, []runtime.RawExtension{{Object: &crq}}, ttr.Spec.TemplateObjects) + } + } + require.FailNowf(t, "Unable to find a TTR with required crq", "ttr:%s CRQ:%+v", ttrName, crq) +} + +// checkThatTTRsHaveParameter verifies that ttrs from the list have the required parameter +func checkThatTTRsHaveParameter(t *testing.T, tier *tiers.CustomNSTemplateTier, ttrs []toolchainv1alpha1.TierTemplateRevision, parameters toolchainv1alpha1.Parameter) { + for _, ttr := range ttrs { + // if the ttr is still in the revisions field we check that it contains the required parameters + if ttrNameInRev, ttrFound := tier.Status.Revisions[ttr.GetLabels()[toolchainv1alpha1.TemplateRefLabelKey]]; ttrFound && ttrNameInRev == ttr.Name { + assert.Contains(t, ttr.Spec.Parameters, parameters, "Unable to find required parameters:%+v in the TTR:%s", parameters, ttr.Name) + } + } +} From 488ef7b218d9681d4fad641fbc1d436da11602ee Mon Sep 17 00:00:00 2001 From: Devtools Date: Thu, 6 Feb 2025 08:40:40 +0100 Subject: [PATCH 09/10] check parameters and crq changes in ttrs --- test/e2e/parallel/nstemplatetier_test.go | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/test/e2e/parallel/nstemplatetier_test.go b/test/e2e/parallel/nstemplatetier_test.go index 9bea0d3af..5f64d5515 100644 --- a/test/e2e/parallel/nstemplatetier_test.go +++ b/test/e2e/parallel/nstemplatetier_test.go @@ -391,14 +391,14 @@ func TestTierTemplateRevision(t *testing.T) { tiers.WithSpaceRoles(t, baseTier, updateTierTemplateObjects), tiers.WithParameter("DEPLOYMENT_QUOTA", "60")) // when - // we verify the counters in the status.history for 'tierUsingTierTemplateRevisions' tier - // and verify that TierTemplateRevision CRs were created, since all the tiertemplates now have templateObjects field populated - _, err = hostAwait.WaitForNSTemplateTierAndCheckTemplates(t, "ttr", + // we verify that TierTemplateRevision CRs were created, since all the tiertemplates now have templateObjects field populated + tier, err := hostAwait.WaitForNSTemplateTierAndCheckTemplates(t, "ttr", wait.HasStatusTierTemplateRevisions(tiers.GetTemplateRefs(t, hostAwait, "ttr").Flatten())) require.NoError(t, err) + customTier.NSTemplateTier = tier // then - // check the expected total number of ttr matches + // check the expected total number of ttr matches, // we IDEALLY expect one TTR per each tiertemplate to be created (clusterresource, namespace and spacerole), thus a total of 3 TTRs ideally. // But since the creation of a TTR could be very quick and could trigger another reconcile of the NSTemplateTier before the status is actually updated with the reference, // this might generate some copies of the TTRs. This is not a problem in production since the cleanup mechanism of TTRs will remove the extra ones but could cause some flakiness with the test, @@ -411,6 +411,7 @@ func TestTierTemplateRevision(t *testing.T) { // that the tiertemplates and nstemlpatetier are provisioned from the parent test ttrToBeModified, found := customTier.Status.Revisions[customTier.Spec.ClusterResources.TemplateRef] require.True(t, found) + // check that it has the crq before updating it checkThatTTRContainsCRQ(t, ttrToBeModified, ttrs, crq) // when @@ -437,17 +438,22 @@ func TestTierTemplateRevision(t *testing.T) { t.Run("update of the NSTemplateTier parameters should trigger creation of new TTR", func(t *testing.T) { // given // that the TierTemplates and NSTemplateTier are provisioned from the parent test + // and they have the initial parameter value for deployment quota + checkThatTTRsHaveParameter(t, customTier, updatedTTRs, toolchainv1alpha1.Parameter{ + Name: "DEPLOYMENT_QUOTA", + Value: "60", + }) // when - // we update a parameter in the NSTemplateTier - // by increasing the deployment quota + // we increase the parameter for the deployment quota customTier = tiers.UpdateCustomNSTemplateTier(t, hostAwait, customTier, tiers.WithParameter("DEPLOYMENT_QUOTA", "100")) require.NoError(t, err) // then - // an additional TTR was created + // an additional TTR will be created ttrsWithNewParams, err := hostAwait.WaitForTTRs(t, customTier.Name, wait.GreaterOrEqual(len(updatedTTRs)+1)) require.NoError(t, err) + // and the parameter is updated in all the ttrs checkThatTTRsHaveParameter(t, customTier, ttrsWithNewParams, toolchainv1alpha1.Parameter{ Name: "DEPLOYMENT_QUOTA", Value: "100", From 719c2a1270f34ea03793a8a4e97fdf57c2715b38 Mon Sep 17 00:00:00 2001 From: Devtools Date: Thu, 6 Feb 2025 14:35:48 +0100 Subject: [PATCH 10/10] fix ttr test --- test/e2e/parallel/nstemplatetier_test.go | 20 ++++++++++++++++---- testsupport/tiers/tier_setup.go | 2 +- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/test/e2e/parallel/nstemplatetier_test.go b/test/e2e/parallel/nstemplatetier_test.go index 5f64d5515..377f2fbf6 100644 --- a/test/e2e/parallel/nstemplatetier_test.go +++ b/test/e2e/parallel/nstemplatetier_test.go @@ -13,6 +13,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "github.com/gofrs/uuid" + "k8s.io/client-go/kubernetes/scheme" toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" "github.com/codeready-toolchain/toolchain-common/pkg/states" @@ -403,6 +404,7 @@ func TestTierTemplateRevision(t *testing.T) { // But since the creation of a TTR could be very quick and could trigger another reconcile of the NSTemplateTier before the status is actually updated with the reference, // this might generate some copies of the TTRs. This is not a problem in production since the cleanup mechanism of TTRs will remove the extra ones but could cause some flakiness with the test, // thus we assert the number of TTRs doesn't exceed the double of the expected number. + // TODO check for exact match or remove the *2 and check for not empty revisions list, once we implement the cleanup controller ttrs, err := hostAwait.WaitForTTRs(t, customTier.Name, wait.LessOrEqual(len(tiers.GetTemplateRefs(t, hostAwait, "ttr").Flatten())*2)) require.NoError(t, err) @@ -426,6 +428,7 @@ func TestTierTemplateRevision(t *testing.T) { // then // a new TTR was created + // TODO check for exact match or remove the +1 once we implement the cleanup controller updatedTTRs, err := hostAwait.WaitForTTRs(t, customTier.Name, wait.GreaterOrEqual(len(ttrs)+1)) require.NoError(t, err) // get the updated nstemplatetier @@ -451,8 +454,12 @@ func TestTierTemplateRevision(t *testing.T) { // then // an additional TTR will be created + // TODO check for exact match or remove the +1 once we implement the cleanup controller ttrsWithNewParams, err := hostAwait.WaitForTTRs(t, customTier.Name, wait.GreaterOrEqual(len(updatedTTRs)+1)) require.NoError(t, err) + // retrieve new tier once the ttrs were created and the revision field updated + customTier.NSTemplateTier, err = hostAwait.WaitForNSTemplateTier(t, customTier.Name) + require.NoError(t, err) // and the parameter is updated in all the ttrs checkThatTTRsHaveParameter(t, customTier, ttrsWithNewParams, toolchainv1alpha1.Parameter{ Name: "DEPLOYMENT_QUOTA", @@ -472,16 +479,16 @@ func getTestCRQ(podsCount string) unstructured.Unstructured { }, "spec": map[string]interface{}{ "quota": map[string]interface{}{ - "hard": map[string]string{ + "hard": map[string]interface{}{ "count/deploymentconfigs.apps": "{{.DEPLOYMENT_QUOTA}}", "count/deployments.apps": "{{.DEPLOYMENT_QUOTA}}", "count/pods": podsCount, }, }, "selector": map[string]interface{}{ - "annotations": map[string]string{}, + "annotations": map[string]interface{}{}, "labels": map[string]interface{}{ - "matchLabels": map[string]string{ + "matchLabels": map[string]interface{}{ "toolchain.dev.openshift.com/space": "{{.SPACE_NAME}}", }, }, @@ -545,7 +552,11 @@ func checkThatTTRContainsCRQ(t *testing.T, ttrName string, ttrs []toolchainv1alp for _, ttr := range ttrs { if ttr.Name == ttrName { assert.NotEmpty(t, ttr.Spec.TemplateObjects) - assert.Equal(t, []runtime.RawExtension{{Object: &crq}}, ttr.Spec.TemplateObjects) + unstructuredObj := &unstructured.Unstructured{} + _, _, err := scheme.Codecs.UniversalDeserializer().Decode(ttr.Spec.TemplateObjects[0].Raw, nil, unstructuredObj) + require.NoError(t, err) + assert.Equal(t, &crq, unstructuredObj) + return } } require.FailNowf(t, "Unable to find a TTR with required crq", "ttr:%s CRQ:%+v", ttrName, crq) @@ -557,6 +568,7 @@ func checkThatTTRsHaveParameter(t *testing.T, tier *tiers.CustomNSTemplateTier, // if the ttr is still in the revisions field we check that it contains the required parameters if ttrNameInRev, ttrFound := tier.Status.Revisions[ttr.GetLabels()[toolchainv1alpha1.TemplateRefLabelKey]]; ttrFound && ttrNameInRev == ttr.Name { assert.Contains(t, ttr.Spec.Parameters, parameters, "Unable to find required parameters:%+v in the TTR:%s", parameters, ttr.Name) + return } } } diff --git a/testsupport/tiers/tier_setup.go b/testsupport/tiers/tier_setup.go index 82ac46fcf..e5d1c47f6 100644 --- a/testsupport/tiers/tier_setup.go +++ b/testsupport/tiers/tier_setup.go @@ -152,7 +152,7 @@ func UpdateCustomNSTemplateTier(t *testing.T, hostAwait *wait.HostAwaitility, ti err := modify(hostAwait, tier) require.NoError(t, err) } - _, err = wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.NSTemplateTier{}). + tier.NSTemplateTier, err = wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.NSTemplateTier{}). Update(tier.NSTemplateTier.Name, hostAwait.Namespace, func(nstt *toolchainv1alpha1.NSTemplateTier) { nstt.Spec = tier.NSTemplateTier.Spec })