Skip to content

Commit

Permalink
Update controller-runtime to v0.15 and k8s dep to v0.27 (#551)
Browse files Browse the repository at this point in the history
* fixes for controller-runtime upgrade and breaking changes

* unit test changes with deletionTimestamp and fakeClient

* mod tidy, fix toolchain common replace version

* remove commented out code

* remove unused function lint check

* replace deprecated Poll function to fix linter error

* correct replace fork version, should fix snyk error too

* add finalizer, fix lint

* remove emoty status in test

* update versions

* update api and common latest version

* format

* update redhat-cop/operator-utils

* update common version

* update kustome and controller-gen version in makefile

* v5

* remove replace, update versions after merge

* remove replace
  • Loading branch information
ranakan19 authored Nov 15, 2024
1 parent 56327d5 commit ca053bf
Show file tree
Hide file tree
Showing 18 changed files with 239 additions and 1,160 deletions.
7 changes: 4 additions & 3 deletions controllers/idler/idler_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ func TestReconcile(t *testing.T) {
idler := &toolchainv1alpha1.Idler{
ObjectMeta: metav1.ObjectMeta{
Name: "being-deleted",
Finalizers: []string{"toolchain.dev.openshift.com"},
DeletionTimestamp: &now,
},
Spec: toolchainv1alpha1.IdlerSpec{TimeoutSeconds: 30},
Expand Down Expand Up @@ -635,7 +636,7 @@ func TestEnsureIdlingFailed(t *testing.T) {
require.NoError(t, err)

// second reconcile to delete pods and create notification
cl.MockStatusUpdate = func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error {
cl.MockStatusUpdate = func(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error {
return fmt.Errorf("cannot set status to fail")
}
_, err = reconciler.Reconcile(context.TODO(), req)
Expand Down Expand Up @@ -935,7 +936,7 @@ func TestCreateNotification(t *testing.T) {
nsTmplSet := newNSTmplSet(test.MemberOperatorNs, "alex", "advanced", "abcde11", namespaces, usernames)
mur := newMUR("alex")
reconciler, _, cl, _, _ := prepareReconcile(t, idler.Name, getHostCluster, idler, nsTmplSet, mur)
cl.MockStatusUpdate = func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error {
cl.MockStatusUpdate = func(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error {
return errors.New("can't update condition")
}
//when
Expand Down Expand Up @@ -1327,7 +1328,7 @@ func createPods(t *testing.T, r *Reconciler, owner metav1.Object, startTime meta
return podsToTrack
}

func prepareReconcile(t *testing.T, name string, getHostClusterFunc func(fakeClient client.Client) cluster.GetHostClusterFunc, initIdlerObjs ...runtime.Object) (*Reconciler, reconcile.Request, *test.FakeClient, *test.FakeClient, *fakedynamic.FakeDynamicClient) {
func prepareReconcile(t *testing.T, name string, getHostClusterFunc func(fakeClient client.Client) cluster.GetHostClusterFunc, initIdlerObjs ...client.Object) (*Reconciler, reconcile.Request, *test.FakeClient, *test.FakeClient, *fakedynamic.FakeDynamicClient) {
s := scheme.Scheme
err := apis.AddToScheme(s)
require.NoError(t, err)
Expand Down
5 changes: 3 additions & 2 deletions controllers/memberoperatorconfig/mapper.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package memberoperatorconfig

import (
"context"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
Expand All @@ -11,8 +12,8 @@ import (
var mapperLog = ctrl.Log.WithName("SecretToMemberOperatorConfigMapper")

// MapSecretToMemberOperatorConfig maps secrets to the singular instance of MemberOperatorConfig named "config"
func MapSecretToMemberOperatorConfig() func(object client.Object) []reconcile.Request {
return func(obj client.Object) []reconcile.Request {
func MapSecretToMemberOperatorConfig() func(context context.Context, object client.Object) []reconcile.Request {
return func(ctx context.Context, obj client.Object) []reconcile.Request {
if secret, ok := obj.(*corev1.Secret); ok {
mapperLog.Info("Secret mapped to MemberOperatorConfig", "name", secret.Name)
return []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: secret.Namespace, Name: "config"}}}
Expand Down
6 changes: 4 additions & 2 deletions controllers/memberoperatorconfig/mapper_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package memberoperatorconfig

import (
"context"
"testing"

"github.com/codeready-toolchain/toolchain-common/pkg/test"
Expand All @@ -14,6 +15,7 @@ import (

func TestSecretToMemberOperatorConfigMapper(t *testing.T) {
// when
ctx := context.TODO()
secretData := map[string][]byte{
"che-admin-username": []byte("cheadmin"),
"che-admin-password": []byte("password"),
Expand All @@ -24,7 +26,7 @@ func TestSecretToMemberOperatorConfigMapper(t *testing.T) {
secret := newSecret("test-secret", secretData)

// when
req := MapSecretToMemberOperatorConfig()(secret)
req := MapSecretToMemberOperatorConfig()(ctx, secret)

// then
require.Len(t, req, 1)
Expand All @@ -39,7 +41,7 @@ func TestSecretToMemberOperatorConfigMapper(t *testing.T) {
pod := &corev1.Pod{}

// when
req := MapSecretToMemberOperatorConfig()(pod)
req := MapSecretToMemberOperatorConfig()(ctx, pod)

// then
assert.Empty(t, req)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,13 @@ import (
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"
)

// SetupWithManager sets up the controller with the Manager.
func (r *Reconciler) SetupWithManager(mgr manager.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&toolchainv1alpha1.MemberOperatorConfig{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})).
Watches(&source.Kind{Type: &corev1.Secret{}},
Watches(&corev1.Secret{},
handler.EnqueueRequestsFromMapFunc(MapSecretToMemberOperatorConfig())).
Complete(r)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
schedulingv1 "k8s.io/api/scheduling/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes/scheme"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -249,9 +248,9 @@ func TestHandleWebhookDeploy(t *testing.T) {
err := apis.AddToScheme(s)
require.NoError(t, err)
objs, err := deploy.GetTemplateObjects(s, test.MemberOperatorNs, "test/image", []byte("asdfasdfasdf"))
initObjs := []runtime.Object{config}
initObjs := []client.Object{config}
for _, obj := range objs {
initObjs = append(initObjs, obj.DeepCopyObject())
initObjs = append(initObjs, obj.DeepCopyObject().(client.Object))
}
require.NoError(t, err)
controller, cl := prepareReconcile(t, initObjs...)
Expand Down Expand Up @@ -317,7 +316,7 @@ func TestHandleWebhookDeploy(t *testing.T) {
})
}

func prepareReconcile(t *testing.T, initObjs ...runtime.Object) (*Reconciler, client.Client) {
func prepareReconcile(t *testing.T, initObjs ...client.Object) (*Reconciler, client.Client) {
os.Setenv("WATCH_NAMESPACE", test.MemberOperatorNs)
restore := test.SetEnvVarAndRestore(t, "MEMBER_OPERATOR_WEBHOOK_IMAGE", "webhookimage")
t.Cleanup(restore)
Expand Down
7 changes: 3 additions & 4 deletions controllers/memberstatus/memberstatus_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/metrics/pkg/apis/metrics/v1beta1"
Expand Down Expand Up @@ -714,7 +713,7 @@ func newGetHostClusterNotExist(fakeClient client.Client) cluster.GetHostClusterF
return NewGetHostClusterWithProbe(fakeClient, false, corev1.ConditionFalse, metav1.Now())
}

func prepareReconcile(t *testing.T, requestName string, getHostClusterFunc func(fakeClient client.Client) cluster.GetHostClusterFunc, allNamespacesClient *test.FakeClient, lastGitHubAPICall time.Time, mockedGitHubClient commonclient.GetGitHubClientFunc, initObjs ...runtime.Object) (*Reconciler, reconcile.Request, *test.FakeClient) {
func prepareReconcile(t *testing.T, requestName string, getHostClusterFunc func(fakeClient client.Client) cluster.GetHostClusterFunc, allNamespacesClient *test.FakeClient, lastGitHubAPICall time.Time, mockedGitHubClient commonclient.GetGitHubClientFunc, initObjs ...client.Object) (*Reconciler, reconcile.Request, *test.FakeClient) {
logf.SetLogger(zap.New(zap.UseDevMode(true)))
os.Setenv("WATCH_NAMESPACE", test.MemberOperatorNs)
fakeClient := test.NewFakeClient(t, initObjs...)
Expand Down Expand Up @@ -779,8 +778,8 @@ func withMemoryUsage(usage string) nodeMetricsModifier {
}
}

func newNodesAndNodeMetrics(nodeAndMetricsCreators ...nodeAndMetricsCreator) []runtime.Object {
var objects []runtime.Object
func newNodesAndNodeMetrics(nodeAndMetricsCreators ...nodeAndMetricsCreator) []client.Object {
var objects []client.Object
for _, create := range nodeAndMetricsCreators {
node, nodeMetrics := create()
objects = append(objects, node, nodeMetrics)
Expand Down
3 changes: 1 addition & 2 deletions controllers/nstemplateset/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer"
"k8s.io/client-go/kubernetes/scheme"
runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -330,7 +329,7 @@ func newOptionalDeployment(name, owner string) *appsv1.Deployment {
}
}

func prepareAPIClient(t *testing.T, initObjs ...runtime.Object) (*APIClient, *test.FakeClient) {
func prepareAPIClient(t *testing.T, initObjs ...runtimeclient.Object) (*APIClient, *test.FakeClient) {
s := scheme.Scheme
err := apis.AddToScheme(s)
require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion controllers/nstemplateset/cluster_resources_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ func TestDeleteClusterResources(t *testing.T) {
t.Run("delete the second ClusterResourceQuota since the first one has deletion timestamp set", func(t *testing.T) {
// given
nsTmplSet := newNSTmplSet(namespaceName, spacename, "withemptycrq", withNamespaces("abcde11", "dev"), withClusterResources("abcde11"))
crq := newClusterResourceQuota(spacename, "withemptycrq")
crq := newClusterResourceQuota(spacename, "withemptycrq", withFinalizer())
deletionTS := metav1.NewTime(time.Now())
crq.SetDeletionTimestamp(&deletionTS)
emptyCrq := newClusterResourceQuota("empty", "withemptycrq")
Expand Down
3 changes: 2 additions & 1 deletion controllers/nstemplateset/namespaces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ func TestEnsureNamespacesFail(t *testing.T) {
nsTmplSet := newNSTmplSet(namespaceName, spacename, "basic", withNamespaces("abcde11", "dev"))
devNS := newNamespace("advanced", spacename, "dev") // NS exists but is missing the resources
manager, fakeClient := prepareNamespacesManager(t, nsTmplSet, devNS)
fakeClient.MockStatusUpdate = func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error {
fakeClient.MockStatusUpdate = func(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error {
return errors.New("unable to update NSTmplSet")
}

Expand Down Expand Up @@ -752,6 +752,7 @@ func TestDeleteNamespace(t *testing.T) {
// given namespace with deletion timestamp
timeStamp := metav1.Now()
deletedNS := codeNS.DeepCopy()
deletedNS.Finalizers = []string{toolchainv1alpha1.FinalizerName}
deletedNS.DeletionTimestamp = &timeStamp
manager, _ := prepareNamespacesManager(t, nsTmplSet, deletedNS)

Expand Down
8 changes: 4 additions & 4 deletions controllers/nstemplateset/nstemplateset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,14 @@ func (r *Reconciler) SetupWithManager(mgr manager.Manager, allNamespaceCluster r
mapToOwnerByLabel := handler.EnqueueRequestsFromMapFunc(commoncontroller.MapToOwnerByLabel("", toolchainv1alpha1.SpaceLabelKey))
build := ctrl.NewControllerManagedBy(mgr).
For(&toolchainv1alpha1.NSTemplateSet{}, builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, predicate.AnnotationChangedPredicate{}))).
Watches(&source.Kind{Type: &corev1.Namespace{}}, mapToOwnerByLabel).
Watches(source.NewKindWithCache(&rbac.Role{}, allNamespaceCluster.GetCache()), mapToOwnerByLabel, builder.WithPredicates(commonpredicates.LabelsAndGenerationPredicate{})).
Watches(source.NewKindWithCache(&rbac.RoleBinding{}, allNamespaceCluster.GetCache()), mapToOwnerByLabel, builder.WithPredicates(commonpredicates.LabelsAndGenerationPredicate{}))
Watches(&corev1.Namespace{}, mapToOwnerByLabel).
WatchesRawSource(source.Kind(allNamespaceCluster.GetCache(), &rbac.Role{}), mapToOwnerByLabel, builder.WithPredicates(commonpredicates.LabelsAndGenerationPredicate{})).
WatchesRawSource(source.Kind(allNamespaceCluster.GetCache(), &rbac.RoleBinding{}), mapToOwnerByLabel, builder.WithPredicates(commonpredicates.LabelsAndGenerationPredicate{}))
// watch for all cluster resource kinds associated with an NSTemplateSet
for _, clusterResource := range clusterResourceKinds {
// only reconcile generation changes for cluster resources and only when the API group is present in the cluster
if apiGroupIsPresent(apiGroupList.Groups, clusterResource.gvk) {
build = build.Watches(&source.Kind{Type: clusterResource.object}, mapToOwnerByLabel, builder.WithPredicates(commonpredicates.LabelsAndGenerationPredicate{}))
build = build.Watches(clusterResource.object, mapToOwnerByLabel, builder.WithPredicates(commonpredicates.LabelsAndGenerationPredicate{}))
}
}

Expand Down
Loading

0 comments on commit ca053bf

Please sign in to comment.