Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Design document for adding NFS as storage backend #1740

Merged
merged 1 commit into from
Jun 5, 2018

Conversation

rohan47
Copy link
Member

@rohan47 rohan47 commented May 24, 2018

Signed-off-by: rohan47 rohanrgupta1996@gmail.com

Design documentation for the feature request #1551

[skip ci]

design/nfs.md Outdated

## Overview

Rook is an open source orchestrator for distributed storage systems running in kubernetes, currently in alpha state and has focused initially on orchestrating Ceph on top of Kubernetes. This document explores a design to add NFS to Rook. This is a part of the rook feature request [#1551](https://github.com/rook/rook/issues/1502).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your link points to #1502 instead of #1551

design/nfs.md Outdated

## Design

Rook brings storage platforms to Kubernetes with a fully Kubernetes-native design. It integrates deeply into cloud native environments leveraging extension points and providing a seamless experience for scheduling, lifecycle management, resource management, security, monitoring, and user experience.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems you have already mentioned the overview in the Rook architecture section, so maybe you don't need this line here?

design/nfs.md Outdated
3. Rook is notified of the new CRD
- Rook starts the NFS daemon on the desired nodes in the cluster
- Rook creates a storage class
4. The pod which requires NFS storage creates a PVC to consume the volume.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bullet is referring to the client consuming the storage? I would suggest either a different title for this section that indicates an end-to-end flow, or else remove this bullet and keep this as just the flow for creating the NFS share

design/nfs.md Outdated

### Deployed service

When the NFS CRD instance is created, Rook responds to this request by starting the NFS daemon with the configuration and exports stated in the CRD and creates a service to expose NFS.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this section be merged with step 3 in the previous section? There is some overlap.

design/nfs.md Outdated
namespace: rook
spec:
replicas: 1
serverConfig:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could call this just server. All of these are implied "config" settings

design/nfs.md Outdated
| `volumes` | Attach a volume to NFS server Pod for sharing | <none> |
| `volumes.name` | Name of the volume that will be attached to NFS server pod. | <none> |
| `volumes.persistentVolumeClaim` | Claim to get volume(Volume could come from hostPath, cephFS, cephRBD, googlePD, EBS etc.). | <none> |
| `volumes.claimName` | Name of the PVC. | <none> |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be under volumes.persistentVolumeClaim.claimName?

design/nfs.md Outdated
| `replicas` | The no. of NFS daemon to start | `1` |
| `serverConfig` | NFS server configuration parameters | <none> |
| `serverConfig.validHosts` | The host or network to which export is being shared. | Current pod network |
| `serverConfig.accessMode` | Reading and Writing permissions on the share. | `RO` |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What values are possible? Would be nice for the CRD to accept readable values such as ReadOnly or ReadWrite.

Copy link
Contributor

@ajarr ajarr May 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Squash and access modes can be a per export or a per client option. You'll need to allow such tunables as well. So A client IP can have RW and No_Squash access to an export, while an another IP can have RO and Root_Squash access to the same export.

See,
https://github.com/nfs-ganesha/nfs-ganesha/blob/next/src/config_samples/export.txt

design/nfs.md Outdated
| `serverConfig.accessMode` | Reading and Writing permissions on the share. | `RO` |
| `serverConfig.rootSquash` | This prevents root users connected remotely from having root privileges. | `true` |
| `serverConfig.sync` | The NFS server will not reply to requests before changes made by previous requests are written to disk. | `true` |
| `volumes` | Attach a volume to NFS server Pod for sharing | <none> |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could point out that this is a list of volumes to expose

design/nfs.md Outdated
| Parameter | Description | Default |
|---------------------------------|--------------------------------------------------------------------------|----------------------|
| `replicas` | The no. of NFS daemon to start | `1` |
| `serverConfig` | NFS server configuration parameters | <none> |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If multiple volumes are defined in the CRD, they will all have these same server config options, right?

design/nfs.md Outdated

### Examples:

1. For sharing a volume(could be hostPath, cephFS, cephRBD, googlePD, EBS etc.) using NFS, which will be shared as /myshare by the NFS server whose Nfs-Ganesha export entry looks like:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example just uses EBS. I'd suggest you mention all the other types (hostPath, cephFS, cephRBD, etc) as part of the table above, perhaps under the claimName property.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claimName is to provide name for PCV from where to attach volume to NFS for sharing, so it can be any name, will it be good to mention type of volume being attached to NFS server pod eg., volumes.persistentVolumeClaim.volumeType ? But i don't think this option will be useful.

design/nfs.md Outdated
| `serverConfig` | NFS server configuration parameters | <none> |
| `serverConfig.validHosts` | The host or network to which export is being shared. | Current pod network |
| `serverConfig.accessMode` | Reading and Writing permissions on the share. | `RO` |
| `serverConfig.rootSquash` | This prevents root users connected remotely from having root privileges. | `true` |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to rename this as 'squash' option that is not boolean. There are more squash options available than Root_Squash, and No_Root_Squash. See,
nfs-ganesha/nfs-ganesha@9aa84e6e03#diff-c5e45dc21935650ee04682077e967e39R32

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the CRD example

design/nfs.md Outdated
| `replicas` | The no. of NFS daemon to start | `1` |
| `serverConfig` | NFS server configuration parameters | <none> |
| `serverConfig.validHosts` | The host or network to which export is being shared. | Current pod network |
| `serverConfig.accessMode` | Reading and Writing permissions on the share. | `RO` |
Copy link
Contributor

@ajarr ajarr May 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Squash and access modes can be a per export or a per client option. You'll need to allow such tunables as well. So A client IP can have RW and No_Squash access to an export, while an another IP can have RO and Root_Squash access to the same export.

See,
https://github.com/nfs-ganesha/nfs-ganesha/blob/next/src/config_samples/export.txt

design/nfs.md Outdated

When the NFS CRD instance is created, Rook responds to this request by starting the NFS daemon with the configuration and exports stated in the CRD and creates a service to expose NFS.

### NFS CRD
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would one add, remove or update an export of a NFS ganesha server using the CRD? would it be a `kubectl edit/replace nfs-ganesha-1.yaml' ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it will be kubectl edit/replace nfs-ganesha-1.yaml, I have also mentioned this in the updated design.

@rohan47 rohan47 force-pushed the nfs-design-doc branch 2 times, most recently from cedd36b to ac40dc0 Compare May 30, 2018 01:05
design/nfs.md Outdated
1. NFS server storage backend configuration. E.g., configuration for various storage backends(ceph, ebs, azure disk etc) that will be shared using NFS.
2. NFS server configuration
- export (The volume being exported)
- client (The host or network to which the export is being shared)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a dynamic environment like kubernetes, specifying the client here doesn't seem to fit the expected pattern. We don't know what host or IP address the client is going to have. If the client isn't specified, will all clients be allowed? Then we could allow any pod to create a PVC that has access to the storage class.

design/nfs.md Outdated
3. When the NFS CRD instance is created, Rook responds to this request by starting the NFS daemon with the configuration and exports stated in the CRD and creates a service to expose NFS.
- Rook starts the NFS daemon on the desired nodes in the cluster
- NFS service is created
- Rook creates a storage class
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it would be convenient to generate the storage class, the admin may want to have more control about the storage classes. We might want to start with examples for how the admin can create the storage classes and then automate it later after we understand better if the flow really requires that simplification. It's a security tradeoff for simplification. Note that the operator actually doesn't currently have RBAC privileges to create storage classes, so we would have to add the create access.

@rohan47 rohan47 force-pushed the nfs-design-doc branch 3 times, most recently from f9fb46c to ab3ba38 Compare June 1, 2018 00:57
design/nfs.md Outdated

The parameters to configure NFS CRD are:

| Parameter | Description | Default |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this table I think it would be helpful to show an example yaml that uses all the properties. Then the reader can visualize the hierarchy

Copy link
Member Author

@rohan47 rohan47 Jun 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will it be fine if this table is after the example 1?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i was suggesting a pattern of a simple example, followed by the detailed table, followed by more advanced examples. That's fine as well if the table is after all of the examples.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something like this?

apiVersion: rook.io/v1alpha1
kind: NetworkFileSystem
metadata:
  name: nfs-vol
  namespace: rook
spec:
  replicas: 1
  exports:
  - name: nfs-share
    server:
      accessMode: ReadOnly
      squash: root
      allowedClients:
      - name: host1
        clients: 172.17.0.5
          accessMode: ReadOnly
          squash: root
  persistentVolumeClaim:
    claimName: ebs-claim

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, looks good. We might consider not including the allowedClients in the first example though. It could be in a more advanced example later.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, this sounds good.

design/nfs.md Outdated

| Parameter | Description | Default |
|-------------------------------------------|--------------------------------------------------------------------------|----------------------|
| `replicas` | The no. of NFS daemon to start | `1` |
| `volumes` | These are the volumes to be exposed by NFS server | <none> |
| `server` | NFS server configuration | <none> |
| `server.volumeMount.name` | `volume.name` of the attached volume for sharing via NFS server | <none> |
Copy link
Member

@travisn travisn Jun 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For each volume in the volumes element, there is a corresponding volumeMount in the server section, right? What if we combine these into a single list? For example:

  volumes:
  - name: nfs-share
    mount:
       accessMode: ReadWrite
       squash: root
    persistentVolumeClaim:
      claimName: googlePD-claim

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, and instead of mount we can name it options or server

exports:
- name: nfs-share
  server:
      accessMode: ReadWrite
      squash: root
  persistantVolumeClaim:
    claimName: googlePD-claim

and for client specific example it can be

exports:
- name: nfs-share
  server:
    allowedClients:
    - name: host1
      clients: 172.17.0.5
      accessMode: ReadOnly
      squash: root
    - name: host2
      clients:
      - 172.17.0.0/16
      - serverX
      accessMode: ReadWrire
      squash: none
  persistantVolumeClaim:
    claimName: googlePD-claim

Signed-off-by: rohan47 <rohanrgupta1996@gmail.com>
@rohan47 rohan47 force-pushed the nfs-design-doc branch 2 times, most recently from 982c1b8 to 1a613f6 Compare June 4, 2018 22:15
design/nfs.md Outdated
| `exports.persistentVolumeClaim` | Claim to get volume(Volume could come from hostPath, cephFS, cephRBD, googlePD, EBS etc. and these volumes will be exposed by NFS server ). | <none> |
| `exports.persistentVolumeClaim.claimName` | Name of the PVC | <none> |

*note: if `server.volumeMount.accessMode` and `server.volumeMount.squash` options are mentioned, `server.volumeMount.allowedClients.accessMode` and `server.volumeMount.allowedClients.squash` are overridden respectively.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like you need to update these paths

@rohan47 rohan47 force-pushed the nfs-design-doc branch 3 times, most recently from d3a4f3c to e21cded Compare June 5, 2018 00:06
@travisn travisn merged commit f36e7b9 into rook:master Jun 5, 2018
@rohan47 rohan47 deleted the nfs-design-doc branch June 5, 2018 00:50
@rohan47 rohan47 added the nfs label Aug 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants