From 2417ea021964b64eac3af98ce4e134543708a9c5 Mon Sep 17 00:00:00 2001 From: Humble Chirammal Date: Thu, 10 Feb 2022 12:20:25 +0530 Subject: [PATCH 1/3] update OWNERS file Signed-off-by: Humble Chirammal --- OWNERS | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/OWNERS b/OWNERS index b1bd5a24..db880eaa 100644 --- a/OWNERS +++ b/OWNERS @@ -1,9 +1,10 @@ # See the OWNERS docs at https://go.k8s.io/owners approvers: - - wongma7 + - ashishranjan738 + - humblec + - jackielii - jsafrane - kmova - - jackielii - - ashishranjan738 - yonatankahana + - wongma7 From e53fbc3d59799e18c3e4cb974b2070b389616f9f Mon Sep 17 00:00:00 2001 From: Humble Chirammal Date: Thu, 10 Feb 2022 12:20:47 +0530 Subject: [PATCH 2/3] Handle error in a some conditions and also correct error message This commit also does: `Replace` string method changed to `ReplaceALL` Signed-off-by: Humble Chirammal --- .../provisioner.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/cmd/nfs-subdir-external-provisioner/provisioner.go b/cmd/nfs-subdir-external-provisioner/provisioner.go index 2d357c11..1db6acab 100644 --- a/cmd/nfs-subdir-external-provisioner/provisioner.go +++ b/cmd/nfs-subdir-external-provisioner/provisioner.go @@ -62,13 +62,14 @@ func (meta *pvcMetadata) stringParser(str string) string { for _, r := range result { switch r[2] { case "labels": - str = strings.Replace(str, r[0], meta.labels[r[3]], -1) + str = strings.ReplaceAll(str, r[0], meta.labels[r[3]]) case "annotations": - str = strings.Replace(str, r[0], meta.annotations[r[3]], -1) + str = strings.ReplaceAll(str, r[0], meta.annotations[r[3]]) default: - str = strings.Replace(str, r[0], meta.data[r[1]], -1) + str = strings.ReplaceAll(str, r[0], meta.data[r[1]]) } } + return str } @@ -114,7 +115,10 @@ func (p *nfsProvisioner) Provision(ctx context.Context, options controller.Provi if err := os.MkdirAll(fullPath, 0777); err != nil { return nil, controller.ProvisioningFinished, errors.New("unable to create directory to provision new pv: " + err.Error()) } - os.Chmod(fullPath, 0777) + err := os.Chmod(fullPath, 0777) + if err != nil { + return nil, "", err + } pv := &v1.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ @@ -189,11 +193,11 @@ func (p *nfsProvisioner) Delete(ctx context.Context, volume *v1.PersistentVolume // getClassForVolume returns StorageClass func (p *nfsProvisioner) getClassForVolume(ctx context.Context, pv *v1.PersistentVolume) (*storage.StorageClass, error) { if p.client == nil { - return nil, fmt.Errorf("Cannot get kube client") + return nil, fmt.Errorf("cannot get kube client") } className := helper.GetPersistentVolumeClass(pv) if className == "" { - return nil, fmt.Errorf("Volume has no storage class") + return nil, fmt.Errorf("volume has no storage class") } class, err := p.client.StorageV1().StorageClasses().Get(ctx, className, metav1.GetOptions{}) if err != nil { From 7c23a7dd7a65ab57f455183931deb4a4728cfa6e Mon Sep 17 00:00:00 2001 From: Humble Chirammal Date: Thu, 10 Feb 2022 12:27:04 +0530 Subject: [PATCH 3/3] Remove unnecessary conversion of resource storage This commit also does more corrections like error handling, formatting, typos..etc Signed-off-by: Humble Chirammal --- cmd/nfs-subdir-external-provisioner/provisioner.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/cmd/nfs-subdir-external-provisioner/provisioner.go b/cmd/nfs-subdir-external-provisioner/provisioner.go index 1db6acab..cd990e15 100644 --- a/cmd/nfs-subdir-external-provisioner/provisioner.go +++ b/cmd/nfs-subdir-external-provisioner/provisioner.go @@ -112,10 +112,10 @@ func (p *nfsProvisioner) Provision(ctx context.Context, options controller.Provi } glog.V(4).Infof("creating path %s", fullPath) - if err := os.MkdirAll(fullPath, 0777); err != nil { + if err := os.MkdirAll(fullPath, 0o777); err != nil { return nil, controller.ProvisioningFinished, errors.New("unable to create directory to provision new pv: " + err.Error()) } - err := os.Chmod(fullPath, 0777) + err := os.Chmod(fullPath, 0o777) if err != nil { return nil, "", err } @@ -159,14 +159,12 @@ func (p *nfsProvisioner) Delete(ctx context.Context, volume *v1.PersistentVolume } // Determine if the "onDelete" parameter exists. - // If it exists and has a delete value, delete the directory. - // If it exists and has a retain value, safe the directory. + // If it exists and has a `delete` value, delete the directory. + // If it exists and has a `retain` value, safe the directory. onDelete := storageClass.Parameters["onDelete"] switch onDelete { - case "delete": return os.RemoveAll(oldPath) - case "retain": return nil } @@ -190,7 +188,7 @@ func (p *nfsProvisioner) Delete(ctx context.Context, volume *v1.PersistentVolume return os.Rename(oldPath, archivePath) } -// getClassForVolume returns StorageClass +// getClassForVolume returns StorageClass. func (p *nfsProvisioner) getClassForVolume(ctx context.Context, pv *v1.PersistentVolume) (*storage.StorageClass, error) { if p.client == nil { return nil, fmt.Errorf("cannot get kube client") @@ -203,6 +201,7 @@ func (p *nfsProvisioner) getClassForVolume(ctx context.Context, pv *v1.Persisten if err != nil { return nil, err } + return class, nil }