diff --git a/controllers/nstemplatetier/mapper.go b/controllers/nstemplatetier/mapper.go new file mode 100644 index 000000000..bfe1fdba5 --- /dev/null +++ b/controllers/nstemplatetier/mapper.go @@ -0,0 +1,50 @@ +package nstemplatetier + +import ( + "context" + + toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" + + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + runtimeclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +var mapperLog = ctrl.Log.WithName("MapTierTemplateToNSTemplateTier") + +// MapTierTemplateToNSTemplateTier maps the TierTemplate to all the NSTemplateTiers that are referencing it. +func MapTierTemplateToNSTemplateTier(cl runtimeclient.Client) func(ctx context.Context, object runtimeclient.Object) []reconcile.Request { + return func(ctx context.Context, obj runtimeclient.Object) []reconcile.Request { + logger := mapperLog.WithValues("object-name", obj.GetName(), "object-kind", obj.GetObjectKind()) + nsTmplTierList := &toolchainv1alpha1.NSTemplateTierList{} + err := cl.List(context.TODO(), nsTmplTierList, + runtimeclient.InNamespace(obj.GetNamespace())) + if err != nil { + logger.Error(err, "unable to list NSTemplateTier") + return []reconcile.Request{} + } + if len(nsTmplTierList.Items) == 0 { + logger.Info("no NSTemplateTier found for an object") + return []reconcile.Request{} + } + + req := make([]reconcile.Request, 0, len(nsTmplTierList.Items)) + for _, item := range nsTmplTierList.Items { + if len(item.Status.Revisions) == 0 { + // this tier doesn't use the template + continue + } + if _, inUse := item.Status.Revisions[obj.GetName()]; inUse { + // the tier uses this template + req = append(req, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: item.Namespace, + Name: item.Name, + }, + }) + } + } + return req + } +} diff --git a/controllers/nstemplatetier/mapper_test.go b/controllers/nstemplatetier/mapper_test.go new file mode 100644 index 000000000..4e1e2df4e --- /dev/null +++ b/controllers/nstemplatetier/mapper_test.go @@ -0,0 +1,95 @@ +package nstemplatetier_test + +import ( + "context" + "testing" + + toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" + "github.com/codeready-toolchain/host-operator/controllers/nstemplatetier" + tiertest "github.com/codeready-toolchain/host-operator/test/nstemplatetier" + "github.com/codeready-toolchain/toolchain-common/pkg/test" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +func TestMapTierTemplateToNSTemplateTier(t *testing.T) { + t.Run("should return the only NSTemplateTier referencing the TierTemplate", func(t *testing.T) { + // given + base1nsTier := tiertest.Base1nsTier(t, tiertest.CurrentBase1nsTemplates) + prepareTierWithRevisions(base1nsTier) + // and we have a tier without the revisions + otherTier := tiertest.AppStudioEnvTier(t, tiertest.AppStudioEnvTemplates) + cl := test.NewFakeClient(t, base1nsTier, otherTier) + + // when + // for the simplicity of the test, we only try to map one tiertemplate + clusterResourceTierTemplate := createTierTemplate(t, "clusterresources", nil, base1nsTier.Name) + requests := nstemplatetier.MapTierTemplateToNSTemplateTier(cl)(context.TODO(), clusterResourceTierTemplate) + + // then + require.Len(t, requests, 1) + assert.Contains(t, requests, newRequest(base1nsTier.Name)) + }) + + t.Run("should return two NSTemplateTier when both are referencing the same TierTemplate", func(t *testing.T) { + // given + base1nsTier := tiertest.Base1nsTier(t, tiertest.CurrentBase1nsTemplates) + prepareTierWithRevisions(base1nsTier) + // we update the other tier to use the same revisions + otherTier := tiertest.AppStudioEnvTier(t, tiertest.AppStudioEnvTemplates) + prepareTierWithRevisions(otherTier) + cl := test.NewFakeClient(t, base1nsTier, otherTier) + + // when + // for the simplicity of the test we only try to map one tiertemplate + clusterResourceTierTemplate := createTierTemplate(t, "clusterresources", nil, base1nsTier.Name) + requests := nstemplatetier.MapTierTemplateToNSTemplateTier(cl)(context.TODO(), clusterResourceTierTemplate) + + // then + require.Len(t, requests, 2) + assert.Contains(t, requests, newRequest(base1nsTier.Name)) + assert.Contains(t, requests, newRequest(otherTier.Name)) + }) + + t.Run("should return no NSTemplateTier when they don't use the TierTemplate", func(t *testing.T) { + // given + // both tiers are without revisions + base1nsTier := tiertest.Base1nsTier(t, tiertest.CurrentBase1nsTemplates) + otherTier := tiertest.AppStudioEnvTier(t, tiertest.AppStudioEnvTemplates) + prepareTierWithRevisions(otherTier) + delete(otherTier.Status.Revisions, "base1ns-clusterresources-123456new") + cl := test.NewFakeClient(t, base1nsTier, otherTier) + + // when + // for the simplicity of the test, we only try to map one tiertemplate + clusterResourceTierTemplate := createTierTemplate(t, "clusterresources", nil, base1nsTier.Name) + requests := nstemplatetier.MapTierTemplateToNSTemplateTier(cl)(context.TODO(), clusterResourceTierTemplate) + + // then + require.Empty(t, requests) + }) +} + +func prepareTierWithRevisions(tier *toolchainv1alpha1.NSTemplateTier) { + initialRevisions := map[string]string{ + "base1ns-admin-123456new": "base1ns-admin-123456new-abcd", + "base1ns-clusterresources-123456new": "base1ns-clusterresources-123456new-abcd", + "base1ns-code-123456new": "base1ns-code-123456new-abcd", + "base1ns-dev-123456new": "base1ns-dev-123456new-abcd", + "base1ns-edit-123456new": "`base1ns-edit-123456new-abcd", + "base1ns-stage-123456new": "base1ns-stage-123456new-abcd", + "base1ns-viewer-123456new": "base1ns-viewer-123456new-abcd", + } + tier.Status.Revisions = initialRevisions +} + +func newRequest(name string) reconcile.Request { + return reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: test.HostOperatorNs, + Name: name, + }, + } +} diff --git a/controllers/nstemplatetier/nstemplatetier_controller.go b/controllers/nstemplatetier/nstemplatetier_controller.go index c6c729a84..bd14abee3 100644 --- a/controllers/nstemplatetier/nstemplatetier_controller.go +++ b/controllers/nstemplatetier/nstemplatetier_controller.go @@ -3,6 +3,7 @@ package nstemplatetier import ( "context" "fmt" + "reflect" "time" toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" @@ -10,6 +11,7 @@ import ( "github.com/redhat-cop/operator-utils/pkg/util" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" "k8s.io/apimachinery/pkg/api/errors" @@ -26,6 +28,8 @@ import ( func (r *Reconciler) SetupWithManager(mgr manager.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&toolchainv1alpha1.NSTemplateTier{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). + Watches(&toolchainv1alpha1.TierTemplate{}, + handler.EnqueueRequestsFromMapFunc(MapTierTemplateToNSTemplateTier(r.Client))). Complete(r) } @@ -136,30 +140,42 @@ func (r *Reconciler) ensureTTRforTemplate(ctx context.Context, nsTmplTier *toolc return false, "", nil } - if tierTemplateRevisionName, found := nsTmplTier.Status.Revisions[tierTemplate.GetName()]; found { - logger.Info("TTR set in the status.revisions for tiertemplate", "tierTemplate.Name", tierTemplate.GetName(), "ttr.Name", tierTemplateRevisionName) - var tierTemplateRevision toolchainv1alpha1.TierTemplateRevision - if err := r.Client.Get(ctx, types.NamespacedName{Namespace: nsTmplTier.GetNamespace(), Name: tierTemplateRevisionName}, &tierTemplateRevision); err != nil { - if errors.IsNotFound(err) { - // no tierTemplateRevision CR was found, - logger.Info("TTR CR not found", "tierTemplateRevision.Name", tierTemplateRevisionName) - // let's create one - ttrName, err := r.createNewTierTemplateRevision(ctx, nsTmplTier, tierTemplate) - return true, ttrName, err - } else { - // something wrong happened - return false, "", err - } - } - // TODO compare TierTemplate content with TTR content - // if the TierTemplate has changes we need to create new TTR - } else { + tierTemplateRevisionName, found := nsTmplTier.Status.Revisions[tierTemplate.GetName()] + if !found { // no revision was set for this TierTemplate CR, let's create a TTR for it ttrName, err := r.createNewTierTemplateRevision(ctx, nsTmplTier, tierTemplate) return true, ttrName, err } - // nothing changed - return false, "", nil + + logger.Info("TTR set in the status.revisions for tiertemplate", "tierTemplate.Name", tierTemplate.GetName(), "ttr.Name", tierTemplateRevisionName) + var tierTemplateRevision toolchainv1alpha1.TierTemplateRevision + if err := r.Client.Get(ctx, types.NamespacedName{Namespace: nsTmplTier.GetNamespace(), Name: tierTemplateRevisionName}, &tierTemplateRevision); err != nil { + if errors.IsNotFound(err) { + // no tierTemplateRevision CR was found, + logger.Info("TTR CR not found", "tierTemplateRevision.Name", tierTemplateRevisionName) + // let's create one + ttrName, err := r.createNewTierTemplateRevision(ctx, nsTmplTier, tierTemplate) + return true, ttrName, err + } else { + // something wrong happened + return false, "", err + } + } + // if the TierTemplate has changes we need to create new TTR + // if the TierTemplate objects are the same as they are in the TTR + // and the parameters are the same, too, we don't have to create + // a new revision. + if reflect.DeepEqual(tierTemplate.Spec.TemplateObjects, tierTemplateRevision.Spec.TemplateObjects) && reflect.DeepEqual(nsTmplTier.Spec.Parameters, tierTemplateRevision.Spec.Parameters) { + return false, tierTemplateRevisionName, nil + } + + // there are some changes, a new TTR is needed + ttrName, err := r.createNewTierTemplateRevision(ctx, nsTmplTier, tierTemplate) + if err != nil { + return false, "", err + } + return true, ttrName, nil + } func (r *Reconciler) createNewTierTemplateRevision(ctx context.Context, nsTmplTier *toolchainv1alpha1.NSTemplateTier, tierTemplate *toolchainv1alpha1.TierTemplate) (string, error) { diff --git a/controllers/nstemplatetier/nstemplatetier_controller_test.go b/controllers/nstemplatetier/nstemplatetier_controller_test.go index 538259764..64f04f261 100644 --- a/controllers/nstemplatetier/nstemplatetier_controller_test.go +++ b/controllers/nstemplatetier/nstemplatetier_controller_test.go @@ -27,8 +27,6 @@ import ( "k8s.io/apimachinery/pkg/util/validation" "k8s.io/client-go/kubernetes/scheme" runtimeclient "sigs.k8s.io/controller-runtime/pkg/client" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -38,8 +36,6 @@ const ( func TestReconcile(t *testing.T) { // given - logf.SetLogger(zap.New(zap.UseDevMode(true))) - t.Run("failures", func(t *testing.T) { t.Run("unable to get NSTemplateTier", func(t *testing.T) { @@ -129,29 +125,7 @@ func TestReconcile(t *testing.T) { t.Run("using TTR as revisions", func(t *testing.T) { // initialize tier templates with templateObjects field populated // for simplicity we initialize all of them with the same objects - var 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]interface{}{ - "count/deploymentconfigs.apps": "{{.DEPLOYMENT_QUOTA}}", - "count/deployments.apps": "{{.DEPLOYMENT_QUOTA}}", - "count/pods": "600", - }, - }, - "selector": map[string]interface{}{ - "annotations": map[string]interface{}{}, - "labels": map[string]interface{}{ - "matchLabels": map[string]interface{}{ - "toolchain.dev.openshift.com/space": "'{{.SPACE_NAME}}'", - }, - }, - }, - }, - }} + crq := newTestCRQ("600") t.Run("add revisions when they are missing ", func(t *testing.T) { // given tierTemplates := initTierTemplates(t, withTemplateObjects(crq), base1nsTier.Name) @@ -338,6 +312,118 @@ func TestReconcile(t *testing.T) { } +func TestUpdateNSTemplateTier(t *testing.T) { + // given + base1nsTier := tiertest.Base1nsTier(t, tiertest.CurrentBase1nsTemplates, + // the tiertemplate revision CR should have a copy of those parameters + tiertest.WithParameter("DEPLOYMENT_QUOTA", "60"), + ) + tierTemplatesRefs := []string{ + "base1ns-admin-123456new", "base1ns-clusterresources-123456new", "base1ns-code-123456new", "base1ns-dev-123456new", "base1ns-edit-123456new", "base1ns-stage-123456new", "base1ns-viewer-123456new", + } + + // initialize tier templates with templateObjects field populated + // for simplicity we initialize all of them with the same objects + crq := newTestCRQ("600") + // given + tierTemplates := initTierTemplates(t, withTemplateObjects(crq), base1nsTier.Name) + r, req, cl := prepareReconcile(t, base1nsTier.Name, append(tierTemplates, base1nsTier)...) + // when + res, err := r.Reconcile(context.TODO(), req) + // then + require.NoError(t, err) + require.Equal(t, reconcile.Result{RequeueAfter: time.Second}, res) // explicit requeue after the adding revisions in `status.revisions` + // check that revisions field was populated + oldNSTemplateTier := tiertest.AssertThatNSTemplateTier(t, "base1ns", cl). + HasStatusTierTemplateRevisions(tierTemplatesRefs).Tier() + + t.Run("revision field is set but TierTemplate content has changed, new ttr should be created", func(t *testing.T) { + // given + // the NSTemplateTier already has the revisions from parent test, + // we update the cluster resource tier template content by setting a higher number of pods + tierTemplate := &toolchainv1alpha1.TierTemplate{} + err = cl.Get(context.TODO(), types.NamespacedName{Namespace: operatorNamespace, Name: "base1ns-clusterresources-123456new"}, tierTemplate) + require.NoError(t, err) + updatedCRQ := newTestCRQ("700") + tierTemplate.Spec.TemplateObjects = withTemplateObjects(updatedCRQ) + err = cl.Update(context.TODO(), tierTemplate) + require.NoError(t, err) + + // when + res, err = r.Reconcile(context.TODO(), req) + + // then + require.NoError(t, err) + // revisions values should be different compared to the previous ones + newNSTmplTier := tiertest.AssertThatNSTemplateTier(t, "base1ns", cl). + HasStatusTierTemplateRevisions(tierTemplatesRefs).Tier() + require.NotEqual(t, oldNSTemplateTier.Status.Revisions, newNSTmplTier.Status.Revisions) + // there should be one new ttr created by the change in the TierTemplate + tiertemplaterevision.AssertThatTTRs(t, cl, newNSTmplTier.GetNamespace()). + NumberOfPresentCRs(len(tierTemplatesRefs) + 1) + + t.Run("new ttr should be created also when parameters are changed in the NSTemplateTier", func(t *testing.T) { + // given + // the NSTemplateTier already has the revisions from previous test, + // but we update the parameters in the NSTemplateTier + // let's increase the quota parameter + newNSTmplTier.Spec.Parameters = []toolchainv1alpha1.Parameter{{Name: "DEPLOYMENT_QUOTA", Value: "100"}} + err = cl.Update(context.TODO(), newNSTmplTier) + require.NoError(t, err) + + // when + res, err = r.Reconcile(context.TODO(), req) + + // then + require.NoError(t, err) + // revisions values should be different compared to the previous ones + // ensure the old revisions are not there anymore + newNSTmplTier = tiertest.AssertThatNSTemplateTier(t, "base1ns", cl). + HasStatusTierTemplateRevisions(tierTemplatesRefs).Tier() + require.NotEqual(t, oldNSTemplateTier.Status.Revisions, newNSTmplTier.Status.Revisions) + // check if the change was propagated to the ttrs + tiertemplaterevision.AssertThatTTRs(t, cl, newNSTmplTier.GetNamespace()). + // a new set of TTRs should be created due the parameter change in the NSTemplateTier + // thus we now have double the initial ttrs plus the one created in the parent test. + NumberOfPresentCRs((len(tierTemplatesRefs) * 2) + 1). + // check that the NSTemplateTier parameter was propagated to all the TTRs from the NSTemplateTier + ForEach(func(ttr *toolchainv1alpha1.TierTemplateRevision) { + // if the ttr is being used by the NSTemplateTier we compare the parameters + if ttrInUse, found := newNSTmplTier.Status.Revisions[ttr.GetLabels()[toolchainv1alpha1.TemplateRefLabelKey]]; found && ttrInUse == ttr.GetName() { + assert.Equal(t, newNSTmplTier.Spec.Parameters, ttr.Spec.Parameters) + } + }) + }) + }) +} + +func newTestCRQ(podsCount string) unstructured.Unstructured { + var 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]interface{}{ + "count/deploymentconfigs.apps": "{{.DEPLOYMENT_QUOTA}}", + "count/deployments.apps": "{{.DEPLOYMENT_QUOTA}}", + "count/pods": podsCount, + }, + }, + "selector": map[string]interface{}{ + "annotations": map[string]interface{}{}, + "labels": map[string]interface{}{ + "matchLabels": map[string]interface{}{ + "toolchain.dev.openshift.com/space": "'{{.SPACE_NAME}}'", + }, + }, + }, + }, + }} + return crq +} + // initTierTemplates creates the TierTemplates objects for the base1ns tier func initTierTemplates(t *testing.T, withTemplateObjects []runtime.RawExtension, tierName string) []runtimeclient.Object { s := scheme.Scheme diff --git a/test/nstemplatetier/assertion.go b/test/nstemplatetier/assertion.go index 93bfae4be..48f5d7e27 100644 --- a/test/nstemplatetier/assertion.go +++ b/test/nstemplatetier/assertion.go @@ -19,6 +19,10 @@ type Assertion struct { t test.T } +func (a *Assertion) Tier() *toolchainv1alpha1.NSTemplateTier { + return a.tier +} + func (a *Assertion) loadResource() error { tier := &toolchainv1alpha1.NSTemplateTier{} err := a.client.Get(context.TODO(), a.namespacedName, tier) @@ -42,8 +46,9 @@ func (a *Assertion) HasStatusTierTemplateRevisions(revisions []string) *Assertio // check that each TierTemplate REF has a TierTemplateRevision set for _, tierTemplateRef := range revisions { require.NotNil(a.t, a.tier.Status.Revisions) - _, ok := a.tier.Status.Revisions[tierTemplateRef] + value, ok := a.tier.Status.Revisions[tierTemplateRef] require.True(a.t, ok) + require.NotEmpty(a.t, value) } return a }