Merge pull request #85 from xrstf/split-controllers

Split controllers into dedicated packages, remove Ginkgo

Author: kcp-ci-bot

Makefile

@@ -2,13 +2,10 @@
 # for checking import statements.
 OPENSHIFT_GOIMPORTS_VER := c72f1dc2e3aacfa00aece3391d938c9bc734e791
 RECONCILER_GEN_VER := v0.5.0
-# ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
-ENVTEST_K8S_VERSION = 1.31.0
 ## Tool Versions
 KUBECTL_VERSION ?= v1.32.0
 KUSTOMIZE_VERSION ?= v5.4.3
 CONTROLLER_TOOLS_VERSION ?= v0.16.1
-ENVTEST_VERSION ?= release-0.19
 GOLANGCI_LINT_VERSION ?= 2.1.6
 PROTOKOL_VERSION ?= 0.7.2
 
@@ -63,8 +60,8 @@ vet: ## Run go vet against code.
 	go vet ./...
 
 .PHONY: test
-test: fmt vet envtest ## Run tests.
-	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(TOOLS_DIR) -p path)" go test $$(go list ./... | grep -v /e2e) -coverprofile cover.out
+test: fmt vet ## Run tests.
+	go test $$(go list ./... | grep -v /e2e) -coverprofile cover.out
 
 # Utilize Kind or modify the e2e tests to load the image locally, enabling compatibility with other vendors.
 .PHONY: test-e2e  # Run the e2e tests against a Kind k8s instance that is spun up.
@@ -152,7 +149,6 @@ undeploy: kubectl kustomize ## Undeploy controller from the K8s cluster specifie
 ## Tool Binaries
 KUBECTL ?= $(TOOLS_DIR)/kubectl
 KUSTOMIZE ?= $(TOOLS_DIR)/kustomize
-ENVTEST ?= $(TOOLS_DIR)/setup-envtest
 GOLANGCI_LINT = $(TOOLS_DIR)/golangci-lint
 PROTOKOL = $(TOOLS_DIR)/protokol
 RECONCILER_GEN := $(TOOLS_DIR)/reconciler-gen
@@ -172,13 +168,6 @@ kustomize: $(KUSTOMIZE) ## Download kustomize locally if necessary.
 $(KUSTOMIZE):
 	@GO_MODULE=true hack/download-tool.sh sigs.k8s.io/kustomize/kustomize/v5 kustomize $(KUSTOMIZE_VERSION)
 
-.PHONY: envtest
-envtest: $(ENVTEST) ## Download setup-envtest locally if necessary.
-
-.PHONY: $(ENVTEST)
-$(ENVTEST):
-	@GO_MODULE=true hack/download-tool.sh sigs.k8s.io/controller-runtime/tools/setup-envtest setup-envtest $(ENVTEST_VERSION)
-
 .PHONY: golangci-lint
 golangci-lint: $(GOLANGCI_LINT) ## Download golangci-lint locally if necessary.
 

cmd/main.go

@@ -36,7 +36,11 @@ import (
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 
-	"github.com/kcp-dev/kcp-operator/internal/controller"
+	"github.com/kcp-dev/kcp-operator/internal/controller/cacheserver"
+	"github.com/kcp-dev/kcp-operator/internal/controller/frontproxy"
+	"github.com/kcp-dev/kcp-operator/internal/controller/kubeconfig"
+	"github.com/kcp-dev/kcp-operator/internal/controller/rootshard"
+	"github.com/kcp-dev/kcp-operator/internal/controller/shard"
 	"github.com/kcp-dev/kcp-operator/internal/reconciling"
 	operatorv1alpha1 "github.com/kcp-dev/kcp-operator/sdk/apis/operator/v1alpha1"
 )
@@ -149,35 +153,35 @@ func main() {
 		os.Exit(1)
 	}
 
-	if err = (&controller.RootShardReconciler{
+	if err = (&rootshard.RootShardReconciler{
 		Client: mgr.GetClient(),
 		Scheme: mgr.GetScheme(),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "KCPInstance")
 		os.Exit(1)
 	}
-	if err = (&controller.FrontProxyReconciler{
+	if err = (&frontproxy.FrontProxyReconciler{
 		Client: mgr.GetClient(),
 		Scheme: mgr.GetScheme(),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "FrontProxy")
 		os.Exit(1)
 	}
-	if err = (&controller.ShardReconciler{
+	if err = (&shard.ShardReconciler{
 		Client: mgr.GetClient(),
 		Scheme: mgr.GetScheme(),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "Shard")
 		os.Exit(1)
 	}
-	if err = (&controller.CacheServerReconciler{
+	if err = (&cacheserver.CacheServerReconciler{
 		Client: mgr.GetClient(),
 		Scheme: mgr.GetScheme(),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "CacheServer")
 		os.Exit(1)
 	}
-	if err = (&controller.KubeconfigReconciler{
+	if err = (&kubeconfig.KubeconfigReconciler{
 		Client: mgr.GetClient(),
 		Scheme: mgr.GetScheme(),
 	}).SetupWithManager(mgr); err != nil {

go.mod

@@ -12,8 +12,6 @@ require (
 	github.com/kcp-dev/code-generator/v2 v2.3.1
 	github.com/kcp-dev/kcp-operator/sdk v0.0.0-00010101000000-000000000000
 	github.com/kcp-dev/kcp/sdk v0.27.1
-	github.com/onsi/ginkgo/v2 v2.20.0
-	github.com/onsi/gomega v1.34.1
 	github.com/stretchr/testify v1.9.0
 	go.uber.org/zap v1.27.0
 	k8c.io/reconciler v0.5.0
@@ -46,7 +44,6 @@ require (
 	github.com/go-openapi/jsonpointer v0.21.0 // indirect
 	github.com/go-openapi/jsonreference v0.21.0 // indirect
 	github.com/go-openapi/swag v0.23.0 // indirect
-	github.com/go-task/slim-sprig/v3 v3.0.0 // indirect
 	github.com/gobuffalo/flect v1.0.2 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
@@ -55,7 +52,6 @@ require (
 	github.com/google/gnostic-models v0.6.8 // indirect
 	github.com/google/go-cmp v0.6.0 // indirect
 	github.com/google/gofuzz v1.2.1-0.20210504230335-f78f29fc09ea // indirect
-	github.com/google/pprof v0.0.0-20240727154555-813a5fbdbec8 // indirect
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/grpc-ecosystem/grpc-gateway/v2 v2.20.0 // indirect
 	github.com/imdario/mergo v0.3.16 // indirect
@@ -70,6 +66,7 @@ require (
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
+	github.com/onsi/gomega v1.34.1 // indirect
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
 	github.com/prometheus/client_golang v1.20.4 // indirect
@@ -104,6 +101,7 @@ require (
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20240903143218-8af14fe29dc1 // indirect
 	google.golang.org/grpc v1.66.2 // indirect
 	google.golang.org/protobuf v1.34.2 // indirect
+	gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect

internal/controller/cacheserver_controller.go renamed to internal/controller/cacheserver/controller.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package controller
+package cacheserver
 
 import (
 	"context"

internal/controller/cacheserver/controller_test.go

@@ -0,0 +1,81 @@
+/*
+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 cacheserver
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	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"
+)
+
+func TestReconciling(t *testing.T) {
+	const namespace = "cacheserver-tests"
+
+	testcases := []struct {
+		name        string
+		cacheServer *operatorv1alpha1.CacheServer
+	}{
+		{
+			name: "vanilla",
+			cacheServer: &operatorv1alpha1.CacheServer{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-resource",
+					Namespace: namespace,
+				},
+				Spec: operatorv1alpha1.CacheServerSpec{
+					Etcd: operatorv1alpha1.EtcdConfig{
+						Endpoints: []string{"https://localhost:2379"},
+					},
+				},
+			},
+		},
+	}
+
+	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()
+
+			ctx := context.Background()
+
+			controllerReconciler := &CacheServerReconciler{
+				Client: client,
+				Scheme: client.Scheme(),
+			}
+
+			_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{
+				NamespacedName: ctrlruntimeclient.ObjectKeyFromObject(testcase.cacheServer),
+			})
+			require.NoError(t, err)
+		})
+	}
+}

internal/controller/cacheserver_controller_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package controller
+package frontproxy
 
 import (
 	"context"
@@ -38,6 +38,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	"github.com/kcp-dev/kcp-operator/internal/controller/util"
 	"github.com/kcp-dev/kcp-operator/internal/reconciling"
 	"github.com/kcp-dev/kcp-operator/internal/resources"
 	"github.com/kcp-dev/kcp-operator/internal/resources/frontproxy"
@@ -118,7 +119,7 @@ func (r *FrontProxyReconciler) reconcile(ctx context.Context, frontProxy *operat
 		conditions []metav1.Condition
 	)
 
-	cond, rootShard := fetchRootShard(ctx, r.Client, frontProxy.Namespace, frontProxy.Spec.RootShard.Reference)
+	cond, rootShard := util.FetchRootShard(ctx, r.Client, frontProxy.Namespace, frontProxy.Spec.RootShard.Reference)
 	conditions = append(conditions, cond)
 
 	if rootShard == nil {
@@ -178,7 +179,7 @@ func (r *FrontProxyReconciler) reconcileStatus(ctx context.Context, oldFrontProx
 	var errs []error
 
 	depKey := types.NamespacedName{Namespace: frontProxy.Namespace, Name: resources.GetFrontProxyDeploymentName(frontProxy)}
-	cond, err := getDeploymentAvailableCondition(ctx, r.Client, depKey)
+	cond, err := util.GetDeploymentAvailableCondition(ctx, r.Client, depKey)
 	if err != nil {
 		errs = append(errs, err)
 	} else {
@@ -187,19 +188,18 @@ func (r *FrontProxyReconciler) reconcileStatus(ctx context.Context, oldFrontProx
 
 	for _, condition := range conditions {
 		condition.ObservedGeneration = frontProxy.Generation
-		frontProxy.Status.Conditions = updateCondition(frontProxy.Status.Conditions, condition)
+		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.