Skip to content

Commit

Permalink
Merge branch 'master' into createttr
Browse files Browse the repository at this point in the history 
# Conflicts:
#	controllers/nstemplatetier/nstemplatetier_controller.go
#	controllers/nstemplatetier/nstemplatetier_controller_test.go
#	test/nstemplatetier/assertion.go
#	test/nstemplatetier/nstemplatetier.go
  • Loading branch information
Devtools committed Jan 9, 2025
2 parents ee41c14 + 28b6608 commit fc2d1e3
Show file tree
Hide file tree
Showing 4 changed files with 1 addition and 196 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 @@ -7,7 +7,6 @@ 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"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/validation"
"sigs.k8s.io/controller-runtime/pkg/builder"
Expand All @@ -23,12 +22,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 @@ -47,8 +40,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 @@ -69,15 +61,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
return reconcile.Result{}, fmt.Errorf("unable to get ToolchainConfig: %w", err)
}

Check warning on line 62 in controllers/nstemplatetier/nstemplatetier_controller.go

View check run for this annotation

Codecov / codecov/patch

controllers/nstemplatetier/nstemplatetier_controller.go#L61-L62 

Added lines #L61 - L62 were not covered by tests

// 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{}, fmt.Errorf("unable to insert a new entry in status.updates after NSTemplateTier changed: %w", err)
} else if added {
logger.Info("Requeue after adding a new entry in tier.status.updates")
return reconcile.Result{Requeue: true}, nil
}

// check if the `status.revisions` field is up-to-date and create a TTR for each TierTemplate
if created, err := r.ensureRevision(ctx, tier); err != nil {
// todo add/update ready condition false in the NSTemplateTier when something fails
Expand All @@ -90,37 +73,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
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, fmt.Errorf("unable to append an entry in the `status.updates` for NSTemplateTier '%s' : %w", tier.Name, err)
}
// 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)
}

// ensureRevision ensures that there is a TierTemplateRevision CR for each of the TierTemplate.
// returns `true` if a new TierTemplateRevision CR was created, `err` if something wrong happened
func (r *Reconciler) ensureRevision(ctx context.Context, nsTmplTier *toolchainv1alpha1.NSTemplateTier) (bool, error) {
Expand Down
88 changes: 0 additions & 88 deletions controllers/nstemplatetier/nstemplatetier_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,74 +41,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)
r, req, cl := prepareReconcile(t, base1nsTier.Name, base1nsTier)
// 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",
}))
r, req, cl := prepareReconcile(t, base1nsTier.Name, base1nsTier)
// 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
tierTemplates := initTierTemplates(t, nil, base1nsTier.Name)

r, req, cl := prepareReconcile(t, base1nsTier.Name, append(tierTemplates, base1nsTier)...)
// when
_, err := r.Reconcile(context.TODO(), req)
// then
require.NoError(t, err)
// 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 @@ -149,26 +81,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)
r, req, cl := prepareReconcile(t, base1nsTier.Name, base1nsTier)
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
})
})

})

t.Run("revisions management", func(t *testing.T) {
Expand Down
36 changes: 0 additions & 36 deletions test/nstemplatetier/assertion.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
"github.com/codeready-toolchain/toolchain-common/pkg/test"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/types"
runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -36,14 +35,6 @@ func AssertThatNSTemplateTier(t test.T, name string, client runtimeclient.Client
}
}

// HasStatusUpdatesItems verifies the number of items in `status.updates`
func (a *Assertion) HasStatusUpdatesItems(expected int) *Assertion {
err := a.loadResource()
require.NoError(a.t, err)
require.Len(a.t, a.tier.Status.Updates, expected)
return a
}

// HasStatusTierTemplateRevisions verifies revisions for the given TierTemplates are set in the `NSTemplateTier.Status.Revisions`
func (a *Assertion) HasStatusTierTemplateRevisions(revisions []string) *Assertion {
err := a.loadResource()
Expand All @@ -64,30 +55,3 @@ func (a *Assertion) HasNoStatusTierTemplateRevisions() *Assertion {
require.Nil(a.t, a.tier.Status.Revisions)
return a
}

// HasValidPreviousUpdates verifies the previous `status.updates`
// in particular, it checks that:
// - `StartTime` is not nil
// - `Hash` is not nil
func (a *Assertion) HasValidPreviousUpdates() *Assertion {
err := a.loadResource()
require.NoError(a.t, err)
require.NotEmpty(a.t, a.tier.Status.Updates)
for _, h := range a.tier.Status.Updates[:len(a.tier.Status.Updates)-1] {
assert.NotNil(a.t, h.StartTime)
assert.NotNil(a.t, h.Hash)
}
return a
}

// HasLatestUpdate verifies the latest `status.updates`
func (a *Assertion) HasLatestUpdate(expected toolchainv1alpha1.NSTemplateTierHistory) *Assertion {
err := a.loadResource()
require.NoError(a.t, err)
require.NotEmpty(a.t, a.tier.Status.Updates)
latest := a.tier.Status.Updates[len(a.tier.Status.Updates)-1]
assert.False(a.t, latest.StartTime.IsZero())
assert.Equal(a.t, expected.Hash, latest.Hash)

return a
}
23 changes: 0 additions & 23 deletions test/nstemplatetier/nstemplatetier.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,29 +208,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,
},
)
}
}

// WithParameter appends a parameter to the parameter's list
func WithParameter(name, value string) TierOption {
return func(tier *toolchainv1alpha1.NSTemplateTier) {
Expand Down

0 comments on commit fc2d1e3

Please sign in to comment.