From d715932db8b08e0ab1894643779710c23b8db158 Mon Sep 17 00:00:00 2001 From: D074096 Date: Tue, 10 Oct 2023 15:37:57 +0200 Subject: [PATCH] Allow concurrency in kubernikus controller Supports long grace period drains better --- .github/workflows/test_workflow.yaml | 2 +- Makefile | 5 ++++- e2e/e2e_test.go | 12 ++++++------ kubernikus/node_controller.go | 10 ++++++++++ 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/.github/workflows/test_workflow.yaml b/.github/workflows/test_workflow.yaml index 329cf0d..16ca75a 100644 --- a/.github/workflows/test_workflow.yaml +++ b/.github/workflows/test_workflow.yaml @@ -70,7 +70,7 @@ jobs: - name: Run tests and generate coverage run: | - ENVTEST_K8S_VERSION=1.25.0 make test + ENVTEST_K8S_VERSION=1.25.0 make test -o lint - name: Upload coverage results to coveralls uses: shogo82148/actions-goveralls@v1 diff --git a/Makefile b/Makefile index 194d233..d26bdc7 100644 --- a/Makefile +++ b/Makefile @@ -50,7 +50,7 @@ vet: ## Run go vet against code. go vet ./... ENVTEST_ASSETS_DIR=$(shell pwd)/testbin -test: manifests generate fmt vet ## Run tests. +test: manifests generate fmt vet lint ## Run tests. mkdir -p ${ENVTEST_ASSETS_DIR} test -f ${ENVTEST_ASSETS_DIR}/setup-envtest.sh || curl -sSLo ${ENVTEST_ASSETS_DIR}/setup-envtest.sh https://raw.githubusercontent.com/kubernetes-sigs/controller-runtime/v0.8.3/hack/setup-envtest.sh source ${ENVTEST_ASSETS_DIR}/setup-envtest.sh; fetch_envtest_tools $(ENVTEST_ASSETS_DIR); setup_envtest_env $(ENVTEST_ASSETS_DIR); $(GOBIN)/ginkgo --coverpkg=github.com/sapcc/maintenance-controller/... --randomize-all ./cache ./common ./controllers ./esx ./event ./kubernikus ./metrics ./plugin ./plugin/impl ./state @@ -94,6 +94,9 @@ KUSTOMIZE = $(shell pwd)/bin/kustomize kustomize: ## Download kustomize locally if necessary. $(call go-get-tool,$(KUSTOMIZE),sigs.k8s.io/kustomize/kustomize/v3@v3.8.7) +lint: + golangci-lint run + # go-get-tool will 'go get' any package $2 and install it to $1. PROJECT_DIR := $(shell dirname $(abspath $(lastword $(MAKEFILE_LIST)))) define go-get-tool diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go index 48c2605..7f06dc0 100644 --- a/e2e/e2e_test.go +++ b/e2e/e2e_test.go @@ -53,7 +53,7 @@ func leadingPodName() string { var leaderLease coordiantionv1.Lease err := k8sClient.Get(context.Background(), types.NamespacedName{ Namespace: "kube-system", - Name: "maintenance-controller-leader-election.cloud.sap", + Name: constants.LeaderElectionID, }, &leaderLease) Expect(err).To(Succeed()) Expect(leaderLease.Spec.HolderIdentity).ToNot(BeNil()) @@ -214,7 +214,7 @@ var _ = Describe("The maintenance controller", func() { It("should recreate nodes with the kubernikus controller", func() { By("fetch node names") - nodes := &v1.NodeList{} + nodes := &corev1.NodeList{} Expect(k8sClient.List(context.Background(), nodes)).To(Succeed()) nodeNames := make([]string, 0) for _, node := range nodes.Items { @@ -227,7 +227,7 @@ var _ = Describe("The maintenance controller", func() { Expect(k8sClient.Patch(context.Background(), toDelete, client.MergeFrom(unmodified))).To(Succeed()) By("assert node gets deleted") Eventually(func(g Gomega) []string { - nodes := &v1.NodeList{} + nodes := &corev1.NodeList{} g.Expect(k8sClient.List(context.Background(), nodes)).To(Succeed()) nodeNames := make([]string, 0) for _, node := range nodes.Items { @@ -237,7 +237,7 @@ var _ = Describe("The maintenance controller", func() { }, 5*time.Minute).Should(HaveLen(1)) By("assert an other node gets added") Eventually(func(g Gomega) []string { - nodes := &v1.NodeList{} + nodes := &corev1.NodeList{} g.Expect(k8sClient.List(context.Background(), nodes)).To(Succeed()) nodeNames := make([]string, 0) for _, node := range nodes.Items { @@ -247,11 +247,11 @@ var _ = Describe("The maintenance controller", func() { }, 5*time.Minute).ShouldNot(Equal(nodeNames)) By("assert that all nodes become ready") Eventually(func(g Gomega) bool { - nodes := &v1.NodeList{} + nodes := &corev1.NodeList{} g.Expect(k8sClient.List(context.Background(), nodes)).To(Succeed()) for _, node := range nodes.Items { for _, condition := range node.Status.Conditions { - if condition.Type == v1.NodeReady && condition.Status != v1.ConditionTrue { + if condition.Type == corev1.NodeReady && condition.Status != corev1.ConditionTrue { return false } } diff --git a/kubernikus/node_controller.go b/kubernikus/node_controller.go index 1e82b33..3b45101 100644 --- a/kubernikus/node_controller.go +++ b/kubernikus/node_controller.go @@ -41,8 +41,13 @@ import ( "k8s.io/client-go/rest" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" ) +// According to https://pkg.go.dev/k8s.io/client-go/util/workqueue +// the same node is never reconciled more than once concurrently. +const ConcurrentReconciles = 5 + type Config struct { Intervals struct { Requeue time.Duration `config:"requeue" validate:"required"` @@ -247,6 +252,11 @@ func deleteVM(ctx context.Context, nodeName string) error { // SetupWithManager attaches the controller to the given manager. func (r *NodeReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). + WithOptions(controller.Options{ + // According to https://pkg.go.dev/k8s.io/client-go/util/workqueue + // the same node is never reconciled more than once concurrently. + MaxConcurrentReconciles: ConcurrentReconciles, + }). For(&v1.Node{}). Complete(r) }