From 45f0debd0004504908a4de9591df03f980cf548c Mon Sep 17 00:00:00 2001 From: Ahmad Nurus S Date: Tue, 21 Jul 2020 18:43:58 +0700 Subject: [PATCH 1/3] nfs: nfs provisioner controlled by operator Signed-off-by: Ahmad Nurus S --- cluster/examples/kubernetes/nfs/nfs.yaml | 54 ++++++ cluster/examples/kubernetes/nfs/operator.yaml | 1 + .../examples/kubernetes/nfs/provisioner.yaml | 86 --------- cluster/examples/kubernetes/nfs/sc.yaml | 2 +- pkg/operator/nfs/controller.go | 77 +++++++- pkg/operator/nfs/controller_test.go | 94 +++++++++- pkg/operator/nfs/provisioner.go | 170 +++++++++++------- pkg/operator/nfs/spec.go | 42 +++++ tests/framework/installer/nfs_manifests.go | 145 ++++++--------- tests/integration/nfs_test.go | 2 +- 10 files changed, 424 insertions(+), 249 deletions(-) delete mode 100644 cluster/examples/kubernetes/nfs/provisioner.yaml diff --git a/cluster/examples/kubernetes/nfs/nfs.yaml b/cluster/examples/kubernetes/nfs/nfs.yaml index f88702627..27eb26354 100644 --- a/cluster/examples/kubernetes/nfs/nfs.yaml +++ b/cluster/examples/kubernetes/nfs/nfs.yaml @@ -9,6 +9,60 @@ metadata: name: rook-nfs-server namespace: rook-nfs --- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: rook-nfs-provisioner + namespace: rook-nfs +--- +kind: ClusterRole +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: rook-nfs-provisioner-runner +rules: + - apiGroups: [""] + resources: ["persistentvolumes"] + verbs: ["get", "list", "watch", "create", "delete"] + - apiGroups: [""] + resources: ["persistentvolumeclaims"] + verbs: ["get", "list", "watch", "update"] + - apiGroups: ["storage.k8s.io"] + resources: ["storageclasses"] + verbs: ["get", "list", "watch"] + - apiGroups: [""] + resources: ["events"] + verbs: ["create", "update", "patch"] + - apiGroups: [""] + resources: ["services", "endpoints"] + verbs: ["get"] + - apiGroups: ["extensions"] + resources: ["podsecuritypolicies"] + resourceNames: ["nfs-provisioner"] + verbs: ["use"] + - apiGroups: [""] + resources: ["endpoints"] + verbs: ["get", "list", "watch", "create", "update", "patch"] + - apiGroups: + - nfs.rook.io + resources: + - "*" + verbs: + - "*" +--- +kind: ClusterRoleBinding +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: run-nfs-provisioner +subjects: + - kind: ServiceAccount + name: rook-nfs-provisioner + # replace with namespace where provisioner is deployed + namespace: rook-nfs +roleRef: + kind: ClusterRole + name: rook-nfs-provisioner-runner + apiGroup: rbac.authorization.k8s.io +--- # A default storageclass must be present apiVersion: v1 kind: PersistentVolumeClaim diff --git a/cluster/examples/kubernetes/nfs/operator.yaml b/cluster/examples/kubernetes/nfs/operator.yaml index 22088ef0a..d2aa0483f 100644 --- a/cluster/examples/kubernetes/nfs/operator.yaml +++ b/cluster/examples/kubernetes/nfs/operator.yaml @@ -67,6 +67,7 @@ rules: - apiGroups: - apps resources: + - deployments - statefulsets verbs: - create diff --git a/cluster/examples/kubernetes/nfs/provisioner.yaml b/cluster/examples/kubernetes/nfs/provisioner.yaml deleted file mode 100644 index 5e8bb89ee..000000000 --- a/cluster/examples/kubernetes/nfs/provisioner.yaml +++ /dev/null @@ -1,86 +0,0 @@ -kind: ClusterRole -apiVersion: rbac.authorization.k8s.io/v1 -metadata: - name: rook-nfs-provisioner-runner -rules: - - apiGroups: [""] - resources: ["persistentvolumes"] - verbs: ["get", "list", "watch", "create", "delete"] - - apiGroups: [""] - resources: ["persistentvolumeclaims"] - verbs: ["get", "list", "watch", "update"] - - apiGroups: ["storage.k8s.io"] - resources: ["storageclasses"] - verbs: ["get", "list", "watch"] - - apiGroups: [""] - resources: ["events"] - verbs: ["create", "update", "patch"] - - apiGroups: [""] - resources: ["services", "endpoints"] - verbs: ["get"] - - apiGroups: ["extensions"] - resources: ["podsecuritypolicies"] - resourceNames: ["nfs-provisioner"] - verbs: ["use"] - - apiGroups: [""] - resources: ["endpoints"] - verbs: ["get", "list", "watch", "create", "update", "patch"] - - apiGroups: - - nfs.rook.io - resources: - - "*" - verbs: - - "*" ---- -kind: ClusterRoleBinding -apiVersion: rbac.authorization.k8s.io/v1 -metadata: - name: run-nfs-provisioner -subjects: - - kind: ServiceAccount - name: rook-nfs-provisioner - # replace with namespace where provisioner is deployed - namespace: rook-nfs-system -roleRef: - kind: ClusterRole - name: rook-nfs-provisioner-runner - apiGroup: rbac.authorization.k8s.io ---- -apiVersion: v1 -kind: ServiceAccount -metadata: - name: rook-nfs-provisioner - namespace: rook-nfs-system ---- -kind: Deployment -apiVersion: apps/v1 -metadata: - name: rook-nfs-provisioner - namespace: rook-nfs-system -spec: - selector: - matchLabels: - app: rook-nfs-provisioner - replicas: 1 - strategy: - type: Recreate - template: - metadata: - labels: - app: rook-nfs-provisioner - spec: - serviceAccount: rook-nfs-provisioner - containers: - - name: rook-nfs-provisioner - image: rook/nfs:master - imagePullPolicy: IfNotPresent - args: ["nfs", "provisioner","--provisioner=rook.io/nfs-provisioner"] - env: - - name: POD_NAME - valueFrom: - fieldRef: - fieldPath: metadata.name - - name: POD_NAMESPACE - valueFrom: - fieldRef: - fieldPath: metadata.namespace diff --git a/cluster/examples/kubernetes/nfs/sc.yaml b/cluster/examples/kubernetes/nfs/sc.yaml index 2f25f3c7c..2ad62ed6b 100644 --- a/cluster/examples/kubernetes/nfs/sc.yaml +++ b/cluster/examples/kubernetes/nfs/sc.yaml @@ -8,6 +8,6 @@ parameters: exportName: share1 nfsServerName: rook-nfs nfsServerNamespace: rook-nfs -provisioner: rook.io/nfs-provisioner +provisioner: nfs.rook.io/rook-nfs-provisioner reclaimPolicy: Delete volumeBindingMode: Immediate diff --git a/pkg/operator/nfs/controller.go b/pkg/operator/nfs/controller.go index fc0c16942..30e251d9c 100644 --- a/pkg/operator/nfs/controller.go +++ b/pkg/operator/nfs/controller.go @@ -19,6 +19,7 @@ package nfs import ( "context" "fmt" + "path/filepath" "strings" "time" @@ -32,6 +33,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" @@ -122,6 +124,12 @@ func (r *NFSServerReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, reterr return reconcile.Result{}, err } + if err := r.reconcileNFSProvisioner(ctx, instance); err != nil { + r.Recorder.Eventf(instance, corev1.EventTypeWarning, nfsv1alpha1.EventFailed, "Failed reconciling nfsprovisioner: %+v", err) + r.Log.Errorf("Error reconciling nfsprovisioner: %+v", err) + return reconcile.Result{}, err + } + // Reconcile status state based on statefulset ready replicas. sts := &appsv1.StatefulSet{} if err := r.Client.Get(ctx, req.NamespacedName, sts); err != nil { @@ -158,8 +166,8 @@ func (r *NFSServerReconciler) reconcileNFSServerConfig(ctx context.Context, cr * nfsGaneshaConfig := ` EXPORT { Export_Id = ` + fmt.Sprintf("%v", id) + `; - Path = /` + claimName + `; - Pseudo = /` + claimName + `; + Path = ` + filepath.Join(mountPath, claimName) + `; + Pseudo = ` + filepath.Join(mountPath, claimName) + `; Protocols = 4; Transports = TCP; Sectype = sys; @@ -287,7 +295,7 @@ func (r *NFSServerReconciler) reconcileNFSServer(ctx context.Context, cr *nfsv1a volumeMounts = append(volumeMounts, corev1.VolumeMount{ Name: shareName, - MountPath: "/" + claimName, + MountPath: filepath.Join(mountPath, claimName), }) } @@ -313,3 +321,66 @@ func (r *NFSServerReconciler) reconcileNFSServer(ctx context.Context, cr *nfsv1a return nil } } + +func (r *NFSServerReconciler) reconcileNFSProvisioner(ctx context.Context, cr *nfsv1alpha1.NFSServer) error { + nfsserversvc := &corev1.Service{} + if err := r.Client.Get(ctx, types.NamespacedName{Namespace: cr.Namespace, Name: cr.Name}, nfsserversvc); err != nil { + return err + } + + dep := newDeploymentForNFSProvisioner(cr) + depop, err := controllerutil.CreateOrUpdate(ctx, r.Client, dep, func() error { + + if dep.ObjectMeta.CreationTimestamp.IsZero() { + dep.Spec.Selector = &metav1.LabelSelector{ + MatchLabels: newLabels(cr), + } + } + + if err := controllerutil.SetControllerReference(cr, dep, r.Scheme); err != nil { + return err + } + + var volumes []corev1.Volume + var volumeMounts []corev1.VolumeMount + for _, export := range cr.Spec.Exports { + shareName := export.Name + claimName := export.PersistentVolumeClaim.ClaimName + volumes = append(volumes, corev1.Volume{ + Name: shareName, + VolumeSource: corev1.VolumeSource{ + NFS: &corev1.NFSVolumeSource{ + Server: nfsserversvc.Spec.ClusterIP, + Path: filepath.Join(mountPath, claimName), + }, + }, + }) + + volumeMounts = append(volumeMounts, corev1.VolumeMount{ + Name: shareName, + MountPath: filepath.Join(mountPath, claimName), + }) + } + + dep.Spec.Template.Spec.Containers[0].VolumeMounts = volumeMounts + dep.Spec.Template.Spec.Volumes = volumes + + return nil + }) + + if err != nil { + return err + } + + r.Log.Info("Reconciling NFSProvisioner Deployment", "Operation.Result ", depop) + switch depop { + case controllerutil.OperationResultCreated: + r.Recorder.Eventf(cr, corev1.EventTypeNormal, nfsv1alpha1.EventCreated, "%s nfs-provisioner deployment: %s", strings.Title(string(depop)), dep.Name) + return nil + case controllerutil.OperationResultUpdated: + r.Recorder.Eventf(cr, corev1.EventTypeNormal, nfsv1alpha1.EventUpdated, "%s nfs-provisioner deployment: %s", strings.Title(string(depop)), dep.Name) + return nil + default: + return nil + } +} diff --git a/pkg/operator/nfs/controller_test.go b/pkg/operator/nfs/controller_test.go index d79946877..a962cf439 100644 --- a/pkg/operator/nfs/controller_test.go +++ b/pkg/operator/nfs/controller_test.go @@ -17,12 +17,15 @@ limitations under the License. package nfs import ( + "context" + "path/filepath" "reflect" "testing" nfsv1alpha1 "github.com/rook/rook/pkg/apis/nfs.rook.io/v1alpha1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -30,6 +33,7 @@ import ( "k8s.io/client-go/tools/record" "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -92,7 +96,7 @@ func (r *resource) Generate() *nfsv1alpha1.NFSServer { } func TestNFSServerReconciler_Reconcile(t *testing.T) { - expectedStatefulSet := func(scheme *runtime.Scheme, cr *nfsv1alpha1.NFSServer) *appsv1.StatefulSet { + expectedServerFunc := func(scheme *runtime.Scheme, cr *nfsv1alpha1.NFSServer) *appsv1.StatefulSet { sts := newStatefulSetForNFSServer(cr) sts.Spec.Selector = &metav1.LabelSelector{ MatchLabels: newLabels(cr), @@ -137,7 +141,7 @@ func TestNFSServerReconciler_Reconcile(t *testing.T) { volumeMounts = append(volumeMounts, corev1.VolumeMount{ Name: shareName, - MountPath: "/" + claimName, + MountPath: filepath.Join(mountPath, claimName), }) } sts.Status.ReadyReplicas = int32(cr.Spec.Replicas) @@ -147,6 +151,48 @@ func TestNFSServerReconciler_Reconcile(t *testing.T) { return sts } + expectedServerServiceFunc := func(scheme *runtime.Scheme, cr *nfsv1alpha1.NFSServer) *corev1.Service { + svc := newServiceForNFSServer(cr) + _ = controllerutil.SetControllerReference(cr, svc, scheme) + return svc + } + + expectedProvisionerFunc := func(scheme *runtime.Scheme, cr *nfsv1alpha1.NFSServer) *appsv1.Deployment { + dep := newDeploymentForNFSProvisioner(cr) + dep.Spec.Selector = &metav1.LabelSelector{ + MatchLabels: newLabels(cr), + } + + _ = controllerutil.SetControllerReference(cr, dep, scheme) + + var volumes []corev1.Volume + var volumeMounts []corev1.VolumeMount + for _, export := range cr.Spec.Exports { + shareName := export.Name + claimName := export.PersistentVolumeClaim.ClaimName + volumes = append(volumes, v1.Volume{ + Name: shareName, + VolumeSource: v1.VolumeSource{ + NFS: &v1.NFSVolumeSource{ + Server: expectedServerServiceFunc(scheme, cr).Spec.ClusterIP, + Path: filepath.Join(mountPath, claimName), + }, + }, + }) + + volumeMounts = append(volumeMounts, v1.VolumeMount{ + Name: shareName, + MountPath: filepath.Join(mountPath, claimName), + }) + } + + dep.Status.ReadyReplicas = int32(cr.Spec.Replicas) + dep.Spec.Template.Spec.Volumes = volumes + dep.Spec.Template.Spec.Containers[0].VolumeMounts = volumeMounts + + return dep + } + rr := reconcile.Request{ NamespacedName: types.NamespacedName{ Name: "nfs-server", @@ -208,11 +254,21 @@ func TestNFSServerReconciler_Reconcile(t *testing.T) { scheme := clientgoscheme.Scheme scheme.AddKnownTypes(nfsv1alpha1.SchemeGroupVersion, tt.cr) + expectedServer := expectedServerFunc(scheme, tt.cr) + expectedServerService := expectedServerServiceFunc(scheme, tt.cr) + expectedProvisioner := expectedProvisionerFunc(scheme, tt.cr) + objs := []runtime.Object{ tt.cr, - expectedStatefulSet(scheme, tt.cr), + expectedServer, + expectedServerService, + expectedProvisioner, } + expectedServer.GetObjectKind().SetGroupVersionKind(appsv1.SchemeGroupVersion.WithKind("StatefulSet")) + expectedServerService.GetObjectKind().SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Service")) + expectedProvisioner.GetObjectKind().SetGroupVersionKind(appsv1.SchemeGroupVersion.WithKind("Deployment")) + fc := fake.NewFakeClient(objs...) fr := record.NewFakeRecorder(2) @@ -230,6 +286,38 @@ func TestNFSServerReconciler_Reconcile(t *testing.T) { if !reflect.DeepEqual(got, tt.want) { t.Errorf("NFSServerReconciler.Reconcile() = %v, want %v", got, tt.want) } + + gotServer := &appsv1.StatefulSet{} + if err := fc.Get(context.Background(), tt.args.req.NamespacedName, gotServer); err != nil { + t.Errorf("NFSServerReconciler.Reconcile() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(gotServer, expectedServer) { + t.Errorf("NFSServerReconciler.Reconcile() = %v, want %v", gotServer, expectedServer) + } + + gotServerService := &corev1.Service{} + if err := fc.Get(context.Background(), tt.args.req.NamespacedName, gotServerService); err != nil { + t.Errorf("NFSServerReconciler.Reconcile() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(gotServerService, expectedServerService) { + t.Errorf("NFSServerReconciler.Reconcile() = %v, want %v", gotServerService, expectedServerService) + } + + gotProvisioner := &appsv1.Deployment{} + provisionerObjKey := client.ObjectKey{ + Name: tt.args.req.Name + "-provisioner", + Namespace: tt.args.req.Namespace, + } + if err := fc.Get(context.Background(), provisionerObjKey, gotProvisioner); err != nil { + t.Errorf("NFSServerReconciler.Reconcile() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if !reflect.DeepEqual(gotProvisioner, expectedProvisioner) { + t.Errorf("NFSServerReconciler.Reconcile() = %v, want %v", gotProvisioner, expectedProvisioner) + } }) } } diff --git a/pkg/operator/nfs/provisioner.go b/pkg/operator/nfs/provisioner.go index c255bc684..b3424cb72 100644 --- a/pkg/operator/nfs/provisioner.go +++ b/pkg/operator/nfs/provisioner.go @@ -18,11 +18,14 @@ package nfs import ( "fmt" + "os" + "path/filepath" + "strings" "github.com/pkg/errors" rookclient "github.com/rook/rook/pkg/client/clientset/versioned" v1 "k8s.io/api/core/v1" - storage "k8s.io/api/storage/v1" + storagev1 "k8s.io/api/storage/v1" "k8s.io/client-go/kubernetes" "k8s.io/kubernetes/pkg/apis/core/v1/helper" "sigs.k8s.io/sig-storage-lib-external-provisioner/controller" @@ -30,50 +33,74 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -type nfsProvisioner struct { +const ( + mountPath = "/export" +) + +type Provisioner struct { client kubernetes.Interface rookClient rookclient.Interface } -var _ controller.Provisioner = &nfsProvisioner{} +var _ controller.Provisioner = &Provisioner{} // NewNFSProvisioner returns an instance of nfsProvisioner -func NewNFSProvisioner(clientset kubernetes.Interface, rookClientset rookclient.Interface) *nfsProvisioner { - return &nfsProvisioner{clientset, rookClientset} +func NewNFSProvisioner(clientset kubernetes.Interface, rookClientset rookclient.Interface) *Provisioner { + return &Provisioner{clientset, rookClientset} } -func (p *nfsProvisioner) Provision(options controller.ProvisionOptions) (*v1.PersistentVolume, error) { +func (p *Provisioner) Provision(options controller.ProvisionOptions) (*v1.PersistentVolume, error) { logger.Infof("nfs provisioner: ProvisionOptions %v", options) - // Get the storage class for this volume. - storageClass, err := p.getClassForPVC(options.PVC) - if err != nil { - return nil, err - } - if options.PVC.Spec.Selector != nil { return nil, fmt.Errorf("claim Selector is not supported") } - nfsServerName, present := storageClass.Parameters["nfsServerName"] - if !present { - return nil, errors.Errorf("NFS share Path not found in the storageclass: %v", storageClass.GetName()) - } - - nfsNamespace, present := storageClass.Parameters["nfsServerNamespace"] - if !present { - return nil, errors.Errorf("NFS share Path not found in the storageclass: %v", storageClass.GetName()) - } - - exportName, present := storageClass.Parameters["exportName"] - if !present { - return nil, errors.Errorf("NFS share Path not found in the storageclass: %v", storageClass.GetName()) - } - - nfsVolumeSource, err := p.getNFSVolumeSource(nfsServerName, nfsNamespace, exportName) + sc, err := p.storageClassForPVC(options.PVC) if err != nil { return nil, err } + + serverName, present := sc.Parameters["nfsServerName"] + if !present { + return nil, errors.Errorf("NFS share Path not found in the storageclass: %v", sc.GetName()) + } + + serverNamespace, present := sc.Parameters["nfsServerNamespace"] + if !present { + return nil, errors.Errorf("NFS share Path not found in the storageclass: %v", sc.GetName()) + } + + exportName, present := sc.Parameters["exportName"] + if !present { + return nil, errors.Errorf("NFS share Path not found in the storageclass: %v", sc.GetName()) + } + + nfsserver, err := p.rookClient.NfsV1alpha1().NFSServers(serverNamespace).Get(serverName, metav1.GetOptions{}) + if err != nil { + return nil, err + } + + nfsserversvc, err := p.client.CoreV1().Services(serverNamespace).Get(serverName, metav1.GetOptions{}) + if err != nil { + return nil, err + } + + var exportPath string + for _, export := range nfsserver.Spec.Exports { + if export.Name == exportName { + exportPath = filepath.Join(mountPath, export.PersistentVolumeClaim.ClaimName) + } + } + + pvName := strings.Join([]string{options.PVC.Namespace, options.PVC.Name, options.PVName}, "-") + fullPath := filepath.Join(exportPath, pvName) + if err := os.MkdirAll(fullPath, 0777); err != nil { + return nil, errors.New("unable to create directory to provision new pv: " + err.Error()) + } + + _ = os.Chmod(fullPath, 0777) + pv := &v1.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ Name: options.PVName, @@ -86,19 +113,59 @@ func (p *nfsProvisioner) Provision(options controller.ProvisionOptions) (*v1.Per v1.ResourceName(v1.ResourceStorage): options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)], }, PersistentVolumeSource: v1.PersistentVolumeSource{ - NFS: nfsVolumeSource, + NFS: &v1.NFSVolumeSource{ + Server: nfsserversvc.Spec.ClusterIP, + Path: "/" + fullPath, + ReadOnly: false, + }, }, }, } + return pv, nil } -func (p *nfsProvisioner) Delete(volume *v1.PersistentVolume) error { - return nil +func (p *Provisioner) Delete(volume *v1.PersistentVolume) error { + path := volume.Spec.PersistentVolumeSource.NFS.Path + pvName := filepath.Base(path) + + sc, err := p.storageClassForPV(volume) + if err != nil { + return err + } + + serverName, present := sc.Parameters["nfsServerName"] + if !present { + return errors.Errorf("NFS share Path not found in the storageclass: %v", sc.GetName()) + } + + serverNamespace, present := sc.Parameters["nfsServerNamespace"] + if !present { + return errors.Errorf("NFS share Path not found in the storageclass: %v", sc.GetName()) + } + + exportName, present := sc.Parameters["exportName"] + if !present { + return errors.Errorf("NFS share Path not found in the storageclass: %v", sc.GetName()) + } + + nfsserver, err := p.rookClient.NfsV1alpha1().NFSServers(serverNamespace).Get(serverName, metav1.GetOptions{}) + if err != nil { + return err + } + + var exportPath string + for _, export := range nfsserver.Spec.Exports { + if export.Name == exportName { + exportPath = filepath.Join(mountPath, export.PersistentVolumeClaim.ClaimName) + } + } + + fullPath := filepath.Join(exportPath, pvName) + return os.RemoveAll(fullPath) } -// getClassForPV returns StorageClass -func (p *nfsProvisioner) getClassForPV(pv *v1.PersistentVolume) (*storage.StorageClass, error) { +func (p *Provisioner) storageClassForPV(pv *v1.PersistentVolume) (*storagev1.StorageClass, error) { if p.client == nil { return nil, fmt.Errorf("Cannot get kube client") } @@ -106,15 +173,11 @@ func (p *nfsProvisioner) getClassForPV(pv *v1.PersistentVolume) (*storage.Storag if className == "" { return nil, fmt.Errorf("Volume has no storage class") } - class, err := p.client.StorageV1().StorageClasses().Get(className, metav1.GetOptions{}) - if err != nil { - return nil, err - } - return class, nil + + return p.client.StorageV1().StorageClasses().Get(className, metav1.GetOptions{}) } -// getClassForPVC returns StorageClass -func (p *nfsProvisioner) getClassForPVC(pvc *v1.PersistentVolumeClaim) (*storage.StorageClass, error) { +func (p *Provisioner) storageClassForPVC(pvc *v1.PersistentVolumeClaim) (*storagev1.StorageClass, error) { if p.client == nil { return nil, fmt.Errorf("Cannot get kube client") } @@ -122,31 +185,6 @@ func (p *nfsProvisioner) getClassForPVC(pvc *v1.PersistentVolumeClaim) (*storage if className == "" { return nil, fmt.Errorf("Volume has no storage class") } - class, err := p.client.StorageV1().StorageClasses().Get(className, metav1.GetOptions{}) - if err != nil { - return nil, err - } - return class, nil -} -// getNFSVolumeSource returns the nfs source configuration for the pv -func (p *nfsProvisioner) getNFSVolumeSource(nfsServerName, namespace, exportName string) (*v1.NFSVolumeSource, error) { - nfsServer, err := p.rookClient.NfsV1alpha1().NFSServers(namespace).Get(nfsServerName, metav1.GetOptions{}) - if err != nil { - return nil, err - } - nfsService, err := p.client.CoreV1().Services(namespace).Get(nfsServerName, metav1.GetOptions{}) - if err != nil { - return nil, err - } - for _, export := range nfsServer.Spec.Exports { - if export.Name == exportName { - return &v1.NFSVolumeSource{ - ReadOnly: false, - Server: nfsService.Spec.ClusterIP, - Path: "/" + export.PersistentVolumeClaim.ClaimName, - }, nil - } - } - return nil, errors.Errorf("exportName not Found") + return p.client.StorageV1().StorageClasses().Get(className, metav1.GetOptions{}) } diff --git a/pkg/operator/nfs/spec.go b/pkg/operator/nfs/spec.go index ca2b0f0b6..d454a9bfc 100644 --- a/pkg/operator/nfs/spec.go +++ b/pkg/operator/nfs/spec.go @@ -116,3 +116,45 @@ func newStatefulSetForNFSServer(cr *nfsv1alpha1.NFSServer) *appsv1.StatefulSet { }, } } + +func newDeploymentForNFSProvisioner(cr *nfsv1alpha1.NFSServer) *appsv1.Deployment { + name := cr.Name + "-provisioner" + provisionerName := "nfs.rook.io/" + name + replicas := int32(1) + defaultTerminationGracePeriodSeconds := int64(30) + return &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: cr.Namespace, + Labels: newLabels(cr), + }, + Spec: appsv1.DeploymentSpec{ + Replicas: &replicas, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Name: cr.Name, + Namespace: cr.Namespace, + Labels: newLabels(cr), + }, + Spec: corev1.PodSpec{ + ServiceAccountName: "rook-nfs-provisioner", + Containers: []corev1.Container{ + { + Name: name, + Image: "rook/nfs:master", + ImagePullPolicy: corev1.PullIfNotPresent, + Args: []string{"nfs", "provisioner", "--provisioner=" + provisionerName}, + TerminationMessagePath: "/dev/termination-log", + TerminationMessagePolicy: corev1.TerminationMessageReadFile, + }, + }, + RestartPolicy: corev1.RestartPolicyAlways, + TerminationGracePeriodSeconds: &defaultTerminationGracePeriodSeconds, + DNSPolicy: corev1.DNSClusterFirst, + SecurityContext: &corev1.PodSecurityContext{}, + SchedulerName: corev1.DefaultSchedulerName, + }, + }, + }, + } +} diff --git a/tests/framework/installer/nfs_manifests.go b/tests/framework/installer/nfs_manifests.go index bfda108ab..0ad8e00ca 100644 --- a/tests/framework/installer/nfs_manifests.go +++ b/tests/framework/installer/nfs_manifests.go @@ -240,6 +240,7 @@ rules: - apiGroups: - apps resources: + - deployments - statefulsets verbs: - create @@ -320,93 +321,6 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace ---- -kind: ClusterRole -apiVersion: rbac.authorization.k8s.io/v1 -metadata: - name: rook-nfs-provisioner-runner -rules: - - apiGroups: [""] - resources: ["persistentvolumes"] - verbs: ["get", "list", "watch", "create", "delete"] - - apiGroups: [""] - resources: ["persistentvolumeclaims"] - verbs: ["get", "list", "watch", "update"] - - apiGroups: ["storage.k8s.io"] - resources: ["storageclasses"] - verbs: ["get", "list", "watch"] - - apiGroups: [""] - resources: ["events"] - verbs: ["create", "update", "patch"] - - apiGroups: [""] - resources: ["services", "endpoints"] - verbs: ["get"] - - apiGroups: ["extensions"] - resources: ["podsecuritypolicies"] - resourceNames: ["nfs-provisioner"] - verbs: ["use"] - - apiGroups: [""] - resources: ["endpoints"] - verbs: ["get", "list", "watch", "create", "update", "patch"] - - apiGroups: - - nfs.rook.io - resources: - - "*" - verbs: - - "*" ---- -kind: ClusterRoleBinding -apiVersion: rbac.authorization.k8s.io/v1 -metadata: - name: run-nfs-provisioner -subjects: - - kind: ServiceAccount - name: rook-nfs-provisioner - # replace with namespace where provisioner is deployed - namespace: ` + namespace + ` -roleRef: - kind: ClusterRole - name: rook-nfs-provisioner-runner - apiGroup: rbac.authorization.k8s.io ---- -apiVersion: v1 -kind: ServiceAccount -metadata: - name: rook-nfs-provisioner - namespace: ` + namespace + ` ---- -kind: Deployment -apiVersion: apps/v1 -metadata: - name: rook-nfs-provisioner - namespace: ` + namespace + ` -spec: - selector: - matchLabels: - app: rook-nfs-provisioner - replicas: 1 - strategy: - type: Recreate - template: - metadata: - labels: - app: rook-nfs-provisioner - spec: - serviceAccount: rook-nfs-provisioner - containers: - - name: rook-nfs-provisioner - image: rook/nfs:master - imagePullPolicy: IfNotPresent - args: ["nfs", "provisioner","--provisioner=rook.io/nfs-provisioner"] - env: - - name: POD_NAME - valueFrom: - fieldRef: - fieldPath: metadata.name - - name: POD_NAMESPACE - valueFrom: - fieldRef: - fieldPath: metadata.namespace ` } @@ -462,7 +376,7 @@ parameters: exportName: nfs-share nfsServerName: ` + namespace + ` nfsServerNamespace: ` + namespace + ` -provisioner: rook.io/nfs-provisioner +provisioner: nfs.rook.io/` + namespace + `-provisioner reclaimPolicy: Delete volumeBindingMode: Immediate --- @@ -476,7 +390,7 @@ parameters: exportName: nfs-share1 nfsServerName: ` + namespace + ` nfsServerNamespace: ` + namespace + ` -provisioner: rook.io/nfs-provisioner +provisioner: nfs.rook.io/` + namespace + `-provisioner reclaimPolicy: Delete volumeBindingMode: Immediate --- @@ -516,6 +430,59 @@ metadata: namespace: ` + namespace + ` --- apiVersion: v1 +kind: ServiceAccount +metadata: + name: rook-nfs-provisioner + namespace: ` + namespace + ` +--- +kind: ClusterRole +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: rook-nfs-provisioner-runner +rules: + - apiGroups: [""] + resources: ["persistentvolumes"] + verbs: ["get", "list", "watch", "create", "delete"] + - apiGroups: [""] + resources: ["persistentvolumeclaims"] + verbs: ["get", "list", "watch", "update"] + - apiGroups: ["storage.k8s.io"] + resources: ["storageclasses"] + verbs: ["get", "list", "watch"] + - apiGroups: [""] + resources: ["events"] + verbs: ["create", "update", "patch"] + - apiGroups: [""] + resources: ["services", "endpoints"] + verbs: ["get"] + - apiGroups: ["extensions"] + resources: ["podsecuritypolicies"] + resourceNames: ["nfs-provisioner"] + verbs: ["use"] + - apiGroups: [""] + resources: ["endpoints"] + verbs: ["get", "list", "watch", "create", "update", "patch"] + - apiGroups: + - nfs.rook.io + resources: + - "*" + verbs: + - "*" +--- +kind: ClusterRoleBinding +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: run-nfs-provisioner +subjects: + - kind: ServiceAccount + name: rook-nfs-provisioner + namespace: ` + namespace + ` +roleRef: + kind: ClusterRole + name: rook-nfs-provisioner-runner + apiGroup: rbac.authorization.k8s.io +--- +apiVersion: v1 kind: PersistentVolumeClaim metadata: name: test-claim diff --git a/tests/integration/nfs_test.go b/tests/integration/nfs_test.go index 51964e4e4..17d98d27d 100644 --- a/tests/integration/nfs_test.go +++ b/tests/integration/nfs_test.go @@ -68,7 +68,7 @@ func (suite *NfsSuite) TearDownSuite() { } func (suite *NfsSuite) Setup() { - suite.namespace = "nfs-ns" + suite.namespace = "rook-nfs" suite.systemNamespace = installer.SystemNamespace(suite.namespace) suite.instanceCount = 1 From 92fffca1e429d611d03ada68ca161f339683923a Mon Sep 17 00:00:00 2001 From: Ahmad Nurus S Date: Tue, 4 Aug 2020 11:40:01 +0700 Subject: [PATCH 2/3] nfs: add unit test for nfs provisioner Signed-off-by: Ahmad Nurus S --- pkg/operator/nfs/provisioner.go | 42 +++-- pkg/operator/nfs/provisioner_test.go | 221 +++++++++++++++++++++++++++ 2 files changed, 254 insertions(+), 9 deletions(-) create mode 100644 pkg/operator/nfs/provisioner_test.go diff --git a/pkg/operator/nfs/provisioner.go b/pkg/operator/nfs/provisioner.go index b3424cb72..aae64a32b 100644 --- a/pkg/operator/nfs/provisioner.go +++ b/pkg/operator/nfs/provisioner.go @@ -34,6 +34,12 @@ import ( ) const ( + nfsServerNameSCParam = "nfsServerName" + nfsServerNamespaceSCParam = "nfsServerNamespace" + exportNameSCParam = "exportName" +) + +var ( mountPath = "/export" ) @@ -61,17 +67,17 @@ func (p *Provisioner) Provision(options controller.ProvisionOptions) (*v1.Persis return nil, err } - serverName, present := sc.Parameters["nfsServerName"] + serverName, present := sc.Parameters[nfsServerNameSCParam] if !present { return nil, errors.Errorf("NFS share Path not found in the storageclass: %v", sc.GetName()) } - serverNamespace, present := sc.Parameters["nfsServerNamespace"] + serverNamespace, present := sc.Parameters[nfsServerNamespaceSCParam] if !present { return nil, errors.Errorf("NFS share Path not found in the storageclass: %v", sc.GetName()) } - exportName, present := sc.Parameters["exportName"] + exportName, present := sc.Parameters[exportNameSCParam] if !present { return nil, errors.Errorf("NFS share Path not found in the storageclass: %v", sc.GetName()) } @@ -86,13 +92,22 @@ func (p *Provisioner) Provision(options controller.ProvisionOptions) (*v1.Persis return nil, err } - var exportPath string + var ( + exportPath string + found bool + ) + for _, export := range nfsserver.Spec.Exports { if export.Name == exportName { exportPath = filepath.Join(mountPath, export.PersistentVolumeClaim.ClaimName) + found = true } } + if !found { + return nil, fmt.Errorf("No export name from storageclass is match with NFSServer %s in namespace %s", nfsserver.Name, nfsserver.Namespace) + } + pvName := strings.Join([]string{options.PVC.Namespace, options.PVC.Name, options.PVName}, "-") fullPath := filepath.Join(exportPath, pvName) if err := os.MkdirAll(fullPath, 0777); err != nil { @@ -115,7 +130,7 @@ func (p *Provisioner) Provision(options controller.ProvisionOptions) (*v1.Persis PersistentVolumeSource: v1.PersistentVolumeSource{ NFS: &v1.NFSVolumeSource{ Server: nfsserversvc.Spec.ClusterIP, - Path: "/" + fullPath, + Path: fullPath, ReadOnly: false, }, }, @@ -134,17 +149,17 @@ func (p *Provisioner) Delete(volume *v1.PersistentVolume) error { return err } - serverName, present := sc.Parameters["nfsServerName"] + serverName, present := sc.Parameters[nfsServerNameSCParam] if !present { return errors.Errorf("NFS share Path not found in the storageclass: %v", sc.GetName()) } - serverNamespace, present := sc.Parameters["nfsServerNamespace"] + serverNamespace, present := sc.Parameters[nfsServerNamespaceSCParam] if !present { return errors.Errorf("NFS share Path not found in the storageclass: %v", sc.GetName()) } - exportName, present := sc.Parameters["exportName"] + exportName, present := sc.Parameters[exportNameSCParam] if !present { return errors.Errorf("NFS share Path not found in the storageclass: %v", sc.GetName()) } @@ -154,13 +169,22 @@ func (p *Provisioner) Delete(volume *v1.PersistentVolume) error { return err } - var exportPath string + var ( + exportPath string + found bool + ) + for _, export := range nfsserver.Spec.Exports { if export.Name == exportName { exportPath = filepath.Join(mountPath, export.PersistentVolumeClaim.ClaimName) + found = true } } + if !found { + return fmt.Errorf("No export name from storageclass is match with NFSServer %s in namespace %s", nfsserver.Name, nfsserver.Namespace) + } + fullPath := filepath.Join(exportPath, pvName) return os.RemoveAll(fullPath) } diff --git a/pkg/operator/nfs/provisioner_test.go b/pkg/operator/nfs/provisioner_test.go new file mode 100644 index 000000000..f1da2e4fb --- /dev/null +++ b/pkg/operator/nfs/provisioner_test.go @@ -0,0 +1,221 @@ +/* +Copyright 2019 The Rook Authors. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package nfs + +import ( + "os" + "reflect" + "testing" + + rookclient "github.com/rook/rook/pkg/client/clientset/versioned" + rookclientfake "github.com/rook/rook/pkg/client/clientset/versioned/fake" + corev1 "k8s.io/api/core/v1" + storagev1 "k8s.io/api/storage/v1" + apiresource "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes" + k8sclientfake "k8s.io/client-go/kubernetes/fake" + "sigs.k8s.io/sig-storage-lib-external-provisioner/controller" +) + +func init() { + mountPath = "/tmp/test-rook-nfs" +} + +func newDummyStorageClass(name string, nfsServerNamespacedName types.NamespacedName, reclaimPolicy corev1.PersistentVolumeReclaimPolicy) *storagev1.StorageClass { + return &storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Parameters: map[string]string{ + nfsServerNameSCParam: nfsServerNamespacedName.Name, + nfsServerNamespaceSCParam: nfsServerNamespacedName.Namespace, + exportNameSCParam: name, + }, + ReclaimPolicy: &reclaimPolicy, + } +} + +func newDummyPVC(name, namespace string, capacity apiresource.Quantity, storageClassName string) *corev1.PersistentVolumeClaim { + return &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceName(corev1.ResourceStorage): capacity, + }, + }, + StorageClassName: &storageClassName, + }, + } +} + +func newDummyPV(name, scName, expectedPath string, expectedCapacity apiresource.Quantity, expectedReclaimPolicy corev1.PersistentVolumeReclaimPolicy) *corev1.PersistentVolume { + return &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeReclaimPolicy: expectedReclaimPolicy, + Capacity: corev1.ResourceList{ + corev1.ResourceName(corev1.ResourceStorage): expectedCapacity, + }, + PersistentVolumeSource: corev1.PersistentVolumeSource{ + NFS: &corev1.NFSVolumeSource{ + Path: expectedPath, + }, + }, + StorageClassName: scName, + }, + } +} + +func TestProvisioner_Provision(t *testing.T) { + if err := os.MkdirAll(mountPath, 0755); err != nil { + t.Error("error creating test provisioner directory") + } + + defer os.RemoveAll(mountPath) + + nfsserver := newCustomResource(types.NamespacedName{Name: "test-nfsserver", Namespace: "test-nfsserver"}).WithExports("share-1", "ReadWrite", "none", "test-claim").Generate() + + type fields struct { + client kubernetes.Interface + rookClient rookclient.Interface + } + type args struct { + options controller.ProvisionOptions + } + tests := []struct { + name string + fields fields + args args + want *corev1.PersistentVolume + wantErr bool + }{ + { + name: "success create volume", + fields: fields{ + client: k8sclientfake.NewSimpleClientset( + newServiceForNFSServer(nfsserver), + newDummyStorageClass("share-1", types.NamespacedName{Name: nfsserver.Name, Namespace: nfsserver.Namespace}, corev1.PersistentVolumeReclaimDelete), + ), + rookClient: rookclientfake.NewSimpleClientset( + nfsserver, + ), + }, + args: args{ + options: controller.ProvisionOptions{ + StorageClass: newDummyStorageClass("share-1", types.NamespacedName{Name: nfsserver.Name, Namespace: nfsserver.Namespace}, corev1.PersistentVolumeReclaimDelete), + PVName: "share-1-pvc", + PVC: newDummyPVC("share-1-pvc", "default", apiresource.MustParse("1Mi"), "share-1"), + }, + }, + want: newDummyPV("share-1-pvc", "", "/tmp/test-rook-nfs/test-claim/default-share-1-pvc-share-1-pvc", apiresource.MustParse("1Mi"), corev1.PersistentVolumeReclaimDelete), + }, + { + name: "no matching export", + fields: fields{ + client: k8sclientfake.NewSimpleClientset( + newServiceForNFSServer(nfsserver), + newDummyStorageClass("foo", types.NamespacedName{Name: nfsserver.Name, Namespace: nfsserver.Namespace}, corev1.PersistentVolumeReclaimDelete), + ), + rookClient: rookclientfake.NewSimpleClientset( + nfsserver, + ), + }, + args: args{ + options: controller.ProvisionOptions{ + StorageClass: newDummyStorageClass("foo", types.NamespacedName{Name: nfsserver.Name, Namespace: nfsserver.Namespace}, corev1.PersistentVolumeReclaimDelete), + PVName: "share-1-pvc", + PVC: newDummyPVC("share-1-pvc", "default", apiresource.MustParse("1Mi"), "foo"), + }, + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := &Provisioner{ + client: tt.fields.client, + rookClient: tt.fields.rookClient, + } + got, err := p.Provision(tt.args.options) + if (err != nil) != tt.wantErr { + t.Errorf("Provisioner.Provision() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("Provisioner.Provision() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestProvisioner_Delete(t *testing.T) { + if err := os.MkdirAll(mountPath, 0755); err != nil { + t.Error("error creating test provisioner directory") + } + + defer os.RemoveAll(mountPath) + + nfsserver := newCustomResource(types.NamespacedName{Name: "test-nfsserver", Namespace: "test-nfsserver"}).WithExports("share-1", "ReadWrite", "none", "test-claim").Generate() + type fields struct { + client kubernetes.Interface + rookClient rookclient.Interface + } + type args struct { + volume *corev1.PersistentVolume + } + tests := []struct { + name string + fields fields + args args + wantErr bool + }{ + { + name: "success delete volume", + fields: fields{ + client: k8sclientfake.NewSimpleClientset( + newServiceForNFSServer(nfsserver), + newDummyStorageClass("share-1", types.NamespacedName{Name: nfsserver.Name, Namespace: nfsserver.Namespace}, corev1.PersistentVolumeReclaimDelete), + ), + rookClient: rookclientfake.NewSimpleClientset( + nfsserver, + ), + }, + args: args{ + volume: newDummyPV("share-1-pvc", "share-1", "/tmp/test-rook-nfs/test-claim/default-share-1-pvc-share-1-pvc", apiresource.MustParse("1Mi"), corev1.PersistentVolumeReclaimDelete), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := &Provisioner{ + client: tt.fields.client, + rookClient: tt.fields.rookClient, + } + if err := p.Delete(tt.args.volume); (err != nil) != tt.wantErr { + t.Errorf("Provisioner.Delete() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} From 5cc2ceda2da5642a965c82d8344adaec8f3d0979 Mon Sep 17 00:00:00 2001 From: Ahmad Nurus S Date: Sat, 15 Aug 2020 22:39:45 +0700 Subject: [PATCH 3/3] nfs: make provisioner as sidecar of nfs server Signed-off-by: Ahmad Nurus S --- cluster/examples/kubernetes/nfs/nfs.yaml | 10 +-- cluster/examples/kubernetes/nfs/operator.yaml | 1 - pkg/operator/nfs/controller.go | 84 ++----------------- pkg/operator/nfs/controller_test.go | 65 ++------------ pkg/operator/nfs/provisioner.go | 20 ++--- pkg/operator/nfs/spec.go | 50 ++--------- tests/framework/installer/nfs_manifests.go | 9 +- 7 files changed, 37 insertions(+), 202 deletions(-) diff --git a/cluster/examples/kubernetes/nfs/nfs.yaml b/cluster/examples/kubernetes/nfs/nfs.yaml index 27eb26354..e6648dc06 100644 --- a/cluster/examples/kubernetes/nfs/nfs.yaml +++ b/cluster/examples/kubernetes/nfs/nfs.yaml @@ -9,12 +9,6 @@ metadata: name: rook-nfs-server namespace: rook-nfs --- -apiVersion: v1 -kind: ServiceAccount -metadata: - name: rook-nfs-provisioner - namespace: rook-nfs ---- kind: ClusterRole apiVersion: rbac.authorization.k8s.io/v1 metadata: @@ -52,10 +46,10 @@ rules: kind: ClusterRoleBinding apiVersion: rbac.authorization.k8s.io/v1 metadata: - name: run-nfs-provisioner + name: rook-nfs-provisioner-runner subjects: - kind: ServiceAccount - name: rook-nfs-provisioner + name: rook-nfs-server # replace with namespace where provisioner is deployed namespace: rook-nfs roleRef: diff --git a/cluster/examples/kubernetes/nfs/operator.yaml b/cluster/examples/kubernetes/nfs/operator.yaml index d2aa0483f..22088ef0a 100644 --- a/cluster/examples/kubernetes/nfs/operator.yaml +++ b/cluster/examples/kubernetes/nfs/operator.yaml @@ -67,7 +67,6 @@ rules: - apiGroups: - apps resources: - - deployments - statefulsets verbs: - create diff --git a/pkg/operator/nfs/controller.go b/pkg/operator/nfs/controller.go index 30e251d9c..a957be0f6 100644 --- a/pkg/operator/nfs/controller.go +++ b/pkg/operator/nfs/controller.go @@ -19,7 +19,7 @@ package nfs import ( "context" "fmt" - "path/filepath" + "path" "strings" "time" @@ -33,7 +33,6 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" @@ -124,12 +123,6 @@ func (r *NFSServerReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, reterr return reconcile.Result{}, err } - if err := r.reconcileNFSProvisioner(ctx, instance); err != nil { - r.Recorder.Eventf(instance, corev1.EventTypeWarning, nfsv1alpha1.EventFailed, "Failed reconciling nfsprovisioner: %+v", err) - r.Log.Errorf("Error reconciling nfsprovisioner: %+v", err) - return reconcile.Result{}, err - } - // Reconcile status state based on statefulset ready replicas. sts := &appsv1.StatefulSet{} if err := r.Client.Get(ctx, req.NamespacedName, sts); err != nil { @@ -166,8 +159,8 @@ func (r *NFSServerReconciler) reconcileNFSServerConfig(ctx context.Context, cr * nfsGaneshaConfig := ` EXPORT { Export_Id = ` + fmt.Sprintf("%v", id) + `; - Path = ` + filepath.Join(mountPath, claimName) + `; - Pseudo = ` + filepath.Join(mountPath, claimName) + `; + Path = ` + path.Join("/", claimName) + `; + Pseudo = ` + path.Join("/", claimName) + `; Protocols = 4; Transports = TCP; Sectype = sys; @@ -295,12 +288,16 @@ func (r *NFSServerReconciler) reconcileNFSServer(ctx context.Context, cr *nfsv1a volumeMounts = append(volumeMounts, corev1.VolumeMount{ Name: shareName, - MountPath: filepath.Join(mountPath, claimName), + MountPath: path.Join("/", claimName), }) } sts.Spec.Template.Spec.Volumes = volumes - sts.Spec.Template.Spec.Containers[0].VolumeMounts = volumeMounts + for i, container := range sts.Spec.Template.Spec.Containers { + if container.Name == "nfs-server" || container.Name == "nfs-provisioner" { + sts.Spec.Template.Spec.Containers[i].VolumeMounts = volumeMounts + } + } return nil }) @@ -321,66 +318,3 @@ func (r *NFSServerReconciler) reconcileNFSServer(ctx context.Context, cr *nfsv1a return nil } } - -func (r *NFSServerReconciler) reconcileNFSProvisioner(ctx context.Context, cr *nfsv1alpha1.NFSServer) error { - nfsserversvc := &corev1.Service{} - if err := r.Client.Get(ctx, types.NamespacedName{Namespace: cr.Namespace, Name: cr.Name}, nfsserversvc); err != nil { - return err - } - - dep := newDeploymentForNFSProvisioner(cr) - depop, err := controllerutil.CreateOrUpdate(ctx, r.Client, dep, func() error { - - if dep.ObjectMeta.CreationTimestamp.IsZero() { - dep.Spec.Selector = &metav1.LabelSelector{ - MatchLabels: newLabels(cr), - } - } - - if err := controllerutil.SetControllerReference(cr, dep, r.Scheme); err != nil { - return err - } - - var volumes []corev1.Volume - var volumeMounts []corev1.VolumeMount - for _, export := range cr.Spec.Exports { - shareName := export.Name - claimName := export.PersistentVolumeClaim.ClaimName - volumes = append(volumes, corev1.Volume{ - Name: shareName, - VolumeSource: corev1.VolumeSource{ - NFS: &corev1.NFSVolumeSource{ - Server: nfsserversvc.Spec.ClusterIP, - Path: filepath.Join(mountPath, claimName), - }, - }, - }) - - volumeMounts = append(volumeMounts, corev1.VolumeMount{ - Name: shareName, - MountPath: filepath.Join(mountPath, claimName), - }) - } - - dep.Spec.Template.Spec.Containers[0].VolumeMounts = volumeMounts - dep.Spec.Template.Spec.Volumes = volumes - - return nil - }) - - if err != nil { - return err - } - - r.Log.Info("Reconciling NFSProvisioner Deployment", "Operation.Result ", depop) - switch depop { - case controllerutil.OperationResultCreated: - r.Recorder.Eventf(cr, corev1.EventTypeNormal, nfsv1alpha1.EventCreated, "%s nfs-provisioner deployment: %s", strings.Title(string(depop)), dep.Name) - return nil - case controllerutil.OperationResultUpdated: - r.Recorder.Eventf(cr, corev1.EventTypeNormal, nfsv1alpha1.EventUpdated, "%s nfs-provisioner deployment: %s", strings.Title(string(depop)), dep.Name) - return nil - default: - return nil - } -} diff --git a/pkg/operator/nfs/controller_test.go b/pkg/operator/nfs/controller_test.go index a962cf439..7030c6383 100644 --- a/pkg/operator/nfs/controller_test.go +++ b/pkg/operator/nfs/controller_test.go @@ -18,14 +18,13 @@ package nfs import ( "context" - "path/filepath" + "path" "reflect" "testing" nfsv1alpha1 "github.com/rook/rook/pkg/apis/nfs.rook.io/v1alpha1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -33,7 +32,6 @@ import ( "k8s.io/client-go/tools/record" "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -141,12 +139,16 @@ func TestNFSServerReconciler_Reconcile(t *testing.T) { volumeMounts = append(volumeMounts, corev1.VolumeMount{ Name: shareName, - MountPath: filepath.Join(mountPath, claimName), + MountPath: path.Join("/", claimName), }) } sts.Status.ReadyReplicas = int32(cr.Spec.Replicas) sts.Spec.Template.Spec.Volumes = volumes - sts.Spec.Template.Spec.Containers[0].VolumeMounts = volumeMounts + for i, container := range sts.Spec.Template.Spec.Containers { + if container.Name == "nfs-server" || container.Name == "nfs-provisioner" { + sts.Spec.Template.Spec.Containers[i].VolumeMounts = volumeMounts + } + } return sts } @@ -157,42 +159,6 @@ func TestNFSServerReconciler_Reconcile(t *testing.T) { return svc } - expectedProvisionerFunc := func(scheme *runtime.Scheme, cr *nfsv1alpha1.NFSServer) *appsv1.Deployment { - dep := newDeploymentForNFSProvisioner(cr) - dep.Spec.Selector = &metav1.LabelSelector{ - MatchLabels: newLabels(cr), - } - - _ = controllerutil.SetControllerReference(cr, dep, scheme) - - var volumes []corev1.Volume - var volumeMounts []corev1.VolumeMount - for _, export := range cr.Spec.Exports { - shareName := export.Name - claimName := export.PersistentVolumeClaim.ClaimName - volumes = append(volumes, v1.Volume{ - Name: shareName, - VolumeSource: v1.VolumeSource{ - NFS: &v1.NFSVolumeSource{ - Server: expectedServerServiceFunc(scheme, cr).Spec.ClusterIP, - Path: filepath.Join(mountPath, claimName), - }, - }, - }) - - volumeMounts = append(volumeMounts, v1.VolumeMount{ - Name: shareName, - MountPath: filepath.Join(mountPath, claimName), - }) - } - - dep.Status.ReadyReplicas = int32(cr.Spec.Replicas) - dep.Spec.Template.Spec.Volumes = volumes - dep.Spec.Template.Spec.Containers[0].VolumeMounts = volumeMounts - - return dep - } - rr := reconcile.Request{ NamespacedName: types.NamespacedName{ Name: "nfs-server", @@ -256,18 +222,15 @@ func TestNFSServerReconciler_Reconcile(t *testing.T) { expectedServer := expectedServerFunc(scheme, tt.cr) expectedServerService := expectedServerServiceFunc(scheme, tt.cr) - expectedProvisioner := expectedProvisionerFunc(scheme, tt.cr) objs := []runtime.Object{ tt.cr, expectedServer, expectedServerService, - expectedProvisioner, } expectedServer.GetObjectKind().SetGroupVersionKind(appsv1.SchemeGroupVersion.WithKind("StatefulSet")) expectedServerService.GetObjectKind().SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Service")) - expectedProvisioner.GetObjectKind().SetGroupVersionKind(appsv1.SchemeGroupVersion.WithKind("Deployment")) fc := fake.NewFakeClient(objs...) fr := record.NewFakeRecorder(2) @@ -304,20 +267,6 @@ func TestNFSServerReconciler_Reconcile(t *testing.T) { if !reflect.DeepEqual(gotServerService, expectedServerService) { t.Errorf("NFSServerReconciler.Reconcile() = %v, want %v", gotServerService, expectedServerService) } - - gotProvisioner := &appsv1.Deployment{} - provisionerObjKey := client.ObjectKey{ - Name: tt.args.req.Name + "-provisioner", - Namespace: tt.args.req.Namespace, - } - if err := fc.Get(context.Background(), provisionerObjKey, gotProvisioner); err != nil { - t.Errorf("NFSServerReconciler.Reconcile() error = %v, wantErr %v", err, tt.wantErr) - return - } - - if !reflect.DeepEqual(gotProvisioner, expectedProvisioner) { - t.Errorf("NFSServerReconciler.Reconcile() = %v, want %v", gotProvisioner, expectedProvisioner) - } }) } } diff --git a/pkg/operator/nfs/provisioner.go b/pkg/operator/nfs/provisioner.go index aae64a32b..598470b33 100644 --- a/pkg/operator/nfs/provisioner.go +++ b/pkg/operator/nfs/provisioner.go @@ -19,7 +19,7 @@ package nfs import ( "fmt" "os" - "path/filepath" + "path" "strings" "github.com/pkg/errors" @@ -40,7 +40,7 @@ const ( ) var ( - mountPath = "/export" + mountPath = "/" ) type Provisioner struct { @@ -99,7 +99,7 @@ func (p *Provisioner) Provision(options controller.ProvisionOptions) (*v1.Persis for _, export := range nfsserver.Spec.Exports { if export.Name == exportName { - exportPath = filepath.Join(mountPath, export.PersistentVolumeClaim.ClaimName) + exportPath = path.Join(mountPath, export.PersistentVolumeClaim.ClaimName) found = true } } @@ -109,13 +109,11 @@ func (p *Provisioner) Provision(options controller.ProvisionOptions) (*v1.Persis } pvName := strings.Join([]string{options.PVC.Namespace, options.PVC.Name, options.PVName}, "-") - fullPath := filepath.Join(exportPath, pvName) - if err := os.MkdirAll(fullPath, 0777); err != nil { + fullPath := path.Join(exportPath, pvName) + if err := os.MkdirAll(fullPath, 0700); err != nil { return nil, errors.New("unable to create directory to provision new pv: " + err.Error()) } - _ = os.Chmod(fullPath, 0777) - pv := &v1.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ Name: options.PVName, @@ -141,8 +139,8 @@ func (p *Provisioner) Provision(options controller.ProvisionOptions) (*v1.Persis } func (p *Provisioner) Delete(volume *v1.PersistentVolume) error { - path := volume.Spec.PersistentVolumeSource.NFS.Path - pvName := filepath.Base(path) + nfsPath := volume.Spec.PersistentVolumeSource.NFS.Path + pvName := path.Base(nfsPath) sc, err := p.storageClassForPV(volume) if err != nil { @@ -176,7 +174,7 @@ func (p *Provisioner) Delete(volume *v1.PersistentVolume) error { for _, export := range nfsserver.Spec.Exports { if export.Name == exportName { - exportPath = filepath.Join(mountPath, export.PersistentVolumeClaim.ClaimName) + exportPath = path.Join(mountPath, export.PersistentVolumeClaim.ClaimName) found = true } } @@ -185,7 +183,7 @@ func (p *Provisioner) Delete(volume *v1.PersistentVolume) error { return fmt.Errorf("No export name from storageclass is match with NFSServer %s in namespace %s", nfsserver.Name, nfsserver.Namespace) } - fullPath := filepath.Join(exportPath, pvName) + fullPath := path.Join(exportPath, pvName) return os.RemoveAll(fullPath) } diff --git a/pkg/operator/nfs/spec.go b/pkg/operator/nfs/spec.go index d454a9bfc..3a3ccd904 100644 --- a/pkg/operator/nfs/spec.go +++ b/pkg/operator/nfs/spec.go @@ -67,6 +67,7 @@ func newServiceForNFSServer(cr *nfsv1alpha1.NFSServer) *corev1.Service { } func newStatefulSetForNFSServer(cr *nfsv1alpha1.NFSServer) *appsv1.StatefulSet { + privilaged := true replicas := int32(cr.Spec.Replicas) return &appsv1.StatefulSet{ ObjectMeta: metav1.ObjectMeta{ @@ -87,10 +88,9 @@ func newStatefulSetForNFSServer(cr *nfsv1alpha1.NFSServer) *appsv1.StatefulSet { ServiceAccountName: "rook-nfs-server", Containers: []corev1.Container{ { - ImagePullPolicy: "IfNotPresent", - Name: cr.Name, - Image: "rook/nfs:master", - Args: []string{"nfs", "server", "--ganeshaConfigPath=" + nfsConfigMapPath + "/" + cr.Name}, + Name: "nfs-server", + Image: "rook/nfs:master", + Args: []string{"nfs", "server", "--ganeshaConfigPath=" + nfsConfigMapPath + "/" + cr.Name}, Ports: []corev1.ContainerPort{ { Name: "nfs-port", @@ -110,49 +110,17 @@ func newStatefulSetForNFSServer(cr *nfsv1alpha1.NFSServer) *appsv1.StatefulSet { }, }, }, - }, - }, - }, - }, - } -} - -func newDeploymentForNFSProvisioner(cr *nfsv1alpha1.NFSServer) *appsv1.Deployment { - name := cr.Name + "-provisioner" - provisionerName := "nfs.rook.io/" + name - replicas := int32(1) - defaultTerminationGracePeriodSeconds := int64(30) - return &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: cr.Namespace, - Labels: newLabels(cr), - }, - Spec: appsv1.DeploymentSpec{ - Replicas: &replicas, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Name: cr.Name, - Namespace: cr.Namespace, - Labels: newLabels(cr), - }, - Spec: corev1.PodSpec{ - ServiceAccountName: "rook-nfs-provisioner", - Containers: []corev1.Container{ { - Name: name, + Name: "nfs-provisioner", Image: "rook/nfs:master", - ImagePullPolicy: corev1.PullIfNotPresent, - Args: []string{"nfs", "provisioner", "--provisioner=" + provisionerName}, + Args: []string{"nfs", "provisioner", "--provisioner=" + "nfs.rook.io/" + cr.Name + "-provisioner"}, TerminationMessagePath: "/dev/termination-log", TerminationMessagePolicy: corev1.TerminationMessageReadFile, + SecurityContext: &corev1.SecurityContext{ + Privileged: &privilaged, + }, }, }, - RestartPolicy: corev1.RestartPolicyAlways, - TerminationGracePeriodSeconds: &defaultTerminationGracePeriodSeconds, - DNSPolicy: corev1.DNSClusterFirst, - SecurityContext: &corev1.PodSecurityContext{}, - SchedulerName: corev1.DefaultSchedulerName, }, }, }, diff --git a/tests/framework/installer/nfs_manifests.go b/tests/framework/installer/nfs_manifests.go index 0ad8e00ca..daf81cb61 100644 --- a/tests/framework/installer/nfs_manifests.go +++ b/tests/framework/installer/nfs_manifests.go @@ -240,7 +240,6 @@ rules: - apiGroups: - apps resources: - - deployments - statefulsets verbs: - create @@ -429,12 +428,6 @@ metadata: name: rook-nfs-server namespace: ` + namespace + ` --- -apiVersion: v1 -kind: ServiceAccount -metadata: - name: rook-nfs-provisioner - namespace: ` + namespace + ` ---- kind: ClusterRole apiVersion: rbac.authorization.k8s.io/v1 metadata: @@ -475,7 +468,7 @@ metadata: name: run-nfs-provisioner subjects: - kind: ServiceAccount - name: rook-nfs-provisioner + name: rook-nfs-server namespace: ` + namespace + ` roleRef: kind: ClusterRole