Skip to content

Commit 3152f81

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #1094 from MartinForReal/master
Refactor: adopt dependency injection pattern and extract cloud property
2 parents 74ad3c2 + ad9d27c commit 3152f81

File tree

7 files changed

+39
-104
lines changed

7 files changed

+39
-104
lines changed

‎pkg/blob/azure.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func IsAzureStackCloud(cloud *azure.Cloud) bool {
5252
}
5353

5454
// getCloudProvider get Azure Cloud Provider
55-
func getCloudProvider(kubeconfig, nodeID, secretName, secretNamespace, userAgent string, allowEmptyCloudConfig bool, kubeAPIQPS float64, kubeAPIBurst int) (*azure.Cloud, error) {
55+
func GetCloudProvider(kubeconfig, nodeID, secretName, secretNamespace, userAgent string, allowEmptyCloudConfig bool, kubeAPIQPS float64, kubeAPIBurst int) (*azure.Cloud, error) {
5656
var (
5757
config *azure.Config
5858
kubeClient *clientset.Clientset

‎pkg/blob/azure_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ users:
168168
}
169169
os.Setenv(DefaultAzureCredentialFileEnv, fakeCredFile)
170170
}
171-
cloud, err := getCloudProvider(test.kubeconfig, test.nodeID, "", "", test.userAgent, test.allowEmptyCloudConfig, 25.0, 50)
171+
cloud, err := GetCloudProvider(test.kubeconfig, test.nodeID, "", "", test.userAgent, test.allowEmptyCloudConfig, 25.0, 50)
172172
if !reflect.DeepEqual(err, test.expectedErr) && test.expectedErr != nil && !strings.Contains(err.Error(), test.expectedErr.Error()) {
173173
t.Errorf("desc: %s,\n input: %q, GetCloudProvider err: %v, expectedErr: %v", test.desc, test.kubeconfig, err, test.expectedErr)
174174
}

‎pkg/blob/blob.go‎

Lines changed: 15 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -153,22 +153,15 @@ var (
153153
type DriverOptions struct {
154154
NodeID string
155155
DriverName string
156-
CloudConfigSecretName string
157-
CloudConfigSecretNamespace string
158-
CustomUserAgent string
159-
UserAgentSuffix string
160156
BlobfuseProxyEndpoint string
161157
EnableBlobfuseProxy bool
162158
BlobfuseProxyConnTimout int
163159
EnableBlobMockMount bool
164-
AllowEmptyCloudConfig bool
165160
AllowInlineVolumeKeyAccessWithIdentity bool
166161
EnableGetVolumeStats bool
167162
AppendTimeStampInCacheDir bool
168163
AppendMountErrorHelpLink bool
169164
MountPermissions uint64
170-
KubeAPIQPS float64
171-
KubeAPIBurst int
172165
EnableAznfsMount bool
173166
VolStatsCacheExpireInMinutes int
174167
SasTokenExpirationMinutes int
@@ -178,24 +171,17 @@ type DriverOptions struct {
178171
type Driver struct {
179172
csicommon.CSIDriver
180173

181-
cloud *azure.Cloud
182-
cloudConfigSecretName string
183-
cloudConfigSecretNamespace string
184-
customUserAgent string
185-
userAgentSuffix string
186-
blobfuseProxyEndpoint string
174+
cloud *azure.Cloud
175+
blobfuseProxyEndpoint string
187176
// enableBlobMockMount is only for testing, DO NOT set as true in non-testing scenario
188177
enableBlobMockMount bool
189178
enableBlobfuseProxy bool
190-
allowEmptyCloudConfig bool
191179
enableGetVolumeStats bool
192180
allowInlineVolumeKeyAccessWithIdentity bool
193181
appendTimeStampInCacheDir bool
194182
appendMountErrorHelpLink bool
195183
blobfuseProxyConnTimout int
196184
mountPermissions uint64
197-
kubeAPIQPS float64
198-
kubeAPIBurst int
199185
enableAznfsMount bool
200186
mounter *mount.SafeFormatAndMount
201187
volLockMap *util.LockMap
@@ -220,26 +206,19 @@ type Driver struct {
220206

221207
// NewDriver Creates a NewCSIDriver object. Assumes vendor version is equal to driver version &
222208
// does not support optional driver plugin info manifest field. Refer to CSI spec for more details.
223-
func NewDriver(options *DriverOptions) *Driver {
209+
func NewDriver(options *DriverOptions, cloud *azure.Cloud) *Driver {
224210
d := Driver{
225211
volLockMap: util.NewLockMap(),
226212
subnetLockMap: util.NewLockMap(),
227213
volumeLocks: newVolumeLocks(),
228-
cloudConfigSecretName: options.CloudConfigSecretName,
229-
cloudConfigSecretNamespace: options.CloudConfigSecretNamespace,
230-
customUserAgent: options.CustomUserAgent,
231-
userAgentSuffix: options.UserAgentSuffix,
232214
blobfuseProxyEndpoint: options.BlobfuseProxyEndpoint,
233215
enableBlobfuseProxy: options.EnableBlobfuseProxy,
234216
allowInlineVolumeKeyAccessWithIdentity: options.AllowInlineVolumeKeyAccessWithIdentity,
235217
blobfuseProxyConnTimout: options.BlobfuseProxyConnTimout,
236218
enableBlobMockMount: options.EnableBlobMockMount,
237-
allowEmptyCloudConfig: options.AllowEmptyCloudConfig,
238219
enableGetVolumeStats: options.EnableGetVolumeStats,
239220
appendMountErrorHelpLink: options.AppendMountErrorHelpLink,
240221
mountPermissions: options.MountPermissions,
241-
kubeAPIQPS: options.KubeAPIQPS,
242-
kubeAPIBurst: options.KubeAPIBurst,
243222
enableAznfsMount: options.EnableAznfsMount,
244223
sasTokenExpirationMinutes: options.SasTokenExpirationMinutes,
245224
azcopy: &util.Azcopy{},
@@ -263,25 +242,7 @@ func NewDriver(options *DriverOptions) *Driver {
263242
if d.volStatsCache, err = azcache.NewTimedCache(time.Duration(options.VolStatsCacheExpireInMinutes)*time.Minute, getter, false); err != nil {
264243
klog.Fatalf("%v", err)
265244
}
266-
return &d
267-
}
268-
269-
// Run driver initialization
270-
func (d *Driver) Run(endpoint, kubeconfig string, testBool bool) {
271-
versionMeta, err := GetVersionYAML(d.Name)
272-
if err != nil {
273-
klog.Fatalf("%v", err)
274-
}
275-
klog.Infof("\nDRIVER INFORMATION:\n-------------------\n%s\n\nStreaming logs below:", versionMeta)
276-
277-
userAgent := GetUserAgent(d.Name, d.customUserAgent, d.userAgentSuffix)
278-
klog.V(2).Infof("driver userAgent: %s", userAgent)
279-
d.cloud, err = getCloudProvider(kubeconfig, d.NodeID, d.cloudConfigSecretName, d.cloudConfigSecretNamespace, userAgent, d.allowEmptyCloudConfig, d.kubeAPIQPS, d.kubeAPIBurst)
280-
if err != nil {
281-
klog.Fatalf("failed to get Azure Cloud Provider, error: %v", err)
282-
}
283-
klog.V(2).Infof("cloud: %s, location: %s, rg: %s, VnetName: %s, VnetResourceGroup: %s, SubnetName: %s", d.cloud.Cloud, d.cloud.Location, d.cloud.ResourceGroup, d.cloud.VnetName, d.cloud.VnetResourceGroup, d.cloud.SubnetName)
284-
245+
d.cloud = cloud
285246
d.mounter = &mount.SafeFormatAndMount{
286247
Interface: mount.New(""),
287248
Exec: utilexec.New(),
@@ -316,6 +277,17 @@ func (d *Driver) Run(endpoint, kubeconfig string, testBool bool) {
316277
}
317278
d.AddNodeServiceCapabilities(nodeCap)
318279

280+
return &d
281+
}
282+
283+
// Run driver initialization
284+
func (d *Driver) Run(endpoint string, testBool bool) {
285+
versionMeta, err := GetVersionYAML(d.Name)
286+
if err != nil {
287+
klog.Fatalf("%v", err)
288+
}
289+
klog.Infof("\nDRIVER INFORMATION:\n-------------------\n%s\n\nStreaming logs below:", versionMeta)
290+
319291
s := csicommon.NewNonBlockingGRPCServer()
320292
// Driver d act as IdentityServer, ControllerServer and NodeServer
321293
s.Start(endpoint, d, d, d, testBool)

‎pkg/blob/blob_test.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,10 @@ func NewFakeDriver() *Driver {
5656
BlobfuseProxyConnTimout: 5,
5757
EnableBlobMockMount: false,
5858
}
59-
driver := NewDriver(&driverOptions)
59+
driver := NewDriver(&driverOptions, &azure.Cloud{})
6060
driver.Name = fakeDriverName
6161
driver.Version = vendorVersion
6262
driver.subnetLockMap = util.NewLockMap()
63-
driver.cloud = &azure.Cloud{}
6463
return driver
6564
}
6665

@@ -73,7 +72,7 @@ func TestNewFakeDriver(t *testing.T) {
7372
BlobfuseProxyConnTimout: 5,
7473
EnableBlobMockMount: false,
7574
}
76-
d := NewDriver(&driverOptions)
75+
d := NewDriver(&driverOptions, &azure.Cloud{})
7776
assert.NotNil(t, d)
7877
}
7978

@@ -86,7 +85,7 @@ func TestNewDriver(t *testing.T) {
8685
BlobfuseProxyConnTimout: 5,
8786
EnableBlobMockMount: false,
8887
}
89-
driver := NewDriver(&driverOptions)
88+
driver := NewDriver(&driverOptions, &azure.Cloud{})
9089
fakedriver := NewFakeDriver()
9190
fakedriver.Name = DefaultDriverName
9291
fakedriver.Version = driverVersion
@@ -134,7 +133,7 @@ func TestRun(t *testing.T) {
134133
os.Setenv(DefaultAzureCredentialFileEnv, fakeCredFile)
135134

136135
d := NewFakeDriver()
137-
d.Run("tcp://127.0.0.1:0", "", true)
136+
d.Run("tcp://127.0.0.1:0", true)
138137
},
139138
},
140139
{
@@ -161,7 +160,7 @@ func TestRun(t *testing.T) {
161160
d := NewFakeDriver()
162161
d.cloud = &azure.Cloud{}
163162
d.NodeID = ""
164-
d.Run("tcp://127.0.0.1:0", "", true)
163+
d.Run("tcp://127.0.0.1:0", true)
165164
},
166165
},
167166
}

‎pkg/blob/controllerserver_test.go‎

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -163,18 +163,6 @@ func TestCreateVolume(t *testing.T) {
163163
name string
164164
testFunc func(t *testing.T)
165165
}{
166-
{
167-
name: "invalid create volume req",
168-
testFunc: func(t *testing.T) {
169-
d := NewFakeDriver()
170-
req := &csi.CreateVolumeRequest{}
171-
_, err := d.CreateVolume(context.Background(), req)
172-
expectedErr := status.Error(codes.InvalidArgument, "CREATE_DELETE_VOLUME")
173-
if !reflect.DeepEqual(err, expectedErr) {
174-
t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
175-
}
176-
},
177-
},
178166
{
179167
name: "volume Name missing",
180168
testFunc: func(t *testing.T) {
@@ -893,20 +881,6 @@ func TestDeleteVolume(t *testing.T) {
893881
}
894882
},
895883
},
896-
{
897-
name: "invalid delete volume req",
898-
testFunc: func(t *testing.T) {
899-
d := NewFakeDriver()
900-
req := &csi.DeleteVolumeRequest{
901-
VolumeId: "unit-test",
902-
}
903-
_, err := d.DeleteVolume(context.Background(), req)
904-
expectedErr := status.Errorf(codes.Internal, "invalid delete volume req: volume_id:\"unit-test\" ")
905-
if !reflect.DeepEqual(err, expectedErr) {
906-
t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
907-
}
908-
},
909-
},
910884
{
911885
name: "invalid volume Id",
912886
testFunc: func(t *testing.T) {
@@ -1298,21 +1272,6 @@ func TestControllerExpandVolume(t *testing.T) {
12981272
}
12991273
},
13001274
},
1301-
{
1302-
name: "invalid expand volume req",
1303-
testFunc: func(t *testing.T) {
1304-
d := NewFakeDriver()
1305-
req := &csi.ControllerExpandVolumeRequest{
1306-
VolumeId: "unit-test",
1307-
CapacityRange: &csi.CapacityRange{},
1308-
}
1309-
_, err := d.ControllerExpandVolume(context.Background(), req)
1310-
expectedErr := status.Errorf(codes.Internal, "invalid expand volume req: volume_id:\"unit-test\" capacity_range:<> ")
1311-
if !reflect.DeepEqual(err, expectedErr) {
1312-
t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
1313-
}
1314-
},
1315-
},
13161275
{
13171276
name: "Volume Size exceeds Max Container Size",
13181277
testFunc: func(t *testing.T) {

‎pkg/blobplugin/main.go‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -81,31 +81,34 @@ func handle() {
8181
driverOptions := blob.DriverOptions{
8282
NodeID: *nodeID,
8383
DriverName: *driverName,
84-
CloudConfigSecretName: *cloudConfigSecretName,
85-
CloudConfigSecretNamespace: *cloudConfigSecretNamespace,
8684
BlobfuseProxyEndpoint: *blobfuseProxyEndpoint,
8785
EnableBlobfuseProxy: *enableBlobfuseProxy,
8886
BlobfuseProxyConnTimout: *blobfuseProxyConnTimout,
8987
EnableBlobMockMount: *enableBlobMockMount,
90-
CustomUserAgent: *customUserAgent,
91-
UserAgentSuffix: *userAgentSuffix,
92-
AllowEmptyCloudConfig: *allowEmptyCloudConfig,
9388
EnableGetVolumeStats: *enableGetVolumeStats,
9489
AppendTimeStampInCacheDir: *appendTimeStampInCacheDir,
9590
MountPermissions: *mountPermissions,
9691
AllowInlineVolumeKeyAccessWithIdentity: *allowInlineVolumeKeyAccessWithIdentity,
9792
AppendMountErrorHelpLink: *appendMountErrorHelpLink,
98-
KubeAPIQPS: *kubeAPIQPS,
99-
KubeAPIBurst: *kubeAPIBurst,
10093
EnableAznfsMount: *enableAznfsMount,
10194
VolStatsCacheExpireInMinutes: *volStatsCacheExpireInMinutes,
10295
SasTokenExpirationMinutes: *sasTokenExpirationMinutes,
10396
}
104-
driver := blob.NewDriver(&driverOptions)
97+
98+
userAgent := blob.GetUserAgent(driverOptions.DriverName, *customUserAgent, *userAgentSuffix)
99+
klog.V(2).Infof("driver userAgent: %s", userAgent)
100+
101+
cloud, err := blob.GetCloudProvider(*kubeconfig, driverOptions.NodeID, *cloudConfigSecretName, *cloudConfigSecretNamespace, userAgent, *allowEmptyCloudConfig, *kubeAPIQPS, *kubeAPIBurst)
102+
if err != nil {
103+
klog.Fatalf("failed to get Azure Cloud Provider, error: %v", err)
104+
}
105+
klog.V(2).Infof("cloud: %s, location: %s, rg: %s, VnetName: %s, VnetResourceGroup: %s, SubnetName: %s", cloud.Cloud, cloud.Location, cloud.ResourceGroup, cloud.VnetName, cloud.VnetResourceGroup, cloud.SubnetName)
106+
107+
driver := blob.NewDriver(&driverOptions, cloud)
105108
if driver == nil {
106109
klog.Fatalln("Failed to initialize Azure Blob Storage CSI driver")
107110
}
108-
driver.Run(*endpoint, *kubeconfig, false)
111+
driver.Run(*endpoint, false)
109112
}
110113

111114
func exportMetrics() {

‎test/e2e/suite_test.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ var _ = ginkgo.SynchronizedBeforeSuite(func(ctx ginkgo.SpecContext) []byte {
142142
// spin up a blob driver locally to make use of the azure client and controller service
143143
kubeconfig := os.Getenv(kubeconfigEnvVar)
144144
_, useBlobfuseProxy := os.LookupEnv("ENABLE_BLOBFUSE_PROXY")
145+
os.Setenv("AZURE_CREDENTIAL_FILE", credentials.TempAzureCredentialFilePath)
145146
driverOptions := blob.DriverOptions{
146147
NodeID: os.Getenv("nodeid"),
147148
DriverName: blob.DefaultDriverName,
@@ -150,10 +151,11 @@ var _ = ginkgo.SynchronizedBeforeSuite(func(ctx ginkgo.SpecContext) []byte {
150151
BlobfuseProxyConnTimout: 5,
151152
EnableBlobMockMount: false,
152153
}
153-
blobDriver = blob.NewDriver(&driverOptions)
154+
cloud, err := blob.GetCloudProvider(kubeconfig, driverOptions.NodeID, "", "", "", false, 0, 0)
155+
gomega.Expect(err).NotTo(gomega.HaveOccurred())
156+
blobDriver = blob.NewDriver(&driverOptions, cloud)
154157
go func() {
155-
os.Setenv("AZURE_CREDENTIAL_FILE", credentials.TempAzureCredentialFilePath)
156-
blobDriver.Run(fmt.Sprintf("unix:///tmp/csi-%s.sock", uuid.NewUUID().String()), kubeconfig, false)
158+
blobDriver.Run(fmt.Sprintf("unix:///tmp/csi-%s.sock", uuid.NewUUID().String()), false)
157159
}()
158160
})
159161

0 commit comments

Comments
 (0)