Create new directory in NFS for each Dataset deployment about datashim

[YiannisGkoufas]

Hi @viktoriaas thanks for trying out the framework and glad that it works!
As you mentioned, in our current version it happens the following:

• You have an NFS server on IP 1.1.1.1
• You are exporting a directory /export
• You create a Dataset as:

apiVersion: com.ie.ibm.hpsys/v1alpha1
kind: Dataset
metadata:
  name: example-dataset
spec:
  local:
    type: "NFS"
    server: 1.1.1.1
    share: /export

• You create a pod mounting the resulting PVC. Anything you write on the pvc would be under /export in the NFS server

Instead, the functionality you are describing would be:

• Creating a Dataset as:

apiVersion: com.ie.ibm.hpsys/v1alpha1
kind: Dataset
metadata:
  name: example-dataset
spec:
  local:
    type: "NFS"
    server: 1.1.1.1
    share: /export
    createDirPVC: true <----- that's the flag we can add

• You create a pod mounting the resulting PVC. Anything you write on the pvc would be under /export/pvc-XX-XXX-XXX in the NFS server

Did I capture the requirement correctly?

[viktoriaas]

Hi @YiannisGkoufas ,
yes, that's exactly the feature we would appreciate greatly! I think that having option of either creating new directory or mounting exactly what is specified would benefit many people.

Thanks

[YiannisGkoufas]

Thanks @viktoriaas ! will have a look and let you know when it is done.

[viktoriaas]

And I suppose that if I want multiple PVCs under same share, I just create DataSet for each new PVC with everything same, just different .metadata.name. Is this assumption okay?

[YiannisGkoufas]

Yes, that's correct!

[YiannisGkoufas]

Hi @viktoriaas, in the branch nfs-create-dir I have added the functionality described.
You can install it by calling:

kubectl apply -f https://raw.githubusercontent.com/IBM/dataset-lifecycle-framework/nfs-create-dir/release-tools/manifests/dlf.yaml

Using this branch you can create NFS datasets like this:

apiVersion: com.ie.ibm.hpsys/v1alpha1
kind: Dataset
metadata:
  name: example-dataset
spec:
  local:
    type: "NFS"
    server: 1.1.1.1
    share: /export
    createDirPVC: true

Let me know if it works for you!

[viktoriaas]

Hi,

the functionality doesn't work.
My bad. I haven't realized that whole deployment doesn't support dynamic provisioning. Functionality works as expected, thanks for the feature!

Notes:
After applying whole deployment, I got some warnings, namely:

• Warning: apiextensions.k8s.io/v1beta1 CustomResourceDefinition is deprecated in v1.16+, unavailable in v1.22+; use apiextensions.k8s.io/v1 CustomResourceDefinition
• Warning: storage.k8s.io/v1beta1 CSIDriver is deprecated in v1.19+, unavailable in v1.22+; use storage.k8s.io/v1 CSIDriver
• Warning: admissionregistration.k8s.io/v1beta1 MutatingWebhookConfiguration is deprecated in v1.16+, unavailable in v1.22+; use admissionregistration.k8s.io/v1 MutatingWebhookConfiguration

We are running Kubernetes 1.19.3.

After creating Dataset I got:

• The Dataset "nfs-example-dataset" is invalid: spec.local.createDirPVC: Invalid value: "boolean": spec.local.createDirPVC in body must be of type string: "boolean"
  which is fixed in obvious way but can be changed in template (if you'll provde it in the example templates folder). As I think of it, the desired size of created PVC would be another nice feature. For some deployments I need more than 5GB and for others it's an overkill.

S3
Important thing concerning S3. I haven't realized it beforehand but although S3 PVC did deploy, it didn't work. Debbuging led to conclusion that we need to specify region. So I specified region, PVC is created but can't mount, stays in Pending state. Fast-forward, we discovered that even if I specify region, it just isn't propagated to goofys and therefore the PVC isn't connected right way.

I don't know why in goofys_mounter.go struct is specified correctly (incl. region)

type goofysMounter struct {
	bucket          *bucket
	endpoint        string
	region          string
	accessKeyID     string
	secretAccessKey string
	volumeID		string
	readonly 		bool
}

but later on is completely ignored, mainly in important function func (goofys *goofysMounter) Mount(source string, target string) error . I can provide you with a patch that also adds a region. Goofys default behavious is that if region isn't specified, it adds one, I think "us-west" or something similar. That's the problem because if I specify a region and it isn't propagated, I end up with a invalid endpoint-region combination and it fails.

[YiannisGkoufas]

Hi @viktoriaas thanks so much for digging into both issues!
Regarding NFS
I think the issues that you are facing could be related to this: #37
And to be honest we have tested it up to K8s 1.18
I just pushed one small fix that does a better a check whether the PVC exists already if you want to try it again.
The test I did for the new createDirPVC feature is the following:

kubectl create -f https://raw.githubusercontent.com/kubernetes-csi/csi-driver-nfs/master/deploy/example/nfs-provisioner/nfs-server.yaml

$ kubectl get svc
NAME         TYPE        CLUSTER-IP      EXTERNAL-IP   PORT(S)            AGE
kubernetes   ClusterIP   10.96.0.1       <none>        443/TCP            102s
nfs-server   ClusterIP   10.108.110.47   <none>        2049/TCP,111/UDP   17s

Used the nfs-server address and created the following Dataset:

apiVersion: com.ie.ibm.hpsys/v1alpha1
kind: Dataset
metadata:
  name: example-dataset
spec:
  local:
    type: "NFS"
    server: 10.108.110.47
    share: /
    createDirPVC: "true"

Then when I created a pod using it and writing into the dataset what is written in the host looks like this:
/nfs-vol/example-dataset-23344/newfile.txt
Just to confirm, in your case when you switched from true to "true" did it work as it was supposed to?

Regarding S3 Goofys
Thanks so much for bringing this to my attention! Forgot to populate and use the region value. Please feel free to create a pull request to fix that and I can assist you.

[viktoriaas]

Hi @YiannisGkoufas,
I don't know when you were reading my previous comment but the errors I've encountered were my fault - I haven't realized I have to create a pod to fire up the PVC. So actually I have no issues.
When I switch true to "true" it works. I would consider this feature done and ready to merge as it works as expected.

NFS PVC volume size
I've searched the code a little bit more to find where the pvc size is set up. As I've already mentioned, it would be good to have an option to specify desired size (rn just for NFS). I propse 2 changes in datasetinternal_controller.go and I hope that's the right place in code.

• in function processLocalDatasetNFS add to these commands last one

server := cr.Spec.Local["server"]
share := cr.Spec.Local["share"]
size := cr.Spec.Local["size"]

• on line 303 change fixed value to size. I've understood that this line sets the volume and the MustParse command does conversion to right quantity (according to docs)

corev1.ResourceStorage: resource.MustParse(size), //TODO: use proper size

and same on line 338

corev1.ResourceStorage: resource.MustParse(size), //TODO: use proper size

There might have to be some more checks, I don't know whether storage size can be represented as a decimal point value but for simplicity, the code can request only Integer value or automatically round it to the nearest bigger int.

S3
I'll try to create a PR next week.

[YiannisGkoufas]

Hi @viktoriaas

you are right, I haven't read the edited version of your message before replying. I think I get notifications only for new comments and not edits. Glad that it works!

NFS PVC volume size
We can definitely have this option as well, I can add to parse the size. I think though that it doesn't limit you from using datasets above 5G. I believe that the size is checked only when working with Persistent Volumes and Persistent Volume Claims. Let's say that you create a PV of 50GB, then you can only create PVCs of total size 50GB. In our case we create one PV+one PVC for the dataset so then in your pods you can mount this PVC so it doesn't make any difference if the size is 5GB or more. Let me know if I got something wrong.

S3
Sure let me know if you want help with that!

[viktoriaas]

Hey @YiannisGkoufas,
I think that since implementation creates one PV per PVC (as dynamic provisioning is not supported (yet I hope :) )) nothing stops us from specifing the size for each of them.
If I want to create PV(PVC) with just 1GB, I would write size: 1Gi, if I want 10GB, I'll write size: 10 Gi. In Kubernetes, PV and PVC bind 1-to-1 so if I have a PV of size 50GB I can't bind 10x 5GB PVC to this one PV. Considering I always create 5GB PV+PVC, I end up with quite waste of space if I know beforehand I need only 1GB.

This "feature adding" could also bring specifying access mode. Sometimes we don't want RWX but only RWO access which currently isn't supported. The fix is identical to fixing capacity.

Helm Chart
I'd like to ask whether you plan on creating a Helm chart. I was testing these drivers multiple times and wanted to have them versioned so I created one. If you were interested, I can provide.

[YiannisGkoufas]

Hi @viktoriaas

You are right about the PV<--->PVC one-to-one binding, I was confused thinking that multiple PVCs can bind to one PV.
However, as far as I am aware there is no space wasted no matter what the setting for the size is. Since in the NFS implementation we just mount a directory of an NFS server, we don't allocate space for that. Also (again correct me if I am wrong, I try to verify these as well), Kubernetes doesn't actually check if in a mounted PVC of 5GB you write data of 10GB. Finally its definitely a good idea to add the access modes, can you open another issue to track that?

Helm Chart
That definitely makes sense, I have this issue to track that: #7
Also if you check I have already started that, for instance for the csi-nfs its here: https://github.com/IBM/dataset-lifecycle-framework/tree/master/src/csi-driver-nfs/chart , is your chart similar? If you have improvements you can contribute, it would be highly appreciated! The current flow is that I generate one dlf.yaml file for all the charts in the repo, so the user can just do kubectl apply -f dlf.yaml
Will try to update the installation instructions to include helm-based installation.

[viktoriaas]

@YiannisGkoufas
I don't really know whether Kubernetes checks if mounted PVC has capacity smaller/bigger than what you write out. But it makes sense that it doesn't check (although there's a question now, why there's a capacity at all if Kubernetes doesn't care?)
I've created new issue for access mode.

Helm Chart
I'm sorry I really didn't see all that helm charts down there in src folder. On project's front page there is command for Kubernetes

kubectl apply -f https://raw.githubusercontent.com/IBM/dataset-lifecycle-framework/master/release-tools/manifests/dlf.yaml

I took the URL part and split the files into s3, nfs and general part and specified some key:value pairs in Values.yaml . It looks different than yours as it encaplsules all charts that you have separately in src/*/ folders. I could share my chart files with you somewhere and you could take a look and evaluate whether it's fine or not.

[YiannisGkoufas]

@viktoriaas I believe that the size is useful when the provisioner interacts with an external system which allocates specific size of available storage. Anyway regarding the functionality around the creation new directory in NFS would be merged in master and I really appreciate your input in this!

Helm Chart
I am sorry for not having it yet on the documentation (the helm-based installation). Will add it soon as an alternative installation method. The main helm chart is this one: https://github.com/IBM/dataset-lifecycle-framework/tree/master/chart which includes 3 subcharts located in:

• https://github.com/IBM/dataset-lifecycle-framework/tree/master/src/dataset-operator/chart
• https://github.com/IBM/dataset-lifecycle-framework/tree/master/src/csi-driver-nfs/chart
• https://github.com/IBM/dataset-lifecycle-framework/tree/master/src/csi-s3/chart

Please have a look and if you feel that there is functionality missing or there is a bug, let me know or open an issue. Thanks again!

[viktoriaas]

@YiannisGkoufas
thanks for merging, please let me know when it will be done.

If you are willing to continue with adding features, I would appreciate specifying access mode. Considering size is not importnant on NFS, I leave the decision about adding size to you.
If you want, I can create PR with fixed S3 bug + added access mode. I can do it before holidays so the PR will just wait here. Just say whether you are fine with it or want to do it yourself.

Helm Chart
The chart itsself is really good, I only do not know how it's used. It's a pity it's not available at some repo such as github pages or Google Cloud Storage. It would be convenient e.g. for automated developement when the chart is downloaded through helm provider in Puppet.

[viktoriaas]

@YiannisGkoufas

Hi!
Happy New Year!
Today I created a PR for region fix in S3 and I would be very happy if you merged it.

Also, could you advise me with some issue around S3? We have IBM Spectrum Scale and turned on S3 feature. It kind of works but there are some issues which might need fixes here, in goofys and in openstack swift client.

• goofys keeps on losing connection. When some error happens, this kind of message pops up
  root@csi-s3-vh5rl:/test2# ls ls: cannot open directory '.': Transport endpoint is not connected

To understand the error we tried mkdir some_dir in connected endpoint. This instantly breaks whole connection (which is another problem but of goofys).

Debugging with strace led to this horrible 500 error which goofys can't take and ends connection
GMT\r\nTransfer-Encoding: chunked\r\n\r\n177\r\n<?xml version='1.0' encoding='UTF-8'?>\n<Error><Code>InternalError</Code><Message>We encountered an internal error. Please try again.</Message><RequestId>tx274cb6f6673e480dbdd73-005ff883af</RequestId><Reason>Invalid object name \"fail/\", object path can end with a slash only if it is a directory marker object with \"Content-Type: application/directory\" header</Reason></Error>\r\n0\r\n\r\n

500 might be because csi-s3 container image misses the location where it wants to write the error. But I really don't know, it's just a thought. I have some other s3 endpoint and in its bucket I can do mkdir without failures.

And related to Spectrum Scale (as I see this is an IBM github so someone might know):

• big files are stored in multiple parts. This is not good if you want to use unified_mode and use both NFS and S3 access (these multiple parts can't be put back together in NFS). How can I achieve that big files
• we use unified_mode but all files are always owned by swift user. We programmed a small fix in a swift client in one function but as IBM says the access should be unified, I don't understand why everything is owned by swift

[YiannisGkoufas]

Hi @viktoriaas just got back from my holidays!

Regarding the helm chart there is a PR pending for some minor fixes, will take care of that and update the README with instructions about installing. Thanks for the suggestion for hosting it somewhere publicly, definitely makes sense!

Regarding the S3 region PR I really appreciate it! will take care of it this week as well.

Regarding the Spectrum Scale and S3 I think it needs a bit more investigation. I have only tried to access via hostpath. Maybe we should take it offline and I can include people from Spectrum Scale. Would you like to email me to organize?

[viktoriaas]

@YiannisGkoufas
Thanks very much for taking care about Helm chart and S3 PR. Both will become a huge help in future work.

As to Spectrum Scale and S3 - sure! I've written an e-mail to address [email protected] (found on your profile).

Pod Mounting
Is the Dataset mounting supposed to work? I can only attach created volume via creating PVC and mounting this into pod. Mounting using labels doesn't work, always ends with
The Pod "nginx" is invalid: spec.containers[0].volumeMounts[0].name: Not found: "cerit-s3-dataset-c1"
but can be mounted with (labels aren't neccessary)

kind: Pod
metadata:
  name: nginxcerit-s3
  labels:
    dataset.0.id: "cerit-s3-1"
    dataset.0.useas: "mount"
spec:
  containers:
    - name: nginx
      image: nginx
      volumeMounts:
        - mountPath: "/mount/dataset1" #optional, if not specified it would be mounted in /mnt/datasets/example-dataset
          name: "cerit-s3-1"
  volumes:
    - name: cerit-s3-1
      persistentVolumeClaim:
        claimName: s3-cerit-dataset-c1

Creating new bucket
I tried mounting non-existing bucket. I did 2 experiments - one with ec2 credentials bounded to some user and second try with ec2 credentials bounded to admin user.

Creating new bucket failed with standard user credentials with access denied

GRPC error: failed to create volume pvc-114e4af1-43dd-4c41-a023-8849be5a24b4: Access Denied.

However, I thought that admin credentials can create new bucket but in logs I see

I0111 15:45:37.835856       1 controllerserver.go:145] Bucket newdir does not exist, ignoring request
...
I0111 15:45:38.731743       1 controllerserver.go:67] Got a request to create volume pvc-ed3a24aa-dacb-4a27-8b46-9c3e7ec48c21
E0111 15:45:48.612994       1 utils.go:101] GRPC error: failed to create volume pvc-ed3a24aa-dacb-4a27-8b46-9c3e7ec48c21: We encountered an internal error. Please try again.

This looks like nobody can create a new bucket (it has to exist prior to mounting, won't create if it's not mounted) but I don't understand why there are different error messages. Could you explain?

[YiannisGkoufas]

Hi @viktoriaas thanks again! I am glad that the work is useful!

Regarding Pod mounting
I believe that you might be using a Kubernetes version above 1.19 so that's why it doesn't work. I posted it here #37 and recently found the solution, will merge it in the master. Basically it's just a convenient convention for users to reference datasets in their pods, you can still use Datasets as normal PVCs

Regarding Creating new bucket
There is pending PR about exactly what you describe. Have a look here: #72
We introduce a new field (provision) that creates the bucket in case it doesn't exist

[YiannisGkoufas]

Hi @viktoriaas after some checks we found out the following:

• If the bucket already exists, you can mount it without specifying region in the Dataset spec.
• If the bucket doesn't exist, then you need to specify the region.
  Can you verify the above is the same in your tests as well?

[viktoriaas]

I can't verify.

In our S3 we do not have regions at all. If the bucket exists, I can mount it without it. If I try to mount nonexistent bucket, I get:

• using ordinary ec2 credentials (no admin, just project member)
  • without region: GRPC error: failed to check if bucket nonexistent exists: 400 Bad Request
  • with region: GRPC error: failed to create volume pvc-887fe4f6-c6c1-4bd0-8139-f0cbe2e0cba5: Access Denied.
• using admin ec2 credentials
  • without region: GRPC error: failed to create volume pvc-e0e3cf6e-c4be-493e-a151-1d0353da881b: We encountered an internal error. Please try again.
  • with region: GRPC error: failed to check if bucket nonexistent exists: 400 Bad Request

Anyway, there is big chaos with permissions whatsover. Everybody can mount whatever bucket they want 🤔

[viktoriaas]

Hi @YiannisGkoufas ,
did something happen to that NFS feature of creating a directory? It suddenly stopped working, whole export is mounted even when I specify createDirPVC: "true".
I have redeployed whole dlf and also deployed kubectl create -f https://raw.githubusercontent.com/kubernetes-csi/csi-driver-nfs/master/deploy/example/nfs-provisioner/nfs-server.yaml

[YiannisGkoufas]

Hi @viktoriaas apologies we have been merging various PRs lately so we might have broken something. Will have a look today.
Also on the issue with bucket creation you mentioned above I am trying to replicate that as well and will get back to you.

[YiannisGkoufas]

I merged the pull request and updated the image, everything should be working now, let me know otherwise.
If createDirPVC works, I will close this and we can track your other issues on another one

[viktoriaas]

Thanks, works now! :)
Might create new issue regarding S3 and permissions.