Skip to content

Commit 68054eb

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #194 from sunnylovestiramisu/leakVolume
Return SelectedNodeName in ProvisionOptions for Leaked Volume Solution
2 parents 20a8b28 + 1ae7dee commit 68054eb

File tree

12 files changed

+27
-102
lines changed

12 files changed

+27
-102
lines changed

‎allocator/doc.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,4 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package allocator // import "sigs.k8s.io/sig-storage-lib-external-provisioner/v12/allocator"
17+
package allocator // import "sigs.k8s.io/sig-storage-lib-external-provisioner/v13/allocator"

‎controller/controller.go‎

Lines changed: 8 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -51,16 +51,15 @@ import (
5151
"k8s.io/client-go/kubernetes"
5252
"k8s.io/client-go/kubernetes/scheme"
5353
corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
54-
corelistersv1 "k8s.io/client-go/listers/core/v1"
5554
"k8s.io/client-go/tools/cache"
5655
"k8s.io/client-go/tools/leaderelection"
5756
"k8s.io/client-go/tools/leaderelection/resourcelock"
5857
"k8s.io/client-go/tools/record"
5958
ref "k8s.io/client-go/tools/reference"
6059
"k8s.io/client-go/util/workqueue"
6160
klog "k8s.io/klog/v2"
62-
"sigs.k8s.io/sig-storage-lib-external-provisioner/v12/controller/metrics"
63-
"sigs.k8s.io/sig-storage-lib-external-provisioner/v12/util"
61+
"sigs.k8s.io/sig-storage-lib-external-provisioner/v13/controller/metrics"
62+
"sigs.k8s.io/sig-storage-lib-external-provisioner/v13/util"
6463
)
6564

6665
// This annotation is added to a PV that has been dynamically provisioned by
@@ -125,7 +124,6 @@ type ProvisionController struct {
125124
volumeInformer cache.SharedInformer
126125
volumes cache.Store
127126
classInformer cache.SharedInformer
128-
nodeLister corelistersv1.NodeLister
129127
classes cache.Store
130128

131129
// To determine if the informer is internal or external
@@ -517,22 +515,6 @@ func ClassesInformer(informer cache.SharedInformer) func(*ProvisionController) e
517515
}
518516
}
519517

520-
// NodesLister sets the informer to use for accessing Nodes.
521-
// This is needed only for PVCs which have a selected node.
522-
// Defaults to using a GET instead of an informer.
523-
//
524-
// Which approach is better depends on factors like cluster size and
525-
// ratio of PVCs with a selected node.
526-
func NodesLister(nodeLister corelistersv1.NodeLister) func(*ProvisionController) error {
527-
return func(c *ProvisionController) error {
528-
if c.HasRun() {
529-
return errRuntime
530-
}
531-
c.nodeLister = nodeLister
532-
return nil
533-
}
534-
}
535-
536518
// MetricsInstance defines which metrics collection to update. Default: metrics.Metrics.
537519
func MetricsInstance(m metrics.Metrics) func(*ProvisionController) error {
538520
return func(c *ProvisionController) error {
@@ -1483,31 +1465,17 @@ func (ctrl *ProvisionController) provisionClaimOperation(ctx context.Context, cl
14831465
return ProvisioningFinished, errStopProvision
14841466
}
14851467

1486-
var selectedNode *v1.Node
1468+
var selectedNodeName string
14871469
// Get SelectedNode
14881470
if nodeName, ok := getString(claim.Annotations, annSelectedNode, annAlphaSelectedNode); ok {
1489-
if ctrl.nodeLister != nil {
1490-
selectedNode, err = ctrl.nodeLister.Get(nodeName)
1491-
} else {
1492-
selectedNode, err = ctrl.client.CoreV1().Nodes().Get(ctx, nodeName, metav1.GetOptions{}) // TODO (verult) cache Nodes
1493-
}
1494-
if err != nil {
1495-
// if node does not exist, reschedule and remove volume.kubernetes.io/selected-node annotation
1496-
if apierrs.IsNotFound(err) {
1497-
ctx2 := klog.NewContext(ctx, logger)
1498-
return ctrl.provisionVolumeErrorHandling(ctx2, ProvisioningReschedule, err, claim)
1499-
}
1500-
err = fmt.Errorf("failed to get target node: %v", err)
1501-
ctrl.eventRecorder.Event(claim, v1.EventTypeWarning, "ProvisioningFailed", err.Error())
1502-
return ProvisioningNoChange, err
1503-
}
1471+
selectedNodeName = nodeName
15041472
}
15051473

15061474
options := ProvisionOptions{
1507-
StorageClass: class,
1508-
PVName: pvName,
1509-
PVC: claim,
1510-
SelectedNode: selectedNode,
1475+
StorageClass: class,
1476+
PVName: pvName,
1477+
PVC: claim,
1478+
SelectedNodeName: selectedNodeName,
15111479
}
15121480

15131481
ctrl.eventRecorder.Event(claim, v1.EventTypeNormal, "Provisioning", fmt.Sprintf("External provisioner is provisioning volume for claim %q", klog.KObj(claim)))

‎controller/controller_test.go‎

Lines changed: 6 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ import (
4949
klog "k8s.io/klog/v2"
5050
"k8s.io/klog/v2/ktesting"
5151
_ "k8s.io/klog/v2/ktesting/init"
52-
"sigs.k8s.io/sig-storage-lib-external-provisioner/v12/controller/metrics"
53-
"sigs.k8s.io/sig-storage-lib-external-provisioner/v12/util"
52+
"sigs.k8s.io/sig-storage-lib-external-provisioner/v13/controller/metrics"
53+
"sigs.k8s.io/sig-storage-lib-external-provisioner/v13/util"
5454
)
5555

5656
const (
@@ -561,24 +561,6 @@ func TestController(t *testing.T) {
561561
},
562562
},
563563
},
564-
{
565-
name: "remove selectedNode if no node exists",
566-
objs: []runtime.Object{
567-
newStorageClassWithVolumeBindingMode("class-1", "foo.bar/baz", &modeWait),
568-
newClaim("claim-1", "uid-1-1", "class-1", "foo.bar/baz", "", map[string]string{annBetaStorageProvisioner: "foo.bar/baz", annSelectedNode: "node-wrong"}),
569-
},
570-
provisionerName: "foo.bar/baz",
571-
provisioner: newBadTestProvisioner(),
572-
expectedClaims: []v1.PersistentVolumeClaim{
573-
*newClaim("claim-1", "uid-1-1", "class-1", "foo.bar/baz", "", map[string]string{annBetaStorageProvisioner: "foo.bar/baz"}),
574-
},
575-
expectedClaimsInProgress: nil, // not in progress anymore
576-
expectedMetrics: testMetrics{
577-
provisioned: counts{
578-
"class-1": count{failed: 1},
579-
},
580-
},
581-
},
582564
{
583565
name: "do not remove selectedNode if nothing changes",
584566
objs: []runtime.Object{
@@ -597,23 +579,6 @@ func TestController(t *testing.T) {
597579
},
598580
},
599581
},
600-
{
601-
name: "remove selectedNode if nothing changes and no node exists",
602-
objs: []runtime.Object{
603-
newStorageClassWithVolumeBindingMode("class-1", "foo.bar/baz", &modeWait),
604-
newClaim("claim-1", "uid-1-1", "class-1", "foo.bar/baz", "", map[string]string{annBetaStorageProvisioner: "foo.bar/baz", annSelectedNode: "node-wrong"}),
605-
},
606-
provisionerName: "foo.bar/baz",
607-
provisioner: newNoChangeTestProvisioner(),
608-
expectedClaims: []v1.PersistentVolumeClaim{
609-
*newClaim("claim-1", "uid-1-1", "class-1", "foo.bar/baz", "", map[string]string{annBetaStorageProvisioner: "foo.bar/baz"}),
610-
},
611-
expectedMetrics: testMetrics{
612-
provisioned: counts{
613-
"class-1": count{failed: 1},
614-
},
615-
},
616-
},
617582
{
618583
name: "do not remove selectedNode while in progress",
619584
objs: []runtime.Object{
@@ -931,7 +896,7 @@ func TestTopologyParams(t *testing.T) {
931896
newClaim("claim-1", "uid-1-1", "class-1", "foo.bar/baz", "", map[string]string{annSelectedNode: "node-1"}),
932897
},
933898
expectedParams: &provisionParams{
934-
selectedNode: newNode("node-1"),
899+
selectedNodeName: "node-1",
935900
},
936901
},
937902
{
@@ -947,16 +912,8 @@ func TestTopologyParams(t *testing.T) {
947912
},
948913
expectedParams: &provisionParams{
949914
allowedTopologies: dummyAllowedTopology,
950-
selectedNode: newNode("node-1"),
951-
},
952-
},
953-
{
954-
name: "provision with selected node, but node does not exist",
955-
objs: []runtime.Object{
956-
newStorageClassWithVolumeBindingMode("class-1", "foo.bar/baz", &modeWait),
957-
newClaim("claim-1", "uid-1-1", "class-1", "foo.bar/baz", "", map[string]string{annSelectedNode: "node-1"}),
915+
selectedNodeName: "node-1",
958916
},
959-
expectedParams: nil,
960917
},
961918
}
962919
for _, test := range tests {
@@ -2084,7 +2041,7 @@ func newNode(nodeName string) *v1.Node {
20842041
}
20852042

20862043
type provisionParams struct {
2087-
selectedNode *v1.Node
2044+
selectedNodeName string
20882045
allowedTopologies []v1.TopologySelectorTerm
20892046
}
20902047

@@ -2136,7 +2093,7 @@ func (p *testBlockProvisioner) SupportsBlock(ctx context.Context) bool {
21362093

21372094
func (p *testProvisioner) Provision(ctx context.Context, options ProvisionOptions) (*v1.PersistentVolume, ProvisioningState, error) {
21382095
p.provisionCalls <- provisionParams{
2139-
selectedNode: options.SelectedNode,
2096+
selectedNodeName: options.SelectedNodeName,
21402097
allowedTopologies: options.StorageClass.AllowedTopologies,
21412098
}
21422099

‎controller/doc.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,4 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package controller // import "sigs.k8s.io/sig-storage-lib-external-provisioner/v12/controller"
17+
package controller // import "sigs.k8s.io/sig-storage-lib-external-provisioner/v13/controller"

‎controller/metrics/doc.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,4 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package metrics // import "sigs.k8s.io/sig-storage-lib-external-provisioner/v12/controller/metrics"
17+
package metrics // import "sigs.k8s.io/sig-storage-lib-external-provisioner/v13/controller/metrics"

‎controller/volume.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020
"context"
2121
"fmt"
2222

23-
"k8s.io/api/core/v1"
23+
v1 "k8s.io/api/core/v1"
2424
storageapis "k8s.io/api/storage/v1"
2525
)
2626

@@ -129,5 +129,5 @@ type ProvisionOptions struct {
129129
PVC *v1.PersistentVolumeClaim
130130

131131
// Node selected by the scheduler for the volume.
132-
SelectedNode *v1.Node
132+
SelectedNodeName string
133133
}

‎examples/hostpath-provisioner/hostpath-provisioner.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424
"path"
2525
"syscall"
2626

27-
"sigs.k8s.io/sig-storage-lib-external-provisioner/v12/controller"
27+
"sigs.k8s.io/sig-storage-lib-external-provisioner/v13/controller"
2828

2929
v1 "k8s.io/api/core/v1"
3030
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

‎gidallocator/doc.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,4 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package gidallocator // import "sigs.k8s.io/sig-storage-lib-external-provisioner/v12/gidallocator"
17+
package gidallocator // import "sigs.k8s.io/sig-storage-lib-external-provisioner/v13/gidallocator"

‎gidallocator/gidallocator.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ import (
2828
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2929
"k8s.io/client-go/kubernetes"
3030
klog "k8s.io/klog/v2"
31-
"sigs.k8s.io/sig-storage-lib-external-provisioner/v12/allocator"
32-
"sigs.k8s.io/sig-storage-lib-external-provisioner/v12/controller"
33-
"sigs.k8s.io/sig-storage-lib-external-provisioner/v12/util"
31+
"sigs.k8s.io/sig-storage-lib-external-provisioner/v13/allocator"
32+
"sigs.k8s.io/sig-storage-lib-external-provisioner/v13/controller"
33+
"sigs.k8s.io/sig-storage-lib-external-provisioner/v13/util"
3434
)
3535

3636
const (

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
module sigs.k8s.io/sig-storage-lib-external-provisioner/v12
1+
module sigs.k8s.io/sig-storage-lib-external-provisioner/v13
22

33
go 1.24.0
44

0 commit comments

Comments
 (0)