fix status handling for RootShards/FrontProxies, fix scheme

On-behalf-of: @SAP [email protected]

Author: xrstf

internal/controller/cacheserver/controller_test.go

@@ -20,15 +20,14 @@ import (
 	"context"
 	"testing"
 
-	certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
 	"github.com/stretchr/testify/require"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client"
 	ctrlruntimefakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	"github.com/kcp-dev/kcp-operator/internal/controller/util"
 	operatorv1alpha1 "github.com/kcp-dev/kcp-operator/sdk/apis/operator/v1alpha1"
 )
 
@@ -55,15 +54,14 @@ func TestReconciling(t *testing.T) {
 		},
 	}
 
-	scheme := runtime.NewScheme()
-	require.Nil(t, operatorv1alpha1.AddToScheme(scheme))
-	require.Nil(t, certmanagerv1.AddToScheme(scheme))
+	scheme := util.GetTestScheme()
 
 	for _, testcase := range testcases {
 		t.Run(testcase.name, func(t *testing.T) {
 			client := ctrlruntimefakeclient.
 				NewClientBuilder().
 				WithScheme(scheme).
+				WithStatusSubresource(testcase.cacheServer).
 				WithObjects(testcase.cacheServer).
 				Build()
 
@@ -77,7 +75,7 @@ func TestReconciling(t *testing.T) {
 			_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{
 				NamespacedName: ctrlruntimeclient.ObjectKeyFromObject(testcase.cacheServer),
 			})
-			require.Nil(t, err)
+			require.NoError(t, err)
 		})
 	}
 }

internal/controller/frontproxy/controller.go

@@ -191,16 +191,15 @@ func (r *FrontProxyReconciler) reconcileStatus(ctx context.Context, oldFrontProx
 		frontProxy.Status.Conditions = util.UpdateCondition(frontProxy.Status.Conditions, condition)
 	}
 
-	availableCond := apimeta.FindStatusCondition(frontProxy.Status.Conditions, string(operatorv1alpha1.ConditionTypeAvailable))
-	switch {
-	case availableCond.Status == metav1.ConditionTrue:
-		frontProxy.Status.Phase = operatorv1alpha1.FrontProxyPhaseRunning
-
-	case frontProxy.DeletionTimestamp != nil:
+	if frontProxy.DeletionTimestamp != nil {
 		frontProxy.Status.Phase = operatorv1alpha1.FrontProxyPhaseDeleting
-
-	case frontProxy.Status.Phase == "":
-		frontProxy.Status.Phase = operatorv1alpha1.FrontProxyPhaseProvisioning
+	} else {
+		availableCond := apimeta.FindStatusCondition(frontProxy.Status.Conditions, string(operatorv1alpha1.ConditionTypeAvailable))
+		if availableCond != nil && availableCond.Status == metav1.ConditionTrue {
+			frontProxy.Status.Phase = operatorv1alpha1.FrontProxyPhaseRunning
+		} else {
+			frontProxy.Status.Phase = operatorv1alpha1.FrontProxyPhaseProvisioning
+		}
 	}
 
 	// only patch the status if there are actual changes.

internal/controller/frontproxy/controller_test.go

@@ -20,16 +20,15 @@ import (
 	"context"
 	"testing"
 
-	certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
 	"github.com/stretchr/testify/require"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client"
 	ctrlruntimefakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	"github.com/kcp-dev/kcp-operator/internal/controller/util"
 	operatorv1alpha1 "github.com/kcp-dev/kcp-operator/sdk/apis/operator/v1alpha1"
 )
 
@@ -76,15 +75,14 @@ func TestReconciling(t *testing.T) {
 		},
 	}
 
-	scheme := runtime.NewScheme()
-	require.Nil(t, operatorv1alpha1.AddToScheme(scheme))
-	require.Nil(t, certmanagerv1.AddToScheme(scheme))
+	scheme := util.GetTestScheme()
 
 	for _, testcase := range testcases {
 		t.Run(testcase.name, func(t *testing.T) {
 			client := ctrlruntimefakeclient.
 				NewClientBuilder().
 				WithScheme(scheme).
+				WithStatusSubresource(testcase.rootShard, testcase.frontProxy).
 				WithObjects(testcase.rootShard, testcase.frontProxy).
 				Build()
 
@@ -98,7 +96,7 @@ func TestReconciling(t *testing.T) {
 			_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{
 				NamespacedName: ctrlruntimeclient.ObjectKeyFromObject(testcase.frontProxy),
 			})
-			require.Nil(t, err)
+			require.NoError(t, err)
 		})
 	}
 }

internal/controller/kubeconfig/controller_test.go

@@ -21,16 +21,15 @@ import (
 	"testing"
 	"time"
 
-	certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
 	"github.com/stretchr/testify/require"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client"
 	ctrlruntimefakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	"github.com/kcp-dev/kcp-operator/internal/controller/util"
 	operatorv1alpha1 "github.com/kcp-dev/kcp-operator/sdk/apis/operator/v1alpha1"
 )
 
@@ -81,15 +80,14 @@ func TestReconciling(t *testing.T) {
 		},
 	}
 
-	scheme := runtime.NewScheme()
-	require.Nil(t, operatorv1alpha1.AddToScheme(scheme))
-	require.Nil(t, certmanagerv1.AddToScheme(scheme))
+	scheme := util.GetTestScheme()
 
 	for _, testcase := range testcases {
 		t.Run(testcase.name, func(t *testing.T) {
 			client := ctrlruntimefakeclient.
 				NewClientBuilder().
 				WithScheme(scheme).
+				WithStatusSubresource(testcase.rootShard).
 				WithObjects(testcase.rootShard, testcase.kubeConfig).
 				Build()
 
@@ -103,7 +101,7 @@ func TestReconciling(t *testing.T) {
 			_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{
 				NamespacedName: ctrlruntimeclient.ObjectKeyFromObject(testcase.kubeConfig),
 			})
-			require.Nil(t, err)
+			require.NoError(t, err)
 		})
 	}
 }

internal/controller/rootshard/controller.go

@@ -206,16 +206,15 @@ func (r *RootShardReconciler) reconcileStatus(ctx context.Context, oldRootShard
 		rootShard.Status.Conditions = util.UpdateCondition(rootShard.Status.Conditions, condition)
 	}
 
-	availableCond := apimeta.FindStatusCondition(rootShard.Status.Conditions, string(operatorv1alpha1.ConditionTypeAvailable))
-	switch {
-	case availableCond.Status == metav1.ConditionTrue:
-		rootShard.Status.Phase = operatorv1alpha1.RootShardPhaseRunning
-
-	case rootShard.DeletionTimestamp != nil:
+	if rootShard.DeletionTimestamp != nil {
 		rootShard.Status.Phase = operatorv1alpha1.RootShardPhaseDeleting
-
-	case rootShard.Status.Phase == "":
-		rootShard.Status.Phase = operatorv1alpha1.RootShardPhaseProvisioning
+	} else {
+		availableCond := apimeta.FindStatusCondition(rootShard.Status.Conditions, string(operatorv1alpha1.ConditionTypeAvailable))
+		if availableCond != nil && availableCond.Status == metav1.ConditionTrue {
+			rootShard.Status.Phase = operatorv1alpha1.RootShardPhaseRunning
+		} else {
+			rootShard.Status.Phase = operatorv1alpha1.RootShardPhaseProvisioning
+		}
 	}
 
 	shards, err := util.GetRootShardChildren(ctx, r.Client, rootShard)

internal/controller/rootshard/controller_test.go

@@ -20,15 +20,14 @@ import (
 	"context"
 	"testing"
 
-	certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
 	"github.com/stretchr/testify/require"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client"
 	ctrlruntimefakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	"github.com/kcp-dev/kcp-operator/internal/controller/util"
 	operatorv1alpha1 "github.com/kcp-dev/kcp-operator/sdk/apis/operator/v1alpha1"
 )
 
@@ -61,15 +60,14 @@ func TestReconciling(t *testing.T) {
 		},
 	}
 
-	scheme := runtime.NewScheme()
-	require.Nil(t, operatorv1alpha1.AddToScheme(scheme))
-	require.Nil(t, certmanagerv1.AddToScheme(scheme))
+	scheme := util.GetTestScheme()
 
 	for _, testcase := range testcases {
 		t.Run(testcase.name, func(t *testing.T) {
 			client := ctrlruntimefakeclient.
 				NewClientBuilder().
 				WithScheme(scheme).
+				WithStatusSubresource(testcase.rootShard).
 				WithObjects(testcase.rootShard).
 				Build()
 
@@ -83,7 +81,7 @@ func TestReconciling(t *testing.T) {
 			_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{
 				NamespacedName: ctrlruntimeclient.ObjectKeyFromObject(testcase.rootShard),
 			})
-			require.Nil(t, err)
+			require.NoError(t, err)
 		})
 	}
 }

internal/controller/shard/controller_test.go

@@ -90,7 +90,8 @@ func TestReconciling(t *testing.T) {
 			client := ctrlruntimefakeclient.
 				NewClientBuilder().
 				WithScheme(scheme).
-				WithObjects(testcase.rootShard).
+				WithStatusSubresource(testcase.rootShard, testcase.shard).
+				WithObjects(testcase.rootShard, testcase.shard).
 				Build()
 
 			ctx := context.Background()
@@ -103,7 +104,7 @@ func TestReconciling(t *testing.T) {
 			_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{
 				NamespacedName: ctrlruntimeclient.ObjectKeyFromObject(testcase.rootShard),
 			})
-			require.Nil(t, err)
+			require.NoError(t, err)
 		})
 	}
 }

internal/controller/util/test.go

@@ -0,0 +1,40 @@
+/*
+Copyright 2024 The KCP Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+import (
+	certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
+
+	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"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+
+	operatorv1alpha1 "github.com/kcp-dev/kcp-operator/sdk/apis/operator/v1alpha1"
+)
+
+func GetTestScheme() *runtime.Scheme {
+	scheme := runtime.NewScheme()
+	utilruntime.Must(metav1.AddMetaToScheme(scheme))
+	utilruntime.Must(corev1.AddToScheme(scheme))
+	utilruntime.Must(operatorv1alpha1.AddToScheme(scheme))
+	utilruntime.Must(certmanagerv1.AddToScheme(scheme))
+	utilruntime.Must(appsv1.AddToScheme(scheme))
+
+	return scheme
+}