From ea957ee7b21aee8d51de3de3de97e9718bff0edc Mon Sep 17 00:00:00 2001
From: Jan Grant <jang@ioctl.org>
Date: Fri, 7 Jun 2024 10:06:19 +0100
Subject: [PATCH] Remove the assumption that a node's name == its hostname

As per https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#built-in-node-labels
[[[
Note:
The value of these labels is cloud provider specific and is not guaranteed to be reliable. For example, the value of kubernetes.io/hostname may be the same as the node name in some environments and a different value in other environments.
]]]
---
 provisioner.go | 61 ++++++++++++++++++++++++++------------------------
 1 file changed, 32 insertions(+), 29 deletions(-)

diff --git a/provisioner.go b/provisioner.go
index ba22720e..e91db38b 100644
--- a/provisioner.go
+++ b/provisioner.go
@@ -422,20 +422,16 @@ func (p *LocalPathProvisioner) provisionFor(opts pvController.ProvisionOptions,
 		// affinity, as path is accessible from any node
 		nodeAffinity = nil
 	} else {
-		valueNode, ok := node.GetLabels()[KeyNode]
-		if !ok {
-			valueNode = nodeName
-		}
 		nodeAffinity = &v1.VolumeNodeAffinity{
 			Required: &v1.NodeSelector{
 				NodeSelectorTerms: []v1.NodeSelectorTerm{
 					{
-						MatchExpressions: []v1.NodeSelectorRequirement{
+						MatchFields: []v1.NodeSelectorRequirement{
 							{
-								Key:      KeyNode,
+								Key:      metav1.ObjectNameField,
 								Operator: v1.NodeSelectorOpIn,
 								Values: []string{
-									valueNode,
+									node.Name,
 								},
 							},
 						},
@@ -475,7 +471,7 @@ func (p *LocalPathProvisioner) deleteFor(pv *v1.PersistentVolume, c *StorageClas
 		err = errors.Wrapf(err, "failed to delete volume %v", pv.Name)
 	}()
 
-	path, node, err := p.getPathAndNodeForPV(pv, c)
+	path, node, nodeLabels, err := p.getPathAndNodeForPV(pv, c)
 	if err != nil {
 		return err
 	}
@@ -498,6 +494,7 @@ func (p *LocalPathProvisioner) deleteFor(pv *v1.PersistentVolume, c *StorageClas
 			Mode:        *pv.Spec.VolumeMode,
 			SizeInBytes: storage.Value(),
 			Node:        node,
+			NodeLabels:  nodeLabels,
 		}, c); err != nil {
 			logrus.Infof("clean up volume %v failed: %v", pv.Name, err)
 			return err
@@ -508,7 +505,7 @@ func (p *LocalPathProvisioner) deleteFor(pv *v1.PersistentVolume, c *StorageClas
 	return nil
 }
 
-func (p *LocalPathProvisioner) getPathAndNodeForPV(pv *v1.PersistentVolume, cfg *StorageClassConfig) (path, node string, err error) {
+func (p *LocalPathProvisioner) getPathAndNodeForPV(pv *v1.PersistentVolume, cfg *StorageClassConfig) (path, node string, nodeLabels map[string]string, err error) {
 	defer func() {
 		err = errors.Wrapf(err, "failed to delete volume %v", pv.Name)
 	}()
@@ -519,49 +516,55 @@ func (p *LocalPathProvisioner) getPathAndNodeForPV(pv *v1.PersistentVolume, cfg
 	} else if volumeSource.Local != nil && volumeSource.HostPath == nil {
 		path = volumeSource.Local.Path
 	} else {
-		return "", "", fmt.Errorf("no path set")
+		return "", "", nil, fmt.Errorf("no path set")
 	}
 
 	sharedFS, err := p.isSharedFilesystem(cfg)
 	if err != nil {
-		return "", "", err
+		return "", "", nil, err
 	}
 
 	if sharedFS {
 		// We don't have affinity and can use any node
-		return path, "", nil
+		return path, "", nil, nil
 	}
 
 	// Dealing with local filesystem
 
 	nodeAffinity := pv.Spec.NodeAffinity
 	if nodeAffinity == nil {
-		return "", "", fmt.Errorf("no NodeAffinity set")
+		return "", "", nil, fmt.Errorf("no NodeAffinity set")
 	}
 	required := nodeAffinity.Required
 	if required == nil {
-		return "", "", fmt.Errorf("no NodeAffinity.Required set")
+		return "", "", nil, fmt.Errorf("no NodeAffinity.Required set")
 	}
 
-	node = ""
+	// If we have an explicit node, use that; otherwise use the selector.
+	for _, selectorTerm := range required.NodeSelectorTerms {
+		for _, expression := range selectorTerm.MatchFields {
+			if expression.Key == metav1.ObjectNameField && expression.Operator == v1.NodeSelectorOpIn {
+				if len(expression.Values) != 1 {
+					return "", "", nil, fmt.Errorf("multiple values for the node affinity")
+				}
+				return path, expression.Values[0], nil, nil
+			}
+		}
+	}
+	// The scheduler must use the PV's node selector to schedule a helper pod.
 	for _, selectorTerm := range required.NodeSelectorTerms {
 		for _, expression := range selectorTerm.MatchExpressions {
 			if expression.Key == KeyNode && expression.Operator == v1.NodeSelectorOpIn {
 				if len(expression.Values) != 1 {
-					return "", "", fmt.Errorf("multiple values for the node affinity")
+					return "", "", nil, fmt.Errorf("multiple values for the node affinity")
 				}
-				node = expression.Values[0]
-				break
+				return path, "", map[string]string{
+					KeyNode: expression.Values[0],
+				}, nil
 			}
 		}
-		if node != "" {
-			break
-		}
 	}
-	if node == "" {
-		return "", "", fmt.Errorf("cannot find affinited node")
-	}
-	return path, node, nil
+	return "", "", nil, fmt.Errorf("cannot find affinited node")
 }
 
 type volumeOptions struct {
@@ -570,6 +573,7 @@ type volumeOptions struct {
 	Mode        v1.PersistentVolumeMode
 	SizeInBytes int64
 	Node        string
+	NodeLabels  map[string]string
 }
 
 func (p *LocalPathProvisioner) createHelperPod(action ActionType, cmd []string, o volumeOptions, cfg *StorageClassConfig) (err error) {
@@ -580,7 +584,7 @@ func (p *LocalPathProvisioner) createHelperPod(action ActionType, cmd []string,
 	if err != nil {
 		return err
 	}
-	if o.Name == "" || o.Path == "" || (!sharedFS && o.Node == "") {
+	if o.Name == "" || o.Path == "" || (!sharedFS && o.Node == "" && o.NodeLabels == nil) {
 		return fmt.Errorf("invalid empty name or path or node")
 	}
 	if !filepath.IsAbs(o.Path) {
@@ -661,9 +665,8 @@ func (p *LocalPathProvisioner) createHelperPod(action ActionType, cmd []string,
 		helperPod.Name = helperPod.Name[:HelperPodNameMaxLength]
 	}
 	helperPod.Namespace = p.namespace
-	if o.Node != "" {
-		helperPod.Spec.NodeName = o.Node
-	}
+	helperPod.Spec.NodeName = o.Node
+	helperPod.Spec.NodeSelector = o.NodeLabels
 	helperPod.Spec.ServiceAccountName = p.serviceAccountName
 	helperPod.Spec.RestartPolicy = v1.RestartPolicyNever
 	helperPod.Spec.Tolerations = append(helperPod.Spec.Tolerations, lpvTolerations...)
-- 
GitLab