Skip to content

Commit cf062f9

Browse files
metlosmetlos
committed
simplify nstemplatetiers.GenerateTiers to not distiniguish between create/update in the EnsureObject func. This is in preparation for using the SSA client that doesn't do that either.
1 parent 109b36e commit cf062f9

File tree

2 files changed

+28
-70
lines changed

2 files changed

+28
-70
lines changed

‎pkg/template/nstemplatetiers/nstemplatetier_generator.go‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import (
2121

2222
var log = logf.Log.WithName("templates")
2323

24-
type EnsureObject func(toEnsure runtimeclient.Object, canUpdate bool, tierName string) (bool, error)
24+
type EnsureObject func(toEnsure runtimeclient.Object, tierName string) error
2525

2626
type TierGenerator struct {
2727
ensureObject EnsureObject
@@ -76,7 +76,6 @@ func GenerateTiers(s *runtime.Scheme, ensureObject EnsureObject, namespace strin
7676

7777
// newNSTemplateTierGenerator loads templates from the provided assets and processes the tierTemplates and NSTemplateTiers
7878
func newNSTemplateTierGenerator(s *runtime.Scheme, ensureObject EnsureObject, namespace string, metadata map[string]string, files map[string][]byte) (*TierGenerator, error) {
79-
8079
templatesByTier, err := loadTemplatesByTiers(metadata, files)
8180
if err != nil {
8281
return nil, err
@@ -141,7 +140,6 @@ type BasedOnTier struct {
141140
// an optional `template` for the cluster resources (`clusterTemplate`) and the NSTemplateTier resource object.
142141
// Each `template` object contains a `revision` (`string`) and the `content` of the template to apply (`[]byte`)
143142
func loadTemplatesByTiers(metadata map[string]string, files map[string][]byte) (map[string]*tierData, error) {
144-
145143
results := make(map[string]*tierData)
146144
for name, content := range files {
147145
// split the name using the `/` separator
@@ -203,7 +201,6 @@ func loadTemplatesByTiers(metadata map[string]string, files map[string][]byte) (
203201

204202
// initTierTemplates generates all TierTemplate resources, and adds them to the tier map indexed by tier name
205203
func (t *TierGenerator) initTierTemplates() error {
206-
207204
// process tiers in alphabetical order
208205
tiers := make([]string, 0, len(t.templatesByTier))
209206
for tier := range t.templatesByTier {
@@ -279,7 +276,7 @@ func (t *TierGenerator) createTierTemplates() error {
279276
for _, tierTmpl := range tierTmpls.tierTemplates {
280277
log.Info("creating TierTemplate", "namespace", tierTmpl.Namespace, "name", tierTmpl.Name)
281278
// using the "standard" client since we don't need to support updates on such resources, they should be immutable
282-
if _, err := t.ensureObject(tierTmpl, false, tierName); err != nil {
279+
if err := t.ensureObject(tierTmpl, tierName); err != nil {
283280
return errors.Wrapf(err, "unable to create the '%s' TierTemplate in namespace '%s'", tierTmpl.Name, tierTmpl.Namespace)
284281
}
285282
log.Info("TierTemplate resource created", "namespace", tierTmpl.Namespace, "name", tierTmpl.Name)
@@ -336,7 +333,6 @@ func newTierTemplateName(tier, kind, revision string) string {
336333

337334
// newNSTemplateTiers generates all NSTemplateTier resources and adds them to the tier map
338335
func (t *TierGenerator) initNSTemplateTiers() error {
339-
340336
for tierName, tierData := range t.templatesByTier {
341337
nsTemplateTier := tierData.rawTemplates.nsTemplateTier
342338
tierTemplates := tierData.tierTemplates
@@ -360,7 +356,6 @@ func (t *TierGenerator) initNSTemplateTiers() error {
360356

361357
// createNSTemplateTiers creates the NSTemplateTier resources from the tier map
362358
func (t *TierGenerator) createNSTemplateTiers() error {
363-
364359
for tierName, tierData := range t.templatesByTier {
365360
if len(tierData.objects) != 1 {
366361
return fmt.Errorf("there is an unexpected number of NSTemplateTier object to be applied for tier name '%s'; expected: 1; actual: %d", tierName, len(tierData.objects))
@@ -380,7 +375,7 @@ func (t *TierGenerator) createNSTemplateTiers() error {
380375
labels = make(map[string]string)
381376
}
382377
labels[toolchainv1alpha1.ProviderLabelKey] = toolchainv1alpha1.ProviderLabelValue
383-
updated, err := t.ensureObject(tier, true, tierName)
378+
err := t.ensureObject(tier, tierName)
384379
if err != nil {
385380
return errors.Wrapf(err, "unable to create or update the '%s' NSTemplateTier", tierName)
386381
}
@@ -394,11 +389,7 @@ func (t *TierGenerator) createNSTemplateTiers() error {
394389
for role, nsTemplate := range tier.Spec.SpaceRoles {
395390
tierLog = tierLog.WithValues(fmt.Sprintf("spaceRoleTemplate-%s", role), nsTemplate.TemplateRef)
396391
}
397-
if updated {
398-
tierLog.Info("NSTemplateTier was either updated or created")
399-
} else {
400-
tierLog.Info("NSTemplateTier wasn't updated nor created: the spec was already set as expected")
401-
}
392+
tierLog.Info("NSTemplateTier was patched")
402393
}
403394
return nil
404395
}

‎pkg/template/nstemplatetiers/nstemplatetier_generator_test.go‎

Lines changed: 24 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"fmt"
88
"io/fs"
99
"path/filepath"
10+
"reflect"
1011
"regexp"
1112
"testing"
1213
texttemplate "text/template"
@@ -17,8 +18,6 @@ import (
1718
commonclient "github.com/codeready-toolchain/toolchain-common/pkg/client"
1819
"github.com/codeready-toolchain/toolchain-common/pkg/test"
1920
"github.com/pkg/errors"
20-
apierrors "k8s.io/apimachinery/pkg/api/errors"
21-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2221
"k8s.io/apimachinery/pkg/types"
2322
runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
2423

@@ -141,12 +140,10 @@ func getTestTemplates(t *testing.T) map[string][]byte {
141140
}
142141

143142
func TestGenerateTiers(t *testing.T) {
144-
145143
s := addToScheme(t)
146144
logf.SetLogger(zap.New(zap.UseDevMode(true)))
147145

148146
t.Run("ok", func(t *testing.T) {
149-
150147
expectedTemplateRefs := map[string]map[string]interface{}{
151148
"advanced": {
152149
"clusterresources": "advanced-clusterresources-abcd123-654321a",
@@ -280,19 +277,29 @@ func TestGenerateTiers(t *testing.T) {
280277
// when
281278
err := GenerateTiers(s, ensureObjectFuncForClient(clt), namespace, getTestMetadata(), getTestTemplates(t))
282279
require.NoError(t, err)
280+
// here we're just capturing the state after the first call so that we can compare with the state after the second call
281+
tierTmpls := toolchainv1alpha1.TierTemplateList{}
282+
err = clt.List(context.TODO(), &tierTmpls, runtimeclient.InNamespace(namespace))
283+
require.NoError(t, err)
284+
require.Len(t, tierTmpls.Items, 16) // 4 items for advanced and base tiers + 3 for nocluster tier + 4 for appstudio
285+
origTemplates := map[string]*toolchainv1alpha1.TierTemplate{}
286+
for _, tmpl := range tierTmpls.Items {
287+
origTemplates[tmpl.Name] = &tmpl
288+
}
283289

284290
// when calling CreateOrUpdateResources a second time
285291
err = GenerateTiers(s, ensureObjectFuncForClient(clt), namespace, getTestMetadata(), getTestTemplates(t))
286292

287293
// then
288294
require.NoError(t, err)
289295
// verify that all TierTemplate CRs were updated
290-
tierTmpls := toolchainv1alpha1.TierTemplateList{}
296+
tierTmpls = toolchainv1alpha1.TierTemplateList{}
291297
err = clt.List(context.TODO(), &tierTmpls, runtimeclient.InNamespace(namespace))
292298
require.NoError(t, err)
293299
require.Len(t, tierTmpls.Items, 16) // 4 items for advanced and base tiers + 3 for nocluster tier + 4 for appstudio
300+
// check that the tier templates are unchanged
294301
for _, tierTmpl := range tierTmpls.Items {
295-
assert.Equal(t, int64(1), tierTmpl.Generation) // unchanged
302+
assert.True(t, reflect.DeepEqual(origTemplates[tierTmpl.Name].Spec, tierTmpl.Spec))
296303
}
297304

298305
// verify that 4 NSTemplateTier CRs were created:
@@ -446,27 +453,25 @@ func TestGenerateTiers(t *testing.T) {
446453
})
447454

448455
t.Run("failures", func(t *testing.T) {
449-
450456
namespace := "host-operator" + uuid.NewString()[:7]
451457

452458
t.Run("nstemplatetiers", func(t *testing.T) {
453-
454-
t.Run("failed to create nstemplatetiers", func(t *testing.T) {
459+
t.Run("failed to patch nstemplatetiers", func(t *testing.T) {
455460
// given
456461
clt := test.NewFakeClient(t)
457-
clt.MockCreate = func(ctx context.Context, obj runtimeclient.Object, opts ...runtimeclient.CreateOption) error {
458-
if obj.GetObjectKind().GroupVersionKind().Kind == "NSTemplateTier" {
462+
clt.MockPatch = func(ctx context.Context, obj runtimeclient.Object, patch runtimeclient.Patch, opts ...runtimeclient.PatchOption) error {
463+
if _, ok := obj.(*toolchainv1alpha1.NSTemplateTier); ok {
459464
// simulate a client/server error
460465
return errors.Errorf("an error")
461466
}
462-
return clt.Client.Create(ctx, obj, opts...)
467+
return test.Patch(ctx, clt, obj, patch, opts...)
463468
}
464469

465470
// when
466471
err := GenerateTiers(s, ensureObjectFuncForClient(clt), namespace, getTestMetadata(), getTestTemplates(t))
467472
// then
468473
require.Error(t, err)
469-
assert.Regexp(t, "unable to create or update the '\\w+' NSTemplateTier: unable to create resource of kind: NSTemplateTier, version: v1alpha1: an error", err.Error())
474+
assert.Regexp(t, "unable to create NSTemplateTiers: unable to create or update the '\\w+' NSTemplateTier: unable to patch 'toolchain.dev.openshift.com/v1alpha1, Kind=NSTemplateTier' called '\\w+' in namespace '[a-zA-Z0-9-]+': an error", err.Error())
470475
})
471476

472477
t.Run("missing tier.yaml file", func(t *testing.T) {
@@ -481,40 +486,15 @@ func TestGenerateTiers(t *testing.T) {
481486
require.EqualError(t, err, "unable to init NSTemplateTier generator: tier appstudio is missing a tier.yaml file")
482487
})
483488

484-
t.Run("failed to update nstemplatetiers", func(t *testing.T) {
485-
// given
486-
// initialize the client with an existing `advanced` NSTemplatetier
487-
clt := test.NewFakeClient(t, &toolchainv1alpha1.NSTemplateTier{
488-
ObjectMeta: metav1.ObjectMeta{
489-
Namespace: namespace,
490-
Name: "advanced",
491-
},
492-
})
493-
clt.MockUpdate = func(ctx context.Context, obj runtimeclient.Object, opts ...runtimeclient.UpdateOption) error {
494-
if obj.GetObjectKind().GroupVersionKind().Kind == "NSTemplateTier" {
495-
// simulate a client/server error
496-
return errors.Errorf("an error")
497-
}
498-
return clt.Client.Update(ctx, obj, opts...)
499-
}
500-
501-
// when
502-
err := GenerateTiers(s, ensureObjectFuncForClient(clt), namespace, getTestMetadata(), getTestTemplates(t))
503-
504-
// then
505-
require.Error(t, err)
506-
assert.Contains(t, err.Error(), "unable to create NSTemplateTiers: unable to create or update the 'advanced' NSTemplateTier: unable to create resource of kind: NSTemplateTier, version: v1alpha1: unable to update the resource")
507-
})
508-
509-
t.Run("failed to create nstemplatetiers", func(t *testing.T) {
489+
t.Run("failed to patch nstemplatetiers", func(t *testing.T) {
510490
// given
511491
clt := test.NewFakeClient(t)
512-
clt.MockCreate = func(ctx context.Context, obj runtimeclient.Object, opts ...runtimeclient.CreateOption) error {
492+
clt.MockPatch = func(ctx context.Context, obj runtimeclient.Object, patch runtimeclient.Patch, opts ...runtimeclient.PatchOption) error {
513493
if _, ok := obj.(*toolchainv1alpha1.TierTemplate); ok {
514494
// simulate a client/server error
515495
return errors.Errorf("an error")
516496
}
517-
return clt.Client.Create(ctx, obj, opts...)
497+
return test.Patch(ctx, clt, obj, patch, opts...)
518498
}
519499

520500
// when
@@ -529,7 +509,6 @@ func TestGenerateTiers(t *testing.T) {
529509
}
530510

531511
func TestLoadTemplatesByTiers(t *testing.T) {
532-
533512
logf.SetLogger(zap.New(zap.UseDevMode(true)))
534513

535514
t.Run("ok", func(t *testing.T) {
@@ -591,7 +570,6 @@ func TestLoadTemplatesByTiers(t *testing.T) {
591570
})
592571

593572
t.Run("failures", func(t *testing.T) {
594-
595573
t.Run("unparseable content", func(t *testing.T) {
596574
// given
597575
testTemplates := getTestTemplates(t)
@@ -663,13 +641,11 @@ func TestLoadTemplatesByTiers(t *testing.T) {
663641
}
664642

665643
func TestNewNSTemplateTier(t *testing.T) {
666-
667644
s := scheme.Scheme
668645
err := toolchainv1alpha1.AddToScheme(s)
669646
require.NoError(t, err)
670647

671648
t.Run("ok", func(t *testing.T) {
672-
673649
t.Run("with test assets", func(t *testing.T) {
674650
// given
675651
namespace := "host-operator-" + uuid.NewString()[:7]
@@ -740,7 +716,6 @@ func TestNewTierTemplate(t *testing.T) {
740716
namespace := "host-operator-" + uuid.NewString()[:7]
741717

742718
t.Run("ok", func(t *testing.T) {
743-
744719
t.Run("with test assets", func(t *testing.T) {
745720
// given
746721

@@ -780,7 +755,6 @@ func TestNewTierTemplate(t *testing.T) {
780755
})
781756

782757
t.Run("failures", func(t *testing.T) {
783-
784758
t.Run("invalid template", func(t *testing.T) {
785759
// given
786760
testTemplates := getTestTemplates(t)
@@ -796,15 +770,9 @@ func TestNewTierTemplate(t *testing.T) {
796770
}
797771

798772
func ensureObjectFuncForClient(cl runtimeclient.Client) EnsureObject {
799-
return func(toEnsure runtimeclient.Object, canUpdate bool, _ string) (bool, error) {
800-
if !canUpdate {
801-
if err := cl.Create(context.TODO(), toEnsure); err != nil && !apierrors.IsAlreadyExists(err) {
802-
return false, err
803-
}
804-
return true, nil
805-
}
806-
applyCl := commonclient.NewApplyClient(cl)
807-
return applyCl.ApplyObject(context.TODO(), toEnsure, commonclient.ForceUpdate(true))
773+
return func(toEnsure runtimeclient.Object, _ string) error {
774+
applyCl := commonclient.NewSSAApplyClient(cl, "testFieldManager")
775+
return applyCl.ApplyObject(context.TODO(), toEnsure)
808776
}
809777
}
810778

@@ -862,7 +830,6 @@ func expectedTemplateFromBasedOnTierConfig(t *testing.T, tier, templateFileName
862830
}
863831

864832
func TestNewNSTemplateTiers(t *testing.T) {
865-
866833
// given
867834
s := addToScheme(t)
868835

0 commit comments

Comments
 (0)