[Bugfix] Fix LocalStorage WaitForFirstConsumer mode (#1219)

Author: ajanikow

CHANGELOG.md

@@ -7,6 +7,7 @@
 - (Feature) Add tolerations runtime rotation
 - (Feature) Promote Version Check Feature
 - (Bugfix) Ensure PDBs Consistency
+- (Bugfix) Fix LocalStorage WaitForFirstConsumer mode
 
 ## [1.2.22](https://github.com/arangodb/kube-arangodb/tree/1.2.22) (2022-12-13)
 - (Bugfix) Do not manage ports in managed ExternalAccess mode

chart/kube-arangodb/templates/storage-operator/cluster-role.yaml

@@ -20,7 +20,7 @@ rules:
       verbs: ["get", "list", "watch"]
     - apiGroups: [""]
       resources: ["namespaces", "nodes"]
-      verbs: ["get", "list"]
+      verbs: ["get", "list", "watch"]
     - apiGroups: ["storage.k8s.io"]
       resources: ["storageclasses"]
       verbs: ["*"]

chart/kube-arangodb/templates/storage-operator/role.yaml

@@ -15,7 +15,7 @@ metadata:
 rules:
     - apiGroups: [""]
       resources: ["pods"]
-      verbs: ["get", "update"]
+      verbs: ["get", "update", "watch", "list"]
     - apiGroups: [""]
       resources: ["secrets"]
       verbs: ["get"]

pkg/apis/storage/v1alpha/local_storage_spec.go

@@ -23,6 +23,8 @@ package v1alpha
 import (
 	"strings"
 
+	core "k8s.io/api/core/v1"
+
 	"github.com/arangodb/kube-arangodb/pkg/util/errors"
 )
 
@@ -32,6 +34,7 @@ type LocalStorageSpec struct {
 	StorageClass StorageClassSpec `json:"storageClass"`
 	LocalPath    []string         `json:"localPath,omitempty"`
 
+	Tolerations  []core.Toleration `json:"tolerations,omitempty"`
 	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
 	Privileged   *bool             `json:"privileged,omitempty"`
 

pkg/apis/storage/v1alpha/zz_generated.deepcopy.go

@@ -23,16 +23,51 @@ package storage
 import (
 	"context"
 	"fmt"
+	"time"
 
 	"github.com/arangodb/kube-arangodb/pkg/storage/provisioner"
 	"github.com/arangodb/kube-arangodb/pkg/storage/provisioner/client"
 	"github.com/arangodb/kube-arangodb/pkg/util/errors"
 	"github.com/arangodb/kube-arangodb/pkg/util/k8sutil"
 )
 
+type Clients map[string]provisioner.API
+
+func (c Clients) Copy() Clients {
+	r := make(Clients, len(c))
+
+	for k, v := range c {
+		r[k] = v
+	}
+
+	return r
+}
+
+func (c Clients) Filter(f func(node string, client provisioner.API) bool) Clients {
+	r := make(Clients, len(c))
+
+	for k, v := range c {
+		if f(k, v) {
+			r[k] = v
+		}
+	}
+
+	return r
+}
+
+func (c Clients) Keys() []string {
+	r := make([]string, 0, len(c))
+
+	for k := range c {
+		r = append(r, k)
+	}
+
+	return r
+}
+
 // createProvisionerClients creates a list of clients for all known
 // provisioners.
-func (ls *LocalStorage) createProvisionerClients() ([]provisioner.API, error) {
+func (ls *LocalStorage) createProvisionerClients(ctx context.Context) (Clients, error) {
 	// Find provisioner endpoints
 	ns := ls.apiObject.GetNamespace()
 	listOptions := k8sutil.LocalStorageListOpt(ls.apiObject.GetName(), roleProvisioner)
@@ -46,31 +81,38 @@ func (ls *LocalStorage) createProvisionerClients() ([]provisioner.API, error) {
 		return nil, nil
 	}
 	// Create clients for endpoints
-	clients := make([]provisioner.API, len(addrs))
-	for i, addr := range addrs {
-		var err error
-		clients[i], err = client.New(fmt.Sprintf("http://%s", addr))
+	clients := make(map[string]provisioner.API, len(addrs))
+	for _, addr := range addrs {
+		c, err := client.New(fmt.Sprintf("http://%s", addr))
 		if err != nil {
 			return nil, errors.WithStack(err)
 		}
+
+		if info, err := ls.fetchClientNodeInfo(ctx, c); err == nil {
+			clients[info.NodeName] = c
+		}
 	}
 	return clients, nil
 }
 
+func (ls *LocalStorage) fetchClientNodeInfo(ctx context.Context, c provisioner.API) (provisioner.NodeInfo, error) {
+	nctx, cancel := context.WithTimeout(ctx, 3*time.Second)
+	defer cancel()
+	return c.GetNodeInfo(nctx)
+}
+
 // GetClientByNodeName looks for a client that serves the given node name.
 // Returns an error if no such client is found.
-func (ls *LocalStorage) GetClientByNodeName(nodeName string) (provisioner.API, error) {
-	clients, err := ls.createProvisionerClients()
+func (ls *LocalStorage) GetClientByNodeName(ctx context.Context, nodeName string) (provisioner.API, error) {
+	clients, err := ls.createProvisionerClients(ctx)
 	if err != nil {
 		return nil, errors.WithStack(err)
 	}
 
 	// Find matching client
-	for _, c := range clients {
-		ctx := context.Background()
-		if info, err := c.GetNodeInfo(ctx); err == nil && info.NodeName == nodeName {
-			return c, nil
-		}
+	if c, ok := clients[nodeName]; ok {
+		return c, nil
 	}
+
 	return nil, errors.WithStack(errors.Newf("No client found for node name '%s'", nodeName))
 }

pkg/generated/clientset/versioned/fake/register.go

@@ -94,6 +94,7 @@ func (ls *LocalStorage) ensureDaemonSet(apiObject *api.ArangoLocalStorage) error
 				NodeSelector:     apiObject.Spec.NodeSelector,
 				ImagePullSecrets: ls.imagePullSecrets,
 				Priority:         apiObject.Spec.PodCustomization.GetPriority(),
+				Tolerations:      apiObject.Spec.Tolerations,
 			},
 		},
 	}
@@ -153,6 +154,7 @@ func (ls *LocalStorage) ensureDaemonSet(apiObject *api.ArangoLocalStorage) error
 		// Update it
 		current.Spec = dsSpec
 		if _, err := ls.deps.Client.Kubernetes().AppsV1().DaemonSets(ns).Update(context.Background(), current, meta.UpdateOptions{}); kerrors.IsConflict(err) && attempt < 10 {
+			ls.log.Err(err).Debug("failed to patch DaemonSet spec")
 			// Failed to update, try again
 			continue
 		} else if err != nil {

pkg/generated/clientset/versioned/scheme/register.go

@@ -259,9 +259,11 @@ func (ls *LocalStorage) run() {
 				}
 				if createNow {
 					ctx := context.Background()
-					if err := ls.createPVs(ctx, ls.apiObject, unboundPVCs); err != nil {
+					if retry, err := ls.createPVs(ctx, ls.apiObject, unboundPVCs); err != nil {
 						hasError = true
 						ls.createEvent(k8sutil.NewErrorEvent("PV creation failed", err, ls.apiObject))
+					} else if retry {
+						inspectionInterval = minInspectionInterval
 					}
 				}
 			}