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

XFS quota for emptyDir volumes #9261

Merged
merged 1 commit into from
Jun 7, 2018
Merged

Conversation

mburke5678
Copy link
Contributor

Seth:
As of 3.10, we no longer use the atomic-openshift-node binary. All the functions it wrapped (ovn, ovs, kubelet) have all been broken out into separate services. The kubelet is now the hyperkube binary built from openshift/kubernetes.

This means all the custom stuff we had wrapping the kubelet for our purposes is no long in effect. The node-config.yaml file is not used directly by anything. The "openshift start --write-flags" command is used to convert the node-config.yaml into a list of flags appropriate for the kubelet. Among these patched-in features that don't translate in the new setup is XFS quota via the LocalQuota option in the node-config.yaml

https://docs.openshift.com/container-platform/3.9/install_config/master_node_configuration.html#master-node-config-volume-config

https://docs.openshift.com/container-platform/3.9/install_config/master_node_configuration.html#node-configuration-files

I converted that patch to a carry in our openshift/kubernetes in this PR
openshift/origin#19533

However, because the kubelet does not understand the node-config.yaml and there is no flag by which the XFS quota configuration can be conveyed, the patch reads a new config file at a fixed location to configure the XFS quota.

The new config file path is
/var/lib/origin/openshift.local.volumes/volume-config.yaml

The file has the format:

apiVersion: kubelet.config.openshift.io/v1
kind: VolumeConfig
localQuota:
perFSGroupInGiB: 1

This feature is very lightly documented. The only two places I can find it mentioned are the links above. The primary user, to our knowledge, is Online.

Michael, can you open a PR for 3.10 that updates the documentation to reflect this change?

Thanks and let me know if you have any questions.

FYI @sjenning

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 15, 2018
@mburke5678 mburke5678 added this to the Future Release milestone May 15, 2018
You can use the link:https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7/html/storage_administration_guide/ch-xfs[XFS quota subsystem]
to limit the size of `emptyDir` volumes and volumes based on an `emptyDir` volume, such as secrets and configmaps, on each node.

Enable a quota on local storage use per unique FSGroup ID usin g the *_/var/lib/origin/openshift.local.volumes/volume-config.yaml_* file.

Choose a reason for hiding this comment

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

s/usin g/using/

---

<1> Contains options for controlling local volume quota on the node.
<2> Set this value to a resource quantity representing the desired quota per

Choose a reason for hiding this comment

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

Due to some complications in the code, this value is now numeric, not a resource quantity. It is the quota in GBs.

<1> Contains options for controlling local volume quota on the node.
<2> Set this value to a resource quantity representing the desired quota per
xref:../install_config/persistent_storage/pod_security_context.adoc#fsgroup[FSGroup], per node (such as 1Gi, 512Mi, etc).
Requires the *volumeDirectory* to be on an XFS filesystem mounted with the 'gquota' option. The matching security context contraint *fsGroup* type must be
Copy link

@sjenning sjenning May 15, 2018

Choose a reason for hiding this comment

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

s/gquota/grpquota/

gquota is a alias for grpquota, but grpquota is the real option

@sjenning
Copy link

Thanks @mburke5678 for the quick response on this!
/cc @derekwaynecarr

Look good overall. I had a few comments.

@mburke5678
Copy link
Contributor Author

@sjenning Is the perFSGroupInGiB setting in GB (gigabyte) or GiB (gibibyte) as the parameter has GiB in its name (or, does it not really matter for our purposes?).

@mburke5678 mburke5678 changed the title [WIP] XFS quota for emptyDir volumes XFS quota for emptyDir volumes May 16, 2018
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 16, 2018
@sjenning
Copy link

standby on this. we might be changing how this is done.

https://bugzilla.redhat.com/show_bug.cgi?id=1572285#c5

i'll keep you posted.

@sjenning
Copy link

sjenning commented May 18, 2018

follow-on PR openshift/origin#19773

  • Transforms perFSGroupInGB back to perFSGroup with a Quantity value.
  • Changes the config file location to /etc/origin/node/volume-config.yaml so that it can be a second key to the existing node-config-compute configmap and everything will Just Work (tm).

@mburke5678
Copy link
Contributor Author

mburke5678 commented May 18, 2018

@sjenning I changed the parameter to perFSGroup: 50 and the text to: "Sets the maximum number of emptyDir volumes for each unique FSGroup ID." Also replaced the path to the yaml.
PTAL

@mburke5678
Copy link
Contributor Author

@mdshuai Can someone on your team review this PR?

1 similar comment
@mburke5678
Copy link
Contributor Author

@mdshuai Can someone on your team review this PR?

@@ -262,7 +262,7 @@ be configured:
NOTE: If the XFS filesystem hosting that folder is mounted with the
`gquota` option in the `/etc/fstab`
* the matching security context contraint’s `fsGroup` type set to `MustRunAs`
* The `/etc/origin/node/node-config.yml` is configured as:
* The xref:../install_config/master_node_configuration.adoc#node-config-volume-config`/var/lib/origin/openshift.local.volumes/volume-config.yaml` is configured] as:

Choose a reason for hiding this comment

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

This file is now /etc/origin/node/volume-config.yaml

apiVersion: kubelet.config.openshift.io/v1
kind: VolumeConfig
localQuota: <1>
perFSGroup: 50 <2>

Choose a reason for hiding this comment

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

needs a unit. use 1Gi or 512Mi as an example

---

<1> Contains the options for controlling local volume quota on the node.
<2> Sets the maximum number of `emptyDir` volumes for each unique FSGroup ID.

Choose a reason for hiding this comment

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

this is not right.

Specifies the maximum amount of storage that can be used for disk-based emptyDir volumes on the node for each unique FSGroup ID

@sjenning
Copy link

We just got tasked with auditing the docs for references to /etc/origin/node/node-config.yaml because that file should no longer be directly edited.

Instead, cluster ops should edit the node-config.yaml key in the node-config-<role> configmap in openshift-node namespace.

Really the same thing applies for this as it will be deployed using the volume-config.yaml key in the same configmap (openshift/openshift-ansible#8450).

to limit the number of `emptyDir` volumes and volumes based on an `emptyDir` volume, such as secrets and configmaps, on each node.

Enable a quota on local storage for each unique xref:../architecture/additional_concepts/authorization.adoc#security-context-constraints[FSGroup ID]
using the *_/etc/origin/node/volume-config.yaml_* file.

Choose a reason for hiding this comment

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

Probably add this here:

NOTE: The volume-config.yaml file should not be edited directly as it is created from the node-config-compute configmap in the openshift-node namespace. Creating or editing volume-config.yaml should be done via the configmap and not directly on the node.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sjenning What does the code in the config map configure perFSGroup look like?

Copy link

Choose a reason for hiding this comment

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

An example:

apiVersion: kubelet.config.openshift.io/v1
kind: VolumeConfig
localQuota:
  perFSGroup: 1Gi

@mburke5678
Copy link
Contributor Author

mburke5678 commented Jun 1, 2018

@sjenning What does the code in the config map configure perFSGroup look like?

@mburke5678
Copy link
Contributor Author

@chao007 PTAL

<2> Set this value to a resource quantity representing the desired quota per
xref:../install_config/persistent_storage/pod_security_context.adoc#fsgroup[FSGroup], per node (such as 1Gi, 512Mi, etc).
Requires the *volumeDirectory* to be on an XFS filesystem mounted with the 'gquota' option. The matching security context contraint *fsGroup* type must be
<2> Set ththe desired quota per
Copy link

Choose a reason for hiding this comment

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

"ththe"? spelling error?

@mburke5678
Copy link
Contributor Author

@chao007 I do not see ththe in the source. Can you verify if ththe is no longer present?


[NOTE]
====
You should not edit the *_/var/lib/origin/openshift.local.volumes/volume-config.yaml_* file directly. The file is created from the *_node-config-compute_* configuration map.
Copy link

Choose a reason for hiding this comment

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

/etc/origin/node/volume-config.yaml

kind: VolumeConfig
localQuota: <1>
perFSGroupInGiB: 1Gi <2>
----
Copy link

Choose a reason for hiding this comment

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

Is this updated to "perFSGroup" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chao007 Fixed. Changed to perFSGroup: 1Gi per Seth's comment. #9261 (comment)

@sjenning
Copy link

sjenning commented Jun 5, 2018

lgtm, thanks 👍

@mburke5678 mburke5678 force-pushed the XFS-quota branch 3 times, most recently from fe6aab1 to 1eca68c Compare June 6, 2018 15:48
@mburke5678
Copy link
Contributor Author

@openshift/team-documentation PTAL

Copy link
Contributor

@kalexand-rh kalexand-rh left a comment

Choose a reason for hiding this comment

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

I'm slightly confused by some of the legacy language and have a few picks.

@@ -260,9 +260,9 @@ is lost. The location of the data in the *{rhocp}* node is located in
be configured:

NOTE: If the XFS filesystem hosting that folder is mounted with the
`gquota` option in the `/etc/fstab`
`grpquota` option in the `/etc/fstab`
* the matching security context contraint’s `fsGroup` type set to `MustRunAs`
Copy link
Contributor

Choose a reason for hiding this comment

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

The structure of this list is confusing, and I'm not sure what you mean by "The using the ... configuration map."

Maybe


grpquota option in the /etc/fstab file:

  • Set the matching security context contraint’s fsGroup type to MustRunAs.
  • xref:../install_config/master_node_configuration.adoc#node-config-volume-config[Configure the volume] using the node-config-compute configuration map in the openshift-node project.
    (and remove the example volumeconfig because it's not really helping here.)

Copy link
Contributor Author

@mburke5678 mburke5678 Jun 6, 2018

Choose a reason for hiding this comment

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

@kalexand-rh Looking around, I don't think this topic is being used at this time. It was existing text. I made your changes, but did not remove the example. I am not sure why it is there.
I did a search and found gquota and changed to grpquota per Seth's comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, @mburke5678!
@bfallonf, do you know if this file should be used or removed? It looks like it was commented out of the day_two_guide/host_level_tasks topic.


<1> Contains options for controlling local volume quota on the node.
<2> Set this value to a resource quantity representing the desired quota per
[FSGroup], per node, such as 1Gi, 512Mi, and so forth.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd set the values off in backticks to emphasize that they're example parameter values. (Otherwise, you need spaces between the value and abbreviation because of Style.)


[NOTE]
====
You should not edit the *_/etc/origin/node/volume-config.yaml_* file directly. The file is created from the *_node-config-compute_* configuration map.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/You should/Do

[NOTE]
====
You should not edit the *_/etc/origin/node/volume-config.yaml_* file directly. The file is created from the *_node-config-compute_* configuration map.
Use the *_node-config-compute_* configuration map to create or edit the paramaters in the *_volume-config.yaml_*.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/volume-config.yaml/volume-config.yaml file

@@ -1082,13 +1082,10 @@ If -1, there is no limit on requests.
|A boolean to enable or disable dynamic provisioning. Default is *true*.

|FSGroup
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd consider adding emphasis on FSGroup.

@@ -1082,13 +1082,10 @@ If -1, there is no limit on requests.
|A boolean to enable or disable dynamic provisioning. Default is *true*.

|FSGroup
|Can be specified to enable a quota on local storage use per unique FSGroup ID.
|Can be specified to xref:node-config-volume-config[enable a quota on local storage] use per unique FSGroup ID.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is confusing. Do you mean "Can be specified to xref:node-config-volume-config[enable a quota on local storage]. You must enable quota for each unique FSGroup ID." ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The feature sets a blanket quota for all FSgroups on each node.
"Enables xref:node-config-volume-config[local storage quotas] on each node for each FSGroup."

@kalexand-rh kalexand-rh added the peer-review-done Signifies that the peer review team has reviewed this PR label Jun 6, 2018
@mburke5678 mburke5678 merged commit ee6e798 into openshift:master Jun 7, 2018
@mburke5678
Copy link
Contributor Author

/cherrypick enterprise-3.10

@openshift-cherrypick-robot

@mburke5678: new pull request created: #9907

In response to this:

/cherrypick enterprise-3.10

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-3.10 peer-review-done Signifies that the peer review team has reviewed this PR size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants