Skip to content

Commit 2739535

Browse files
MartinForRealMartinForReal
committed
move kubeclient to Driver struct
1 parent 1e73071 commit 2739535

File tree

6 files changed

+103
-159
lines changed

6 files changed

+103
-159
lines changed

‎pkg/azurefile/azure.go

+2-23
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828

2929
"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2022-07-01/network"
3030
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
31+
"k8s.io/client-go/kubernetes"
3132
clientset "k8s.io/client-go/kubernetes"
3233
"k8s.io/client-go/rest"
3334
"k8s.io/client-go/tools/clientcmd"
@@ -65,37 +66,15 @@ func getRuntimeClassForPod(ctx context.Context, kubeClient clientset.Interface,
6566
}
6667

6768
// getCloudProvider get Azure Cloud Provider
68-
func getCloudProvider(ctx context.Context, kubeconfig, nodeID, secretName, secretNamespace, userAgent string, allowEmptyCloudConfig, enableWindowsHostProcess bool, kubeAPIQPS float64, kubeAPIBurst int) (*azure.Cloud, error) {
69+
func getCloudProvider(ctx context.Context, kubeClient kubernetes.Interface, nodeID, secretName, secretNamespace, userAgent string, allowEmptyCloudConfig bool) (*azure.Cloud, error) {
6970
var (
7071
config *azureconfig.Config
71-
kubeClient *clientset.Clientset
7272
fromSecret bool
7373
)
7474

7575
az := &azure.Cloud{}
7676
var err error
7777

78-
// for sanity test: if kubeconfig is set as "no-need-kubeconfig", kubeClient will be nil
79-
if kubeconfig == "no-need-kubeconfig" {
80-
klog.V(2).Infof("kubeconfig is set as no-need-kubeconfig, kubeClient will be nil")
81-
} else {
82-
kubeCfg, err := getKubeConfig(kubeconfig, enableWindowsHostProcess)
83-
if err == nil && kubeCfg != nil {
84-
klog.V(2).Infof("set QPS(%f) and QPS Burst(%d) for driver kubeClient", float32(kubeAPIQPS), kubeAPIBurst)
85-
kubeCfg.QPS = float32(kubeAPIQPS)
86-
kubeCfg.Burst = kubeAPIBurst
87-
kubeClient, err = clientset.NewForConfig(kubeCfg)
88-
if err != nil {
89-
klog.Warningf("NewForConfig failed with error: %v", err)
90-
}
91-
} else {
92-
klog.Warningf("get kubeconfig(%s) failed with error: %v", kubeconfig, err)
93-
if !os.IsNotExist(err) && !errors.Is(err, rest.ErrNotInCluster) {
94-
return az, fmt.Errorf("failed to get KubeClient: %v", err)
95-
}
96-
}
97-
}
98-
9978
if kubeClient != nil {
10079
klog.V(2).Infof("reading cloud config from secret %s/%s", secretNamespace, secretName)
10180
config, err = configloader.Load[azureconfig.Config](ctx, &configloader.K8sSecretLoaderConfig{

‎pkg/azurefile/azure_test.go

+53-107
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535

3636
corev1 "k8s.io/api/core/v1"
3737
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
38+
"k8s.io/client-go/kubernetes"
3839
fake "k8s.io/client-go/kubernetes/fake"
3940
azureprovider "sigs.k8s.io/cloud-provider-azure/pkg/provider"
4041
azureconfig "sigs.k8s.io/cloud-provider-azure/pkg/provider/config"
@@ -116,169 +117,114 @@ func TestGetRuntimeClassForPod(t *testing.T) {
116117
// To run this unit test successfully, need to ensure /etc/kubernetes/azure.json nonexistent.
117118
func TestGetCloudProvider(t *testing.T) {
118119
var (
119-
fakeCredFile = testutil.GetWorkDirPath("fake-cred-file.json", t)
120-
fakeKubeConfig = testutil.GetWorkDirPath("fake-kube-config", t)
121-
emptyKubeConfig = testutil.GetWorkDirPath("empty-kube-config", t)
122-
notExistKubeConfig = testutil.GetWorkDirPath("non-exist.json", t)
120+
fakeCredFile = testutil.GetWorkDirPath("fake-cred-file.json", t)
123121
)
124-
125-
fakeContent := `apiVersion: v1
126-
clusters:
127-
- cluster:
128-
server: https://localhost:8080
129-
name: foo-cluster
130-
contexts:
131-
- context:
132-
cluster: foo-cluster
133-
user: foo-user
134-
namespace: bar
135-
name: foo-context
136-
current-context: foo-context
137-
kind: Config
138-
users:
139-
- name: foo-user
140-
user:
141-
exec:
142-
apiVersion: client.authentication.k8s.io/v1beta1
143-
args:
144-
- arg-1
145-
- arg-2
146-
command: foo-command
147-
`
148-
149-
if err := createTestFile(emptyKubeConfig); err != nil {
150-
t.Error(err)
151-
}
152-
defer func() {
153-
if err := os.Remove(emptyKubeConfig); err != nil {
154-
t.Error(err)
155-
}
156-
}()
157-
158122
tests := []struct {
159123
desc string
160124
createFakeCredFile bool
161-
createFakeKubeConfig bool
162125
setFederatedWorkloadIdentityEnv bool
163-
kubeconfig string
126+
kubeclient kubernetes.Interface
164127
userAgent string
165128
allowEmptyCloudConfig bool
166129
aadFederatedTokenFile string
167130
useFederatedWorkloadIdentityExtension bool
168131
aadClientID string
169132
tenantID string
170-
expectedErr testutil.TestError
133+
expectedErr *testutil.TestError
171134
}{
172135
{
173136
desc: "out of cluster, no kubeconfig, no credential file",
174-
kubeconfig: "",
137+
kubeclient: nil,
175138
allowEmptyCloudConfig: true,
176-
expectedErr: testutil.TestError{},
139+
expectedErr: nil,
177140
},
178141
{
179142
desc: "[failure][disallowEmptyCloudConfig] out of cluster, no kubeconfig, no credential file",
180-
kubeconfig: "",
143+
kubeclient: nil,
181144
allowEmptyCloudConfig: false,
182-
expectedErr: testutil.TestError{
145+
expectedErr: &testutil.TestError{
183146
DefaultError: fmt.Errorf("no cloud config provided, error"),
184147
},
185148
},
186149
{
187150
desc: "[failure] out of cluster & in cluster, specify a non-exist kubeconfig, no credential file",
188-
kubeconfig: notExistKubeConfig,
151+
kubeclient: nil,
189152
allowEmptyCloudConfig: true,
190-
expectedErr: testutil.TestError{},
191-
},
192-
{
193-
desc: "[failure] out of cluster & in cluster, specify a empty kubeconfig, no credential file",
194-
kubeconfig: emptyKubeConfig,
195-
allowEmptyCloudConfig: true,
196-
expectedErr: testutil.TestError{
197-
DefaultError: fmt.Errorf("failed to get KubeClient: invalid configuration: no configuration has been provided, try setting KUBERNETES_MASTER environment variable"),
198-
},
153+
expectedErr: nil,
199154
},
200155
{
201156
desc: "[failure] out of cluster & in cluster, specify a fake kubeconfig, no credential file",
202-
createFakeKubeConfig: true,
203-
kubeconfig: fakeKubeConfig,
157+
kubeclient: fake.NewSimpleClientset(),
204158
allowEmptyCloudConfig: true,
205-
expectedErr: testutil.TestError{},
159+
expectedErr: nil,
206160
},
207161
{
208162
desc: "[success] out of cluster & in cluster, no kubeconfig, a fake credential file",
209163
createFakeCredFile: true,
210-
kubeconfig: "",
164+
kubeclient: nil,
211165
userAgent: "useragent",
212166
allowEmptyCloudConfig: true,
213-
expectedErr: testutil.TestError{},
167+
expectedErr: nil,
214168
},
215169
{
216170
desc: "[success] get azure client with workload identity",
217-
createFakeKubeConfig: true,
218171
createFakeCredFile: true,
219172
setFederatedWorkloadIdentityEnv: true,
220-
kubeconfig: fakeKubeConfig,
173+
kubeclient: fake.NewSimpleClientset(),
221174
userAgent: "useragent",
222175
useFederatedWorkloadIdentityExtension: true,
223176
aadFederatedTokenFile: "fake-token-file",
224177
aadClientID: "fake-client-id",
225178
tenantID: "fake-tenant-id",
226-
expectedErr: testutil.TestError{},
179+
expectedErr: nil,
227180
},
228181
}
229182

230183
for _, test := range tests {
231-
if test.createFakeKubeConfig {
232-
if err := createTestFile(fakeKubeConfig); err != nil {
233-
t.Error(err)
234-
}
235-
defer func() {
236-
if err := os.Remove(fakeKubeConfig); err != nil && !os.IsNotExist(err) {
184+
t.Run(test.desc, func(t *testing.T) {
185+
if test.createFakeCredFile {
186+
if err := createTestFile(fakeCredFile); err != nil {
237187
t.Error(err)
238188
}
239-
}()
240-
241-
if err := os.WriteFile(fakeKubeConfig, []byte(fakeContent), 0666); err != nil {
242-
t.Error(err)
189+
defer func() {
190+
if err := os.Remove(fakeCredFile); err != nil && !os.IsNotExist(err) {
191+
t.Error(err)
192+
}
193+
}()
194+
195+
originalCredFile, ok := os.LookupEnv(DefaultAzureCredentialFileEnv)
196+
if ok {
197+
defer os.Setenv(DefaultAzureCredentialFileEnv, originalCredFile)
198+
} else {
199+
defer os.Unsetenv(DefaultAzureCredentialFileEnv)
200+
}
201+
os.Setenv(DefaultAzureCredentialFileEnv, fakeCredFile)
243202
}
244-
}
245-
if test.createFakeCredFile {
246-
if err := createTestFile(fakeCredFile); err != nil {
247-
t.Error(err)
203+
if test.setFederatedWorkloadIdentityEnv {
204+
t.Setenv("AZURE_TENANT_ID", test.tenantID)
205+
t.Setenv("AZURE_CLIENT_ID", test.aadClientID)
206+
t.Setenv("AZURE_FEDERATED_TOKEN_FILE", test.aadFederatedTokenFile)
248207
}
249-
defer func() {
250-
if err := os.Remove(fakeCredFile); err != nil && !os.IsNotExist(err) {
251-
t.Error(err)
252-
}
253-
}()
254208

255-
originalCredFile, ok := os.LookupEnv(DefaultAzureCredentialFileEnv)
256-
if ok {
257-
defer os.Setenv(DefaultAzureCredentialFileEnv, originalCredFile)
209+
cloud, err := getCloudProvider(context.Background(), test.kubeclient, "", "", "", test.userAgent, test.allowEmptyCloudConfig)
210+
if test.expectedErr != nil {
211+
if err == nil {
212+
t.Errorf("desc: %s,\n input: %q, getCloudProvider err: %v, expectedErr: %v", test.desc, test.kubeclient, err, test.expectedErr)
213+
}
214+
if !testutil.AssertError(err, test.expectedErr) && !strings.Contains(err.Error(), test.expectedErr.DefaultError.Error()) {
215+
t.Errorf("desc: %s,\n input: %q, getCloudProvider err: %v, expectedErr: %v", test.desc, test.kubeclient, err, test.expectedErr)
216+
}
217+
}
218+
if cloud == nil {
219+
t.Errorf("return value of getCloudProvider should not be nil even there is error")
258220
} else {
259-
defer os.Unsetenv(DefaultAzureCredentialFileEnv)
221+
assert.Equal(t, test.userAgent, cloud.UserAgent)
222+
assert.Equal(t, cloud.AADFederatedTokenFile, test.aadFederatedTokenFile)
223+
assert.Equal(t, cloud.UseFederatedWorkloadIdentityExtension, test.useFederatedWorkloadIdentityExtension)
224+
assert.Equal(t, cloud.AADClientID, test.aadClientID)
225+
assert.Equal(t, cloud.TenantID, test.tenantID)
260226
}
261-
os.Setenv(DefaultAzureCredentialFileEnv, fakeCredFile)
262-
}
263-
if test.setFederatedWorkloadIdentityEnv {
264-
t.Setenv("AZURE_TENANT_ID", test.tenantID)
265-
t.Setenv("AZURE_CLIENT_ID", test.aadClientID)
266-
t.Setenv("AZURE_FEDERATED_TOKEN_FILE", test.aadFederatedTokenFile)
267-
}
268-
269-
cloud, err := getCloudProvider(context.Background(), test.kubeconfig, "", "", "", test.userAgent, test.allowEmptyCloudConfig, false, 5, 10)
270-
if !testutil.AssertError(err, &test.expectedErr) && !strings.Contains(err.Error(), test.expectedErr.DefaultError.Error()) {
271-
t.Errorf("desc: %s,\n input: %q, getCloudProvider err: %v, expectedErr: %v", test.desc, test.kubeconfig, err, test.expectedErr)
272-
}
273-
if cloud == nil {
274-
t.Errorf("return value of getCloudProvider should not be nil even there is error")
275-
} else {
276-
assert.Equal(t, test.userAgent, cloud.UserAgent)
277-
assert.Equal(t, cloud.AADFederatedTokenFile, test.aadFederatedTokenFile)
278-
assert.Equal(t, cloud.UseFederatedWorkloadIdentityExtension, test.useFederatedWorkloadIdentityExtension)
279-
assert.Equal(t, cloud.AADClientID, test.aadClientID)
280-
assert.Equal(t, cloud.TenantID, test.tenantID)
281-
}
227+
})
282228
}
283229
}
284230

‎pkg/azurefile/azurefile.go

+30-11
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"fmt"
2525
"net/http"
2626
"net/url"
27+
"os"
2728
"strconv"
2829
"strings"
2930
"sync"
@@ -45,6 +46,9 @@ import (
4546
apierrors "k8s.io/apimachinery/pkg/api/errors"
4647
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
4748
"k8s.io/apimachinery/pkg/util/wait"
49+
"k8s.io/client-go/kubernetes"
50+
clientset "k8s.io/client-go/kubernetes"
51+
"k8s.io/client-go/rest"
4852
"k8s.io/klog/v2"
4953
"k8s.io/kubernetes/pkg/volume/util"
5054
mount "k8s.io/mount-utils"
@@ -231,8 +235,6 @@ type Driver struct {
231235
enableVolumeMountGroup bool
232236
appendMountErrorHelpLink bool
233237
mountPermissions uint64
234-
kubeAPIQPS float64
235-
kubeAPIBurst int
236238
enableWindowsHostProcess bool
237239
removeSMBMountOnWindows bool
238240
appendClosetimeoOption bool
@@ -281,7 +283,7 @@ type Driver struct {
281283
// azcopy for provide exec mock for ut
282284
azcopy *fileutil.Azcopy
283285

284-
kubeconfig string
286+
kubeClient kubernetes.Interface
285287
endpoint string
286288
resolver Resolver
287289
directVolume DirectVolume
@@ -307,8 +309,6 @@ func NewDriver(options *DriverOptions) *Driver {
307309
driver.appendMountErrorHelpLink = options.AppendMountErrorHelpLink
308310
driver.mountPermissions = options.MountPermissions
309311
driver.fsGroupChangePolicy = options.FSGroupChangePolicy
310-
driver.kubeAPIQPS = options.KubeAPIQPS
311-
driver.kubeAPIBurst = options.KubeAPIBurst
312312
driver.enableWindowsHostProcess = options.EnableWindowsHostProcess
313313
driver.removeSMBMountOnWindows = options.RemoveSMBMountOnWindows
314314
driver.appendClosetimeoOption = options.AppendClosetimeoOption
@@ -322,7 +322,6 @@ func NewDriver(options *DriverOptions) *Driver {
322322
driver.subnetLockMap = newLockMap()
323323
driver.volumeLocks = newVolumeLocks()
324324
driver.azcopy = &fileutil.Azcopy{}
325-
driver.kubeconfig = options.KubeConfig
326325
driver.endpoint = options.Endpoint
327326
driver.resolver = new(NetResolver)
328327
driver.directVolume = new(directVolume)
@@ -376,6 +375,26 @@ func NewDriver(options *DriverOptions) *Driver {
376375
klog.Fatalf("%v", err)
377376
}
378377

378+
// for sanity test: if kubeconfig is set as "no-need-kubeconfig", kubeClient will be nil
379+
if options.KubeConfig == "no-need-kubeconfig" {
380+
klog.V(2).Infof("kubeconfig is set as no-need-kubeconfig, kubeClient will be nil")
381+
} else {
382+
kubeCfg, err := getKubeConfig(options.KubeConfig, options.EnableWindowsHostProcess)
383+
if err == nil && kubeCfg != nil {
384+
klog.V(2).Infof("set QPS(%f) and QPS Burst(%d) for driver kubeClient", float32(options.KubeAPIQPS), options.KubeAPIBurst)
385+
kubeCfg.QPS = float32(options.KubeAPIQPS)
386+
kubeCfg.Burst = options.KubeAPIBurst
387+
driver.kubeClient, err = clientset.NewForConfig(kubeCfg)
388+
if err != nil {
389+
klog.Warningf("NewForConfig failed with error: %v", err)
390+
}
391+
} else {
392+
klog.Warningf("get kubeconfig(%s) failed with error: %v", options.KubeConfig, err)
393+
if !os.IsNotExist(err) && !errors.Is(err, rest.ErrNotInCluster) {
394+
klog.Fatalf("failed to get KubeClient: %v", err)
395+
}
396+
}
397+
}
379398
return &driver
380399
}
381400

@@ -394,7 +413,7 @@ func (d *Driver) Run(ctx context.Context) error {
394413

395414
userAgent := GetUserAgent(d.Name, d.customUserAgent, d.userAgentSuffix)
396415
klog.V(2).Infof("driver userAgent: %s", userAgent)
397-
d.cloud, err = getCloudProvider(context.Background(), d.kubeconfig, d.NodeID, d.cloudConfigSecretName, d.cloudConfigSecretNamespace, userAgent, d.allowEmptyCloudConfig, d.enableWindowsHostProcess, d.kubeAPIQPS, d.kubeAPIBurst)
416+
d.cloud, err = getCloudProvider(context.Background(), d.kubeClient, d.NodeID, d.cloudConfigSecretName, d.cloudConfigSecretNamespace, userAgent, d.allowEmptyCloudConfig)
398417
if err != nil {
399418
klog.Fatalf("failed to get Azure Cloud Provider, error: %v", err)
400419
}
@@ -1150,11 +1169,11 @@ func (d *Driver) GetStorageAccesskey(ctx context.Context, accountOptions *azure.
11501169
// GetStorageAccountFromSecret get storage account key from k8s secret
11511170
// return <accountName, accountKey, error>
11521171
func (d *Driver) GetStorageAccountFromSecret(ctx context.Context, secretName, secretNamespace string) (string, string, error) {
1153-
if d.cloud.KubeClient == nil {
1172+
if d.kubeClient == nil {
11541173
return "", "", fmt.Errorf("could not get account key from secret(%s): KubeClient is nil", secretName)
11551174
}
11561175

1157-
secret, err := d.cloud.KubeClient.CoreV1().Secrets(secretNamespace).Get(ctx, secretName, metav1.GetOptions{})
1176+
secret, err := d.kubeClient.CoreV1().Secrets(secretNamespace).Get(ctx, secretName, metav1.GetOptions{})
11581177
if err != nil {
11591178
return "", "", fmt.Errorf("could not get secret(%v): %v", secretName, err)
11601179
}
@@ -1205,7 +1224,7 @@ func (d *Driver) useDataPlaneAPI(ctx context.Context, volumeID, accountName stri
12051224
}
12061225

12071226
func (d *Driver) SetAzureCredentials(ctx context.Context, accountName, accountKey, secretName, secretNamespace string) (string, error) {
1208-
if d.cloud.KubeClient == nil {
1227+
if d.kubeClient == nil {
12091228
klog.Warningf("could not create secret: kubeClient is nil")
12101229
return "", nil
12111230
}
@@ -1226,7 +1245,7 @@ func (d *Driver) SetAzureCredentials(ctx context.Context, accountName, accountKe
12261245
},
12271246
Type: "Opaque",
12281247
}
1229-
_, err := d.cloud.KubeClient.CoreV1().Secrets(secretNamespace).Create(ctx, secret, metav1.CreateOptions{})
1248+
_, err := d.kubeClient.CoreV1().Secrets(secretNamespace).Create(ctx, secret, metav1.CreateOptions{})
12301249
if apierrors.IsAlreadyExists(err) {
12311250
err = nil
12321251
}

0 commit comments

Comments
 (0)