Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KUBESAW-248: Drop NSTemplateTier.Status.Updates field #1115

Merged
merged 5 commits into from
Jan 9, 2025
Merged

KUBESAW-248: Drop NSTemplateTier.Status.Updates field #1115

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
495db1f
KUBESAW-248: Drop NSTemplateTier.Status.Updates field
fbm3307 Jan 6, 2025
8721a30
go-lint
fbm3307 Jan 6, 2025
299e1fd
keeping the assert function for other PR
fbm3307 Jan 7, 2025
820cf22
Merge branch 'master' into ks248nsupdatefield
mfrancisc Jan 7, 2025
0e53387
ignoring golint unused error for now
fbm3307 Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 1 addition & 49 deletions controllers/nstemplatetier/nstemplatetier_controller.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop

This file was deleted.

23 changes: 0 additions & 23 deletions test/nstemplatetier/nstemplatetier.go
View file
Open in desktop
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
Loading