Skip to content

Commit

Permalink
Allow concurrency in kubernikus controller
Browse files Browse the repository at this point in the history
Supports long grace period drains better
  • Loading branch information
Nuckal777 committed Oct 10, 2023
1 parent bc73654 commit d715932
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 8 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test_workflow.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions e2e/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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
}
}
Expand Down
10 changes: 10 additions & 0 deletions kubernikus/node_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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)
}

0 comments on commit d715932

Please sign in to comment.