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

feat(node affinity): add support to specify node affinity rules of NFS Server #59

Merged
merged 6 commits into from
Jul 9, 2021

Conversation

mittachaitu
Copy link
Contributor

@mittachaitu mittachaitu commented Jul 5, 2021

Pull Request template

Why is this PR required? What issue does it fix?:
This PR adds support to specify node affinity rules via NFS-Provisioner
ENV to schedule NFS Server on a specific set of nodes. It fixes #57

What this PR does?:
This PR adds a feature to propagate node affinity rules to NFS Server
instance via Provisioner ENV.

Does this PR require any upgrade changes?:
No

How to use?:

  • Add 'OPENEBS_IO_NFS_SERVER_NODE_AFFINITY' ENV in NFS-Provisioner deployment in following manner:
    - name: OPENEBS_IO_NFS_SERVER_NODE_AFFINITY
      value: "kubernetes.io/hostname:[172.17.0.1],kubernetes.io/os:[linux]"
  • To schedule NFS Server instance on storage & nfs nodes
    - name: OPENEBS_IO_NFS_SERVER_NODE_AFFINITY
      value: "kubernetes.io/storage,kubernetes.io/nfs-node"
    Ex: Propagation to NFS Server deployment
    ...
    ...
    ...
           nodeAffinity:
           requiredDuringSchedulingIgnoredDuringExecution:
             nodeSelectorTerms:
             - matchExpressions:
               - key: kubernetes.io/storage-node
                 operator: Exists
               - key: kubernetes.io/arch
                 operator: Exists

How it is propagated to NFS Server instance:

  • During boot-up time of provisioner instance, provisioner will read
    OPENEBS_IO_NFS_SERVER_NODE_AFFINITY ENV then parse
    affinity rules and store them under the affinity rules in form of Go structure[in-memory].
  • When volume is provisioned NFS-Provisioner will propagate this affinity
    rules to NFS Server instance.
    Example propagation:
...
...
...
      affinity:
        nodeAffinity:
          requiredDuringSchedulingIgnoredDuringExecution:
            nodeSelectorTerms:
            - matchExpressions:
              - key: kubernetes.io/hostname
                operator: In
                values:
                - 172.17.0.1
              - key: kubernetes.io/os
                operator: In
                values:
                - linux

If the changes in this PR are manually verified, list down the scenarios covered::

Any additional information for your reviewer? :
Mention if this PR is part of any design or a continuation of previous PRs

Checklist:

Signed-off-by: mittachaitu [email protected]

@codecov-commenter
Copy link

codecov-commenter commented Jul 5, 2021

Codecov Report

Merging #59 (5adecb5) into develop (e0a9e18) will decrease coverage by 11.53%.
The diff coverage is 44.64%.

Impacted file tree graph

@@             Coverage Diff              @@
##           develop      #59       +/-   ##
============================================
- Coverage    48.37%   36.83%   -11.54%     
============================================
  Files           18       27        +9     
  Lines         1598     2234      +636     
============================================
+ Hits           773      823       +50     
- Misses         781     1366      +585     
- Partials        44       45        +1     
Impacted Files Coverage Δ
...tes/api/core/v1/podtemplatespec/podtemplatespec.go 37.23% <0.00%> (-4.64%) ⬇️
provisioner/helper_kernel_nfs_server.go 0.00% <0.00%> (ø)
provisioner/provisioner.go 0.00% <0.00%> (ø)
provisioner/node_affinity.go 96.07% <96.07%> (ø)
provisioner/env.go 14.28% <100.00%> (ø)
provisioner/signal_handler.go 0.00% <0.00%> (ø)
provisioner/provisioner_kernel_nfs_server.go 0.00% <0.00%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e0a9e18...5adecb5. Read the comment docs.

@kmova
Copy link
Collaborator

kmova commented Jul 6, 2021

@mittachaitu -- is it feasible to modify the schema for specifying the affinity a litte for the cases where there no values. for example is it possible to just use:

- name: NODEAFFINITY
  value: "kubernetes.io/storage"

or a comma separated values like this:

- name: NODEAFFINITY
  value: "kubernetes.io/storage, openebs.io/nfsnodes"

@mittachaitu
Copy link
Contributor Author

@mittachaitu -- is it feasible to modify the schema for specifying the affinity a litte for the cases where there no values. for example is it possible to just use:

- name: NODEAFFINITY
  value: "kubernetes.io/storage"

or a comma separated values like this:

- name: NODEAFFINITY
  value: "kubernetes.io/storage, openebs.io/nfsnodes"

Question: As of now we are supporting only In & Exists operations. Do we also need to support NotIn & DoesnNotExist cases? If yes without values how can we take negate input?

If we follow with key-value pairs even though there is no value then-current implementation is easy to extend for negation. For example negation config

- name: NODEAFFINITY
  value: "kubernetes.io/application:~[], kubernetes.io/zones:~[zone-2,zone-3]"

can be converted to

            - matchExpressions:
              - key: kubernetes.io/application
                operator: DoesNotExist
              - key: kubernetes.io/zones
                operator: NotIn
                values:
                - zone-2
                - zone-3

@mittachaitu mittachaitu requested a review from mynktl July 7, 2021 14:39
@kmova
Copy link
Collaborator

kmova commented Jul 8, 2021

This should be merged after #58 - as there are some helm chart files with conflicting changes.

mittachaitu added 3 commits July 8, 2021 15:58
…S Server

This commit adds support to specify node affinity rules via NFS-Provisioner
ENV to schedule scheduling NFS Server on set of nodes.

**How to use?**:
- Add 'NODEAFFINITY' ENV in NFS-Provisioner deployment in following manner:
  ```sh
  - name: NODEAFFINITY
    value: "kubernetes.io/hostname:[172.17.0.1],kubernetes.io/os:[linux]"
  ```
- To schedule NFS Server instance on storage nodes
  ```sh
  - name: NODEAFFINITY
    value: "kubernetes.io/storage:[]"
  ```

**How it is propogated to NFS Server instance**:
- During boot up time of provisioner instance, provisioner will read
  NODEAFFINITY ENV then parse affinity rules and store them under affinity
  rules in form of Go structure[in-memory].
- When volume is provisioned NFS-Provisioner will propogate these affinity
  rules to NFS Server instance.
Example propogation:
```yaml
...
...
...
      affinity:
        nodeAffinity:
          requiredDuringSchedulingIgnoredDuringExecution:
            nodeSelectorTerms:
            - matchExpressions:
              - key: kubernetes.io/hostname
                operator: In
                values:
                - 172.17.0.1
              - key: kubernetes.io/os
                operator: In
                values:
                - linux
```

Signed-off-by: mittachaitu <[email protected]>
This commit also address review comments

Signed-off-by: mittachaitu <[email protected]>
provisioner/env.go Outdated Show resolved Hide resolved
provisioner/env.go Outdated Show resolved Hide resolved
@kmova kmova merged commit cbb6dc2 into openebs-archive:develop Jul 9, 2021
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.

support to provision nfs-volume using the node information provided via storageclass or env in provisioner
4 participants