Skip to content

Commit

Permalink
Testing: Ensure testing logger is passed for every reconciler; use `t…
Browse files Browse the repository at this point in the history
….Context()` (#151)

`t.Context()` was introduced with Go 1.24. We use `logr/testr` as the test logger.
  • Loading branch information
bastjan authored Feb 18, 2025
1 parent e43f7fb commit 9e51c6d
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 37 deletions.
5 changes: 3 additions & 2 deletions controllers/clusterversion_controller_test.go
Original file line number Diff line number Diff line change
@@ -1,24 +1,25 @@
package controllers

import (
"context"
"testing"
"time"

"github.com/go-logr/logr/testr"
configv1 "github.com/openshift/api/config/v1"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

managedupgradev1beta1 "github.com/appuio/openshift-upgrade-controller/api/v1beta1"
)

func Test_ClusterVersionReconciler_Reconcile(t *testing.T) {
clock := mockClock{now: time.Date(2022, 12, 4, 22, 45, 0, 0, time.UTC)}
ctx := context.Background()
ctx := log.IntoContext(t.Context(), testr.New(t))

upstream := &configv1.ClusterVersion{
ObjectMeta: metav1.ObjectMeta{
Expand Down
5 changes: 3 additions & 2 deletions controllers/node_controller_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package controllers

import (
"context"
"testing"

"github.com/go-logr/logr/testr"
"github.com/prometheus/client_golang/prometheus/testutil"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
Expand All @@ -13,10 +13,11 @@ import (
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/log"
)

func Test_NodeReconciler_Reconcile(t *testing.T) {
ctx := context.Background()
ctx := log.IntoContext(t.Context(), testr.New(t))

scheme := runtime.NewScheme()
require.NoError(t, clientgoscheme.AddToScheme(scheme))
Expand Down
11 changes: 6 additions & 5 deletions controllers/node_force_drain_controller_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package controllers

import (
"context"
"testing"
"time"

Expand All @@ -19,7 +18,7 @@ import (
)

func Test_NodeForceDrainReconciler_Reconcile_E2E(t *testing.T) {
ctx := log.IntoContext(context.Background(), testr.New(t))
ctx := log.IntoContext(t.Context(), testr.New(t))

clock := mockClock{now: time.Date(2022, time.April, 4, 8, 0, 0, 0, time.Local)}
t.Log("Now: ", clock.Now())
Expand Down Expand Up @@ -227,7 +226,7 @@ func Test_NodeForceDrainReconciler_Reconcile_E2E(t *testing.T) {
}

func Test_NodeForceDrainReconciler_Reconcile_DrainIgnoreActiveDaemonsSetsStaticPods(t *testing.T) {
ctx := log.IntoContext(context.Background(), testr.New(t))
ctx := log.IntoContext(t.Context(), testr.New(t))

clock := mockClock{now: time.Date(2022, time.April, 4, 8, 0, 0, 0, time.Local)}
t.Log("Now: ", clock.Now())
Expand Down Expand Up @@ -378,7 +377,7 @@ func Test_NodeForceDrainReconciler_Reconcile_DrainIgnoreActiveDaemonsSetsStaticP
}

func Test_NodeForceDrainReconciler_Reconcile_MaxIntervalDuringActiveDrain(t *testing.T) {
ctx := log.IntoContext(context.Background(), testr.New(t))
ctx := log.IntoContext(t.Context(), testr.New(t))

clock := mockClock{now: time.Date(2022, time.April, 4, 8, 0, 0, 0, time.Local)}
t.Log("Now: ", clock.Now())
Expand Down Expand Up @@ -451,6 +450,8 @@ func Test_NodeForceDrainReconciler_Reconcile_MaxIntervalDuringActiveDrain(t *tes
}

func Test_NodeToNodeForceDrainMapper(t *testing.T) {
ctx := log.IntoContext(t.Context(), testr.New(t))

fds := []client.Object{
&managedupgradev1beta1.NodeForceDrain{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -517,7 +518,7 @@ func Test_NodeToNodeForceDrainMapper(t *testing.T) {
},
}

require.ElementsMatch(t, subject(context.Background(), node), []reconcile.Request{
require.ElementsMatch(t, subject(ctx, node), []reconcile.Request{
{NamespacedName: client.ObjectKeyFromObject(fds[0])},
{NamespacedName: client.ObjectKeyFromObject(fds[1])},
}, "should only return NodeForceDrain objects matching the input node")
Expand Down
7 changes: 3 additions & 4 deletions controllers/upgrade_information_collector_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package controllers

import (
"context"
"fmt"
"io"
"strings"
Expand Down Expand Up @@ -244,17 +243,17 @@ func Test_ClusterUpgradingMetric(t *testing.T) {
StartedTime: metav1.Now(),
Version: version.Spec.DesiredUpdate.Version,
})
require.NoError(t, c.Status().Update(context.Background(), version))
require.NoError(t, c.Status().Update(t.Context(), version))
workerPool.Status.UpdatedMachineCount = workerPool.Status.MachineCount - 1
require.NoError(t, c.Status().Update(context.Background(), workerPool))
require.NoError(t, c.Status().Update(t.Context(), workerPool))

require.NoError(t,
testutil.CollectAndCompare(subject, expectedUpgradingMetrics(true, false, true), expectedMetricNames...),
"upgrading should be true if cluster version is progressing or a machine config pool is not fully upgraded",
)

workerPool.Status.UpdatedMachineCount = workerPool.Status.MachineCount
require.NoError(t, c.Status().Update(context.Background(), workerPool))
require.NoError(t, c.Status().Update(t.Context(), workerPool))

require.NoError(t,
testutil.CollectAndCompare(subject, expectedUpgradingMetrics(false, false, false), expectedMetricNames...),
Expand Down
12 changes: 7 additions & 5 deletions controllers/upgradeconfig_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"testing"
"time"

"github.com/go-logr/logr/testr"
configv1 "github.com/openshift/api/config/v1"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand All @@ -18,13 +19,14 @@ import (
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

managedupgradev1beta1 "github.com/appuio/openshift-upgrade-controller/api/v1beta1"
)

func Test_UpgradeConfigReconciler_Reconcile_E2E(t *testing.T) {
ctx := context.Background()
ctx := log.IntoContext(t.Context(), testr.New(t))
clock := mockClock{now: time.Date(2022, time.April, 4, 8, 0, 0, 0, time.UTC)}
t.Log("Now: ", clock.Now())
require.Equal(t, 14, func() int { _, isoweek := clock.Now().ISOWeek(); return isoweek }())
Expand Down Expand Up @@ -215,7 +217,7 @@ func Test_UpgradeConfigReconciler_Reconcile_E2E(t *testing.T) {
}

func Test_UpgradeConfigReconciler_Reconcile_AddNextWindowsToStatus(t *testing.T) {
ctx := context.Background()
ctx := log.IntoContext(t.Context(), testr.New(t))
clock := mockClock{now: time.Date(2022, time.April, 4, 8, 0, 0, 0, time.UTC)}
t.Log("Now: ", clock.Now())
require.Equal(t, 14, func() int { _, isoweek := clock.Now().ISOWeek(); return isoweek }())
Expand Down Expand Up @@ -284,7 +286,7 @@ func Test_UpgradeConfigReconciler_Reconcile_AddNextWindowsToStatus(t *testing.T)
}

func Test_UpgradeConfigReconciler_Reconcile_SuspendedByWindow(t *testing.T) {
ctx := context.Background()
ctx := log.IntoContext(t.Context(), testr.New(t))
clock := mockClock{now: time.Date(2022, time.April, 4, 8, 0, 0, 0, time.UTC)}
t.Log("Now: ", clock.Now())
require.Equal(t, 14, func() int { _, isoweek := clock.Now().ISOWeek(); return isoweek }())
Expand Down Expand Up @@ -414,7 +416,7 @@ func requireEventMatches(t *testing.T, recorder *record.FakeRecorder, substrings
}

func Test_UpgradeConfigReconciler_Reconcile_CleanupSuccessfulJobs(t *testing.T) {
ctx := context.Background()
ctx := log.IntoContext(t.Context(), testr.New(t))
clock := mockClock{now: time.Date(2022, time.April, 4, 8, 0, 0, 0, time.UTC)}

ucv := &configv1.ClusterVersion{
Expand Down Expand Up @@ -525,7 +527,7 @@ func requireTimeEqual(t *testing.T, expected, actual time.Time, msgAndArgs ...an
func listJobs(t *testing.T, c client.Client, namespace string) []managedupgradev1beta1.UpgradeJob {
t.Helper()
var jobs managedupgradev1beta1.UpgradeJobList
require.NoError(t, c.List(context.Background(), &jobs, client.InNamespace(namespace)))
require.NoError(t, c.List(t.Context(), &jobs, client.InNamespace(namespace)))
return jobs.Items
}

Expand Down
38 changes: 21 additions & 17 deletions controllers/upgradejob_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"testing"
"time"

"github.com/go-logr/logr/testr"
configv1 "github.com/openshift/api/config/v1"
machineconfigurationv1 "github.com/openshift/api/machineconfiguration/v1"
"github.com/stretchr/testify/require"
Expand All @@ -23,12 +24,13 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/log"

managedupgradev1beta1 "github.com/appuio/openshift-upgrade-controller/api/v1beta1"
)

func Test_UpgradeJobReconciler_Reconcile_E2E_Upgrade(t *testing.T) {
ctx := context.Background()
ctx := log.IntoContext(t.Context(), testr.New(t))
clock := mockClock{now: time.Date(2022, 12, 4, 22, 45, 0, 0, time.UTC)}

ucv := &configv1.ClusterVersion{
Expand Down Expand Up @@ -326,7 +328,7 @@ func Test_UpgradeJobReconciler_Reconcile_E2E_Upgrade(t *testing.T) {
}

func Test_UpgradeJobReconciler_Reconcile_Skipped_Job(t *testing.T) {
ctx := context.Background()
ctx := log.IntoContext(t.Context(), testr.New(t))
clock := mockClock{now: time.Date(2022, 12, 4, 22, 45, 0, 0, time.UTC)}

ucv := &configv1.ClusterVersion{
Expand Down Expand Up @@ -441,7 +443,7 @@ func Test_UpgradeJobReconciler_Reconcile_Skipped_Job(t *testing.T) {
}

func Test_UpgradeJobReconciler_Reconcile_EmptyDesiredVersion(t *testing.T) {
ctx := context.Background()
ctx := log.IntoContext(t.Context(), testr.New(t))
clock := mockClock{now: time.Date(2022, 12, 4, 22, 45, 0, 0, time.UTC)}

upgradeJob := &managedupgradev1beta1.UpgradeJob{
Expand Down Expand Up @@ -503,7 +505,7 @@ func Test_UpgradeJobReconciler_Reconcile_EmptyDesiredVersion(t *testing.T) {
}

func Test_UpgradeJobReconciler_Reconcile_HookFailed(t *testing.T) {
ctx := context.Background()
ctx := log.IntoContext(t.Context(), testr.New(t))
clock := mockClock{now: time.Date(2022, 12, 4, 22, 45, 0, 0, time.UTC)}

upgradeJob := &managedupgradev1beta1.UpgradeJob{
Expand Down Expand Up @@ -791,7 +793,7 @@ func Test_UpgradeJobReconciler_Reconcile_Disruptive(t *testing.T) {
}

func Test_UpgradeJobReconciler_Reconcile_ClaimNextHook(t *testing.T) {
ctx := context.Background()
ctx := log.IntoContext(t.Context(), testr.New(t))
clock := mockClock{now: time.Date(2022, 12, 4, 22, 45, 0, 0, time.UTC)}

upgradeJob := &managedupgradev1beta1.UpgradeJob{
Expand Down Expand Up @@ -882,7 +884,7 @@ func requireEnv(t *testing.T, list []corev1.EnvVar, name string, valueMatcher fu
}

func Test_UpgradeJobReconciler_Reconcile_Expired(t *testing.T) {
ctx := context.Background()
ctx := log.IntoContext(t.Context(), testr.New(t))
clock := mockClock{now: time.Date(2022, 12, 4, 22, 45, 0, 0, time.UTC)}

upgradeJob := &managedupgradev1beta1.UpgradeJob{
Expand Down Expand Up @@ -916,7 +918,7 @@ func Test_UpgradeJobReconciler_Reconcile_Expired(t *testing.T) {
}

func Test_UpgradeJobReconciler_Reconcile_UpgradeWithdrawn(t *testing.T) {
ctx := context.Background()
ctx := log.IntoContext(t.Context(), testr.New(t))
clock := mockClock{now: time.Date(2022, 12, 4, 22, 45, 0, 0, time.UTC)}

ucv := &configv1.ClusterVersion{
Expand Down Expand Up @@ -976,7 +978,7 @@ func Test_UpgradeJobReconciler_Reconcile_UpgradeWithdrawn(t *testing.T) {
}

func Test_UpgradeJobReconciler_Reconcile_Timeout(t *testing.T) {
ctx := context.Background()
ctx := log.IntoContext(t.Context(), testr.New(t))
clock := mockClock{now: time.Date(2022, 12, 4, 22, 45, 0, 0, time.UTC)}

ucv := &configv1.ClusterVersion{
Expand Down Expand Up @@ -1031,7 +1033,7 @@ func Test_UpgradeJobReconciler_Reconcile_Timeout(t *testing.T) {
}

func Test_UpgradeJobReconciler_Reconcile_PreHealthCheckTimeout(t *testing.T) {
ctx := context.Background()
ctx := log.IntoContext(t.Context(), testr.New(t))
clock := mockClock{now: time.Date(2022, 12, 4, 22, 45, 0, 0, time.UTC)}

ucv := &configv1.ClusterVersion{
Expand Down Expand Up @@ -1095,7 +1097,7 @@ func Test_UpgradeJobReconciler_Reconcile_PreHealthCheckTimeout(t *testing.T) {
}

func Test_UpgradeJobReconciler_Reconcile_PostHealthCheckTimeout(t *testing.T) {
ctx := context.Background()
ctx := log.IntoContext(t.Context(), testr.New(t))
clock := mockClock{now: time.Date(2022, 12, 4, 22, 45, 0, 0, time.UTC)}

ucv := &configv1.ClusterVersion{
Expand Down Expand Up @@ -1168,7 +1170,7 @@ func Test_UpgradeJobReconciler_Reconcile_PostHealthCheckTimeout(t *testing.T) {
}

func Test_UpgradeJobReconciler_Reconcile_PausedMachineConfigPools(t *testing.T) {
ctx := context.Background()
ctx := log.IntoContext(t.Context(), testr.New(t))
clock := mockClock{now: time.Date(2022, 12, 4, 22, 45, 0, 0, time.UTC)}

ucv := &configv1.ClusterVersion{
Expand Down Expand Up @@ -1365,7 +1367,7 @@ func Test_UpgradeJobReconciler_Reconcile_PausedMachineConfigPools(t *testing.T)
}

func Test_UpgradeJobReconciler_Reconcile_PausedMachineConfigPools_UnpauseExpire(t *testing.T) {
ctx := context.Background()
ctx := log.IntoContext(t.Context(), testr.New(t))
clock := mockClock{now: time.Date(2022, 12, 4, 22, 45, 0, 0, time.UTC)}

ucv := &configv1.ClusterVersion{
Expand Down Expand Up @@ -1486,7 +1488,7 @@ func Test_UpgradeJobReconciler_Reconcile_PausedMachineConfigPools_UnpauseExpire(
// Test_UpgradeJobReconciler_Reconcile_PausedMachineConfigPools_EnsureUnpause tests that the upgrade job reconciler
// will unpause machine config pools at the end of an upgrade even if they did not require any upgrades
func Test_UpgradeJobReconciler_Reconcile_PausedMachineConfigPools_EnsureUnpause(t *testing.T) {
ctx := context.Background()
ctx := log.IntoContext(t.Context(), testr.New(t))
clock := mockClock{now: time.Date(2022, 12, 4, 22, 45, 0, 0, time.UTC)}

ucv := &configv1.ClusterVersion{
Expand Down Expand Up @@ -1621,6 +1623,8 @@ func Test_UpgradeJobReconciler_Reconcile_PausedMachineConfigPools_EnsureUnpause(
}

func Test_JobFromClusterVersionHandler(t *testing.T) {
ctx := log.IntoContext(t.Context(), testr.New(t))

ucv := &configv1.ClusterVersion{
ObjectMeta: metav1.ObjectMeta{
Name: "version",
Expand All @@ -1630,14 +1634,14 @@ func Test_JobFromClusterVersionHandler(t *testing.T) {
client := controllerClient(t, ucv)
subject := JobFromClusterVersionMapper(client, "version")

require.Len(t, subject(context.Background(), nil), 0, "should not return a reconcile request if clusterversion is not locked")
require.Len(t, subject(ctx, nil), 0, "should not return a reconcile request if clusterversion is not locked")

ucv.Annotations = map[string]string{
JobLockAnnotation: "ns/upgrade-1234-4-5-13",
}
require.NoError(t, client.Update(context.Background(), ucv))
require.NoError(t, client.Update(ctx, ucv))

reqs := subject(context.Background(), nil)
reqs := subject(ctx, nil)
require.Len(t, reqs, 1, "should return a reconcile request if clusterversion is locked")
require.Equal(t, types.NamespacedName{Namespace: "ns", Name: "upgrade-1234-4-5-13"}, reqs[0].NamespacedName)
}
Expand Down Expand Up @@ -1738,7 +1742,7 @@ func nodeNameIndexer(obj client.Object) []string {

func checkAndCompleteHook(t *testing.T, c client.WithWatch, subject *UpgradeJobReconciler, upgradeJob *managedupgradev1beta1.UpgradeJob, upgradeJobHook *managedupgradev1beta1.UpgradeJobHook, event managedupgradev1beta1.UpgradeEvent, trackingKey string, fail bool) batchv1.Job {
t.Helper()
ctx := context.Background()
ctx := t.Context()

var jobs batchv1.JobList

Expand Down
5 changes: 3 additions & 2 deletions controllers/upgradesuspensionwindow_controller_test.go
Original file line number Diff line number Diff line change
@@ -1,20 +1,21 @@
package controllers

import (
"context"
"testing"
"time"

managedupgradev1beta1 "github.com/appuio/openshift-upgrade-controller/api/v1beta1"
"github.com/go-logr/logr/testr"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"
)

func Test_UpgradeSuspensionWindowReconciler_Reconcile(t *testing.T) {
ctx := context.Background()
ctx := log.IntoContext(t.Context(), testr.New(t))

j1 := &managedupgradev1beta1.UpgradeJob{
ObjectMeta: metav1.ObjectMeta{
Expand Down

0 comments on commit 9e51c6d

Please sign in to comment.