From 92db1250c0b803b823d1127ea8e3ae6690f630aa Mon Sep 17 00:00:00 2001 From: vsoch Date: Fri, 14 Jun 2024 12:58:55 -0600 Subject: [PATCH 1/4] feat: add support for empty directory Increasing the shared memory of the pod is important for IPC, which is important for MPI! Adding empty directory support here with type Medium should do that. Signed-off-by: vsoch --- api/v1alpha2/minicluster_types.go | 9 ++++ .../flux-framework.org_miniclusters.yaml | 12 +++++ config/manager/kustomization.yaml | 1 + controllers/flux/containers.go | 19 ++++++-- controllers/flux/volumes.go | 18 +++++++- .../custom-resource-definition.md | 44 +++++++++++++++++++ examples/tests/emptydir/minicluster.yaml | 18 ++++++++ 7 files changed, 116 insertions(+), 5 deletions(-) create mode 100644 examples/tests/emptydir/minicluster.yaml diff --git a/api/v1alpha2/minicluster_types.go b/api/v1alpha2/minicluster_types.go index 0b3b9977..bd62bea3 100644 --- a/api/v1alpha2/minicluster_types.go +++ b/api/v1alpha2/minicluster_types.go @@ -250,6 +250,15 @@ type ContainerVolume struct { // +default=false // +optional ReadOnly bool `json:"readOnly,omitempty"` + + // Add an empty directory custom type + // +optional + EmptyDirMedium string `json:"emptyDirMedium,omitempty"` + + // +kubebuilder:default=false + // +default=false + // +optional + EmptyDir bool `json:"emptyDir,omitempty"` } type FluxSpec struct { diff --git a/config/crd/bases/flux-framework.org_miniclusters.yaml b/config/crd/bases/flux-framework.org_miniclusters.yaml index eda43389..d9f163a0 100644 --- a/config/crd/bases/flux-framework.org_miniclusters.yaml +++ b/config/crd/bases/flux-framework.org_miniclusters.yaml @@ -226,6 +226,12 @@ spec: Config map name if the existing volume is a config map You should also define items if you are using this type: string + emptyDir: + default: false + type: boolean + emptyDirMedium: + description: Add an empty directory custom type + type: string hostPath: description: An existing hostPath to bind to path type: string @@ -723,6 +729,12 @@ spec: Config map name if the existing volume is a config map You should also define items if you are using this type: string + emptyDir: + default: false + type: boolean + emptyDirMedium: + description: Add an empty directory custom type + type: string hostPath: description: An existing hostPath to bind to path type: string diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 67ba7813..1d8b5340 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -13,3 +13,4 @@ kind: Kustomization images: - name: controller newName: ghcr.io/flux-framework/flux-operator + newTag: test diff --git a/controllers/flux/containers.go b/controllers/flux/containers.go index a1028c7e..7d8051d3 100644 --- a/controllers/flux/containers.go +++ b/controllers/flux/containers.go @@ -97,10 +97,21 @@ func getContainers( // Add on existing volumes/claims for volumeName, volume := range container.Volumes { - mount := corev1.VolumeMount{ - Name: volumeName, - MountPath: volume.Path, - ReadOnly: volume.ReadOnly, + + var mount corev1.VolumeMount + if volume.EmptyDir { + + // We assume bindings to /dev/shm + mount = corev1.VolumeMount{ + Name: volumeName, + MountPath: "/dev/shm", + } + } else { + mount = corev1.VolumeMount{ + Name: volumeName, + MountPath: volume.Path, + ReadOnly: volume.ReadOnly, + } } mounts = append(mounts, mount) } diff --git a/controllers/flux/volumes.go b/controllers/flux/volumes.go index 0bb2b453..f875d460 100644 --- a/controllers/flux/volumes.go +++ b/controllers/flux/volumes.go @@ -132,11 +132,27 @@ func getExistingVolumes(existing map[string]api.ContainerVolume) []corev1.Volume }, } + } else if volumeMeta.EmptyDir { + + // The Flux Operator supports default and memory + medium := corev1.StorageMediumDefault + if volumeMeta.EmptyDirMedium == "memory" { + medium = corev1.StorageMediumMemory + } + newVolume = corev1.Volume{ + Name: volumeName, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{ + Medium: medium, + }, + }, + } + } else if volumeMeta.HostPath != "" { newVolume = corev1.Volume{ Name: volumeName, VolumeSource: corev1.VolumeSource{ - // Empath path for type means no checks are done + // Empty path for type means no checks are done HostPath: &corev1.HostPathVolumeSource{ Path: volumeMeta.Path, }, diff --git a/docs/getting_started/custom-resource-definition.md b/docs/getting_started/custom-resource-definition.md index 4f7e34e2..b9266ed3 100644 --- a/docs/getting_started/custom-resource-definition.md +++ b/docs/getting_started/custom-resource-definition.md @@ -1115,6 +1115,7 @@ We did this because volumes are complex and it was challenging to support every the volumes and persistent volume claims that the MiniCluster needs, and simply tell it about them. A volume (that must exist) can be: - a hostpath (good for local development) + - an empty directory (and of a particular custom type, such as Memory) - a persistent volume claim (PVC) and persistent volume (PV) that you've created - a secret that you've created - a config map that you've created @@ -1171,6 +1172,49 @@ spec: An example is provided in the [volumes test](https://github.com/flux-framework/flux-operator/tree/main/examples/tests/volumes). +#### emptyDir example + +A standard empty directory might look like this: + +```yaml +apiVersion: flux-framework.org/v1alpha2 +kind: MiniCluster +metadata: + name: flux-sample +spec: + size: 2 + containers: + - image: rockylinux:9 + command: ls /data + volumes: + # must be lowercase! + my-empty-dir: + emptyDir: true +``` + +And one for shared memory (to inherit the host) like this: + + +```yaml +apiVersion: flux-framework.org/v1alpha2 +kind: MiniCluster +metadata: + name: flux-sample +spec: + size: 2 + containers: + - image: rockylinux:9 + command: ls /data + volumes: + # must be lowercase! + my-empty-dir: + emptyDir: true + emptyDirMedium: "memory" +``` + +The default binds to the path `/dev/shm` and is not customizable. This can be changed if needed. + + #### persistent volume claim example As an example, the IBM plugin we use to setup takes this approach, and then we define the existing volume on the level of the container: diff --git a/examples/tests/emptydir/minicluster.yaml b/examples/tests/emptydir/minicluster.yaml new file mode 100644 index 00000000..74256ec0 --- /dev/null +++ b/examples/tests/emptydir/minicluster.yaml @@ -0,0 +1,18 @@ +apiVersion: flux-framework.org/v1alpha2 +kind: MiniCluster +metadata: + name: flux-sample +spec: + size: 2 + logging: + quiet: true + + # This is a list because a pod can support multiple containers + containers: + - image: rockylinux:9 + command: ls /data + volumes: + # This must be all lowercase! + my-empty-dir: + emptyDir: true + emptyDirMedium: "memory" From 7dcc7477f248826cbc7d74fde3fa4ed9c385137c Mon Sep 17 00:00:00 2001 From: vsoch Date: Fri, 14 Jun 2024 13:06:01 -0600 Subject: [PATCH 2/4] prepush Signed-off-by: vsoch --- api/v1alpha2/swagger.json | 8 +++ api/v1alpha2/zz_generated.openapi.go | 14 +++++ chart/templates/minicluster-crd.yaml | 12 ++++ config/manager/kustomization.yaml | 1 - examples/dist/flux-operator-arm.yaml | 12 ++++ examples/dist/flux-operator.yaml | 12 ++++ examples/tests/emptydir/minicluster.yaml | 2 +- sdk/python/v1alpha2/docs/ContainerVolume.md | 2 + .../fluxoperator/models/container_volume.py | 56 ++++++++++++++++++- 9 files changed, 116 insertions(+), 3 deletions(-) diff --git a/api/v1alpha2/swagger.json b/api/v1alpha2/swagger.json index 06d3b036..7e57f3fe 100644 --- a/api/v1alpha2/swagger.json +++ b/api/v1alpha2/swagger.json @@ -121,6 +121,14 @@ "description": "Config map name if the existing volume is a config map You should also define items if you are using this", "type": "string" }, + "emptyDir": { + "type": "boolean", + "default": false + }, + "emptyDirMedium": { + "description": "Add an empty directory custom type", + "type": "string" + }, "hostPath": { "description": "An existing hostPath to bind to path", "type": "string" diff --git a/api/v1alpha2/zz_generated.openapi.go b/api/v1alpha2/zz_generated.openapi.go index 49b0dff5..b2b352d7 100644 --- a/api/v1alpha2/zz_generated.openapi.go +++ b/api/v1alpha2/zz_generated.openapi.go @@ -304,6 +304,20 @@ func schema_flux_framework_flux_operator_api_v1alpha2_ContainerVolume(ref common Format: "", }, }, + "emptyDirMedium": { + SchemaProps: spec.SchemaProps{ + Description: "Add an empty directory custom type", + Type: []string{"string"}, + Format: "", + }, + }, + "emptyDir": { + SchemaProps: spec.SchemaProps{ + Default: false, + Type: []string{"boolean"}, + Format: "", + }, + }, }, }, }, diff --git a/chart/templates/minicluster-crd.yaml b/chart/templates/minicluster-crd.yaml index 978141b3..290d5a1c 100644 --- a/chart/templates/minicluster-crd.yaml +++ b/chart/templates/minicluster-crd.yaml @@ -227,6 +227,12 @@ spec: Config map name if the existing volume is a config map You should also define items if you are using this type: string + emptyDir: + default: false + type: boolean + emptyDirMedium: + description: Add an empty directory custom type + type: string hostPath: description: An existing hostPath to bind to path type: string @@ -720,6 +726,12 @@ spec: Config map name if the existing volume is a config map You should also define items if you are using this type: string + emptyDir: + default: false + type: boolean + emptyDirMedium: + description: Add an empty directory custom type + type: string hostPath: description: An existing hostPath to bind to path type: string diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 1d8b5340..67ba7813 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -13,4 +13,3 @@ kind: Kustomization images: - name: controller newName: ghcr.io/flux-framework/flux-operator - newTag: test diff --git a/examples/dist/flux-operator-arm.yaml b/examples/dist/flux-operator-arm.yaml index 3459e0ad..493eeee4 100644 --- a/examples/dist/flux-operator-arm.yaml +++ b/examples/dist/flux-operator-arm.yaml @@ -232,6 +232,12 @@ spec: Config map name if the existing volume is a config map You should also define items if you are using this type: string + emptyDir: + default: false + type: boolean + emptyDirMedium: + description: Add an empty directory custom type + type: string hostPath: description: An existing hostPath to bind to path type: string @@ -729,6 +735,12 @@ spec: Config map name if the existing volume is a config map You should also define items if you are using this type: string + emptyDir: + default: false + type: boolean + emptyDirMedium: + description: Add an empty directory custom type + type: string hostPath: description: An existing hostPath to bind to path type: string diff --git a/examples/dist/flux-operator.yaml b/examples/dist/flux-operator.yaml index f557007c..8c01f467 100644 --- a/examples/dist/flux-operator.yaml +++ b/examples/dist/flux-operator.yaml @@ -232,6 +232,12 @@ spec: Config map name if the existing volume is a config map You should also define items if you are using this type: string + emptyDir: + default: false + type: boolean + emptyDirMedium: + description: Add an empty directory custom type + type: string hostPath: description: An existing hostPath to bind to path type: string @@ -729,6 +735,12 @@ spec: Config map name if the existing volume is a config map You should also define items if you are using this type: string + emptyDir: + default: false + type: boolean + emptyDirMedium: + description: Add an empty directory custom type + type: string hostPath: description: An existing hostPath to bind to path type: string diff --git a/examples/tests/emptydir/minicluster.yaml b/examples/tests/emptydir/minicluster.yaml index 74256ec0..96fbb789 100644 --- a/examples/tests/emptydir/minicluster.yaml +++ b/examples/tests/emptydir/minicluster.yaml @@ -10,7 +10,7 @@ spec: # This is a list because a pod can support multiple containers containers: - image: rockylinux:9 - command: ls /data + command: cat /proc/meminfo volumes: # This must be all lowercase! my-empty-dir: diff --git a/sdk/python/v1alpha2/docs/ContainerVolume.md b/sdk/python/v1alpha2/docs/ContainerVolume.md index b2f8fac3..32e70e90 100644 --- a/sdk/python/v1alpha2/docs/ContainerVolume.md +++ b/sdk/python/v1alpha2/docs/ContainerVolume.md @@ -6,6 +6,8 @@ Name | Type | Description | Notes ------------ | ------------- | ------------- | ------------- **claim_name** | **str** | Claim name if the existing volume is a PVC | [optional] **config_map_name** | **str** | Config map name if the existing volume is a config map You should also define items if you are using this | [optional] +**empty_dir** | **bool** | | [optional] [default to False] +**empty_dir_medium** | **str** | Add an empty directory custom type | [optional] **host_path** | **str** | An existing hostPath to bind to path | [optional] **items** | **dict[str, str]** | Items (key and paths) for the config map | [optional] **path** | **str** | Path and claim name are always required if a secret isn't defined | [optional] diff --git a/sdk/python/v1alpha2/fluxoperator/models/container_volume.py b/sdk/python/v1alpha2/fluxoperator/models/container_volume.py index 16ad9986..9ef83c80 100644 --- a/sdk/python/v1alpha2/fluxoperator/models/container_volume.py +++ b/sdk/python/v1alpha2/fluxoperator/models/container_volume.py @@ -38,6 +38,8 @@ class ContainerVolume(object): openapi_types = { 'claim_name': 'str', 'config_map_name': 'str', + 'empty_dir': 'bool', + 'empty_dir_medium': 'str', 'host_path': 'str', 'items': 'dict[str, str]', 'path': 'str', @@ -48,6 +50,8 @@ class ContainerVolume(object): attribute_map = { 'claim_name': 'claimName', 'config_map_name': 'configMapName', + 'empty_dir': 'emptyDir', + 'empty_dir_medium': 'emptyDirMedium', 'host_path': 'hostPath', 'items': 'items', 'path': 'path', @@ -55,7 +59,7 @@ class ContainerVolume(object): 'secret_name': 'secretName' } - def __init__(self, claim_name=None, config_map_name=None, host_path=None, items=None, path=None, read_only=False, secret_name=None, local_vars_configuration=None): # noqa: E501 + def __init__(self, claim_name=None, config_map_name=None, empty_dir=False, empty_dir_medium=None, host_path=None, items=None, path=None, read_only=False, secret_name=None, local_vars_configuration=None): # noqa: E501 """ContainerVolume - a model defined in OpenAPI""" # noqa: E501 if local_vars_configuration is None: local_vars_configuration = Configuration.get_default_copy() @@ -63,6 +67,8 @@ def __init__(self, claim_name=None, config_map_name=None, host_path=None, items= self._claim_name = None self._config_map_name = None + self._empty_dir = None + self._empty_dir_medium = None self._host_path = None self._items = None self._path = None @@ -74,6 +80,10 @@ def __init__(self, claim_name=None, config_map_name=None, host_path=None, items= self.claim_name = claim_name if config_map_name is not None: self.config_map_name = config_map_name + if empty_dir is not None: + self.empty_dir = empty_dir + if empty_dir_medium is not None: + self.empty_dir_medium = empty_dir_medium if host_path is not None: self.host_path = host_path if items is not None: @@ -131,6 +141,50 @@ def config_map_name(self, config_map_name): self._config_map_name = config_map_name + @property + def empty_dir(self): + """Gets the empty_dir of this ContainerVolume. # noqa: E501 + + + :return: The empty_dir of this ContainerVolume. # noqa: E501 + :rtype: bool + """ + return self._empty_dir + + @empty_dir.setter + def empty_dir(self, empty_dir): + """Sets the empty_dir of this ContainerVolume. + + + :param empty_dir: The empty_dir of this ContainerVolume. # noqa: E501 + :type empty_dir: bool + """ + + self._empty_dir = empty_dir + + @property + def empty_dir_medium(self): + """Gets the empty_dir_medium of this ContainerVolume. # noqa: E501 + + Add an empty directory custom type # noqa: E501 + + :return: The empty_dir_medium of this ContainerVolume. # noqa: E501 + :rtype: str + """ + return self._empty_dir_medium + + @empty_dir_medium.setter + def empty_dir_medium(self, empty_dir_medium): + """Sets the empty_dir_medium of this ContainerVolume. + + Add an empty directory custom type # noqa: E501 + + :param empty_dir_medium: The empty_dir_medium of this ContainerVolume. # noqa: E501 + :type empty_dir_medium: str + """ + + self._empty_dir_medium = empty_dir_medium + @property def host_path(self): """Gets the host_path of this ContainerVolume. # noqa: E501 From a835aad6f35ab9cc70fd2d5cde59e6ec779a8447 Mon Sep 17 00:00:00 2001 From: vsoch Date: Fri, 14 Jun 2024 15:08:13 -0600 Subject: [PATCH 3/4] add without example Signed-off-by: vsoch --- examples/tests/emptydir/minicluster-without.yaml | 13 +++++++++++++ examples/tests/emptydir/minicluster.yaml | 4 ++-- 2 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 examples/tests/emptydir/minicluster-without.yaml diff --git a/examples/tests/emptydir/minicluster-without.yaml b/examples/tests/emptydir/minicluster-without.yaml new file mode 100644 index 00000000..314b4d30 --- /dev/null +++ b/examples/tests/emptydir/minicluster-without.yaml @@ -0,0 +1,13 @@ +apiVersion: flux-framework.org/v1alpha2 +kind: MiniCluster +metadata: + name: flux-sample +spec: + size: 2 + logging: + quiet: true + + # This example shows the default shared memory (64M) without the empty dir + containers: + - image: rockylinux:9 + command: df -h /dev/shm \ No newline at end of file diff --git a/examples/tests/emptydir/minicluster.yaml b/examples/tests/emptydir/minicluster.yaml index 96fbb789..ef19e493 100644 --- a/examples/tests/emptydir/minicluster.yaml +++ b/examples/tests/emptydir/minicluster.yaml @@ -7,10 +7,10 @@ spec: logging: quiet: true - # This is a list because a pod can support multiple containers + # This example should show all the shared memory available on the node containers: - image: rockylinux:9 - command: cat /proc/meminfo + command: df -h /dev/shm volumes: # This must be all lowercase! my-empty-dir: From 33560de9d5905ae6de25b566f1c0dbb64f39403a Mon Sep 17 00:00:00 2001 From: vsoch Date: Fri, 14 Jun 2024 15:24:07 -0600 Subject: [PATCH 4/4] add additional note Signed-off-by: vsoch --- .../custom-resource-definition.md | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/docs/getting_started/custom-resource-definition.md b/docs/getting_started/custom-resource-definition.md index b9266ed3..484ef37a 100644 --- a/docs/getting_started/custom-resource-definition.md +++ b/docs/getting_started/custom-resource-definition.md @@ -1185,7 +1185,7 @@ spec: size: 2 containers: - image: rockylinux:9 - command: ls /data + command: df -h /dev/shm volumes: # must be lowercase! my-empty-dir: @@ -1212,8 +1212,25 @@ spec: emptyDirMedium: "memory" ``` -The default binds to the path `/dev/shm` and is not customizable. This can be changed if needed. +The default binds to the path `/dev/shm` and is not customizable. This can be changed if needed. When you have the "memory" medium added, +you should see all the shared memory from the host, which is [calculated here](https://github.com/kubernetes/kubernetes/blob/e6616033cb844516b1e91b3ec7cd30f8c5d1ea50/pkg/volume/emptydir/empty_dir.go#L148-L157). +As an example, here is output from a local run with kind when shared memory is added: + +```console +$ kubectl logs flux-sample-0-smflk +Defaulted container "flux-sample" out of: flux-sample, flux-view (init) +Filesystem Size Used Avail Use% Mounted on +tmpfs 32G 0 32G 0% /dev/shm +``` +And here is the same MiniCluster with the volume removed (64M is the default): + +```console +$ kubectl logs flux-sample-0-4bwjf -f +Defaulted container "flux-sample" out of: flux-sample, flux-view (init) +Filesystem Size Used Avail Use% Mounted on +shm 64M 0 64M 0% /dev/shm +``` #### persistent volume claim example