Skip to content

Commit 92cb06a

Browse files
committed
Refactor CSI Translation Library into a struct that is injected into various components to simplify unit testing in future
1 parent 67d928a commit 92cb06a

File tree

20 files changed

+168
-94
lines changed

20 files changed

+168
-94
lines changed

cmd/kube-controller-manager/app/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ go_library(
141141
"//staging/src/k8s.io/component-base/cli/globalflag:go_default_library",
142142
"//staging/src/k8s.io/component-base/version:go_default_library",
143143
"//staging/src/k8s.io/component-base/version/verflag:go_default_library",
144+
"//staging/src/k8s.io/csi-translation-lib:go_default_library",
144145
"//staging/src/k8s.io/metrics/pkg/client/clientset/versioned/typed/metrics/v1beta1:go_default_library",
145146
"//staging/src/k8s.io/metrics/pkg/client/custom_metrics:go_default_library",
146147
"//staging/src/k8s.io/metrics/pkg/client/external_metrics:go_default_library",

cmd/kube-controller-manager/app/core.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
clientset "k8s.io/client-go/kubernetes"
3737
"k8s.io/client-go/metadata"
3838
restclient "k8s.io/client-go/rest"
39+
csitrans "k8s.io/csi-translation-lib"
3940
"k8s.io/kubernetes/pkg/controller"
4041
cloudcontroller "k8s.io/kubernetes/pkg/controller/cloud"
4142
endpointcontroller "k8s.io/kubernetes/pkg/controller/endpoint"
@@ -309,7 +310,8 @@ func startVolumeExpandController(ctx ControllerContext) (http.Handler, bool, err
309310
ctx.InformerFactory.Core().V1().PersistentVolumes(),
310311
ctx.InformerFactory.Storage().V1().StorageClasses(),
311312
ctx.Cloud,
312-
ProbeExpandableVolumePlugins(ctx.ComponentConfig.PersistentVolumeBinderController.VolumeConfiguration))
313+
ProbeExpandableVolumePlugins(ctx.ComponentConfig.PersistentVolumeBinderController.VolumeConfiguration),
314+
csitrans.New())
313315

314316
if expandControllerErr != nil {
315317
return nil, true, fmt.Errorf("failed to start volume expand controller : %v", expandControllerErr)

pkg/controller/volume/expand/BUILD

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ go_library(
3232
"//staging/src/k8s.io/client-go/tools/record:go_default_library",
3333
"//staging/src/k8s.io/client-go/util/workqueue:go_default_library",
3434
"//staging/src/k8s.io/cloud-provider:go_default_library",
35-
"//staging/src/k8s.io/csi-translation-lib:go_default_library",
3635
"//vendor/k8s.io/klog:go_default_library",
3736
],
3837
)
@@ -75,6 +74,7 @@ go_test(
7574
"//staging/src/k8s.io/client-go/informers:go_default_library",
7675
"//staging/src/k8s.io/client-go/testing:go_default_library",
7776
"//staging/src/k8s.io/component-base/featuregate/testing:go_default_library",
77+
"//staging/src/k8s.io/csi-translation-lib:go_default_library",
7878
"//staging/src/k8s.io/csi-translation-lib/plugins:go_default_library",
7979
],
8080
)

pkg/controller/volume/expand/expand_controller.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ import (
4141
"k8s.io/client-go/tools/record"
4242
"k8s.io/client-go/util/workqueue"
4343
cloudprovider "k8s.io/cloud-provider"
44-
csitranslation "k8s.io/csi-translation-lib"
4544
v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper"
4645
"k8s.io/kubernetes/pkg/controller/volume/events"
4746
"k8s.io/kubernetes/pkg/util/mount"
@@ -62,6 +61,11 @@ type ExpandController interface {
6261
Run(stopCh <-chan struct{})
6362
}
6463

64+
// CSINameTranslator can get the CSI Driver name based on the in-tree plugin name
65+
type CSINameTranslator interface {
66+
GetCSINameFromInTreeName(pluginName string) (string, error)
67+
}
68+
6569
type expandController struct {
6670
// kubeClient is the kube API client used by volumehost to communicate with
6771
// the API server.
@@ -92,6 +96,8 @@ type expandController struct {
9296
operationGenerator operationexecutor.OperationGenerator
9397

9498
queue workqueue.RateLimitingInterface
99+
100+
translator CSINameTranslator
95101
}
96102

97103
// NewExpandController expands the pvs
@@ -101,7 +107,8 @@ func NewExpandController(
101107
pvInformer coreinformers.PersistentVolumeInformer,
102108
scInformer storageclassinformer.StorageClassInformer,
103109
cloud cloudprovider.Interface,
104-
plugins []volume.VolumePlugin) (ExpandController, error) {
110+
plugins []volume.VolumePlugin,
111+
translator CSINameTranslator) (ExpandController, error) {
105112

106113
expc := &expandController{
107114
kubeClient: kubeClient,
@@ -113,6 +120,7 @@ func NewExpandController(
113120
classLister: scInformer.Lister(),
114121
classListerSynced: scInformer.Informer().HasSynced,
115122
queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "volume_expand"),
123+
translator: translator,
116124
}
117125

118126
if err := expc.volumePluginMgr.InitPlugins(plugins, nil, expc); err != nil {
@@ -255,7 +263,7 @@ func (expc *expandController) syncHandler(key string) error {
255263
if volumePlugin.IsMigratedToCSI() {
256264
msg := fmt.Sprintf("CSI migration enabled for %s; waiting for external resizer to expand the pvc", volumeResizerName)
257265
expc.recorder.Event(pvc, v1.EventTypeNormal, events.ExternalExpanding, msg)
258-
csiResizerName, err := csitranslation.GetCSINameFromInTreeName(class.Provisioner)
266+
csiResizerName, err := expc.translator.GetCSINameFromInTreeName(class.Provisioner)
259267
if err != nil {
260268
errorMsg := fmt.Sprintf("error getting CSI driver name for pvc %s, with error %v", util.ClaimToClaimKey(pvc), err)
261269
expc.recorder.Event(pvc, v1.EventTypeWarning, events.ExternalExpanding, errorMsg)

pkg/controller/volume/expand/expand_controller_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"k8s.io/client-go/informers"
3434
coretesting "k8s.io/client-go/testing"
3535
featuregatetesting "k8s.io/component-base/featuregate/testing"
36+
csitrans "k8s.io/csi-translation-lib"
3637
csitranslationplugins "k8s.io/csi-translation-lib/plugins"
3738
"k8s.io/kubernetes/pkg/controller"
3839
controllervolumetesting "k8s.io/kubernetes/pkg/controller/volume/attachdetach/testing"
@@ -123,7 +124,7 @@ func TestSyncHandler(t *testing.T) {
123124
if tc.storageClass != nil {
124125
informerFactory.Storage().V1().StorageClasses().Informer().GetIndexer().Add(tc.storageClass)
125126
}
126-
expc, err := NewExpandController(fakeKubeClient, pvcInformer, pvInformer, storageClassInformer, nil, allPlugins)
127+
expc, err := NewExpandController(fakeKubeClient, pvcInformer, pvInformer, storageClassInformer, nil, allPlugins, csitrans.New())
127128
if err != nil {
128129
t.Fatalf("error creating expand controller : %v", err)
129130
}

pkg/controller/volume/persistentvolume/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ go_test(
101101
"//staging/src/k8s.io/client-go/tools/record:go_default_library",
102102
"//staging/src/k8s.io/client-go/tools/reference:go_default_library",
103103
"//staging/src/k8s.io/component-base/featuregate/testing:go_default_library",
104+
"//staging/src/k8s.io/csi-translation-lib:go_default_library",
104105
"//vendor/k8s.io/klog:go_default_library",
105106
],
106107
)

pkg/controller/volume/persistentvolume/framework_test.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,12 @@ func wrapTestWithProvisionCalls(expectedProvisionCalls []provisionCall, toWrap t
521521
return wrapTestWithPluginCalls(nil, nil, expectedProvisionCalls, toWrap)
522522
}
523523

524+
type fakeCSINameTranslator struct{}
525+
526+
func (t fakeCSINameTranslator) GetCSINameFromInTreeName(pluginName string) (string, error) {
527+
return "vendor.com/MockCSIPlugin", nil
528+
}
529+
524530
// wrapTestWithCSIMigrationProvisionCalls returns a testCall that:
525531
// - configures controller with a volume plugin that emulates CSI migration
526532
// - calls given testCall
@@ -530,9 +536,7 @@ func wrapTestWithCSIMigrationProvisionCalls(toWrap testCall) testCall {
530536
isMigratedToCSI: true,
531537
}
532538
ctrl.volumePluginMgr.InitPlugins([]vol.VolumePlugin{plugin}, nil /* prober */, ctrl)
533-
ctrl.csiNameFromIntreeNameHook = func(string) (string, error) {
534-
return "vendor.com/MockCSIPlugin", nil
535-
}
539+
ctrl.translator = fakeCSINameTranslator{}
536540
return toWrap(ctrl, reactor, test)
537541
}
538542
}

pkg/controller/volume/persistentvolume/pv_controller.go

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ import (
3939
"k8s.io/client-go/util/workqueue"
4040
cloudprovider "k8s.io/cloud-provider"
4141
volerr "k8s.io/cloud-provider/volume/errors"
42-
csitranslation "k8s.io/csi-translation-lib"
4342
v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper"
4443
"k8s.io/kubernetes/pkg/controller/volume/events"
4544
"k8s.io/kubernetes/pkg/controller/volume/persistentvolume/metrics"
@@ -134,6 +133,11 @@ const createProvisionedPVRetryCount = 5
134133
// Interval between retries when we create a PV object for a provisioned volume.
135134
const createProvisionedPVInterval = 10 * time.Second
136135

136+
// CSINameTranslator can get the CSI Driver name based on the in-tree plugin name
137+
type CSINameTranslator interface {
138+
GetCSINameFromInTreeName(pluginName string) (string, error)
139+
}
140+
137141
// PersistentVolumeController is a controller that synchronizes
138142
// PersistentVolumeClaims and PersistentVolumes. It starts two
139143
// cache.Controllers that watch PersistentVolume and PersistentVolumeClaim
@@ -200,10 +204,6 @@ type PersistentVolumeController struct {
200204
createProvisionedPVRetryCount int
201205
createProvisionedPVInterval time.Duration
202206

203-
// For testing only: hook to intercept CSI driver name <=> Intree plugin name mapping
204-
// Not used when set to nil
205-
csiNameFromIntreeNameHook func(pluginName string) (string, error)
206-
207207
// operationTimestamps caches start timestamp of operations
208208
// (currently provision + binding/deletion) for metric recording.
209209
// Detailed lifecyle/key for each operation
@@ -225,6 +225,8 @@ type PersistentVolumeController struct {
225225
// the corresponding timestamp entry will be deleted from cache
226226
// abort: N.A.
227227
operationTimestamps metrics.OperationStartTimeCache
228+
229+
translator CSINameTranslator
228230
}
229231

230232
// syncClaim is the main controller method to decide what to do with a claim.
@@ -1355,13 +1357,6 @@ func (ctrl *PersistentVolumeController) provisionClaim(claim *v1.PersistentVolum
13551357
return nil
13561358
}
13571359

1358-
func (ctrl *PersistentVolumeController) getCSINameFromIntreeName(pluginName string) (string, error) {
1359-
if ctrl.csiNameFromIntreeNameHook != nil {
1360-
return ctrl.csiNameFromIntreeNameHook(pluginName)
1361-
}
1362-
return csitranslation.GetCSINameFromInTreeName(pluginName)
1363-
}
1364-
13651360
// provisionClaimOperation provisions a volume. This method is running in
13661361
// standalone goroutine and already has all necessary locks.
13671362
func (ctrl *PersistentVolumeController) provisionClaimOperation(
@@ -1571,7 +1566,7 @@ func (ctrl *PersistentVolumeController) provisionClaimOperationExternal(
15711566
provisionerName := storageClass.Provisioner
15721567
if plugin != nil {
15731568
// update the provisioner name to use the CSI in-tree name
1574-
provisionerName, err = ctrl.getCSINameFromIntreeName(storageClass.Provisioner)
1569+
provisionerName, err = ctrl.translator.GetCSINameFromInTreeName(storageClass.Provisioner)
15751570
if err != nil {
15761571
strerr := fmt.Sprintf("error getting CSI name for In tree plugin %s: %v", storageClass.Provisioner, err)
15771572
klog.V(2).Infof("%s", strerr)
@@ -1732,7 +1727,7 @@ func (ctrl *PersistentVolumeController) getProvisionerNameFromVolume(volume *v1.
17321727
return "N/A"
17331728
}
17341729
if plugin != nil {
1735-
provisionerName, err := ctrl.getCSINameFromIntreeName(class.Provisioner)
1730+
provisionerName, err := ctrl.translator.GetCSINameFromInTreeName(class.Provisioner)
17361731
if err == nil {
17371732
return provisionerName
17381733
}
@@ -1747,7 +1742,7 @@ func (ctrl *PersistentVolumeController) getProvisionerName(plugin vol.Provisiona
17471742
return plugin.GetPluginName()
17481743
} else if plugin != nil {
17491744
// get the CSI in-tree name from storage class provisioner name
1750-
provisionerName, err := ctrl.getCSINameFromIntreeName(storageClass.Provisioner)
1745+
provisionerName, err := ctrl.translator.GetCSINameFromInTreeName(storageClass.Provisioner)
17511746
if err != nil {
17521747
return "N/A"
17531748
}

pkg/controller/volume/persistentvolume/pv_controller_base.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import (
3838
"k8s.io/client-go/tools/record"
3939
"k8s.io/client-go/util/workqueue"
4040
cloudprovider "k8s.io/cloud-provider"
41+
csitrans "k8s.io/csi-translation-lib"
4142
"k8s.io/kubernetes/pkg/controller"
4243
"k8s.io/kubernetes/pkg/controller/volume/persistentvolume/metrics"
4344
pvutil "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/util"
@@ -93,6 +94,7 @@ func NewController(p ControllerParameters) (*PersistentVolumeController, error)
9394
volumeQueue: workqueue.NewNamed("volumes"),
9495
resyncPeriod: p.SyncPeriod,
9596
operationTimestamps: metrics.NewOperationStartTimeCache(),
97+
translator: csitrans.New(),
9698
}
9799

98100
// Prober is nil because PV is not aware of Flexvolume.

pkg/controller/volume/persistentvolume/pv_controller_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
storagelisters "k8s.io/client-go/listers/storage/v1"
3131
core "k8s.io/client-go/testing"
3232
"k8s.io/client-go/tools/cache"
33+
csitrans "k8s.io/csi-translation-lib"
3334
"k8s.io/klog"
3435
"k8s.io/kubernetes/pkg/controller"
3536
pvtesting "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/testing"
@@ -438,6 +439,7 @@ func TestDelayBindingMode(t *testing.T) {
438439
classInformer := informerFactory.Storage().V1().StorageClasses()
439440
ctrl := &PersistentVolumeController{
440441
classLister: classInformer.Lister(),
442+
translator: csitrans.New(),
441443
}
442444

443445
for _, class := range classes {

0 commit comments

Comments
 (0)