Skip to content

Commit

Permalink
KUBESAW-248: Drop NSTemplateTier.Status.Updates field (#1115)
Browse files Browse the repository at this point in the history 
* KUBESAW-248: Drop NSTemplateTier.Status.Updates field

Signed-off-by: Feny Mehta <fbm3307@gmail.com>

* go-lint

Signed-off-by: Feny Mehta <fbm3307@gmail.com>

* keeping the assert function for other PR

Signed-off-by: Feny Mehta <fbm3307@gmail.com>

* ignoring golint unused error for now

Signed-off-by: Feny Mehta <fbm3307@gmail.com>

---------

Signed-off-by: Feny Mehta <fbm3307@gmail.com>
Co-authored-by: Francisc Munteanu <fmuntean@redhat.com>
  • Loading branch information
@fbm3307 @mfrancisc
fbm3307 and mfrancisc authored Jan 9, 2025
1 parent 2b06cc8 commit 69a195d
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 237 deletions.
50 changes: 1 addition & 49 deletions controllers/nstemplatetier/nstemplatetier_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,11 @@ import (

toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
"github.com/codeready-toolchain/host-operator/controllers/toolchainconfig"
"github.com/codeready-toolchain/toolchain-common/pkg/hash"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/log"

errs "github.com/pkg/errors"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -20,12 +18,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

// ----------------------------------------------------------------------------------------------------------------------------
// NSTemplateTier Controller Reconciler:
// . in case of a new NSTemplateTier update to process:
// .. inserts a new record in the `status.updates` history
// ----------------------------------------------------------------------------------------------------------------------------

// SetupWithManager sets up the controller with the Manager.
func (r *Reconciler) SetupWithManager(mgr manager.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
Expand All @@ -44,8 +36,7 @@ type Reconciler struct {
//+kubebuilder:rbac:groups=toolchain.dev.openshift.com,resources=nstemplatetiers/status,verbs=get;update;patch
//+kubebuilder:rbac:groups=toolchain.dev.openshift.com,resources=nstemplatetiers/finalizers,verbs=update

// Reconcile takes care of:
// - inserting a new entry in the `status.updates`
// Reconcile takes care of fetching the NSTemplateTier
func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
logger := log.FromContext(ctx)

Expand All @@ -66,44 +57,5 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
return reconcile.Result{}, errs.Wrapf(err, "unable to get ToolchainConfig")
}

// create a new entry in the `status.history` if needed
if added, err := r.ensureStatusUpdateRecord(ctx, tier); err != nil {
logger.Error(err, "unable to insert a new entry in status.updates after NSTemplateTier changed")
return reconcile.Result{}, errs.Wrap(err, "unable to insert a new entry in status.updates after NSTemplateTier changed")
} else if added {
logger.Info("Requeing after adding a new entry in tier.status.updates")
return reconcile.Result{Requeue: true}, nil
}
return reconcile.Result{}, nil
}

// ensureStatusUpdateRecord adds a new entry in the `status.updates` with the current date/time
// if needed and the hash of the NSTemplateTier
// returns `true` if an entry was added, `err` if something wrong happened
func (r *Reconciler) ensureStatusUpdateRecord(ctx context.Context, tier *toolchainv1alpha1.NSTemplateTier) (bool, error) {
hash, err := hash.ComputeHashForNSTemplateTier(tier)
if err != nil {
return false, errs.Wrapf(err, "unable to append an entry in the `status.updates` for NSTemplateTier '%s'", tier.Name)
}
// if there was no previous status:
if len(tier.Status.Updates) == 0 {
return true, r.addNewTierUpdate(ctx, tier, hash)
}

// check whether the entry was already added
logger := log.FromContext(ctx)
if tier.Status.Updates[len(tier.Status.Updates)-1].Hash == hash {
logger.Info("current tier template already exists in tier.status.updates")
return false, nil
}
logger.Info("Adding a new entry in tier.status.updates")
return true, r.addNewTierUpdate(ctx, tier, hash)
}

func (r *Reconciler) addNewTierUpdate(ctx context.Context, tier *toolchainv1alpha1.NSTemplateTier, hash string) error {
tier.Status.Updates = append(tier.Status.Updates, toolchainv1alpha1.NSTemplateTierHistory{
StartTime: metav1.Now(),
Hash: hash,
})
return r.Client.Status().Update(ctx, tier)
}
93 changes: 0 additions & 93 deletions controllers/nstemplatetier/nstemplatetier_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes/scheme"
Expand All @@ -34,77 +33,6 @@ func TestReconcile(t *testing.T) {
// given
logf.SetLogger(zap.New(zap.UseDevMode(true)))

t.Run("controller should add entry in tier.status.updates", func(t *testing.T) {

t.Run("without previous entry", func(t *testing.T) {
// given
base1nsTier := tiertest.Base1nsTier(t, tiertest.CurrentBase1nsTemplates)
initObjs := []runtimeclient.Object{base1nsTier}
r, req, cl := prepareReconcile(t, base1nsTier.Name, initObjs...)
// when
res, err := r.Reconcile(context.TODO(), req)
// then
require.NoError(t, err)
require.Equal(t, reconcile.Result{Requeue: true}, res) // explicit requeue after the adding an entry in `status.updates`
// check that an entry was added in `status.updates`
tiertest.AssertThatNSTemplateTier(t, "base1ns", cl).
HasStatusUpdatesItems(1).
HasLatestUpdate(toolchainv1alpha1.NSTemplateTierHistory{
Hash: base1nsTier.Labels["toolchain.dev.openshift.com/base1ns-tier-hash"],
})
})

t.Run("with previous entries", func(t *testing.T) {
// given
base1nsTier := tiertest.Base1nsTier(t, tiertest.CurrentBase1nsTemplates, tiertest.WithPreviousUpdates(
toolchainv1alpha1.NSTemplateTierHistory{
StartTime: metav1.Now(),
Hash: "abc123",
},
toolchainv1alpha1.NSTemplateTierHistory{
StartTime: metav1.Now(),
Hash: "def456",
}))
initObjs := []runtimeclient.Object{base1nsTier}
r, req, cl := prepareReconcile(t, base1nsTier.Name, initObjs...)
// when
res, err := r.Reconcile(context.TODO(), req)
// then
require.NoError(t, err)
require.Equal(t, reconcile.Result{Requeue: true}, res) // explicit requeue after the adding an entry in `status.updates`
// check that an entry was added in `status.updates`
tiertest.AssertThatNSTemplateTier(t, "base1ns", cl).
HasStatusUpdatesItems(3).
HasValidPreviousUpdates().
HasLatestUpdate(toolchainv1alpha1.NSTemplateTierHistory{
Hash: base1nsTier.Labels["toolchain.dev.openshift.com/base1ns-tier-hash"],
})
})
})

t.Run("controller should NOT add entry in tier.status.updates", func(t *testing.T) {

t.Run("last entry exists with matching hash", func(t *testing.T) {
// given
base1nsTier := tiertest.Base1nsTier(t, tiertest.CurrentBase1nsTemplates,
tiertest.WithCurrentUpdate()) // current update already exists

initObjs := []runtimeclient.Object{base1nsTier}
r, req, cl := prepareReconcile(t, base1nsTier.Name, initObjs...)
// when
res, err := r.Reconcile(context.TODO(), req)
// then
require.NoError(t, err)
require.Equal(t, reconcile.Result{}, res) // no explicit requeue
// check that no entry was added in `status.updates`
tiertest.AssertThatNSTemplateTier(t, "base1ns", cl).
HasStatusUpdatesItems(1). // same number of entries
HasLatestUpdate(toolchainv1alpha1.NSTemplateTierHistory{
Hash: base1nsTier.Labels["toolchain.dev.openshift.com/base1ns-tier-hash"],
})
})
})

t.Run("failures", func(t *testing.T) {

t.Run("unable to get NSTemplateTier", func(t *testing.T) {
Expand Down Expand Up @@ -147,27 +75,6 @@ func TestReconcile(t *testing.T) {
})
})

t.Run("unable to update NSTemplateTier status", func(t *testing.T) {

t.Run("when adding new update", func(t *testing.T) {
// given
base1nsTier := tiertest.Base1nsTier(t, tiertest.CurrentBase1nsTemplates)
initObjs := []runtimeclient.Object{base1nsTier}
r, req, cl := prepareReconcile(t, base1nsTier.Name, initObjs...)
cl.MockStatusUpdate = func(ctx context.Context, obj runtimeclient.Object, opts ...runtimeclient.SubResourceUpdateOption) error {
if _, ok := obj.(*toolchainv1alpha1.NSTemplateTier); ok {
return fmt.Errorf("mock error")
}
return cl.Client.Status().Update(ctx, obj, opts...)
}
// when
res, err := r.Reconcile(context.TODO(), req)
// then
require.EqualError(t, err, "unable to insert a new entry in status.updates after NSTemplateTier changed: mock error")
assert.Equal(t, reconcile.Result{}, res) // no explicit requeue
})
})

})

}
Expand Down
72 changes: 0 additions & 72 deletions test/nstemplatetier/assertion.go
View file

This file was deleted.

35 changes: 35 additions & 0 deletions test/nstemplatetier/assertions.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package nstemplatetier

import (
"context"

toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
"github.com/codeready-toolchain/toolchain-common/pkg/test"

"k8s.io/apimachinery/pkg/types"
runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
)

// Assertion an assertion helper for an NSTemplateTier
type Assertion struct {
tier *toolchainv1alpha1.NSTemplateTier //nolint:golint,unused
client runtimeclient.Client
namespacedName types.NamespacedName
t test.T
}

func (a *Assertion) loadResource() error { //nolint:golint,unused
tier := &toolchainv1alpha1.NSTemplateTier{}
err := a.client.Get(context.TODO(), a.namespacedName, tier)
a.tier = tier
return err
}

// AssertThatNSTemplateTier helper func to begin with the assertions on an NSTemplateTier
func AssertThatNSTemplateTier(t test.T, name string, client runtimeclient.Client) *Assertion {
return &Assertion{
client: client,
namespacedName: test.NamespacedName(test.HostOperatorNs, name),
t: t,
}
}
23 changes: 0 additions & 23 deletions test/nstemplatetier/nstemplatetier.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,29 +178,6 @@ func WithoutClusterResources() TierOption {
}
}

// WithPreviousUpdates adds the given entries in the `status.updates`
func WithPreviousUpdates(entries ...toolchainv1alpha1.NSTemplateTierHistory) TierOption {
return func(tier *toolchainv1alpha1.NSTemplateTier) {
tier.Status.Updates = entries
}
}

// WithCurrentUpdate appends an entry in the `status.updates` for the current tier
func WithCurrentUpdate() TierOption {
return func(tier *toolchainv1alpha1.NSTemplateTier) {
hash, _ := hash.ComputeHashForNSTemplateTier(tier)
if tier.Status.Updates == nil {
tier.Status.Updates = []toolchainv1alpha1.NSTemplateTierHistory{}
}
tier.Status.Updates = append(tier.Status.Updates,
toolchainv1alpha1.NSTemplateTierHistory{
StartTime: metav1.Now(),
Hash: hash,
},
)
}
}

// OtherTier returns an "other" NSTemplateTier
func OtherTier() *toolchainv1alpha1.NSTemplateTier {
return &toolchainv1alpha1.NSTemplateTier{
Expand Down

0 comments on commit 69a195d

Please sign in to comment.