From 0c42c858370f210f46a5e08c251320849516f45f Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Mon, 31 Aug 2026 16:29:29 +0200 Subject: [PATCH 01/15] CH-288 add support for RWX volumes --- .../helm/templates/_helpers.tpl | 50 +++++ .../auto-database-postgres-operator.yaml | 4 + .../helm/templates/auto-database.yaml | 8 + .../helm/templates/auto-deployments.yaml | 23 +- .../helm/templates/auto-volumes.yaml | 22 +- deployment-configuration/value-template.yaml | 16 +- docs/applications/databases.md | 7 + docs/applications/volumes.md | 104 ++++++++- docs/model/DatabaseDeploymentConfig.md | 1 + docs/model/DeploymentAutoArtifactConfig.md | 3 +- docs/model/DeploymentVolumeSpec.md | 4 +- .../cloudharness/workflows/operations.py | 6 +- .../cloudharness/workflows/utils.py | 38 +++- .../tests/test_workflow.py | 34 +++ .../cloudharness-common/tests/values.yaml | 11 + libraries/models/api/openapi.yaml | 42 +++- .../models/database_deployment_config.py | 9 +- .../models/deployment_auto_artifact_config.py | 10 +- .../models/deployment_volume_spec.py | 10 +- .../ch_cli_tools/configurationgenerator.py | 21 ++ tools/deployment-cli-tools/tests/test_helm.py | 210 ++++++++++++++++++ 21 files changed, 582 insertions(+), 51 deletions(-) diff --git a/deployment-configuration/helm/templates/_helpers.tpl b/deployment-configuration/helm/templates/_helpers.tpl index a88324fbf..de1099729 100644 --- a/deployment-configuration/helm/templates/_helpers.tpl +++ b/deployment-configuration/helm/templates/_helpers.tpl @@ -94,6 +94,56 @@ heritage: {{ $.Release.Service | quote }} {{- end }} +{{/* +Tells whether a harness.deployment.volume is ReadWriteMany: nfs volumes (legacy `usenfs` flag) +always are, otherwise `writeMany` decides. Renders "true" or nothing, so the result can be used +directly in a condition. Accepts a nil volume. +Usage: {{ if include "deploy_utils.volumeWriteMany" $volume }} +*/}} +{{- define "deploy_utils.volumeWriteMany" -}} +{{- if . }}{{ if .usenfs }}true{{ else if .writeMany }}true{{ end }}{{ end }} +{{- end -}} + +{{/* +Storage class of a harness.deployment.volume claim: nfs volumes always use the class created by +the nfsserver application, otherwise the volume `storageClass` wins on the deployment default +(harness.deployment.storageClass). A null default renders nothing, leaving the claim to the +cluster default storage class; `standard` is used when the deployment does not declare the key +at all (values generated before the setting existed). +Usage: {{ include "deploy_utils.volumeStorageClass" (dict "root" .root "deployment" $deployment) }} +*/}} +{{- define "deploy_utils.volumeStorageClass" -}} +{{- $volume := .deployment.volume -}} +{{- if $volume.usenfs }}{{ printf "%s-%s" .root.Values.namespace .root.Values.apps.nfsserver.storageClass.name }}{{ else if $volume.storageClass }}{{ $volume.storageClass }}{{ else if .deployment.storageClass }}{{ .deployment.storageClass }}{{ else if not (hasKey .deployment "storageClass") }}standard{{ end }} +{{- end -}} + +{{/* +Storage class of a database volume claim: harness.database.storageClass. A null value renders +nothing, leaving the claim to the cluster default storage class; `standard` is used when the +database does not declare the key at all (values generated before the setting existed). +Usage: {{ include "deploy_utils.databaseStorageClass" .app.harness.database }} +*/}} +{{- define "deploy_utils.databaseStorageClass" -}} +{{- if .storageClass }}{{ .storageClass }}{{ else if not (hasKey . "storageClass") }}standard{{ end }} +{{- end -}} + +{{/* +Render the spec of a claim (PersistentVolumeClaim or statefulset volumeClaimTemplate) for a +harness.deployment.volume. +Usage: {{ include "deploy_utils.volumeClaimSpec" (dict "root" .root "deployment" $deployment) | nindent 2 }} +*/}} +{{- define "deploy_utils.volumeClaimSpec" -}} +{{- $storageClass := include "deploy_utils.volumeStorageClass" (dict "root" .root "deployment" .deployment) -}} +accessModes: + - {{ if include "deploy_utils.volumeWriteMany" .deployment.volume }}ReadWriteMany{{ else }}ReadWriteOnce{{ end }} +{{- if $storageClass }} +storageClassName: {{ $storageClass }} +{{- end }} +resources: + requests: + storage: {{ .deployment.volume.size }} +{{- end -}} + {{/* Render volumeMounts block for a container. Usage: {{ include "deploy_utils.volumeMounts" (dict "app" .app "root" .root) }} diff --git a/deployment-configuration/helm/templates/auto-database-postgres-operator.yaml b/deployment-configuration/helm/templates/auto-database-postgres-operator.yaml index c8e80a984..b071b3e8a 100644 --- a/deployment-configuration/helm/templates/auto-database-postgres-operator.yaml +++ b/deployment-configuration/helm/templates/auto-database-postgres-operator.yaml @@ -46,6 +46,10 @@ spec: storage: size: {{ .app.harness.database.size }} + {{- $storageClass := include "deploy_utils.databaseStorageClass" .app.harness.database }} + {{- if $storageClass }} + storageClass: {{ $storageClass }} + {{- end }} {{- with .app.harness.database.resources }} resources: diff --git a/deployment-configuration/helm/templates/auto-database.yaml b/deployment-configuration/helm/templates/auto-database.yaml index b221255e9..69d2c8c68 100644 --- a/deployment-configuration/helm/templates/auto-database.yaml +++ b/deployment-configuration/helm/templates/auto-database.yaml @@ -42,6 +42,10 @@ metadata: spec: accessModes: - ReadWriteOnce + {{- $storageClass := include "deploy_utils.databaseStorageClass" .app.harness.database }} + {{- if $storageClass }} + storageClassName: {{ $storageClass }} + {{- end }} resources: requests: storage: {{ .app.harness.database.size }} @@ -138,6 +142,10 @@ spec: spec: accessModes: - ReadWriteOnce + {{- $storageClass := include "deploy_utils.databaseStorageClass" .app.harness.database }} + {{- if $storageClass }} + storageClassName: {{ $storageClass }} + {{- end }} resources: requests: storage: {{ .app.harness.database.size }} diff --git a/deployment-configuration/helm/templates/auto-deployments.yaml b/deployment-configuration/helm/templates/auto-deployments.yaml index db013c67a..19a16b09f 100644 --- a/deployment-configuration/helm/templates/auto-deployments.yaml +++ b/deployment-configuration/helm/templates/auto-deployments.yaml @@ -1,15 +1,17 @@ {{- define "deploy_utils.deployment" }} {{- $isStatefulSet := .app.harness.deployment.statefulset | default false }} {{- $volume := .app.harness.deployment.volume }} -{{- /* $rwoVolume: a ReadWriteOnce (non-nfs) volume pins the pod to a single node — it drives both - the Recreate strategy and the podAffinity. $ownVolume additionally requires the PVC to be - managed here (auto), which a statefulset turns into a volumeClaimTemplate. The checks are - nested so hasKey/index never runs on a nil volume: helm < 3.10 (go < 1.18) does not - short-circuit 'and'/'or', so a flat expression would fail for volume-less apps. */}} +{{- /* $rwoVolume: a ReadWriteOnce volume pins the pod to a single node — it drives both the + Recreate strategy and the podAffinity. ReadWriteMany volumes (`writeMany`, or the legacy + `usenfs` flag) attach to several nodes at once, hence need neither. $ownVolume additionally + requires the PVC to be managed here (auto), which a statefulset turns into a + volumeClaimTemplate. The checks are nested so hasKey/index never runs on a nil volume: + helm < 3.10 (go < 1.18) does not short-circuit 'and'/'or', so a flat expression would fail + for volume-less apps. */}} {{- $rwoVolume := false }} {{- $ownVolume := false }} {{- if $volume }} -{{- if or (not (hasKey $volume "usenfs")) (not $volume.usenfs) }} +{{- if not (include "deploy_utils.volumeWriteMany" $volume) }} {{- $rwoVolume = true }} {{- if or (not (hasKey $volume "auto")) $volume.auto }} {{- $ownVolume = true }} @@ -47,7 +49,7 @@ spec: # A ReadWriteOnce volume attaches to a single node and the pod is pinned to the # volume's node via podAffinity (see below). Recreate terminates the old pod # before starting the new one, avoiding an unschedulable pod when the node can't - # fit both. NFS (ReadWriteMany) volumes have no pinning and can roll normally. + # fit both. ReadWriteMany volumes have no pinning and can roll normally. strategy: type: Recreate {{- end }} @@ -210,12 +212,7 @@ spec: - metadata: name: {{ $volume.name }} spec: - accessModes: - - ReadWriteOnce - storageClassName: standard - resources: - requests: - storage: {{ $volume.size }} + {{- include "deploy_utils.volumeClaimSpec" (dict "root" .root "deployment" .app.harness.deployment) | nindent 8 }} {{- end }} {{- if $legacyClaim }} {{- include "deploy_utils.volumeMigration" (dict "root" .root "name" .app.harness.deployment.name "pvc" $volume.name) }} diff --git a/deployment-configuration/helm/templates/auto-volumes.yaml b/deployment-configuration/helm/templates/auto-volumes.yaml index 15edb91c7..17a445612 100644 --- a/deployment-configuration/helm/templates/auto-volumes.yaml +++ b/deployment-configuration/helm/templates/auto-volumes.yaml @@ -1,11 +1,16 @@ {{- define "deploy_utils.pvolume" }} {{- $volume := .app.harness.deployment.volume }} {{- $isStatefulSet := .app.harness.deployment.statefulset | default false }} +{{- /* ReadWriteMany volumes are shared by all the pods using them: they are never provisioned + per replica through volumeClaimTemplates, hence never owned by a statefulset. */}} +{{- $writeMany := include "deploy_utils.volumeWriteMany" $volume }} {{- $ownVolume := false }} -{{- if and (or (not (hasKey $volume "usenfs")) (not $volume.usenfs)) (or (not (hasKey $volume "auto")) $volume.auto) }} +{{- if not $writeMany }} +{{- if or (not (hasKey $volume "auto")) $volume.auto }} {{- $ownVolume = true }} {{- end }} -{{- /* Statefulsets own their (non-nfs) volume through volumeClaimTemplates: the standalone PVC +{{- end }} +{{- /* Statefulsets own their (ReadWriteOnce) volume through volumeClaimTemplates: the standalone PVC is only kept while a legacy one exists, so that its data can be migrated. Delete the legacy PVC once migrated. */}} {{- $legacyClaim := false }} @@ -27,18 +32,7 @@ metadata: labels: app: {{ .app.harness.deployment.name| quote }} spec: - resources: - requests: - storage: {{ .app.harness.deployment.volume.size }} - - accessModes: -{{- if or (not (hasKey .app.harness.deployment.volume "usenfs")) (not .app.harness.deployment.volume.usenfs) }} - - ReadWriteOnce - storageClassName: standard -{{- else }} - - ReadWriteMany - storageClassName: {{ printf "%s-%s" .root.Values.namespace .root.Values.apps.nfsserver.storageClass.name }} -{{- end }} + {{- include "deploy_utils.volumeClaimSpec" (dict "root" .root "deployment" .app.harness.deployment) | nindent 2 }} {{- end }} --- {{- end }} diff --git a/deployment-configuration/value-template.yaml b/deployment-configuration/value-template.yaml index a36c72371..724fc64a7 100644 --- a/deployment-configuration/value-template.yaml +++ b/deployment-configuration/value-template.yaml @@ -36,8 +36,20 @@ harness: name: # -- Deployment port. port: 8080 - # -- volume specification + # -- Default storage class of the deployment volume claim. Set to null to use the cluster default storage class. + storageClass: standard + # -- volume specification. + # `writeMany: true` creates and mounts the volume as ReadWriteMany: the volume attaches to several + # nodes at once, hence pods using it are not pinned to the volume's node. Requires a storage class + # supporting ReadWriteMany, set through the volume `storageClass` (which overrides the deployment default). volume: + # example: + # name: my-volume + # mountpath: /usr/src/app/myvolume + # auto: true + # size: 5Gi + # writeMany: true + # storageClass: efs-sc # -- When true, the deployment is rendered as a StatefulSet instead of a Deployment. Recommended for deployments with a (non-nfs) volume: updates terminate the old pod before creating the new one. The volume is provisioned per replica through volumeClaimTemplates; data of a pre-existing PVC named after the volume is copied into each statefulset volume by a migration job (delete the legacy PVC once migrated). statefulset: false # -- Deployment resources. @@ -93,6 +105,8 @@ harness: # -- supported db types: mongo, postgres, neo4j type: size: 1Gi + # -- Storage class of the database volume claim. Set to null to use the cluster default storage class. + storageClass: standard # -- database username user: mnp # -- database password diff --git a/docs/applications/databases.md b/docs/applications/databases.md index 3e1563aff..954a8d460 100644 --- a/docs/applications/databases.md +++ b/docs/applications/databases.md @@ -31,6 +31,13 @@ harness: `size`: Size of the persistent volume that the database container mounts, default is set to `1Gi` +`storageClass`: Storage class of the database volume claim, default is set to `standard`. Set it to +null to omit the storage class from the claim, so that the cluster default storage class is used. +It applies to the plain and statefulset database volumes as well as to the storage of a +`postgres.operator` cluster. Note that the storage class is immutable on an existing claim: on a +cluster whose default class is not `standard`, set this value (or null) before upgrading a release +that already has a database volume. + `resources`: Set the database pod resources `image_ref`: Optional setting, used for referencing a base/static image from the build. The complete image name with tag will automagically being generated from the values.yaml file. This setting overrides the `image` setting specific for the database type (e.g. postgres/image). Note: the referenced image must be included as a build dependency in order to be built by the pipelines. diff --git a/docs/applications/volumes.md b/docs/applications/volumes.md index b2694b51b..80ae7c95e 100644 --- a/docs/applications/volumes.md +++ b/docs/applications/volumes.md @@ -34,17 +34,79 @@ harness: A Volume can be mounted by one or more pods (shared Volume). Be careful: only one of the deployments should create the Volume, the other deployment should only mount it. -Cloudharness uses the `standard` StorageClass for the volume and ReadWriteOnce +By default Cloudharness uses the `standard` StorageClass for the volume and ReadWriteOnce mount strategy. In order to support volume sharing affinity rules are added so that all pods using the same volume end up in the same node. This strategy works for basic use cases but can easily cause deadlocks if more than one node is available on the cluster and other affinity rules or taints are present. -A better support for volume sharing is achieved by using a Network File System (NFS). -In order to use the nfs, the NFS must be added to the deployment (e.g. as a dependency and `usenfs` must be set to true. +### Storage class +The storage class of the volume claim is `harness.deployment.storageClass`, which defaults to +`standard`. Set it to null to omit the storage class from the claim, so that the cluster default +storage class is used: + +```yaml +harness: + ... + deployment: + # the cluster default storage class provisions the volume + storageClass: null + volume: + name: my-volume + mountpath: /usr/src/app/myvolume + auto: true + size: 5Gi +``` + +The volume's own `storageClass` overrides the deployment default, and takes precedence for both +ReadWriteOnce and ReadWriteMany volumes. A null on the volume means "not set", hence inherits the +deployment default: use the deployment `storageClass: null` to provision the volume on the cluster +default class. The same setting is available for database volumes as +`harness.database.storageClass` (see [databases](databases.md)). + +### ReadWriteMany volumes + +Setting `writeMany: true` creates and mounts the volume as ReadWriteMany. A ReadWriteMany volume +attaches to several nodes at the same time, hence the pods using it are not pinned to the volume's +node: no podAffinity is added, and deployments roll normally instead of being recreated. + +ReadWriteMany requires a storage class supporting it (e.g. AWS EFS, Azure Files, CephFS, +the nfs provisioner). The class is resolved as above — `standard` is normally ReadWriteOnce only, +so set the volume `storageClass`, or the deployment default, to a ReadWriteMany capable class +(or to null when the cluster default one supports ReadWriteMany). + +```yaml +harness: + ... + deployment: + ... + volume: + name: my-shared-volume + mountpath: /usr/src/app/myvolume + auto: true + size: 5Gi + writeMany: true + storageClass: efs-sc ``` + +When a volume is shared by several deployments, declare the same `writeMany` on all of them: the +claim is created once (by the deployment declaring `auto: true`), but each deployment decides on +its own declaration whether its pods are pinned to the volume's node. + +Note that both the access mode and the storage class are immutable on an existing +PersistentVolumeClaim: changing `writeMany` or the storage class on a live volume requires +deleting and recreating the claim, and the data is not migrated. + +### Using the NFS server application + +Volume sharing can also be achieved by using the Network File System provided by the `nfsserver` +application. In order to use the nfs, the nfs server must be added to the deployment (e.g. as a +dependency) and `usenfs` must be set to true: the volume is created as ReadWriteMany on the storage +class of the nfs provisioner. + +```yaml harness: ... dependencies: @@ -62,10 +124,36 @@ harness: usenfs: true ``` +`usenfs` is equivalent to `writeMany: true` with the nfs provisioner storage class, and is kept +for backwards compatibility: on a cluster providing a ReadWriteMany storage class, prefer +`writeMany` with `storageClass`. + +The nfs server settings prevail on the volume ones: an `usenfs` volume is always created on the +nfs provisioner storage class and mounted ReadWriteMany, whatever `storageClass` and `writeMany` +say. `harness-deployment` logs a warning when they collide: + +``` +WARNING Volume my-shared-volume of application samples sets usenfs and storageClass efs-sc: the nfs server storage class prevails. +``` + +### Volumes mounted by Argo workflows + +Argo workflow pods mounting an application volume (`:`, see +[Argo workflows](../argo-workflows.md)) are pinned to the volume's node in the same way +deployments are. ReadWriteMany application volumes are recognized from the application +configuration, so their workflows get no node pinning. + +Volumes that are not declared by an application can be marked as ReadWriteMany explicitly with +the `rwx` mount mode, which also disables the pinning: + +```python +operations.PipelineOperation('my-op-', tasks, shared_directory='my-claim:/mnt/shared:rwx') +``` + ## Deploying as a StatefulSet -By default, a deployment with a (non-nfs) volume is rendered as a Kubernetes `Deployment` with a -`Recreate` update strategy and podAffinity pinning it to the node holding the volume, since a +By default, a deployment with a ReadWriteOnce volume is rendered as a Kubernetes `Deployment` with +a `Recreate` update strategy and podAffinity pinning it to the node holding the volume, since a ReadWriteOnce volume can only attach to one node at a time. Setting `harness.deployment.statefulset: true` renders it as a `StatefulSet` instead. StatefulSet @@ -86,9 +174,9 @@ harness: ``` The volume is provisioned per replica through `volumeClaimTemplates` (PVCs named -`--`). Exceptions: nfs volumes (`usenfs: true`) and externally managed -volumes (`auto: false`) keep mounting their common PVC by name — per-replica claims would -un-share them. +`--`). Exceptions: ReadWriteMany volumes (`writeMany: true` or +`usenfs: true`) and externally managed volumes (`auto: false`) keep mounting their common PVC by +name — per-replica claims would un-share them. **Migrating from an existing Deployment**: if a PVC named after the volume exists in the cluster at deploy time (left over from the pre-statefulset Deployment), it is treated as a legacy volume: diff --git a/docs/model/DatabaseDeploymentConfig.md b/docs/model/DatabaseDeploymentConfig.md index b7aea5546..4d72a6b86 100644 --- a/docs/model/DatabaseDeploymentConfig.md +++ b/docs/model/DatabaseDeploymentConfig.md @@ -10,6 +10,7 @@ Name | Type | Description | Notes **name** | **str** | | [optional] **type** | **str** | Define the database type. One of (mongo, postgres, neo4j, sqlite3) | [optional] **size** | **str** | Specify database disk size | [optional] +**storage_class** | **str** | Storage class of the database volume claim. Set to null to omit the storage class from the claim, so that the cluster default storage class is used. | [optional] **user** | **str** | database username | [optional] **var_pass** | **str** | Database password | [optional] **image_ref** | **str** | Used for referencing images from the build | [optional] diff --git a/docs/model/DeploymentAutoArtifactConfig.md b/docs/model/DeploymentAutoArtifactConfig.md index 1b5de8692..3ecf3ac8c 100644 --- a/docs/model/DeploymentAutoArtifactConfig.md +++ b/docs/model/DeploymentAutoArtifactConfig.md @@ -13,7 +13,8 @@ Name | Type | Description | Notes **image** | **str** | Image name to use in the deployment. Leave it blank to set from the application's Docker file | [optional] **resources** | [**DeploymentResourcesConf**](DeploymentResourcesConf.md) | | [optional] **volume** | [**DeploymentVolumeSpec**](DeploymentVolumeSpec.md) | | [optional] -**statefulset** | **bool** | When true, the workload is rendered as a Kubernetes StatefulSet instead of a Deployment. Recommended for deployments with a ReadWriteOnce volume: updates terminate the old pod before creating the new one, so no Recreate strategy or node pinning is needed. The volume, unless nfs-shared or externally managed (auto false), is provisioned per replica through volumeClaimTemplates. A pre-existing PVC named after the volume (left over from a previous Deployment) is migrated automatically: a migration job streams its data into each statefulset volume through the Kubernetes API, so the volumes are never mounted by the same pod (works on multi-zone clusters); delete the legacy PVC once migrated. | [optional] +**storage_class** | **str** | Default storage class of the deployment volume claim, used when the volume does not define its own `storageClass`. Set to null to omit the storage class from the claim, so that the cluster default storage class is used. | [optional] +**statefulset** | **bool** | When true, the workload is rendered as a Kubernetes StatefulSet instead of a Deployment. Recommended for deployments with a ReadWriteOnce volume: updates terminate the old pod before creating the new one, so no Recreate strategy or node pinning is needed. The volume, unless ReadWriteMany or externally managed (auto false), is provisioned per replica through volumeClaimTemplates. A pre-existing PVC named after the volume (left over from a previous Deployment) is migrated automatically: a migration job streams its data into each statefulset volume through the Kubernetes API, so the volumes are never mounted by the same pod (works on multi-zone clusters); delete the legacy PVC once migrated. | [optional] **network** | [**NetworkConfig**](NetworkConfig.md) | | [optional] **extra_containers** | [**Dict[str, ExtraContainerConfig]**](ExtraContainerConfig.md) | Extra containers (init containers and sidecars) for the deployment. Each key is a container name mapping to an ExtraContainerConfig. | [optional] diff --git a/docs/model/DeploymentVolumeSpec.md b/docs/model/DeploymentVolumeSpec.md index a22a912ba..8b2726a50 100644 --- a/docs/model/DeploymentVolumeSpec.md +++ b/docs/model/DeploymentVolumeSpec.md @@ -10,7 +10,9 @@ Name | Type | Description | Notes **name** | **str** | | [optional] **mountpath** | **str** | The mount path for the volume | **size** | **object** | The volume size. E.g. 5Gi | [optional] -**usenfs** | **bool** | Set to `true` to use the nfs on the created volume and mount as ReadWriteMany. | [optional] +**usenfs** | **bool** | Deprecated: use `writeMany` with the nfs storage class instead. Set to `true` to use the nfs on the created volume and mount as ReadWriteMany. | [optional] +**write_many** | **bool** | Set to `true` to create and mount the volume as ReadWriteMany. ReadWriteMany volumes attach to several nodes at once, hence pods using them are not pinned to the volume's node. Requires a storage class supporting ReadWriteMany: set `storageClass`, unless the cluster default one supports it. | [optional] +**storage_class** | **str** | The storage class used to create the volume claim. Overrides the deployment default (`harness.deployment.storageClass`). | [optional] ## Example diff --git a/libraries/cloudharness-common/cloudharness/workflows/operations.py b/libraries/cloudharness-common/cloudharness/workflows/operations.py index d97ea5919..e1297e13f 100644 --- a/libraries/cloudharness-common/cloudharness/workflows/operations.py +++ b/libraries/cloudharness-common/cloudharness/workflows/operations.py @@ -11,7 +11,7 @@ from cloudharness.utils import env, config from . import argo_service from .tasks import Task, SendResultTask, CustomTask -from .utils import PodExecutionContext, affinity_spec, is_accounts_present, name_from_path, volume_mount_template +from .utils import PodExecutionContext, affinity_spec, is_accounts_present, name_from_path, volume_mount_template, volume_requires_affinity from argo.workflows.client import V1Toleration POLLING_WAIT_SECONDS = 1 @@ -97,8 +97,8 @@ def __init__(self, basename: str, pod_context: Union[PodExecutionContext, list, else: self.volumes = tuple() - self.pod_contexts += [PodExecutionContext('usesvolume', v.split(':')[0], True) for v in self.volumes if - ':' in v and (len(v.split(':')) < 3 or v.split(':')[2] != "rwx")] + self.pod_contexts += [PodExecutionContext('usesvolume', v.split(':')[0], True) + for v in self.volumes if volume_requires_affinity(v)] def task_list(self) -> List[Task]: raise NotImplementedError() diff --git a/libraries/cloudharness-common/cloudharness/workflows/utils.py b/libraries/cloudharness-common/cloudharness/workflows/utils.py index f65d65917..28c3deb18 100644 --- a/libraries/cloudharness-common/cloudharness/workflows/utils.py +++ b/libraries/cloudharness-common/cloudharness/workflows/utils.py @@ -2,6 +2,7 @@ from cloudharness import applications from cloudharness.events.client import EventClient +from cloudharness.utils.config import CloudharnessConfig from cloudharness.utils.env import get_variable WORKFLOW_NAME_VARIABLE_NAME = "CH_WORKFLOW_NAME" @@ -28,8 +29,43 @@ def get_workflow_name(): return name[0:-len(remove) - 1] +def deployment_volumes(): + """Yields the volume specs of all the application (and sub-application) deployments""" + def walk(configurations: dict): + for name, configuration in configurations.items(): + if name == 'harness' or not isinstance(configuration, dict): + continue + harness = configuration.get('harness') + if isinstance(harness, dict): + volume = (harness.get('deployment') or {}).get('volume') + if volume: + yield volume + yield from walk(configuration) + + yield from walk(CloudharnessConfig.get_configuration().get('apps') or {}) + + +def volume_is_write_many(claim_name): + """Tells whether the claim belongs to an application volume declared ReadWriteMany + (`harness.deployment.volume.writeMany`, or the legacy `usenfs` flag)""" + for volume in deployment_volumes(): + if volume.get('name') == claim_name: + return bool(volume.get('usenfs') or volume.get('writeMany')) + return False + + def volume_requires_affinity(v): - return ':' in v and 'rwx' not in v[-4:] + """Tells whether a volume mount (`claim:path[:mode]`) requires pod affinity. + + Pods sharing a ReadWriteOnce volume must all run on the volume's node. ReadWriteMany + volumes attach to several nodes at once, hence require no affinity: they are recognized + either from the application declaring the volume, or from the explicit `rwx` mount mode. + A mount without a claim prefix is provisioned for the workflow itself and is not matched + by the `usesvolume` affinity. + """ + if ':' not in v or 'rwx' in v[-4:]: + return False + return not volume_is_write_many(v.split(':')[0]) def get_shared_directory(): diff --git a/libraries/cloudharness-common/tests/test_workflow.py b/libraries/cloudharness-common/tests/test_workflow.py index 61b85171f..47d70c7d4 100644 --- a/libraries/cloudharness-common/tests/test_workflow.py +++ b/libraries/cloudharness-common/tests/test_workflow.py @@ -31,6 +31,15 @@ def test_volume_affinity_check(): assert utils.volume_requires_affinity("a:b") assert utils.volume_requires_affinity("a:b:ro") assert not utils.volume_requires_affinity("a:b:rwx") + # ReadWriteMany application volumes are recognized from the configuration + assert utils.volume_requires_affinity("my-shared-volume:/tmp/myvolume") + assert not utils.volume_requires_affinity("volumemanager-files:/tmp/myvolume") + + +def test_volume_is_write_many(): + assert not utils.volume_is_write_many("unknown-volume") + assert not utils.volume_is_write_many("my-shared-volume") + assert utils.volume_is_write_many("volumemanager-files") def test_sync_workflow(): @@ -200,6 +209,31 @@ def test_single_task_shared_rwx(): assert not 'affinity' in wf['spec'], "Pod affinity should not be added for rwx volumes" +def test_single_task_shared_application_rwx_volume(): + """A ReadWriteMany application volume attaches to any node: no pinning, no `rwx` marker needed""" + shared_directory = 'volumemanager-files:/mnt/shared' + task_write = operations.CustomTask('download-file', 'workflows-extract-download', + url='https://raw.githubusercontent.com/openworm/org.geppetto/master/README.md') + op = operations.SingleTaskOperation('test-custom-connected-op-', task_write, + shared_directory=shared_directory, shared_volume_size=100) + wf = op.to_workflow() + + accounts_offset = 1 if is_accounts_present() else 0 + assert wf['spec']['volumes'][1 + accounts_offset]['persistentVolumeClaim']['claimName'] == 'volumemanager-files' + assert 'affinity' not in wf['spec'], "Pod affinity should not be added for ReadWriteMany volumes" + assert 'usesvolume' not in wf['spec']['templates'][0]['metadata']['labels'] + + +def test_task_volume_mount_application_rwx_volume(): + """Task level volume mounts of a ReadWriteMany application volume are not pinned either""" + task = operations.CustomTask('download-file', 'workflows-extract-download', + volume_mounts=["volumemanager-files:/mnt/shared", "my-shared-volume:/mnt/other"], + url='https://raw.githubusercontent.com/openworm/org.geppetto/master/README.md') + assert task.external_volumes == ["my-shared-volume"] + assert task.metadata_spec()['labels']['usesvolume'] == 'my-shared-volume' + assert 'usesvolume-volumemanager-files' not in task.metadata_spec()['labels'] + + def test_single_task_volume_notshared(): task_write = operations.CustomTask('download-file', 'workflows-extract-download', volume_mounts=["a:b"], diff --git a/libraries/cloudharness-common/tests/values.yaml b/libraries/cloudharness-common/tests/values.yaml index ec96b5632..ce1561ecb 100644 --- a/libraries/cloudharness-common/tests/values.yaml +++ b/libraries/cloudharness-common/tests/values.yaml @@ -30,6 +30,12 @@ apps: image: cloudharness/volumemanager:latest name: volumemanager port: 8080 + volume: + name: volumemanager-files + mountpath: /usr/src/app/files + auto: true + size: 10Mi + writeMany: true resources: &id001 requests: memory: 32Mi @@ -232,6 +238,11 @@ apps: image: cloudharness/samples:latest name: samples port: 8080 + volume: + name: my-shared-volume + mountpath: /tmp/myvolume + auto: true + size: 10Mi resources: &id003 requests: memory: 32Mi diff --git a/libraries/models/api/openapi.yaml b/libraries/models/api/openapi.yaml index 18c495ba4..d683d50a3 100644 --- a/libraries/models/api/openapi.yaml +++ b/libraries/models/api/openapi.yaml @@ -684,6 +684,14 @@ components: description: Specify database disk size type: string example: 1Gi + storageClass: + description: |- + Storage class of the database volume claim. + + Set to null to omit the storage class from the claim, so that the cluster + default storage class is used. + type: string + nullable: true user: description: database username type: string @@ -813,8 +821,26 @@ components: E.g. 5Gi usenfs: - description: Set to `true` to use the nfs on the created volume and mount as ReadWriteMany. + description: |- + Deprecated: use `writeMany` with the nfs storage class instead. + + Set to `true` to use the nfs on the created volume and mount as ReadWriteMany. + type: boolean + writeMany: + description: |- + Set to `true` to create and mount the volume as ReadWriteMany. + + ReadWriteMany volumes attach to several nodes at once, hence pods using them + are not pinned to the volume's node. + Requires a storage class supporting ReadWriteMany: set `storageClass`, unless + the cluster default one supports it. type: boolean + storageClass: + description: |- + The storage class used to create the volume claim. + + Overrides the deployment default (`harness.deployment.storageClass`). + type: string - $ref: '#/components/schemas/AutoArtifactSpec' example: @@ -822,7 +848,8 @@ components: mountpath: /usr/src/app/persistent name: my-files size: 5Gi - usenfs: true + writeMany: true + storageClass: efs-sc CDCEvent: description: |- A message sent to the orchestration queue. @@ -1322,12 +1349,21 @@ components: volume: $ref: '#/components/schemas/DeploymentVolumeSpec' description: Volume specification + storageClass: + description: |- + Default storage class of the deployment volume claim, used when the volume + does not define its own `storageClass`. + + Set to null to omit the storage class from the claim, so that the cluster + default storage class is used. + type: string + nullable: true statefulset: description: >- When true, the workload is rendered as a Kubernetes StatefulSet instead of a Deployment. Recommended for deployments with a ReadWriteOnce volume: updates terminate the old pod before creating the new one, so no Recreate strategy or - node pinning is needed. The volume, unless nfs-shared or externally managed + node pinning is needed. The volume, unless ReadWriteMany or externally managed (auto false), is provisioned per replica through volumeClaimTemplates. A pre-existing PVC named after the volume (left over from a previous Deployment) is migrated automatically: a migration job streams its data into each diff --git a/libraries/models/cloudharness_model/models/database_deployment_config.py b/libraries/models/cloudharness_model/models/database_deployment_config.py index 1f8ada043..76a219504 100644 --- a/libraries/models/cloudharness_model/models/database_deployment_config.py +++ b/libraries/models/cloudharness_model/models/database_deployment_config.py @@ -35,6 +35,7 @@ class DatabaseDeploymentConfig(CloudHarnessBaseModel): name: Optional[StrictStr] = None type: Optional[Annotated[str, Field(strict=True)]] = Field(default=None, description="Define the database type. One of (mongo, postgres, neo4j, sqlite3)") size: Optional[StrictStr] = Field(default=None, description="Specify database disk size") + storage_class: Optional[StrictStr] = Field(default=None, description="Storage class of the database volume claim. Set to null to omit the storage class from the claim, so that the cluster default storage class is used.", alias="storageClass") user: Optional[StrictStr] = Field(default=None, description="database username") var_pass: Optional[StrictStr] = Field(default=None, description="Database password", alias="pass") image_ref: Optional[StrictStr] = Field(default=None, description="Used for referencing images from the build") @@ -45,7 +46,7 @@ class DatabaseDeploymentConfig(CloudHarnessBaseModel): resources: Optional[DeploymentResourcesConf] = None connect_string: Optional[StrictStr] = Field(default=None, description="Specify if the database is external. If not null, auto deployment if set will not be used. Leave it as an empty string and the connect string will be provided as a secret to be provided at CI/CD (recommended)") additional_properties: Dict[str, Any] = {} - __properties: ClassVar[List[str]] = ["auto", "name", "type", "size", "user", "pass", "image_ref", "statefulset", "mongo", "postgres", "neo4j", "resources", "connect_string"] + __properties: ClassVar[List[str]] = ["auto", "name", "type", "size", "storageClass", "user", "pass", "image_ref", "statefulset", "mongo", "postgres", "neo4j", "resources", "connect_string"] @field_validator('type') def type_validate_regular_expression(cls, value): @@ -85,6 +86,11 @@ def to_dict(self) -> Dict[str, Any]: for _key, _value in self.additional_properties.items(): _dict[_key] = _value + # set to None if storage_class (nullable) is None + # and model_fields_set contains the field + if self.storage_class is None and "storage_class" in self.model_fields_set: + _dict['storageClass'] = None + # set to None if neo4j (nullable) is None # and model_fields_set contains the field if self.neo4j is None and "neo4j" in self.model_fields_set: @@ -106,6 +112,7 @@ def from_dict(cls, obj: Optional[Dict[str, Any]]) -> Optional[Self]: "name": obj.get("name"), "type": obj.get("type"), "size": obj.get("size"), + "storageClass": obj.get("storageClass"), "user": obj.get("user"), "pass": obj.get("pass"), "image_ref": obj.get("image_ref"), diff --git a/libraries/models/cloudharness_model/models/deployment_auto_artifact_config.py b/libraries/models/cloudharness_model/models/deployment_auto_artifact_config.py index ed08a588a..a4724415c 100644 --- a/libraries/models/cloudharness_model/models/deployment_auto_artifact_config.py +++ b/libraries/models/cloudharness_model/models/deployment_auto_artifact_config.py @@ -41,11 +41,12 @@ class DeploymentAutoArtifactConfig(CloudHarnessBaseModel): image: Optional[Annotated[str, Field(strict=True)]] = Field(default=None, description="Image name to use in the deployment. Leave it blank to set from the application's Docker file") resources: Optional[DeploymentResourcesConf] = None volume: Optional[DeploymentVolumeSpec] = None - statefulset: Optional[StrictBool] = Field(default=None, description="When true, the workload is rendered as a Kubernetes StatefulSet instead of a Deployment. Recommended for deployments with a ReadWriteOnce volume: updates terminate the old pod before creating the new one, so no Recreate strategy or node pinning is needed. The volume, unless nfs-shared or externally managed (auto false), is provisioned per replica through volumeClaimTemplates. A pre-existing PVC named after the volume (left over from a previous Deployment) is migrated automatically: a migration job streams its data into each statefulset volume through the Kubernetes API, so the volumes are never mounted by the same pod (works on multi-zone clusters); delete the legacy PVC once migrated.") + storage_class: Optional[StrictStr] = Field(default=None, description="Default storage class of the deployment volume claim, used when the volume does not define its own `storageClass`. Set to null to omit the storage class from the claim, so that the cluster default storage class is used.", alias="storageClass") + statefulset: Optional[StrictBool] = Field(default=None, description="When true, the workload is rendered as a Kubernetes StatefulSet instead of a Deployment. Recommended for deployments with a ReadWriteOnce volume: updates terminate the old pod before creating the new one, so no Recreate strategy or node pinning is needed. The volume, unless ReadWriteMany or externally managed (auto false), is provisioned per replica through volumeClaimTemplates. A pre-existing PVC named after the volume (left over from a previous Deployment) is migrated automatically: a migration job streams its data into each statefulset volume through the Kubernetes API, so the volumes are never mounted by the same pod (works on multi-zone clusters); delete the legacy PVC once migrated.") network: Optional[NetworkConfig] = None extra_containers: Optional[Dict[str, ExtraContainerConfig]] = Field(default=None, description="Extra containers (init containers and sidecars) for the deployment. Each key is a container name mapping to an ExtraContainerConfig.", alias="extraContainers") additional_properties: Dict[str, Any] = {} - __properties: ClassVar[List[str]] = ["auto", "name", "port", "replicas", "image", "resources", "volume", "statefulset", "network", "extraContainers"] + __properties: ClassVar[List[str]] = ["auto", "name", "port", "replicas", "image", "resources", "volume", "storageClass", "statefulset", "network", "extraContainers"] @field_validator('image') def image_validate_regular_expression(cls, value): @@ -83,6 +84,10 @@ def to_dict(self) -> Dict[str, Any]: # override the default output from pydantic by calling `to_dict()` of volume if self.volume: _dict['volume'] = self.volume.to_dict() + # set to None if storage_class (nullable) is None + # and model_fields_set contains the field + if self.storage_class is None and "storage_class" in self.model_fields_set: + _dict['storageClass'] = None # override the default output from pydantic by calling `to_dict()` of network if self.network: _dict['network'] = self.network.to_dict() @@ -122,6 +127,7 @@ def from_dict(cls, obj: Optional[Dict[str, Any]]) -> Optional[Self]: "image": obj.get("image"), "resources": DeploymentResourcesConf.from_dict(obj["resources"]) if obj.get("resources") is not None else None, "volume": DeploymentVolumeSpec.from_dict(obj["volume"]) if obj.get("volume") is not None else None, + "storageClass": obj.get("storageClass"), "statefulset": obj.get("statefulset"), "network": NetworkConfig.from_dict(obj["network"]) if obj.get("network") is not None else None, "extraContainers": dict( diff --git a/libraries/models/cloudharness_model/models/deployment_volume_spec.py b/libraries/models/cloudharness_model/models/deployment_volume_spec.py index e38b01871..ff268ea77 100644 --- a/libraries/models/cloudharness_model/models/deployment_volume_spec.py +++ b/libraries/models/cloudharness_model/models/deployment_volume_spec.py @@ -34,9 +34,11 @@ class DeploymentVolumeSpec(CloudHarnessBaseModel): name: Optional[StrictStr] = None mountpath: StrictStr = Field(description="The mount path for the volume") size: Optional[Any] = Field(default=None, description="The volume size. E.g. 5Gi") - usenfs: Optional[StrictBool] = Field(default=None, description="Set to `true` to use the nfs on the created volume and mount as ReadWriteMany.") + usenfs: Optional[StrictBool] = Field(default=None, description="Deprecated: use `writeMany` with the nfs storage class instead. Set to `true` to use the nfs on the created volume and mount as ReadWriteMany.") + write_many: Optional[StrictBool] = Field(default=None, description="Set to `true` to create and mount the volume as ReadWriteMany. ReadWriteMany volumes attach to several nodes at once, hence pods using them are not pinned to the volume's node. Requires a storage class supporting ReadWriteMany: set `storageClass`, unless the cluster default one supports it.", alias="writeMany") + storage_class: Optional[StrictStr] = Field(default=None, description="The storage class used to create the volume claim. Overrides the deployment default (`harness.deployment.storageClass`).", alias="storageClass") additional_properties: Dict[str, Any] = {} - __properties: ClassVar[List[str]] = ["auto", "name", "mountpath", "size", "usenfs"] + __properties: ClassVar[List[str]] = ["auto", "name", "mountpath", "size", "usenfs", "writeMany", "storageClass"] def to_dict(self) -> Dict[str, Any]: """Return the dictionary representation of the model using alias. @@ -84,7 +86,9 @@ def from_dict(cls, obj: Optional[Dict[str, Any]]) -> Optional[Self]: "name": obj.get("name"), "mountpath": obj.get("mountpath"), "size": obj.get("size"), - "usenfs": obj.get("usenfs") + "usenfs": obj.get("usenfs"), + "writeMany": obj.get("writeMany"), + "storageClass": obj.get("storageClass") }) # store additional fields in additional_properties for _key in obj.keys(): diff --git a/tools/deployment-cli-tools/ch_cli_tools/configurationgenerator.py b/tools/deployment-cli-tools/ch_cli_tools/configurationgenerator.py index b8193b3d9..85fb9506e 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/configurationgenerator.py +++ b/tools/deployment-cli-tools/ch_cli_tools/configurationgenerator.py @@ -663,6 +663,27 @@ class ValuesValidationException(Exception): def validate_helm_values(values): validate_dependencies(values) validate_secrets(values) + validate_volumes(values) + + +def validate_volumes(values): + """Warns when a volume configuration collides with the nfs server settings. + + On an `usenfs` volume the nfs server storage class and its ReadWriteMany access mode always + prevail: any storage class or access mode set on the volume itself is ignored. + """ + for app, app_values in values["apps"].items(): + volume = (app_values[KEY_HARNESS].get(KEY_DEPLOYMENT) or {}).get("volume") or {} + if not volume.get("usenfs"): + continue + if volume.get("storageClass"): + logging.warning( + f"Volume {volume.get('name')} of application {app} sets usenfs and storageClass " + f"{volume['storageClass']}: the nfs server storage class prevails.") + if volume.get("writeMany") is False: + logging.warning( + f"Volume {volume.get('name')} of application {app} sets usenfs and writeMany false: " + "nfs volumes are always mounted ReadWriteMany.") def validate_secrets(values): diff --git a/tools/deployment-cli-tools/tests/test_helm.py b/tools/deployment-cli-tools/tests/test_helm.py index fb18d81d5..a88f1acfb 100644 --- a/tools/deployment-cli-tools/tests/test_helm.py +++ b/tools/deployment-cli-tools/tests/test_helm.py @@ -2,6 +2,7 @@ from ch_cli_tools.configurationgenerator import * from ch_cli_tools import configurationgenerator from ch_cli_tools.preprocessing import preprocess_build_overrides, generate_hash_based_image_tags +import logging import pytest import shutil import subprocess @@ -524,6 +525,215 @@ def test_statefulset_option(tmp_path): assert 'myapp-data' in claims +def test_volume_write_many(tmp_path): + out_folder = tmp_path / 'test_volume_write_many' + # nfsserver is deliberately not included: a ReadWriteMany volume must not rely on it + create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, domain="my.local", + env='withpostgres', local=False, include=["myapp"], exclude=["legacy"]) + + helm_path = out_folder / HELM_CHART_PATH + shutil.rmtree(helm_path / 'charts') + values_path = helm_path / 'values.yaml' + with open(values_path, 'r') as values_file: + values = yaml.safe_load(values_file) + + harness = values['apps']['myapp']['harness'] + dep_name = harness['deployment']['name'] + + harness['deployment']['auto'] = True + volume = {'name': 'myapp-data', 'mountpath': '/data', 'size': '1Gi', 'auto': True} + harness['deployment']['volume'] = volume + + def render(): + with open(values_path, 'w') as values_file: + yaml.safe_dump(values, values_file) + return render_helm_chart(helm_path) + + # `standard` is the deployment default, coming from the application values + assert harness['deployment']['storageClass'] == 'standard' + manifests = render() + pvc = find_manifest(manifests, 'PersistentVolumeClaim', 'myapp-data') + assert pvc['spec']['storageClassName'] == 'standard' + + # a null default omits the storage class, so the cluster default one is used + harness['deployment']['storageClass'] = None + manifests = render() + pvc = find_manifest(manifests, 'PersistentVolumeClaim', 'myapp-data') + assert 'storageClassName' not in pvc['spec'] + + # the deployment default applies to the volume claim of every deployment volume + harness['deployment']['storageClass'] = 'gp2' + manifests = render() + pvc = find_manifest(manifests, 'PersistentVolumeClaim', 'myapp-data') + assert pvc['spec']['storageClassName'] == 'gp2' + + # the volume storage class overrides the deployment default; ReadWriteOnce volumes keep + # the node pinning + volume['storageClass'] = 'gp3' + manifests = render() + pvc = find_manifest(manifests, 'PersistentVolumeClaim', 'myapp-data') + assert pvc['spec']['accessModes'] == ['ReadWriteOnce'] + assert pvc['spec']['storageClassName'] == 'gp3' + dep = find_manifest(manifests, 'Deployment', dep_name) + assert dep['spec']['strategy']['type'] == 'Recreate' + assert 'affinity' in dep['spec']['template']['spec'] + + # writeMany without a volume storage class inherits the deployment default (`gp2` here), and + # the pod is neither pinned to a node nor recreated on update + volume.pop('storageClass') + volume['writeMany'] = True + manifests = render() + pvc = find_manifest(manifests, 'PersistentVolumeClaim', 'myapp-data') + assert pvc['spec']['accessModes'] == ['ReadWriteMany'] + assert pvc['spec']['storageClassName'] == 'gp2' + dep = find_manifest(manifests, 'Deployment', dep_name) + assert 'strategy' not in dep['spec'] + assert 'affinity' not in dep['spec']['template']['spec'] + + # writeMany with an explicit ReadWriteMany capable storage class + volume['storageClass'] = 'efs-sc' + manifests = render() + pvc = find_manifest(manifests, 'PersistentVolumeClaim', 'myapp-data') + assert pvc['spec']['accessModes'] == ['ReadWriteMany'] + assert pvc['spec']['storageClassName'] == 'efs-sc' + + # a null deployment default omits the storage class from a ReadWriteMany claim too + volume.pop('storageClass') + harness['deployment']['storageClass'] = None + manifests = render() + pvc = find_manifest(manifests, 'PersistentVolumeClaim', 'myapp-data') + assert 'storageClassName' not in pvc['spec'] + harness['deployment']['storageClass'] = 'standard' + volume['storageClass'] = 'efs-sc' + + # ReadWriteMany volumes are shared: a statefulset keeps mounting the common PVC by + # claimName instead of provisioning one per replica + harness['deployment']['statefulset'] = True + manifests = render() + sts = find_manifest(manifests, 'StatefulSet', dep_name) + assert 'volumeClaimTemplates' not in sts['spec'] + claims = [v['persistentVolumeClaim']['claimName'] + for v in sts['spec']['template']['spec']['volumes'] if 'persistentVolumeClaim' in v] + assert 'myapp-data' in claims + pvc = find_manifest(manifests, 'PersistentVolumeClaim', 'myapp-data') + assert pvc['spec']['accessModes'] == ['ReadWriteMany'] + + # the storage class of a per-replica statefulset volume is configurable too + volume['writeMany'] = False + volume['storageClass'] = 'gp3' + manifests = render() + sts = find_manifest(manifests, 'StatefulSet', dep_name) + claim_template = sts['spec']['volumeClaimTemplates'][0] + assert claim_template['metadata']['name'] == 'myapp-data' + assert claim_template['spec']['accessModes'] == ['ReadWriteOnce'] + assert claim_template['spec']['storageClassName'] == 'gp3' + + +def test_volume_usenfs_prevails(tmp_path): + out_folder = tmp_path / 'test_volume_usenfs_prevails' + create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, domain="my.local", + env='withpostgres', local=False, include=["myapp", "nfsserver"], exclude=["legacy"]) + + helm_path = out_folder / HELM_CHART_PATH + shutil.rmtree(helm_path / 'charts') + values_path = helm_path / 'values.yaml' + with open(values_path, 'r') as values_file: + values = yaml.safe_load(values_file) + + harness = values['apps']['myapp']['harness'] + harness['deployment']['auto'] = True + # colliding settings: the nfs server storage class and access mode prevail + harness['deployment']['storageClass'] = 'gp2' + harness['deployment']['volume'] = { + 'name': 'myapp-data', 'mountpath': '/data', 'size': '1Gi', 'auto': True, + 'usenfs': True, 'writeMany': False, 'storageClass': 'efs-sc', + } + with open(values_path, 'w') as values_file: + yaml.safe_dump(values, values_file) + + manifests = render_helm_chart(helm_path) + pvc = find_manifest(manifests, 'PersistentVolumeClaim', 'myapp-data') + nfs_class = f"{values['namespace']}-{values['apps']['nfsserver']['storageClass']['name']}" + assert pvc['spec']['storageClassName'] == nfs_class + assert pvc['spec']['accessModes'] == ['ReadWriteMany'] + dep = find_manifest(manifests, 'Deployment', harness['deployment']['name']) + assert 'affinity' not in dep['spec']['template']['spec'] + + +def test_validate_volumes_warns_on_nfs_collisions(caplog): + volume = {'name': 'myapp-data', 'usenfs': True, 'writeMany': False, 'storageClass': 'efs-sc'} + values = {'apps': {'myapp': {KEY_HARNESS: {'deployment': {'volume': volume}}}}} + + with caplog.at_level(logging.WARNING): + validate_volumes(values) + assert 'the nfs server storage class prevails' in caplog.text + assert 'always mounted ReadWriteMany' in caplog.text + + # no collision: nothing to warn about + caplog.clear() + with caplog.at_level(logging.WARNING): + validate_volumes({'apps': {'myapp': {KEY_HARNESS: {'deployment': {'volume': { + 'name': 'myapp-data', 'usenfs': True, 'writeMany': True}}}}}}) + validate_volumes({'apps': {'myapp': {KEY_HARNESS: {'deployment': {'volume': { + 'name': 'myapp-data', 'storageClass': 'efs-sc', 'writeMany': True}}}}}}) + validate_volumes({'apps': {'myapp': {KEY_HARNESS: {'deployment': {}}}}}) + assert not caplog.text + + +def test_database_storage_class(tmp_path): + out_folder = tmp_path / 'test_database_storage_class' + create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, domain="my.local", + env='withpostgres', local=False, include=["myapp"], exclude=["legacy"]) + + helm_path = out_folder / HELM_CHART_PATH + shutil.rmtree(helm_path / 'charts') + values_path = helm_path / 'values.yaml' + with open(values_path, 'r') as values_file: + values = yaml.safe_load(values_file) + + database = values['apps']['myapp']['harness']['database'] + db_name = database['name'] + + def render(): + with open(values_path, 'w') as values_file: + yaml.safe_dump(values, values_file) + return render_helm_chart(helm_path) + + # `standard` is the default, coming from the application values + assert database['storageClass'] == 'standard' + manifests = render() + assert find_manifest(manifests, 'PersistentVolumeClaim', db_name)['spec']['storageClassName'] == 'standard' + + # a null storage class is omitted, so the cluster default one is used + database['storageClass'] = None + manifests = render() + assert 'storageClassName' not in find_manifest(manifests, 'PersistentVolumeClaim', db_name)['spec'] + + database['storageClass'] = 'gp3' + manifests = render() + assert find_manifest(manifests, 'PersistentVolumeClaim', db_name)['spec']['storageClassName'] == 'gp3' + + # statefulset databases provision their volume through volumeClaimTemplates + database['statefulset'] = True + manifests = render() + sts = find_manifest(manifests, 'StatefulSet', db_name) + assert sts['spec']['volumeClaimTemplates'][0]['spec']['storageClassName'] == 'gp3' + database['storageClass'] = None + manifests = render() + sts = find_manifest(manifests, 'StatefulSet', db_name) + assert 'storageClassName' not in sts['spec']['volumeClaimTemplates'][0]['spec'] + + # the postgres operator cluster storage honours the same setting + database['statefulset'] = False + database['postgres']['operator'] = True + database['storageClass'] = 'gp3' + manifests = render() + assert find_manifest(manifests, 'Cluster', db_name)['spec']['storage']['storageClass'] == 'gp3' + database['storageClass'] = None + manifests = render() + assert 'storageClass' not in find_manifest(manifests, 'Cluster', db_name)['spec']['storage'] + + def test_statefulset_leader_service(tmp_path): out_folder = tmp_path / 'test_statefulset_leader_service' create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, domain="my.local", From 4233bca59e7b9a5ab7db80e41e17ba5ec12ba0a8 Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Mon, 31 Aug 2026 16:34:58 +0200 Subject: [PATCH 02/15] CH-288 linting fix --- libraries/cloudharness-common/tests/test_workflow.py | 2 +- tools/deployment-cli-tools/tests/test_helm.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/libraries/cloudharness-common/tests/test_workflow.py b/libraries/cloudharness-common/tests/test_workflow.py index 47d70c7d4..83eddb8e8 100644 --- a/libraries/cloudharness-common/tests/test_workflow.py +++ b/libraries/cloudharness-common/tests/test_workflow.py @@ -215,7 +215,7 @@ def test_single_task_shared_application_rwx_volume(): task_write = operations.CustomTask('download-file', 'workflows-extract-download', url='https://raw.githubusercontent.com/openworm/org.geppetto/master/README.md') op = operations.SingleTaskOperation('test-custom-connected-op-', task_write, - shared_directory=shared_directory, shared_volume_size=100) + shared_directory=shared_directory, shared_volume_size=100) wf = op.to_workflow() accounts_offset = 1 if is_accounts_present() else 0 diff --git a/tools/deployment-cli-tools/tests/test_helm.py b/tools/deployment-cli-tools/tests/test_helm.py index a88f1acfb..c08ed197e 100644 --- a/tools/deployment-cli-tools/tests/test_helm.py +++ b/tools/deployment-cli-tools/tests/test_helm.py @@ -481,7 +481,7 @@ def test_statefulset_option(tmp_path): assert 'myapp-data' not in claims assert sts['spec']['volumeClaimTemplates'][0]['metadata']['name'] == 'myapp-data' assert not any(m for m in manifests - if m.get('kind') == 'PersistentVolumeClaim' and m.get('metadata', {}).get('name') == 'myapp-data') + if m.get('kind') == 'PersistentVolumeClaim' and m.get('metadata', {}).get('name') == 'myapp-data') db_sts = find_manifest(manifests, 'StatefulSet', db_name) assert db_sts['spec']['serviceName'] == db_name @@ -490,7 +490,7 @@ def test_statefulset_option(tmp_path): assert 'initContainers' not in db_sts['spec']['template']['spec'] assert db_sts['spec']['volumeClaimTemplates'][0]['metadata']['name'] == db_name assert not any(m for m in manifests - if m.get('kind') == 'PersistentVolumeClaim' and m.get('metadata', {}).get('name') == db_name) + if m.get('kind') == 'PersistentVolumeClaim' and m.get('metadata', {}).get('name') == db_name) find_manifest(manifests, 'Service', db_name) # without a legacy PVC no migration resources are rendered assert not any(m for m in manifests if 'volume-migration' in m.get('metadata', {}).get('name', '')) @@ -766,9 +766,9 @@ def ingress_paths(manifests): manifests = render_helm_chart(helm_path) assert not any(m for m in manifests - if m.get('kind') == 'Service' and m.get('metadata', {}).get('name') == rw_name) + if m.get('kind') == 'Service' and m.get('metadata', {}).get('name') == rw_name) assert not any(p for p in ingress_paths(manifests) - if p['backend']['service']['name'] == rw_name) + if p['backend']['service']['name'] == rw_name) harness['deployment']['statefulset'] = True with open(values_path, 'w') as values_file: From 7d254964ddfcf86d8ec1198083d3a6d21e101c6b Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Mon, 31 Aug 2026 16:42:58 +0200 Subject: [PATCH 03/15] CH-288 remove unnecessary configuration level --- .../helm/templates/_helpers.tpl | 27 +++++++++---------- .../helm/templates/auto-deployments.yaml | 2 +- .../helm/templates/auto-volumes.yaml | 2 +- deployment-configuration/value-template.yaml | 8 +++--- docs/applications/volumes.md | 23 +++++++--------- docs/model/DeploymentAutoArtifactConfig.md | 1 - docs/model/DeploymentVolumeSpec.md | 2 +- libraries/models/api/openapi.yaml | 16 ++++------- .../models/deployment_auto_artifact_config.py | 8 +----- .../models/deployment_volume_spec.py | 7 ++++- tools/deployment-cli-tools/tests/test_helm.py | 27 ++++++------------- 11 files changed, 50 insertions(+), 73 deletions(-) diff --git a/deployment-configuration/helm/templates/_helpers.tpl b/deployment-configuration/helm/templates/_helpers.tpl index de1099729..78edc3d88 100644 --- a/deployment-configuration/helm/templates/_helpers.tpl +++ b/deployment-configuration/helm/templates/_helpers.tpl @@ -106,21 +106,20 @@ Usage: {{ if include "deploy_utils.volumeWriteMany" $volume }} {{/* Storage class of a harness.deployment.volume claim: nfs volumes always use the class created by -the nfsserver application, otherwise the volume `storageClass` wins on the deployment default -(harness.deployment.storageClass). A null default renders nothing, leaving the claim to the -cluster default storage class; `standard` is used when the deployment does not declare the key -at all (values generated before the setting existed). -Usage: {{ include "deploy_utils.volumeStorageClass" (dict "root" .root "deployment" $deployment) }} +the nfsserver application, otherwise the volume `storageClass`, `standard` when the volume does +not specify it. A null `storageClass` renders nothing, leaving the claim to the cluster default +storage class. +Usage: {{ include "deploy_utils.volumeStorageClass" (dict "root" .root "volume" $volume) }} */}} {{- define "deploy_utils.volumeStorageClass" -}} -{{- $volume := .deployment.volume -}} -{{- if $volume.usenfs }}{{ printf "%s-%s" .root.Values.namespace .root.Values.apps.nfsserver.storageClass.name }}{{ else if $volume.storageClass }}{{ $volume.storageClass }}{{ else if .deployment.storageClass }}{{ .deployment.storageClass }}{{ else if not (hasKey .deployment "storageClass") }}standard{{ end }} +{{- $volume := .volume -}} +{{- if $volume.usenfs }}{{ printf "%s-%s" .root.Values.namespace .root.Values.apps.nfsserver.storageClass.name }}{{ else if $volume.storageClass }}{{ $volume.storageClass }}{{ else if not (hasKey $volume "storageClass") }}standard{{ end }} {{- end -}} {{/* -Storage class of a database volume claim: harness.database.storageClass. A null value renders -nothing, leaving the claim to the cluster default storage class; `standard` is used when the -database does not declare the key at all (values generated before the setting existed). +Storage class of a database volume claim: harness.database.storageClass, `standard` when the +database does not specify it. A null value renders nothing, leaving the claim to the cluster +default storage class. Usage: {{ include "deploy_utils.databaseStorageClass" .app.harness.database }} */}} {{- define "deploy_utils.databaseStorageClass" -}} @@ -130,18 +129,18 @@ Usage: {{ include "deploy_utils.databaseStorageClass" .app.harness.database }} {{/* Render the spec of a claim (PersistentVolumeClaim or statefulset volumeClaimTemplate) for a harness.deployment.volume. -Usage: {{ include "deploy_utils.volumeClaimSpec" (dict "root" .root "deployment" $deployment) | nindent 2 }} +Usage: {{ include "deploy_utils.volumeClaimSpec" (dict "root" .root "volume" $volume) | nindent 2 }} */}} {{- define "deploy_utils.volumeClaimSpec" -}} -{{- $storageClass := include "deploy_utils.volumeStorageClass" (dict "root" .root "deployment" .deployment) -}} +{{- $storageClass := include "deploy_utils.volumeStorageClass" (dict "root" .root "volume" .volume) -}} accessModes: - - {{ if include "deploy_utils.volumeWriteMany" .deployment.volume }}ReadWriteMany{{ else }}ReadWriteOnce{{ end }} + - {{ if include "deploy_utils.volumeWriteMany" .volume }}ReadWriteMany{{ else }}ReadWriteOnce{{ end }} {{- if $storageClass }} storageClassName: {{ $storageClass }} {{- end }} resources: requests: - storage: {{ .deployment.volume.size }} + storage: {{ .volume.size }} {{- end -}} {{/* diff --git a/deployment-configuration/helm/templates/auto-deployments.yaml b/deployment-configuration/helm/templates/auto-deployments.yaml index 19a16b09f..dd98c4a84 100644 --- a/deployment-configuration/helm/templates/auto-deployments.yaml +++ b/deployment-configuration/helm/templates/auto-deployments.yaml @@ -212,7 +212,7 @@ spec: - metadata: name: {{ $volume.name }} spec: - {{- include "deploy_utils.volumeClaimSpec" (dict "root" .root "deployment" .app.harness.deployment) | nindent 8 }} + {{- include "deploy_utils.volumeClaimSpec" (dict "root" .root "volume" $volume) | nindent 8 }} {{- end }} {{- if $legacyClaim }} {{- include "deploy_utils.volumeMigration" (dict "root" .root "name" .app.harness.deployment.name "pvc" $volume.name) }} diff --git a/deployment-configuration/helm/templates/auto-volumes.yaml b/deployment-configuration/helm/templates/auto-volumes.yaml index 17a445612..f783e363f 100644 --- a/deployment-configuration/helm/templates/auto-volumes.yaml +++ b/deployment-configuration/helm/templates/auto-volumes.yaml @@ -32,7 +32,7 @@ metadata: labels: app: {{ .app.harness.deployment.name| quote }} spec: - {{- include "deploy_utils.volumeClaimSpec" (dict "root" .root "deployment" .app.harness.deployment) | nindent 2 }} + {{- include "deploy_utils.volumeClaimSpec" (dict "root" .root "volume" $volume) | nindent 2 }} {{- end }} --- {{- end }} diff --git a/deployment-configuration/value-template.yaml b/deployment-configuration/value-template.yaml index 724fc64a7..184a43607 100644 --- a/deployment-configuration/value-template.yaml +++ b/deployment-configuration/value-template.yaml @@ -36,12 +36,12 @@ harness: name: # -- Deployment port. port: 8080 - # -- Default storage class of the deployment volume claim. Set to null to use the cluster default storage class. - storageClass: standard # -- volume specification. # `writeMany: true` creates and mounts the volume as ReadWriteMany: the volume attaches to several - # nodes at once, hence pods using it are not pinned to the volume's node. Requires a storage class - # supporting ReadWriteMany, set through the volume `storageClass` (which overrides the deployment default). + # nodes at once, hence pods using it are not pinned to the volume's node. It requires a storage class + # supporting ReadWriteMany, set through `storageClass`. + # `storageClass` is the storage class of the volume claim, `standard` when not specified. Set it to + # null to use the cluster default storage class. volume: # example: # name: my-volume diff --git a/docs/applications/volumes.md b/docs/applications/volumes.md index 80ae7c95e..7296c8d7f 100644 --- a/docs/applications/volumes.md +++ b/docs/applications/volumes.md @@ -43,28 +43,25 @@ one node is available on the cluster and other affinity rules or taints are pres ### Storage class -The storage class of the volume claim is `harness.deployment.storageClass`, which defaults to -`standard`. Set it to null to omit the storage class from the claim, so that the cluster default -storage class is used: +The volume `storageClass` sets the storage class of the claim; when not specified, `standard` is +used. Set it to null to omit the storage class from the claim, so that the cluster default storage +class provisions the volume: ```yaml harness: ... deployment: - # the cluster default storage class provisions the volume - storageClass: null + ... volume: name: my-volume mountpath: /usr/src/app/myvolume auto: true size: 5Gi + storageClass: null # or e.g. gp3 ``` -The volume's own `storageClass` overrides the deployment default, and takes precedence for both -ReadWriteOnce and ReadWriteMany volumes. A null on the volume means "not set", hence inherits the -deployment default: use the deployment `storageClass: null` to provision the volume on the cluster -default class. The same setting is available for database volumes as -`harness.database.storageClass` (see [databases](databases.md)). +The same setting is available for database volumes as `harness.database.storageClass` (see +[databases](databases.md)). ### ReadWriteMany volumes @@ -73,9 +70,9 @@ attaches to several nodes at the same time, hence the pods using it are not pinn node: no podAffinity is added, and deployments roll normally instead of being recreated. ReadWriteMany requires a storage class supporting it (e.g. AWS EFS, Azure Files, CephFS, -the nfs provisioner). The class is resolved as above — `standard` is normally ReadWriteOnce only, -so set the volume `storageClass`, or the deployment default, to a ReadWriteMany capable class -(or to null when the cluster default one supports ReadWriteMany). +the nfs provisioner). The default `standard` class is normally ReadWriteOnce only, so set +`storageClass` to a ReadWriteMany capable class (or to null when the cluster default one supports +ReadWriteMany). ```yaml harness: diff --git a/docs/model/DeploymentAutoArtifactConfig.md b/docs/model/DeploymentAutoArtifactConfig.md index 3ecf3ac8c..6c5560d11 100644 --- a/docs/model/DeploymentAutoArtifactConfig.md +++ b/docs/model/DeploymentAutoArtifactConfig.md @@ -13,7 +13,6 @@ Name | Type | Description | Notes **image** | **str** | Image name to use in the deployment. Leave it blank to set from the application's Docker file | [optional] **resources** | [**DeploymentResourcesConf**](DeploymentResourcesConf.md) | | [optional] **volume** | [**DeploymentVolumeSpec**](DeploymentVolumeSpec.md) | | [optional] -**storage_class** | **str** | Default storage class of the deployment volume claim, used when the volume does not define its own `storageClass`. Set to null to omit the storage class from the claim, so that the cluster default storage class is used. | [optional] **statefulset** | **bool** | When true, the workload is rendered as a Kubernetes StatefulSet instead of a Deployment. Recommended for deployments with a ReadWriteOnce volume: updates terminate the old pod before creating the new one, so no Recreate strategy or node pinning is needed. The volume, unless ReadWriteMany or externally managed (auto false), is provisioned per replica through volumeClaimTemplates. A pre-existing PVC named after the volume (left over from a previous Deployment) is migrated automatically: a migration job streams its data into each statefulset volume through the Kubernetes API, so the volumes are never mounted by the same pod (works on multi-zone clusters); delete the legacy PVC once migrated. | [optional] **network** | [**NetworkConfig**](NetworkConfig.md) | | [optional] **extra_containers** | [**Dict[str, ExtraContainerConfig]**](ExtraContainerConfig.md) | Extra containers (init containers and sidecars) for the deployment. Each key is a container name mapping to an ExtraContainerConfig. | [optional] diff --git a/docs/model/DeploymentVolumeSpec.md b/docs/model/DeploymentVolumeSpec.md index 8b2726a50..cdafce4d0 100644 --- a/docs/model/DeploymentVolumeSpec.md +++ b/docs/model/DeploymentVolumeSpec.md @@ -12,7 +12,7 @@ Name | Type | Description | Notes **size** | **object** | The volume size. E.g. 5Gi | [optional] **usenfs** | **bool** | Deprecated: use `writeMany` with the nfs storage class instead. Set to `true` to use the nfs on the created volume and mount as ReadWriteMany. | [optional] **write_many** | **bool** | Set to `true` to create and mount the volume as ReadWriteMany. ReadWriteMany volumes attach to several nodes at once, hence pods using them are not pinned to the volume's node. Requires a storage class supporting ReadWriteMany: set `storageClass`, unless the cluster default one supports it. | [optional] -**storage_class** | **str** | The storage class used to create the volume claim. Overrides the deployment default (`harness.deployment.storageClass`). | [optional] +**storage_class** | **str** | The storage class used to create the volume claim. Defaults to `standard` when not specified. Set it to null to omit the storage class from the claim, so that the cluster default storage class is used. | [optional] ## Example diff --git a/libraries/models/api/openapi.yaml b/libraries/models/api/openapi.yaml index d683d50a3..0f88adc39 100644 --- a/libraries/models/api/openapi.yaml +++ b/libraries/models/api/openapi.yaml @@ -837,10 +837,13 @@ components: type: boolean storageClass: description: |- - The storage class used to create the volume claim. + The storage class used to create the volume claim. Defaults to `standard` + when not specified. - Overrides the deployment default (`harness.deployment.storageClass`). + Set it to null to omit the storage class from the claim, so that the + cluster default storage class is used. type: string + nullable: true - $ref: '#/components/schemas/AutoArtifactSpec' example: @@ -1349,15 +1352,6 @@ components: volume: $ref: '#/components/schemas/DeploymentVolumeSpec' description: Volume specification - storageClass: - description: |- - Default storage class of the deployment volume claim, used when the volume - does not define its own `storageClass`. - - Set to null to omit the storage class from the claim, so that the cluster - default storage class is used. - type: string - nullable: true statefulset: description: >- When true, the workload is rendered as a Kubernetes StatefulSet instead of a diff --git a/libraries/models/cloudharness_model/models/deployment_auto_artifact_config.py b/libraries/models/cloudharness_model/models/deployment_auto_artifact_config.py index a4724415c..834ad4c31 100644 --- a/libraries/models/cloudharness_model/models/deployment_auto_artifact_config.py +++ b/libraries/models/cloudharness_model/models/deployment_auto_artifact_config.py @@ -41,12 +41,11 @@ class DeploymentAutoArtifactConfig(CloudHarnessBaseModel): image: Optional[Annotated[str, Field(strict=True)]] = Field(default=None, description="Image name to use in the deployment. Leave it blank to set from the application's Docker file") resources: Optional[DeploymentResourcesConf] = None volume: Optional[DeploymentVolumeSpec] = None - storage_class: Optional[StrictStr] = Field(default=None, description="Default storage class of the deployment volume claim, used when the volume does not define its own `storageClass`. Set to null to omit the storage class from the claim, so that the cluster default storage class is used.", alias="storageClass") statefulset: Optional[StrictBool] = Field(default=None, description="When true, the workload is rendered as a Kubernetes StatefulSet instead of a Deployment. Recommended for deployments with a ReadWriteOnce volume: updates terminate the old pod before creating the new one, so no Recreate strategy or node pinning is needed. The volume, unless ReadWriteMany or externally managed (auto false), is provisioned per replica through volumeClaimTemplates. A pre-existing PVC named after the volume (left over from a previous Deployment) is migrated automatically: a migration job streams its data into each statefulset volume through the Kubernetes API, so the volumes are never mounted by the same pod (works on multi-zone clusters); delete the legacy PVC once migrated.") network: Optional[NetworkConfig] = None extra_containers: Optional[Dict[str, ExtraContainerConfig]] = Field(default=None, description="Extra containers (init containers and sidecars) for the deployment. Each key is a container name mapping to an ExtraContainerConfig.", alias="extraContainers") additional_properties: Dict[str, Any] = {} - __properties: ClassVar[List[str]] = ["auto", "name", "port", "replicas", "image", "resources", "volume", "storageClass", "statefulset", "network", "extraContainers"] + __properties: ClassVar[List[str]] = ["auto", "name", "port", "replicas", "image", "resources", "volume", "statefulset", "network", "extraContainers"] @field_validator('image') def image_validate_regular_expression(cls, value): @@ -84,10 +83,6 @@ def to_dict(self) -> Dict[str, Any]: # override the default output from pydantic by calling `to_dict()` of volume if self.volume: _dict['volume'] = self.volume.to_dict() - # set to None if storage_class (nullable) is None - # and model_fields_set contains the field - if self.storage_class is None and "storage_class" in self.model_fields_set: - _dict['storageClass'] = None # override the default output from pydantic by calling `to_dict()` of network if self.network: _dict['network'] = self.network.to_dict() @@ -127,7 +122,6 @@ def from_dict(cls, obj: Optional[Dict[str, Any]]) -> Optional[Self]: "image": obj.get("image"), "resources": DeploymentResourcesConf.from_dict(obj["resources"]) if obj.get("resources") is not None else None, "volume": DeploymentVolumeSpec.from_dict(obj["volume"]) if obj.get("volume") is not None else None, - "storageClass": obj.get("storageClass"), "statefulset": obj.get("statefulset"), "network": NetworkConfig.from_dict(obj["network"]) if obj.get("network") is not None else None, "extraContainers": dict( diff --git a/libraries/models/cloudharness_model/models/deployment_volume_spec.py b/libraries/models/cloudharness_model/models/deployment_volume_spec.py index ff268ea77..06dd6d403 100644 --- a/libraries/models/cloudharness_model/models/deployment_volume_spec.py +++ b/libraries/models/cloudharness_model/models/deployment_volume_spec.py @@ -36,7 +36,7 @@ class DeploymentVolumeSpec(CloudHarnessBaseModel): size: Optional[Any] = Field(default=None, description="The volume size. E.g. 5Gi") usenfs: Optional[StrictBool] = Field(default=None, description="Deprecated: use `writeMany` with the nfs storage class instead. Set to `true` to use the nfs on the created volume and mount as ReadWriteMany.") write_many: Optional[StrictBool] = Field(default=None, description="Set to `true` to create and mount the volume as ReadWriteMany. ReadWriteMany volumes attach to several nodes at once, hence pods using them are not pinned to the volume's node. Requires a storage class supporting ReadWriteMany: set `storageClass`, unless the cluster default one supports it.", alias="writeMany") - storage_class: Optional[StrictStr] = Field(default=None, description="The storage class used to create the volume claim. Overrides the deployment default (`harness.deployment.storageClass`).", alias="storageClass") + storage_class: Optional[StrictStr] = Field(default=None, description="The storage class used to create the volume claim. Defaults to `standard` when not specified. Set it to null to omit the storage class from the claim, so that the cluster default storage class is used.", alias="storageClass") additional_properties: Dict[str, Any] = {} __properties: ClassVar[List[str]] = ["auto", "name", "mountpath", "size", "usenfs", "writeMany", "storageClass"] @@ -70,6 +70,11 @@ def to_dict(self) -> Dict[str, Any]: if self.size is None and "size" in self.model_fields_set: _dict['size'] = None + # set to None if storage_class (nullable) is None + # and model_fields_set contains the field + if self.storage_class is None and "storage_class" in self.model_fields_set: + _dict['storageClass'] = None + return _dict @classmethod diff --git a/tools/deployment-cli-tools/tests/test_helm.py b/tools/deployment-cli-tools/tests/test_helm.py index c08ed197e..0e911942e 100644 --- a/tools/deployment-cli-tools/tests/test_helm.py +++ b/tools/deployment-cli-tools/tests/test_helm.py @@ -549,26 +549,18 @@ def render(): yaml.safe_dump(values, values_file) return render_helm_chart(helm_path) - # `standard` is the deployment default, coming from the application values - assert harness['deployment']['storageClass'] == 'standard' + # `standard` when the volume does not specify a storage class manifests = render() pvc = find_manifest(manifests, 'PersistentVolumeClaim', 'myapp-data') assert pvc['spec']['storageClassName'] == 'standard' - # a null default omits the storage class, so the cluster default one is used - harness['deployment']['storageClass'] = None + # a null storage class is omitted, so the cluster default one is used + volume['storageClass'] = None manifests = render() pvc = find_manifest(manifests, 'PersistentVolumeClaim', 'myapp-data') assert 'storageClassName' not in pvc['spec'] - # the deployment default applies to the volume claim of every deployment volume - harness['deployment']['storageClass'] = 'gp2' - manifests = render() - pvc = find_manifest(manifests, 'PersistentVolumeClaim', 'myapp-data') - assert pvc['spec']['storageClassName'] == 'gp2' - - # the volume storage class overrides the deployment default; ReadWriteOnce volumes keep - # the node pinning + # a storage class can be set on a ReadWriteOnce volume, which keeps the node pinning volume['storageClass'] = 'gp3' manifests = render() pvc = find_manifest(manifests, 'PersistentVolumeClaim', 'myapp-data') @@ -578,14 +570,14 @@ def render(): assert dep['spec']['strategy']['type'] == 'Recreate' assert 'affinity' in dep['spec']['template']['spec'] - # writeMany without a volume storage class inherits the deployment default (`gp2` here), and + # writeMany without a storage class gets the `standard` default like any other volume, and # the pod is neither pinned to a node nor recreated on update volume.pop('storageClass') volume['writeMany'] = True manifests = render() pvc = find_manifest(manifests, 'PersistentVolumeClaim', 'myapp-data') assert pvc['spec']['accessModes'] == ['ReadWriteMany'] - assert pvc['spec']['storageClassName'] == 'gp2' + assert pvc['spec']['storageClassName'] == 'standard' dep = find_manifest(manifests, 'Deployment', dep_name) assert 'strategy' not in dep['spec'] assert 'affinity' not in dep['spec']['template']['spec'] @@ -597,13 +589,11 @@ def render(): assert pvc['spec']['accessModes'] == ['ReadWriteMany'] assert pvc['spec']['storageClassName'] == 'efs-sc' - # a null deployment default omits the storage class from a ReadWriteMany claim too - volume.pop('storageClass') - harness['deployment']['storageClass'] = None + # a null storage class is omitted from a ReadWriteMany claim too + volume['storageClass'] = None manifests = render() pvc = find_manifest(manifests, 'PersistentVolumeClaim', 'myapp-data') assert 'storageClassName' not in pvc['spec'] - harness['deployment']['storageClass'] = 'standard' volume['storageClass'] = 'efs-sc' # ReadWriteMany volumes are shared: a statefulset keeps mounting the common PVC by @@ -643,7 +633,6 @@ def test_volume_usenfs_prevails(tmp_path): harness = values['apps']['myapp']['harness'] harness['deployment']['auto'] = True # colliding settings: the nfs server storage class and access mode prevail - harness['deployment']['storageClass'] = 'gp2' harness['deployment']['volume'] = { 'name': 'myapp-data', 'mountpath': '/data', 'size': '1Gi', 'auto': True, 'usenfs': True, 'writeMany': False, 'storageClass': 'efs-sc', From 9bac9d5d02256ec3d466b2111a44fcc5bdbb07d0 Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Mon, 31 Aug 2026 17:20:26 +0200 Subject: [PATCH 04/15] CH-288 fix defaults --- .../helm/templates/_helpers.tpl | 14 ++--- deployment-configuration/value-template.yaml | 11 ++-- docs/applications/databases.md | 17 +++--- docs/applications/volumes.md | 6 +-- docs/model/DatabaseDeploymentConfig.md | 2 +- docs/model/DeploymentVolumeSpec.md | 2 +- libraries/models/api/openapi.yaml | 9 ++-- .../models/database_deployment_config.py | 2 +- .../models/deployment_volume_spec.py | 2 +- .../ch_cli_tools/configurationgenerator.py | 17 ++++++ .../ch_cli_tools/dockercompose.py | 3 +- .../deployment-cli-tools/ch_cli_tools/helm.py | 2 + tools/deployment-cli-tools/tests/test_helm.py | 54 +++++++++++++++---- 13 files changed, 100 insertions(+), 41 deletions(-) diff --git a/deployment-configuration/helm/templates/_helpers.tpl b/deployment-configuration/helm/templates/_helpers.tpl index 78edc3d88..4e4c0059a 100644 --- a/deployment-configuration/helm/templates/_helpers.tpl +++ b/deployment-configuration/helm/templates/_helpers.tpl @@ -106,24 +106,24 @@ Usage: {{ if include "deploy_utils.volumeWriteMany" $volume }} {{/* Storage class of a harness.deployment.volume claim: nfs volumes always use the class created by -the nfsserver application, otherwise the volume `storageClass`, `standard` when the volume does -not specify it. A null `storageClass` renders nothing, leaving the claim to the cluster default -storage class. +the nfsserver application, otherwise the volume `storageClass` (`standard` by default, see +value-template.yaml). Renders nothing when it is set to null, leaving the claim to the cluster +default storage class. Usage: {{ include "deploy_utils.volumeStorageClass" (dict "root" .root "volume" $volume) }} */}} {{- define "deploy_utils.volumeStorageClass" -}} {{- $volume := .volume -}} -{{- if $volume.usenfs }}{{ printf "%s-%s" .root.Values.namespace .root.Values.apps.nfsserver.storageClass.name }}{{ else if $volume.storageClass }}{{ $volume.storageClass }}{{ else if not (hasKey $volume "storageClass") }}standard{{ end }} +{{- if $volume.usenfs }}{{ printf "%s-%s" .root.Values.namespace .root.Values.apps.nfsserver.storageClass.name }}{{ else if $volume.storageClass }}{{ $volume.storageClass }}{{ end }} {{- end -}} {{/* -Storage class of a database volume claim: harness.database.storageClass, `standard` when the -database does not specify it. A null value renders nothing, leaving the claim to the cluster +Storage class of a database volume claim: harness.database.storageClass (`standard` by default, +see value-template.yaml). Renders nothing when it is set to null, leaving the claim to the cluster default storage class. Usage: {{ include "deploy_utils.databaseStorageClass" .app.harness.database }} */}} {{- define "deploy_utils.databaseStorageClass" -}} -{{- if .storageClass }}{{ .storageClass }}{{ else if not (hasKey . "storageClass") }}standard{{ end }} +{{- if .storageClass }}{{ .storageClass }}{{ end }} {{- end -}} {{/* diff --git a/deployment-configuration/value-template.yaml b/deployment-configuration/value-template.yaml index 184a43607..513b23c9a 100644 --- a/deployment-configuration/value-template.yaml +++ b/deployment-configuration/value-template.yaml @@ -36,13 +36,14 @@ harness: name: # -- Deployment port. port: 8080 - # -- volume specification. + # -- volume specification. `mountpath` is what defines a volume: without it these values are + # dropped and the deployment has no volume. # `writeMany: true` creates and mounts the volume as ReadWriteMany: the volume attaches to several - # nodes at once, hence pods using it are not pinned to the volume's node. It requires a storage class - # supporting ReadWriteMany, set through `storageClass`. - # `storageClass` is the storage class of the volume claim, `standard` when not specified. Set it to - # null to use the cluster default storage class. + # nodes at once, hence pods using it are not pinned to the volume's node. It requires a storage + # class supporting ReadWriteMany, set through `storageClass`. volume: + # -- Storage class of the volume claim. Set to null to use the cluster default storage class. + storageClass: standard # example: # name: my-volume # mountpath: /usr/src/app/myvolume diff --git a/docs/applications/databases.md b/docs/applications/databases.md index 954a8d460..4e5c9c700 100644 --- a/docs/applications/databases.md +++ b/docs/applications/databases.md @@ -31,12 +31,17 @@ harness: `size`: Size of the persistent volume that the database container mounts, default is set to `1Gi` -`storageClass`: Storage class of the database volume claim, default is set to `standard`. Set it to -null to omit the storage class from the claim, so that the cluster default storage class is used. -It applies to the plain and statefulset database volumes as well as to the storage of a -`postgres.operator` cluster. Note that the storage class is immutable on an existing claim: on a -cluster whose default class is not `standard`, set this value (or null) before upgrading a release -that already has a database volume. +`storageClass`: Storage class of the database volume claim, `standard` by default. It applies to the +plain and statefulset database volumes as well as to the storage of a `postgres.operator` cluster. +Set it to null to omit the storage class from the claim, so that the cluster default storage class +is used. + +Note that the storage class is immutable on an existing claim: on a cluster whose default storage +class is not `standard`, a database volume created before this setting existed carries the cluster +default class (Kubernetes records it on the claim), and `helm upgrade` is rejected when the values +ask for a different one. Set `harness.database.storageClass` to the class of the existing claim, or +to null, before upgrading — in a deployment scaffolding this can be done once for all applications +in `deployment-configuration/value-template.yaml`. `resources`: Set the database pod resources diff --git a/docs/applications/volumes.md b/docs/applications/volumes.md index 7296c8d7f..31e1c92ca 100644 --- a/docs/applications/volumes.md +++ b/docs/applications/volumes.md @@ -43,9 +43,9 @@ one node is available on the cluster and other affinity rules or taints are pres ### Storage class -The volume `storageClass` sets the storage class of the claim; when not specified, `standard` is -used. Set it to null to omit the storage class from the claim, so that the cluster default storage -class provisions the volume: +The volume `storageClass` sets the storage class of the claim, and defaults to `standard` (see +`deployment-configuration/value-template.yaml`). Set it to null to omit the storage class from the +claim, so that the cluster default storage class provisions the volume: ```yaml harness: diff --git a/docs/model/DatabaseDeploymentConfig.md b/docs/model/DatabaseDeploymentConfig.md index 4d72a6b86..db031f743 100644 --- a/docs/model/DatabaseDeploymentConfig.md +++ b/docs/model/DatabaseDeploymentConfig.md @@ -10,7 +10,7 @@ Name | Type | Description | Notes **name** | **str** | | [optional] **type** | **str** | Define the database type. One of (mongo, postgres, neo4j, sqlite3) | [optional] **size** | **str** | Specify database disk size | [optional] -**storage_class** | **str** | Storage class of the database volume claim. Set to null to omit the storage class from the claim, so that the cluster default storage class is used. | [optional] +**storage_class** | **str** | Storage class of the database volume claim, `standard` by default. Set it to null to omit the storage class from the claim, so that the cluster default storage class is used. | [optional] **user** | **str** | database username | [optional] **var_pass** | **str** | Database password | [optional] **image_ref** | **str** | Used for referencing images from the build | [optional] diff --git a/docs/model/DeploymentVolumeSpec.md b/docs/model/DeploymentVolumeSpec.md index cdafce4d0..b24e156b9 100644 --- a/docs/model/DeploymentVolumeSpec.md +++ b/docs/model/DeploymentVolumeSpec.md @@ -12,7 +12,7 @@ Name | Type | Description | Notes **size** | **object** | The volume size. E.g. 5Gi | [optional] **usenfs** | **bool** | Deprecated: use `writeMany` with the nfs storage class instead. Set to `true` to use the nfs on the created volume and mount as ReadWriteMany. | [optional] **write_many** | **bool** | Set to `true` to create and mount the volume as ReadWriteMany. ReadWriteMany volumes attach to several nodes at once, hence pods using them are not pinned to the volume's node. Requires a storage class supporting ReadWriteMany: set `storageClass`, unless the cluster default one supports it. | [optional] -**storage_class** | **str** | The storage class used to create the volume claim. Defaults to `standard` when not specified. Set it to null to omit the storage class from the claim, so that the cluster default storage class is used. | [optional] +**storage_class** | **str** | The storage class used to create the volume claim, `standard` by default. Set it to null to omit the storage class from the claim, so that the cluster default storage class is used. | [optional] ## Example diff --git a/libraries/models/api/openapi.yaml b/libraries/models/api/openapi.yaml index 0f88adc39..77c6eedf4 100644 --- a/libraries/models/api/openapi.yaml +++ b/libraries/models/api/openapi.yaml @@ -686,10 +686,10 @@ components: example: 1Gi storageClass: description: |- - Storage class of the database volume claim. + Storage class of the database volume claim, `standard` by default. - Set to null to omit the storage class from the claim, so that the cluster - default storage class is used. + Set it to null to omit the storage class from the claim, so that the + cluster default storage class is used. type: string nullable: true user: @@ -837,8 +837,7 @@ components: type: boolean storageClass: description: |- - The storage class used to create the volume claim. Defaults to `standard` - when not specified. + The storage class used to create the volume claim, `standard` by default. Set it to null to omit the storage class from the claim, so that the cluster default storage class is used. diff --git a/libraries/models/cloudharness_model/models/database_deployment_config.py b/libraries/models/cloudharness_model/models/database_deployment_config.py index 76a219504..909acc997 100644 --- a/libraries/models/cloudharness_model/models/database_deployment_config.py +++ b/libraries/models/cloudharness_model/models/database_deployment_config.py @@ -35,7 +35,7 @@ class DatabaseDeploymentConfig(CloudHarnessBaseModel): name: Optional[StrictStr] = None type: Optional[Annotated[str, Field(strict=True)]] = Field(default=None, description="Define the database type. One of (mongo, postgres, neo4j, sqlite3)") size: Optional[StrictStr] = Field(default=None, description="Specify database disk size") - storage_class: Optional[StrictStr] = Field(default=None, description="Storage class of the database volume claim. Set to null to omit the storage class from the claim, so that the cluster default storage class is used.", alias="storageClass") + storage_class: Optional[StrictStr] = Field(default=None, description="Storage class of the database volume claim, `standard` by default. Set it to null to omit the storage class from the claim, so that the cluster default storage class is used.", alias="storageClass") user: Optional[StrictStr] = Field(default=None, description="database username") var_pass: Optional[StrictStr] = Field(default=None, description="Database password", alias="pass") image_ref: Optional[StrictStr] = Field(default=None, description="Used for referencing images from the build") diff --git a/libraries/models/cloudharness_model/models/deployment_volume_spec.py b/libraries/models/cloudharness_model/models/deployment_volume_spec.py index 06dd6d403..83eb7fc2d 100644 --- a/libraries/models/cloudharness_model/models/deployment_volume_spec.py +++ b/libraries/models/cloudharness_model/models/deployment_volume_spec.py @@ -36,7 +36,7 @@ class DeploymentVolumeSpec(CloudHarnessBaseModel): size: Optional[Any] = Field(default=None, description="The volume size. E.g. 5Gi") usenfs: Optional[StrictBool] = Field(default=None, description="Deprecated: use `writeMany` with the nfs storage class instead. Set to `true` to use the nfs on the created volume and mount as ReadWriteMany.") write_many: Optional[StrictBool] = Field(default=None, description="Set to `true` to create and mount the volume as ReadWriteMany. ReadWriteMany volumes attach to several nodes at once, hence pods using them are not pinned to the volume's node. Requires a storage class supporting ReadWriteMany: set `storageClass`, unless the cluster default one supports it.", alias="writeMany") - storage_class: Optional[StrictStr] = Field(default=None, description="The storage class used to create the volume claim. Defaults to `standard` when not specified. Set it to null to omit the storage class from the claim, so that the cluster default storage class is used.", alias="storageClass") + storage_class: Optional[StrictStr] = Field(default=None, description="The storage class used to create the volume claim, `standard` by default. Set it to null to omit the storage class from the claim, so that the cluster default storage class is used.", alias="storageClass") additional_properties: Dict[str, Any] = {} __properties: ClassVar[List[str]] = ["auto", "name", "mountpath", "size", "usenfs", "writeMany", "storageClass"] diff --git a/tools/deployment-cli-tools/ch_cli_tools/configurationgenerator.py b/tools/deployment-cli-tools/ch_cli_tools/configurationgenerator.py index 85fb9506e..75f8b5e89 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/configurationgenerator.py +++ b/tools/deployment-cli-tools/ch_cli_tools/configurationgenerator.py @@ -666,6 +666,23 @@ def validate_helm_values(values): validate_volumes(values) +def clear_unused_volume_configuration(harness_config): + """Clears the deployment volume defaults (see value-template.yaml) of an application + declaring no volume. + + `mountpath` is what defines a volume: the defaults alone (the storage class) do not make one, + and are dropped so that a volume-less application keeps no volume at all. + """ + deployment_config = harness_config[KEY_DEPLOYMENT] + volume_config = deployment_config.get('volume') or {} + if volume_config.get('mountpath'): + return + if volume_config.get('name') or volume_config.get('size'): + raise ValuesValidationException( + f"Bad volume specified for application {harness_config.get('name')}: mountpath is required") + deployment_config.pop('volume', None) + + def validate_volumes(values): """Warns when a volume configuration collides with the nfs server settings. diff --git a/tools/deployment-cli-tools/ch_cli_tools/dockercompose.py b/tools/deployment-cli-tools/ch_cli_tools/dockercompose.py index bcae37641..2f551434a 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/dockercompose.py +++ b/tools/deployment-cli-tools/ch_cli_tools/dockercompose.py @@ -17,7 +17,7 @@ from .models import HarnessMainConfig from .configurationgenerator import ConfigurationGenerator, \ - validate_helm_values, values_from_legacy, values_set_legacy, get_included_applications, get_included_builds, resolve_task_image_owner, create_env_variables, collect_apps_helm_templates, \ + clear_unused_volume_configuration, validate_helm_values, values_from_legacy, values_set_legacy, get_included_applications, get_included_builds, resolve_task_image_owner, create_env_variables, collect_apps_helm_templates, \ KEY_HARNESS, KEY_SERVICE, KEY_DATABASE, KEY_APPS, KEY_TASK_IMAGES, KEY_TEST_IMAGES, KEY_DEPLOYMENT @@ -225,6 +225,7 @@ def __finish_helm_values(self, values, defer_task_images=False): harness[KEY_DATABASE]['name'] = app_name.strip() + '-db' self._clear_unused_db_configuration(harness) + clear_unused_volume_configuration(harness) values_set_legacy(v) if self.include: diff --git a/tools/deployment-cli-tools/ch_cli_tools/helm.py b/tools/deployment-cli-tools/ch_cli_tools/helm.py index d20fed3e3..8d24e00c8 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/helm.py +++ b/tools/deployment-cli-tools/ch_cli_tools/helm.py @@ -16,6 +16,7 @@ from .models import HarnessMainConfig from .configurationgenerator import ConfigurationGenerator, get_included_builds, validate_helm_values, resolve_task_image_owner, \ + clear_unused_volume_configuration, \ KEY_HARNESS, KEY_SERVICE, KEY_DATABASE, KEY_APPS, KEY_TASK_IMAGES, KEY_TEST_IMAGES, KEY_DEPLOYMENT, DEFAULT_IGNORE, \ values_from_legacy, values_set_legacy, get_included_applications, create_env_variables, collect_apps_helm_templates, generate_tag_from_content, guess_build_dependencies_from_dockerfile @@ -239,6 +240,7 @@ def __finish_helm_values(self, values, defer_task_images=False): harness[KEY_DATABASE]['name'] = app_name.strip() + '-db' self._clear_unused_db_configuration(harness) + clear_unused_volume_configuration(harness) values_set_legacy(v) if self.include: diff --git a/tools/deployment-cli-tools/tests/test_helm.py b/tools/deployment-cli-tools/tests/test_helm.py index 0e911942e..8f23c58f3 100644 --- a/tools/deployment-cli-tools/tests/test_helm.py +++ b/tools/deployment-cli-tools/tests/test_helm.py @@ -549,11 +549,6 @@ def render(): yaml.safe_dump(values, values_file) return render_helm_chart(helm_path) - # `standard` when the volume does not specify a storage class - manifests = render() - pvc = find_manifest(manifests, 'PersistentVolumeClaim', 'myapp-data') - assert pvc['spec']['storageClassName'] == 'standard' - # a null storage class is omitted, so the cluster default one is used volume['storageClass'] = None manifests = render() @@ -570,14 +565,13 @@ def render(): assert dep['spec']['strategy']['type'] == 'Recreate' assert 'affinity' in dep['spec']['template']['spec'] - # writeMany without a storage class gets the `standard` default like any other volume, and - # the pod is neither pinned to a node nor recreated on update - volume.pop('storageClass') + # a writeMany volume keeps its storage class, and its pod is neither pinned to a node nor + # recreated on update volume['writeMany'] = True manifests = render() pvc = find_manifest(manifests, 'PersistentVolumeClaim', 'myapp-data') assert pvc['spec']['accessModes'] == ['ReadWriteMany'] - assert pvc['spec']['storageClassName'] == 'standard' + assert pvc['spec']['storageClassName'] == 'gp3' dep = find_manifest(manifests, 'Deployment', dep_name) assert 'strategy' not in dep['spec'] assert 'affinity' not in dep['spec']['template']['spec'] @@ -595,6 +589,9 @@ def render(): pvc = find_manifest(manifests, 'PersistentVolumeClaim', 'myapp-data') assert 'storageClassName' not in pvc['spec'] volume['storageClass'] = 'efs-sc' + manifests = render() + assert find_manifest(manifests, 'PersistentVolumeClaim', + 'myapp-data')['spec']['storageClassName'] == 'efs-sc' # ReadWriteMany volumes are shared: a statefulset keeps mounting the common PVC by # claimName instead of provisioning one per replica @@ -619,6 +616,43 @@ def render(): assert claim_template['spec']['storageClassName'] == 'gp3' +def test_volume_storage_class_default(tmp_path): + out_folder = tmp_path / 'test_volume_storage_class_default' + # samples declares a volume, myapp does not + values = create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, domain="my.local", + env='withpostgres', local=False, include=["samples", "myapp"], exclude=["legacy"]) + + # the value-template default applies to the volume declared by the application + volume = values[KEY_APPS]['samples'][KEY_HARNESS]['deployment']['volume'] + assert volume['mountpath'] + assert volume['storageClass'] == 'standard' + + # ... and the defaults alone do not make a volume: a volume-less application has none + assert not values[KEY_APPS]['myapp'][KEY_HARNESS]['deployment'].get('volume') + + helm_path = out_folder / HELM_CHART_PATH + shutil.rmtree(helm_path / 'charts') + manifests = render_helm_chart(helm_path) + sts = find_manifest(manifests, 'StatefulSet', values[KEY_APPS]['samples'][KEY_HARNESS]['deployment']['name']) + assert sts['spec']['volumeClaimTemplates'][0]['spec']['storageClassName'] == 'standard' + + +def test_volume_without_mountpath_is_rejected(): + harness = {'name': 'myapp', KEY_DEPLOYMENT: {'volume': {'name': 'myapp-data', 'size': '1Gi'}}} + with pytest.raises(ValuesValidationException): + clear_unused_volume_configuration(harness) + + # the defaults alone are dropped, a declared volume is kept + harness = {'name': 'myapp', KEY_DEPLOYMENT: {'volume': {'storageClass': 'standard'}}} + clear_unused_volume_configuration(harness) + assert 'volume' not in harness[KEY_DEPLOYMENT] + + volume = {'name': 'myapp-data', 'mountpath': '/data', 'storageClass': 'standard'} + harness = {'name': 'myapp', KEY_DEPLOYMENT: {'volume': volume}} + clear_unused_volume_configuration(harness) + assert harness[KEY_DEPLOYMENT]['volume'] == volume + + def test_volume_usenfs_prevails(tmp_path): out_folder = tmp_path / 'test_volume_usenfs_prevails' create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, domain="my.local", @@ -693,7 +727,7 @@ def render(): manifests = render() assert find_manifest(manifests, 'PersistentVolumeClaim', db_name)['spec']['storageClassName'] == 'standard' - # a null storage class is omitted, so the cluster default one is used + # only an explicit null omits the storage class, so that the cluster default one is used database['storageClass'] = None manifests = render() assert 'storageClassName' not in find_manifest(manifests, 'PersistentVolumeClaim', db_name)['spec'] From c49cf6482dc1df1964cd30148d7b91785020cb34 Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Mon, 31 Aug 2026 17:44:36 +0200 Subject: [PATCH 05/15] CH-288 fix defaults on db --- .../helm/templates/_helpers.tpl | 6 +++--- deployment-configuration/value-template.yaml | 7 +++++-- docs/applications/databases.md | 19 ++++++++----------- docs/model/DatabaseDeploymentConfig.md | 2 +- libraries/models/api/openapi.yaml | 6 +++--- .../models/database_deployment_config.py | 2 +- tools/deployment-cli-tools/tests/test_helm.py | 11 ++++------- 7 files changed, 25 insertions(+), 28 deletions(-) diff --git a/deployment-configuration/helm/templates/_helpers.tpl b/deployment-configuration/helm/templates/_helpers.tpl index 4e4c0059a..7f2eb6f72 100644 --- a/deployment-configuration/helm/templates/_helpers.tpl +++ b/deployment-configuration/helm/templates/_helpers.tpl @@ -117,9 +117,9 @@ Usage: {{ include "deploy_utils.volumeStorageClass" (dict "root" .root "volume" {{- end -}} {{/* -Storage class of a database volume claim: harness.database.storageClass (`standard` by default, -see value-template.yaml). Renders nothing when it is set to null, leaving the claim to the cluster -default storage class. +Storage class of a database volume claim: harness.database.storageClass. Renders nothing when it +is not set (the default), leaving the claim to the cluster default storage class: that is how the +database volumes of existing deployments were created, and the storage class is immutable. Usage: {{ include "deploy_utils.databaseStorageClass" .app.harness.database }} */}} {{- define "deploy_utils.databaseStorageClass" -}} diff --git a/deployment-configuration/value-template.yaml b/deployment-configuration/value-template.yaml index 513b23c9a..6b85846d7 100644 --- a/deployment-configuration/value-template.yaml +++ b/deployment-configuration/value-template.yaml @@ -106,8 +106,11 @@ harness: # -- supported db types: mongo, postgres, neo4j type: size: 1Gi - # -- Storage class of the database volume claim. Set to null to use the cluster default storage class. - storageClass: standard + # -- Storage class of the database volume claim. Left null, the claim carries no storage class and + # the cluster default one is used, which is what pre-existing database volumes were created with: + # the storage class is immutable on an existing claim, so setting it on a live database requires + # deleting and recreating the claim. + storageClass: # -- database username user: mnp # -- database password diff --git a/docs/applications/databases.md b/docs/applications/databases.md index 4e5c9c700..28359b796 100644 --- a/docs/applications/databases.md +++ b/docs/applications/databases.md @@ -31,17 +31,14 @@ harness: `size`: Size of the persistent volume that the database container mounts, default is set to `1Gi` -`storageClass`: Storage class of the database volume claim, `standard` by default. It applies to the -plain and statefulset database volumes as well as to the storage of a `postgres.operator` cluster. -Set it to null to omit the storage class from the claim, so that the cluster default storage class -is used. - -Note that the storage class is immutable on an existing claim: on a cluster whose default storage -class is not `standard`, a database volume created before this setting existed carries the cluster -default class (Kubernetes records it on the claim), and `helm upgrade` is rejected when the values -ask for a different one. Set `harness.database.storageClass` to the class of the existing claim, or -to null, before upgrading — in a deployment scaffolding this can be done once for all applications -in `deployment-configuration/value-template.yaml`. +`storageClass`: Storage class of the database volume claim, applied to the plain and statefulset +database volumes as well as to the storage of a `postgres.operator` cluster. Not set by default: +the claim then carries no storage class and Kubernetes provisions it on the cluster default one, +which is how the database volumes of existing deployments were created. + +Note that the storage class is immutable on an existing claim (Kubernetes records the class it was +provisioned with, e.g. `metacell`): setting or changing this value on a live database makes +`helm upgrade` fail, and requires deleting and recreating the claim — the data is not migrated. `resources`: Set the database pod resources diff --git a/docs/model/DatabaseDeploymentConfig.md b/docs/model/DatabaseDeploymentConfig.md index db031f743..3b3fc7c05 100644 --- a/docs/model/DatabaseDeploymentConfig.md +++ b/docs/model/DatabaseDeploymentConfig.md @@ -10,7 +10,7 @@ Name | Type | Description | Notes **name** | **str** | | [optional] **type** | **str** | Define the database type. One of (mongo, postgres, neo4j, sqlite3) | [optional] **size** | **str** | Specify database disk size | [optional] -**storage_class** | **str** | Storage class of the database volume claim, `standard` by default. Set it to null to omit the storage class from the claim, so that the cluster default storage class is used. | [optional] +**storage_class** | **str** | Storage class of the database volume claim. When not set, the claim carries no storage class, so that the cluster default storage class is used. | [optional] **user** | **str** | database username | [optional] **var_pass** | **str** | Database password | [optional] **image_ref** | **str** | Used for referencing images from the build | [optional] diff --git a/libraries/models/api/openapi.yaml b/libraries/models/api/openapi.yaml index 77c6eedf4..3270c4794 100644 --- a/libraries/models/api/openapi.yaml +++ b/libraries/models/api/openapi.yaml @@ -686,10 +686,10 @@ components: example: 1Gi storageClass: description: |- - Storage class of the database volume claim, `standard` by default. + Storage class of the database volume claim. - Set it to null to omit the storage class from the claim, so that the - cluster default storage class is used. + When not set, the claim carries no storage class, so that the cluster + default storage class is used. type: string nullable: true user: diff --git a/libraries/models/cloudharness_model/models/database_deployment_config.py b/libraries/models/cloudharness_model/models/database_deployment_config.py index 909acc997..222bfb72f 100644 --- a/libraries/models/cloudharness_model/models/database_deployment_config.py +++ b/libraries/models/cloudharness_model/models/database_deployment_config.py @@ -35,7 +35,7 @@ class DatabaseDeploymentConfig(CloudHarnessBaseModel): name: Optional[StrictStr] = None type: Optional[Annotated[str, Field(strict=True)]] = Field(default=None, description="Define the database type. One of (mongo, postgres, neo4j, sqlite3)") size: Optional[StrictStr] = Field(default=None, description="Specify database disk size") - storage_class: Optional[StrictStr] = Field(default=None, description="Storage class of the database volume claim, `standard` by default. Set it to null to omit the storage class from the claim, so that the cluster default storage class is used.", alias="storageClass") + storage_class: Optional[StrictStr] = Field(default=None, description="Storage class of the database volume claim. When not set, the claim carries no storage class, so that the cluster default storage class is used.", alias="storageClass") user: Optional[StrictStr] = Field(default=None, description="database username") var_pass: Optional[StrictStr] = Field(default=None, description="Database password", alias="pass") image_ref: Optional[StrictStr] = Field(default=None, description="Used for referencing images from the build") diff --git a/tools/deployment-cli-tools/tests/test_helm.py b/tools/deployment-cli-tools/tests/test_helm.py index 8f23c58f3..0767dc14a 100644 --- a/tools/deployment-cli-tools/tests/test_helm.py +++ b/tools/deployment-cli-tools/tests/test_helm.py @@ -722,13 +722,10 @@ def render(): yaml.safe_dump(values, values_file) return render_helm_chart(helm_path) - # `standard` is the default, coming from the application values - assert database['storageClass'] == 'standard' - manifests = render() - assert find_manifest(manifests, 'PersistentVolumeClaim', db_name)['spec']['storageClassName'] == 'standard' - - # only an explicit null omits the storage class, so that the cluster default one is used - database['storageClass'] = None + # not set by default: the claim carries no storage class, so the cluster default one is used. + # The storage class is immutable on an existing claim, hence never set implicitly: database + # volumes of existing deployments must keep rendering without it. + assert database['storageClass'] is None manifests = render() assert 'storageClassName' not in find_manifest(manifests, 'PersistentVolumeClaim', db_name)['spec'] From 25265c7a2b3a06818f6523bf483afd5dfb05e75e Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Tue, 1 Sep 2026 11:18:05 +0200 Subject: [PATCH 06/15] Fix dates marshaling for flask server --- .../cloudharness/utils/server.py | 10 ++++++++ .../cloudharness-common/tests/test_server.py | 23 +++++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/libraries/cloudharness-common/cloudharness/utils/server.py b/libraries/cloudharness-common/cloudharness/utils/server.py index 3174d18dc..d16e2eb61 100644 --- a/libraries/cloudharness-common/cloudharness/utils/server.py +++ b/libraries/cloudharness-common/cloudharness/utils/server.py @@ -1,6 +1,7 @@ import os import json import traceback +from datetime import date, datetime import flask import connexion @@ -18,6 +19,15 @@ class JSONEncoder(DefaultJSONProvider): include_nulls = False def default(self, o): + # `format: date-time`/`format: date` in the specs mean RFC 3339, but the + # Flask provider we inherit from renders any date as an HTTP date + # ("Mon, 31 Aug 2026 16:44:54 GMT"), so handle them here. Naive + # datetimes are assumed UTC: the offset is not optional for RFC 3339. + # `datetime` first, it is a subclass of `date`. + if isinstance(o, datetime): + return o.isoformat("T") if o.tzinfo else o.isoformat("T") + "Z" + if isinstance(o, date): + return o.isoformat() # Connexion/openapi-generator models: their `to_dict()` keys are the # *Python* attribute names (snake_case), so serializing through it would # break every camelCase property in the spec. `attribute_map` holds the diff --git a/libraries/cloudharness-common/tests/test_server.py b/libraries/cloudharness-common/tests/test_server.py index 652b589ff..64855b35d 100644 --- a/libraries/cloudharness-common/tests/test_server.py +++ b/libraries/cloudharness-common/tests/test_server.py @@ -1,5 +1,6 @@ import datetime import json +import uuid import flask @@ -44,5 +45,23 @@ def test_pydantic_model_keeps_its_aliased_keys(): def test_unknown_types_fall_back_to_the_default_provider(): - encoded = json.loads(encoder().dumps({'date': datetime.date(2026, 7, 29)})) - assert 'Jul 2026' in encoded['date'] + value = uuid.uuid4() + assert json.loads(encoder().dumps({'id': value})) == {'id': str(value)} + + +def test_aware_datetime_is_serialized_as_rfc3339(): + value = datetime.datetime(2026, 8, 31, 16, 44, 54, tzinfo=datetime.timezone.utc) + encoded = json.loads(encoder().dumps({'createTime': value})) + assert encoded == {'createTime': '2026-08-31T16:44:54+00:00'} + + +def test_naive_datetime_is_serialized_as_utc(): + """RFC 3339 makes the offset mandatory, so a naive datetime is assumed UTC.""" + value = datetime.datetime(2026, 8, 31, 16, 44, 54) + encoded = json.loads(encoder().dumps({'createTime': value})) + assert encoded == {'createTime': '2026-08-31T16:44:54Z'} + + +def test_date_is_serialized_as_iso_date(): + encoded = json.loads(encoder().dumps({'day': datetime.date(2026, 7, 29)})) + assert encoded == {'day': '2026-07-29'} From 8432b02bb0c55aba2f06c0496033130c7de6e595 Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Tue, 1 Sep 2026 11:21:49 +0200 Subject: [PATCH 07/15] Refactor flask server utility --- libraries/cloudharness-common/cloudharness/utils/__init__.py | 5 ++++- .../cloudharness/utils/{server.py => flask_server.py} | 0 2 files changed, 4 insertions(+), 1 deletion(-) rename libraries/cloudharness-common/cloudharness/utils/{server.py => flask_server.py} (100%) diff --git a/libraries/cloudharness-common/cloudharness/utils/__init__.py b/libraries/cloudharness-common/cloudharness/utils/__init__.py index b11b3ef80..723cc2046 100644 --- a/libraries/cloudharness-common/cloudharness/utils/__init__.py +++ b/libraries/cloudharness-common/cloudharness/utils/__init__.py @@ -1,5 +1,5 @@ import collections - +import flask_server as server # Backwards compatibility def dict_merge(dct, merge_dct, add_keys=True, merge_none=True): """ Recursive dict merge. Inspired by :meth:``dict.update()``, instead of @@ -37,3 +37,6 @@ def dict_merge(dct, merge_dct, add_keys=True, merge_none=True): dct[k] = merge_dct[k] return dct + + +__all__ = ["dict_merge", "server"] \ No newline at end of file diff --git a/libraries/cloudharness-common/cloudharness/utils/server.py b/libraries/cloudharness-common/cloudharness/utils/flask_server.py similarity index 100% rename from libraries/cloudharness-common/cloudharness/utils/server.py rename to libraries/cloudharness-common/cloudharness/utils/flask_server.py From bea94170c769a23bfa53351107c45b11a73b989a Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Tue, 1 Sep 2026 11:27:34 +0200 Subject: [PATCH 08/15] Regenerate samples application to cover new API --- .../backend/samples/models/__init__.py | 2 + .../samples/models/write_file200_response.py | 113 ++++++++++++++++++ .../samples/models/write_file_request.py | 61 ++++++++++ .../backend/samples/openapi/openapi.yaml | 49 ++++++++ applications/samples/backend/samples/util.py | 10 +- .../frontend/src/components/RestTest.tsx | 2 +- .../{ => samples}/.openapi-generator-ignore | 0 .../src/rest/{ => samples}/apis/AuthApi.ts | 0 .../src/rest/samples/apis/DatabaseApi.ts | 55 +++++++++ .../rest/{ => samples}/apis/ResourceApi.ts | 0 .../src/rest/{ => samples}/apis/TestApi.ts | 47 +++++++- .../rest/{ => samples}/apis/WorkflowsApi.ts | 4 - .../src/rest/{ => samples}/apis/index.ts | 1 + .../frontend/src/rest/{ => samples}/index.ts | 0 .../{ => samples}/models/InlineResponse202.ts | 0 .../models/InlineResponse202Task.ts | 0 .../{ => samples}/models/SampleResource.ts | 0 .../samples/models/WriteFile200Response.ts | 76 ++++++++++++ .../rest/samples/models/WriteFileRequest.ts | 60 ++++++++++ .../src/rest/{ => samples}/models/index.ts | 2 + .../src/rest/{ => samples}/runtime.ts | 0 21 files changed, 470 insertions(+), 12 deletions(-) create mode 100644 applications/samples/backend/samples/models/write_file200_response.py create mode 100644 applications/samples/backend/samples/models/write_file_request.py rename applications/samples/frontend/src/rest/{ => samples}/.openapi-generator-ignore (100%) rename applications/samples/frontend/src/rest/{ => samples}/apis/AuthApi.ts (100%) create mode 100644 applications/samples/frontend/src/rest/samples/apis/DatabaseApi.ts rename applications/samples/frontend/src/rest/{ => samples}/apis/ResourceApi.ts (100%) rename applications/samples/frontend/src/rest/{ => samples}/apis/TestApi.ts (54%) rename applications/samples/frontend/src/rest/{ => samples}/apis/WorkflowsApi.ts (98%) rename applications/samples/frontend/src/rest/{ => samples}/apis/index.ts (83%) rename applications/samples/frontend/src/rest/{ => samples}/index.ts (100%) rename applications/samples/frontend/src/rest/{ => samples}/models/InlineResponse202.ts (100%) rename applications/samples/frontend/src/rest/{ => samples}/models/InlineResponse202Task.ts (100%) rename applications/samples/frontend/src/rest/{ => samples}/models/SampleResource.ts (100%) create mode 100644 applications/samples/frontend/src/rest/samples/models/WriteFile200Response.ts create mode 100644 applications/samples/frontend/src/rest/samples/models/WriteFileRequest.ts rename applications/samples/frontend/src/rest/{ => samples}/models/index.ts (66%) rename applications/samples/frontend/src/rest/{ => samples}/runtime.ts (100%) diff --git a/applications/samples/backend/samples/models/__init__.py b/applications/samples/backend/samples/models/__init__.py index 946173988..547211c10 100644 --- a/applications/samples/backend/samples/models/__init__.py +++ b/applications/samples/backend/samples/models/__init__.py @@ -3,3 +3,5 @@ from samples.models.inline_response202 import InlineResponse202 from samples.models.inline_response202_task import InlineResponse202Task from samples.models.sample_resource import SampleResource +from samples.models.write_file200_response import WriteFile200Response +from samples.models.write_file_request import WriteFileRequest diff --git a/applications/samples/backend/samples/models/write_file200_response.py b/applications/samples/backend/samples/models/write_file200_response.py new file mode 100644 index 000000000..8155ebe60 --- /dev/null +++ b/applications/samples/backend/samples/models/write_file200_response.py @@ -0,0 +1,113 @@ +from datetime import date, datetime # noqa: F401 + +from typing import List, Dict # noqa: F401 + +from samples.models.base_model import Model +from samples import util + + +class WriteFile200Response(Model): + """NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + + Do not edit the class manually. + """ + + def __init__(self, filename=None, path=None, hostname=None): # noqa: E501 + """WriteFile200Response - a model defined in OpenAPI + + :param filename: The filename of this WriteFile200Response. # noqa: E501 + :type filename: str + :param path: The path of this WriteFile200Response. # noqa: E501 + :type path: str + :param hostname: The hostname of this WriteFile200Response. # noqa: E501 + :type hostname: str + """ + self.openapi_types = { + 'filename': str, + 'path': str, + 'hostname': str + } + + self.attribute_map = { + 'filename': 'filename', + 'path': 'path', + 'hostname': 'hostname' + } + + self._filename = filename + self._path = path + self._hostname = hostname + + @classmethod + def from_dict(cls, dikt) -> 'WriteFile200Response': + """Returns the dict as a model + + :param dikt: A dict. + :type: dict + :return: The write_file_200_response of this WriteFile200Response. # noqa: E501 + :rtype: WriteFile200Response + """ + return util.deserialize_model(dikt, cls) + + @property + def filename(self) -> str: + """Gets the filename of this WriteFile200Response. + + + :return: The filename of this WriteFile200Response. + :rtype: str + """ + return self._filename + + @filename.setter + def filename(self, filename: str): + """Sets the filename of this WriteFile200Response. + + + :param filename: The filename of this WriteFile200Response. + :type filename: str + """ + + self._filename = filename + + @property + def path(self) -> str: + """Gets the path of this WriteFile200Response. + + + :return: The path of this WriteFile200Response. + :rtype: str + """ + return self._path + + @path.setter + def path(self, path: str): + """Sets the path of this WriteFile200Response. + + + :param path: The path of this WriteFile200Response. + :type path: str + """ + + self._path = path + + @property + def hostname(self) -> str: + """Gets the hostname of this WriteFile200Response. + + + :return: The hostname of this WriteFile200Response. + :rtype: str + """ + return self._hostname + + @hostname.setter + def hostname(self, hostname: str): + """Sets the hostname of this WriteFile200Response. + + + :param hostname: The hostname of this WriteFile200Response. + :type hostname: str + """ + + self._hostname = hostname diff --git a/applications/samples/backend/samples/models/write_file_request.py b/applications/samples/backend/samples/models/write_file_request.py new file mode 100644 index 000000000..ef6e2b29c --- /dev/null +++ b/applications/samples/backend/samples/models/write_file_request.py @@ -0,0 +1,61 @@ +from datetime import date, datetime # noqa: F401 + +from typing import List, Dict # noqa: F401 + +from samples.models.base_model import Model +from samples import util + + +class WriteFileRequest(Model): + """NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + + Do not edit the class manually. + """ + + def __init__(self, content=None): # noqa: E501 + """WriteFileRequest - a model defined in OpenAPI + + :param content: The content of this WriteFileRequest. # noqa: E501 + :type content: str + """ + self.openapi_types = { + 'content': str + } + + self.attribute_map = { + 'content': 'content' + } + + self._content = content + + @classmethod + def from_dict(cls, dikt) -> 'WriteFileRequest': + """Returns the dict as a model + + :param dikt: A dict. + :type: dict + :return: The write_file_request of this WriteFileRequest. # noqa: E501 + :rtype: WriteFileRequest + """ + return util.deserialize_model(dikt, cls) + + @property + def content(self) -> str: + """Gets the content of this WriteFileRequest. + + + :return: The content of this WriteFileRequest. + :rtype: str + """ + return self._content + + @content.setter + def content(self, content: str): + """Sets the content of this WriteFileRequest. + + + :param content: The content of this WriteFileRequest. + :type content: str + """ + + self._content = content diff --git a/applications/samples/backend/samples/openapi/openapi.yaml b/applications/samples/backend/samples/openapi/openapi.yaml index 3bf1d1c54..3b4b40593 100644 --- a/applications/samples/backend/samples/openapi/openapi.yaml +++ b/applications/samples/backend/samples/openapi/openapi.yaml @@ -292,6 +292,31 @@ paths: tags: - auth x-openapi-router-controller: samples.controllers.auth_controller + /write-file: + post: + description: "Writes a timestamped file on the application volume and returns\ + \ the name of the pod that handled the request. On a statefulset deployment,\ + \ route this endpoint to the leader service to have all writes land on pod\ + \ 0." + operationId: write_file + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/write_file_request' + description: Optional content of the file to write. + required: false + responses: + "200": + content: + application/json: + schema: + $ref: '#/components/schemas/write_file_200_response' + description: The file was written on the application volume + summary: writes a file on the application volume + tags: + - test + x-openapi-router-controller: samples.controllers.test_controller components: schemas: inline_response_202_task: @@ -343,6 +368,30 @@ components: - a title: SampleResource type: object + write_file_request: + properties: + content: + title: content + type: string + title: write_file_request + type: object + write_file_200_response: + example: + path: path + hostname: hostname + filename: filename + properties: + filename: + title: filename + type: string + path: + title: path + type: string + hostname: + title: hostname + type: string + title: write_file_200_response + type: object securitySchemes: bearerAuth: bearerFormat: JWT diff --git a/applications/samples/backend/samples/util.py b/applications/samples/backend/samples/util.py index b802fafda..5b241814f 100644 --- a/applications/samples/backend/samples/util.py +++ b/applications/samples/backend/samples/util.py @@ -67,8 +67,8 @@ def deserialize_date(string): :rtype: date """ if string is None: - return None - + return None + try: from dateutil.parser import parse return parse(string).date() @@ -87,8 +87,8 @@ def deserialize_datetime(string): :rtype: datetime """ if string is None: - return None - + return None + try: from dateutil.parser import parse return parse(string) @@ -144,4 +144,4 @@ def _deserialize_dict(data, boxed_type): :rtype: dict """ return {k: _deserialize(v, boxed_type) - for k, v in data.items()} + for k, v in data.items() } diff --git a/applications/samples/frontend/src/components/RestTest.tsx b/applications/samples/frontend/src/components/RestTest.tsx index c1dc7c002..e9dd79155 100644 --- a/applications/samples/frontend/src/components/RestTest.tsx +++ b/applications/samples/frontend/src/components/RestTest.tsx @@ -1,7 +1,7 @@ import { useState, useEffect } from 'react'; -import { TestApi } from '../rest/apis/TestApi' +import { TestApi } from '../rest/samples/apis/TestApi' const test = new TestApi(); diff --git a/applications/samples/frontend/src/rest/.openapi-generator-ignore b/applications/samples/frontend/src/rest/samples/.openapi-generator-ignore similarity index 100% rename from applications/samples/frontend/src/rest/.openapi-generator-ignore rename to applications/samples/frontend/src/rest/samples/.openapi-generator-ignore diff --git a/applications/samples/frontend/src/rest/apis/AuthApi.ts b/applications/samples/frontend/src/rest/samples/apis/AuthApi.ts similarity index 100% rename from applications/samples/frontend/src/rest/apis/AuthApi.ts rename to applications/samples/frontend/src/rest/samples/apis/AuthApi.ts diff --git a/applications/samples/frontend/src/rest/samples/apis/DatabaseApi.ts b/applications/samples/frontend/src/rest/samples/apis/DatabaseApi.ts new file mode 100644 index 000000000..4b261a02e --- /dev/null +++ b/applications/samples/frontend/src/rest/samples/apis/DatabaseApi.ts @@ -0,0 +1,55 @@ +/* tslint:disable */ +/* eslint-disable */ +/** + * CloudHarness Sample API + * CloudHarness Sample api + * + * The version of the OpenAPI document: 0.1.0 + * Contact: cloudharness@metacell.us + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + + +import * as runtime from '../runtime'; + +/** + * + */ +export class DatabaseApi extends runtime.BaseAPI { + + /** + * Returns the database connection string for the current application. + * Get database connection string + */ + async getDbConnectStringRaw(initOverrides?: RequestInit | runtime.InitOverrideFunction): Promise> { + const queryParameters: any = {}; + + const headerParameters: runtime.HTTPHeaders = {}; + + const response = await this.request({ + path: `/db-connect-string`, + method: 'GET', + headers: headerParameters, + query: queryParameters, + }, initOverrides); + + if (this.isJsonMime(response.headers.get('content-type'))) { + return new runtime.JSONApiResponse(response); + } else { + return new runtime.TextApiResponse(response) as any; + } + } + + /** + * Returns the database connection string for the current application. + * Get database connection string + */ + async getDbConnectString(initOverrides?: RequestInit | runtime.InitOverrideFunction): Promise { + const response = await this.getDbConnectStringRaw(initOverrides); + return await response.value(); + } + +} diff --git a/applications/samples/frontend/src/rest/apis/ResourceApi.ts b/applications/samples/frontend/src/rest/samples/apis/ResourceApi.ts similarity index 100% rename from applications/samples/frontend/src/rest/apis/ResourceApi.ts rename to applications/samples/frontend/src/rest/samples/apis/ResourceApi.ts diff --git a/applications/samples/frontend/src/rest/apis/TestApi.ts b/applications/samples/frontend/src/rest/samples/apis/TestApi.ts similarity index 54% rename from applications/samples/frontend/src/rest/apis/TestApi.ts rename to applications/samples/frontend/src/rest/samples/apis/TestApi.ts index 38f6b820e..ecd3071d9 100644 --- a/applications/samples/frontend/src/rest/apis/TestApi.ts +++ b/applications/samples/frontend/src/rest/samples/apis/TestApi.ts @@ -14,6 +14,20 @@ import * as runtime from '../runtime'; +import type { + WriteFile200Response, + WriteFileRequest, +} from '../models/index'; +import { + WriteFile200ResponseFromJSON, + WriteFile200ResponseToJSON, + WriteFileRequestFromJSON, + WriteFileRequestToJSON, +} from '../models/index'; + +export interface WriteFileOperationRequest { + writeFileRequest?: WriteFileRequest; +} /** * @@ -22,7 +36,6 @@ export class TestApi extends runtime.BaseAPI { /** * test sentry is working - * @deprecated */ async errorRaw(initOverrides?: RequestInit | runtime.InitOverrideFunction): Promise> { const queryParameters: any = {}; @@ -45,7 +58,6 @@ export class TestApi extends runtime.BaseAPI { /** * test sentry is working - * @deprecated */ async error(initOverrides?: RequestInit | runtime.InitOverrideFunction): Promise { const response = await this.errorRaw(initOverrides); @@ -82,4 +94,35 @@ export class TestApi extends runtime.BaseAPI { return await response.value(); } + /** + * Writes a timestamped file on the application volume and returns the name of the pod that handled the request. On a statefulset deployment, route this endpoint to the leader service to have all writes land on pod 0. + * writes a file on the application volume + */ + async writeFileRaw(requestParameters: WriteFileOperationRequest, initOverrides?: RequestInit | runtime.InitOverrideFunction): Promise> { + const queryParameters: any = {}; + + const headerParameters: runtime.HTTPHeaders = {}; + + headerParameters['Content-Type'] = 'application/json'; + + const response = await this.request({ + path: `/write-file`, + method: 'POST', + headers: headerParameters, + query: queryParameters, + body: WriteFileRequestToJSON(requestParameters['writeFileRequest']), + }, initOverrides); + + return new runtime.JSONApiResponse(response, (jsonValue) => WriteFile200ResponseFromJSON(jsonValue)); + } + + /** + * Writes a timestamped file on the application volume and returns the name of the pod that handled the request. On a statefulset deployment, route this endpoint to the leader service to have all writes land on pod 0. + * writes a file on the application volume + */ + async writeFile(requestParameters: WriteFileOperationRequest = {}, initOverrides?: RequestInit | runtime.InitOverrideFunction): Promise { + const response = await this.writeFileRaw(requestParameters, initOverrides); + return await response.value(); + } + } diff --git a/applications/samples/frontend/src/rest/apis/WorkflowsApi.ts b/applications/samples/frontend/src/rest/samples/apis/WorkflowsApi.ts similarity index 98% rename from applications/samples/frontend/src/rest/apis/WorkflowsApi.ts rename to applications/samples/frontend/src/rest/samples/apis/WorkflowsApi.ts index c70e2df62..995e00659 100644 --- a/applications/samples/frontend/src/rest/apis/WorkflowsApi.ts +++ b/applications/samples/frontend/src/rest/samples/apis/WorkflowsApi.ts @@ -60,7 +60,6 @@ export class WorkflowsApi extends runtime.BaseAPI { /** * Send a synchronous operation - * @deprecated */ async submitSyncRaw(initOverrides?: RequestInit | runtime.InitOverrideFunction): Promise> { const queryParameters: any = {}; @@ -79,7 +78,6 @@ export class WorkflowsApi extends runtime.BaseAPI { /** * Send a synchronous operation - * @deprecated */ async submitSync(initOverrides?: RequestInit | runtime.InitOverrideFunction): Promise { const response = await this.submitSyncRaw(initOverrides); @@ -88,7 +86,6 @@ export class WorkflowsApi extends runtime.BaseAPI { /** * Send a synchronous operation and get results using the event queue. Just a sum, but in the cloud - * @deprecated */ async submitSyncWithResultsRaw(requestParameters: SubmitSyncWithResultsRequest, initOverrides?: RequestInit | runtime.InitOverrideFunction): Promise> { if (requestParameters['a'] == null) { @@ -133,7 +130,6 @@ export class WorkflowsApi extends runtime.BaseAPI { /** * Send a synchronous operation and get results using the event queue. Just a sum, but in the cloud - * @deprecated */ async submitSyncWithResults(requestParameters: SubmitSyncWithResultsRequest, initOverrides?: RequestInit | runtime.InitOverrideFunction): Promise { const response = await this.submitSyncWithResultsRaw(requestParameters, initOverrides); diff --git a/applications/samples/frontend/src/rest/apis/index.ts b/applications/samples/frontend/src/rest/samples/apis/index.ts similarity index 83% rename from applications/samples/frontend/src/rest/apis/index.ts rename to applications/samples/frontend/src/rest/samples/apis/index.ts index c90e98f7c..789f346e2 100644 --- a/applications/samples/frontend/src/rest/apis/index.ts +++ b/applications/samples/frontend/src/rest/samples/apis/index.ts @@ -1,6 +1,7 @@ /* tslint:disable */ /* eslint-disable */ export * from './AuthApi'; +export * from './DatabaseApi'; export * from './ResourceApi'; export * from './TestApi'; export * from './WorkflowsApi'; diff --git a/applications/samples/frontend/src/rest/index.ts b/applications/samples/frontend/src/rest/samples/index.ts similarity index 100% rename from applications/samples/frontend/src/rest/index.ts rename to applications/samples/frontend/src/rest/samples/index.ts diff --git a/applications/samples/frontend/src/rest/models/InlineResponse202.ts b/applications/samples/frontend/src/rest/samples/models/InlineResponse202.ts similarity index 100% rename from applications/samples/frontend/src/rest/models/InlineResponse202.ts rename to applications/samples/frontend/src/rest/samples/models/InlineResponse202.ts diff --git a/applications/samples/frontend/src/rest/models/InlineResponse202Task.ts b/applications/samples/frontend/src/rest/samples/models/InlineResponse202Task.ts similarity index 100% rename from applications/samples/frontend/src/rest/models/InlineResponse202Task.ts rename to applications/samples/frontend/src/rest/samples/models/InlineResponse202Task.ts diff --git a/applications/samples/frontend/src/rest/models/SampleResource.ts b/applications/samples/frontend/src/rest/samples/models/SampleResource.ts similarity index 100% rename from applications/samples/frontend/src/rest/models/SampleResource.ts rename to applications/samples/frontend/src/rest/samples/models/SampleResource.ts diff --git a/applications/samples/frontend/src/rest/samples/models/WriteFile200Response.ts b/applications/samples/frontend/src/rest/samples/models/WriteFile200Response.ts new file mode 100644 index 000000000..aa0cc214c --- /dev/null +++ b/applications/samples/frontend/src/rest/samples/models/WriteFile200Response.ts @@ -0,0 +1,76 @@ +/* tslint:disable */ +/* eslint-disable */ +/** + * CloudHarness Sample API + * CloudHarness Sample api + * + * The version of the OpenAPI document: 0.1.0 + * Contact: cloudharness@metacell.us + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + +import { mapValues } from '../runtime'; +/** + * + * @export + * @interface WriteFile200Response + */ +export interface WriteFile200Response { + /** + * + * @type {string} + * @memberof WriteFile200Response + */ + filename?: string; + /** + * + * @type {string} + * @memberof WriteFile200Response + */ + path?: string; + /** + * + * @type {string} + * @memberof WriteFile200Response + */ + hostname?: string; +} + +/** + * Check if a given object implements the WriteFile200Response interface. + */ +export function instanceOfWriteFile200Response(value: object): value is WriteFile200Response { + return true; +} + +export function WriteFile200ResponseFromJSON(json: any): WriteFile200Response { + return WriteFile200ResponseFromJSONTyped(json, false); +} + +export function WriteFile200ResponseFromJSONTyped(json: any, ignoreDiscriminator: boolean): WriteFile200Response { + if (json == null) { + return json; + } + return { + + 'filename': json['filename'] == null ? undefined : json['filename'], + 'path': json['path'] == null ? undefined : json['path'], + 'hostname': json['hostname'] == null ? undefined : json['hostname'], + }; +} + +export function WriteFile200ResponseToJSON(value?: WriteFile200Response | null): any { + if (value == null) { + return value; + } + return { + + 'filename': value['filename'], + 'path': value['path'], + 'hostname': value['hostname'], + }; +} + diff --git a/applications/samples/frontend/src/rest/samples/models/WriteFileRequest.ts b/applications/samples/frontend/src/rest/samples/models/WriteFileRequest.ts new file mode 100644 index 000000000..2ec176fad --- /dev/null +++ b/applications/samples/frontend/src/rest/samples/models/WriteFileRequest.ts @@ -0,0 +1,60 @@ +/* tslint:disable */ +/* eslint-disable */ +/** + * CloudHarness Sample API + * CloudHarness Sample api + * + * The version of the OpenAPI document: 0.1.0 + * Contact: cloudharness@metacell.us + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + +import { mapValues } from '../runtime'; +/** + * + * @export + * @interface WriteFileRequest + */ +export interface WriteFileRequest { + /** + * + * @type {string} + * @memberof WriteFileRequest + */ + content?: string; +} + +/** + * Check if a given object implements the WriteFileRequest interface. + */ +export function instanceOfWriteFileRequest(value: object): value is WriteFileRequest { + return true; +} + +export function WriteFileRequestFromJSON(json: any): WriteFileRequest { + return WriteFileRequestFromJSONTyped(json, false); +} + +export function WriteFileRequestFromJSONTyped(json: any, ignoreDiscriminator: boolean): WriteFileRequest { + if (json == null) { + return json; + } + return { + + 'content': json['content'] == null ? undefined : json['content'], + }; +} + +export function WriteFileRequestToJSON(value?: WriteFileRequest | null): any { + if (value == null) { + return value; + } + return { + + 'content': value['content'], + }; +} + diff --git a/applications/samples/frontend/src/rest/models/index.ts b/applications/samples/frontend/src/rest/samples/models/index.ts similarity index 66% rename from applications/samples/frontend/src/rest/models/index.ts rename to applications/samples/frontend/src/rest/samples/models/index.ts index 63b4e5e46..ff3aa576a 100644 --- a/applications/samples/frontend/src/rest/models/index.ts +++ b/applications/samples/frontend/src/rest/samples/models/index.ts @@ -3,3 +3,5 @@ export * from './InlineResponse202'; export * from './InlineResponse202Task'; export * from './SampleResource'; +export * from './WriteFile200Response'; +export * from './WriteFileRequest'; diff --git a/applications/samples/frontend/src/rest/runtime.ts b/applications/samples/frontend/src/rest/samples/runtime.ts similarity index 100% rename from applications/samples/frontend/src/rest/runtime.ts rename to applications/samples/frontend/src/rest/samples/runtime.ts From e9f3114ef48b92b7cc76a004be50077b9ac8b482 Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Tue, 1 Sep 2026 13:09:26 +0200 Subject: [PATCH 09/15] Fix regression post refactoring --- applications/samples/backend/requirements.txt | 13 ++++++++++--- .../cloudharness/utils/__init__.py | 11 ++++++++++- libraries/cloudharness-common/tests/test_server.py | 2 +- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/applications/samples/backend/requirements.txt b/applications/samples/backend/requirements.txt index 36508e87c..2cb06891c 100644 --- a/applications/samples/backend/requirements.txt +++ b/applications/samples/backend/requirements.txt @@ -1,6 +1,13 @@ -connexion[swagger-ui] >= 2.6.0, <= 2.14.2 -werkzeug >= 2.3.8, < 2.4 +connexion[swagger-ui] >= 2.6.0; python_version>="3.6" +# 2.3 is the last version that supports python 3.4-3.5 +connexion[swagger-ui] <= 2.3.0; python_version=="3.5" or python_version=="3.4" +# prevent breaking dependencies from advent of connexion>=3.0 +connexion[swagger-ui] <= 2.14.2; python_version>"3.4" +# connexion requires werkzeug but connexion < 2.4.0 does not install werkzeug +# we must peg werkzeug versions below to fix connexion +# https://github.com/zalando/connexion/pull/1044 +werkzeug == 0.16.1; python_version=="3.5" or python_version=="3.4" swagger-ui-bundle >= 0.0.2 python_dateutil >= 2.6.0 setuptools >= 21.0.0 -Flask >= 2.3.3, < 2.4 +Flask == 2.1.1 diff --git a/libraries/cloudharness-common/cloudharness/utils/__init__.py b/libraries/cloudharness-common/cloudharness/utils/__init__.py index 723cc2046..5aace16ec 100644 --- a/libraries/cloudharness-common/cloudharness/utils/__init__.py +++ b/libraries/cloudharness-common/cloudharness/utils/__init__.py @@ -1,5 +1,14 @@ import collections -import flask_server as server # Backwards compatibility + + +def __getattr__(name): + # Backwards compatibility: cloudharness.utils.server was renamed to + # flask_server. Import lazily - flask_server pulls in flask/connexion and + # cloudharness.applications, which would be a circular import here. + if name == "server": + from . import flask_server + return flask_server + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") def dict_merge(dct, merge_dct, add_keys=True, merge_none=True): """ Recursive dict merge. Inspired by :meth:``dict.update()``, instead of diff --git a/libraries/cloudharness-common/tests/test_server.py b/libraries/cloudharness-common/tests/test_server.py index 64855b35d..f91547ab9 100644 --- a/libraries/cloudharness-common/tests/test_server.py +++ b/libraries/cloudharness-common/tests/test_server.py @@ -4,7 +4,7 @@ import flask -from cloudharness.utils.server import JSONEncoder +from cloudharness.utils.flask_server import JSONEncoder class ConnexionStyleModel: From 2c8249282acbe8cc84d11f991242d63d0de0720a Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Tue, 1 Sep 2026 14:53:54 +0200 Subject: [PATCH 10/15] Backward compatibility fix --- .../cloudharness/utils/server.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 libraries/cloudharness-common/cloudharness/utils/server.py diff --git a/libraries/cloudharness-common/cloudharness/utils/server.py b/libraries/cloudharness-common/cloudharness/utils/server.py new file mode 100644 index 000000000..da43a6fb8 --- /dev/null +++ b/libraries/cloudharness-common/cloudharness/utils/server.py @@ -0,0 +1,13 @@ +"""Backwards compatibility: this module was renamed to `cloudharness.utils.flask_server`. + +Aliasing through `sys.modules` rather than re-exporting keeps `cloudharness.utils.server` +and `cloudharness.utils.flask_server` the *same* module object, so module level state that +`init_flask` rebinds (notably `app`) stays visible through both names. A `__getattr__` on +the package cannot replace this file: PEP 562 covers attribute access, not the submodule +resolution that `from cloudharness.utils.server import init_flask` goes through. +""" +import sys + +from . import flask_server + +sys.modules[__name__] = flask_server From 6d7ad2ae83635e366bad97051d84757916adfd54 Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Tue, 1 Sep 2026 14:55:35 +0200 Subject: [PATCH 11/15] CH-289 rearrange wait-timeouts on test and dev deployments --- .../flask-server/backend/__APP_NAME__/__main__.py | 2 +- application-templates/webapp/backend/__APP_NAME__/__main__.py | 2 +- applications/common/server/common/__main__.py | 2 +- applications/samples/backend/samples/__main__.py | 2 +- applications/volumemanager/server/volumemanager/__main__.py | 2 +- applications/workflows/server/workflows_api/__main__.py | 2 +- deployment-configuration/codefresh-template-dev.yaml | 3 ++- deployment-configuration/codefresh-template-stage.yaml | 1 + deployment-configuration/codefresh-template-test.yaml | 3 ++- deployment/codefresh-test.yaml | 1 + .../resources_buggy/applications/myapp/myapp_code/__main__.py | 2 +- .../resources_buggy/applications/myapp2/myapp_code/__main__.py | 2 +- 12 files changed, 14 insertions(+), 10 deletions(-) diff --git a/application-templates/flask-server/backend/__APP_NAME__/__main__.py b/application-templates/flask-server/backend/__APP_NAME__/__main__.py index f2f198da0..68fbd1c33 100644 --- a/application-templates/flask-server/backend/__APP_NAME__/__main__.py +++ b/application-templates/flask-server/backend/__APP_NAME__/__main__.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 -from cloudharness.utils.server import init_flask, main +from cloudharness.utils.flask_server import init_flask, main app = init_flask(title="__APP_NAME__", init_app_fn=None, webapp=False) diff --git a/application-templates/webapp/backend/__APP_NAME__/__main__.py b/application-templates/webapp/backend/__APP_NAME__/__main__.py index a4b264cb9..811bcbd0e 100644 --- a/application-templates/webapp/backend/__APP_NAME__/__main__.py +++ b/application-templates/webapp/backend/__APP_NAME__/__main__.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 -from cloudharness.utils.server import init_flask, main +from cloudharness.utils.flask_server import init_flask, main app = init_flask(title="__APP_NAME__", init_app_fn=None, webapp=True) diff --git a/applications/common/server/common/__main__.py b/applications/common/server/common/__main__.py index ffcf7ebd9..44e326eac 100644 --- a/applications/common/server/common/__main__.py +++ b/applications/common/server/common/__main__.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 -from cloudharness.utils.server import init_flask, main +from cloudharness.utils.flask_server import init_flask, main from cloudharness import log diff --git a/applications/samples/backend/samples/__main__.py b/applications/samples/backend/samples/__main__.py index 3118318a7..2eea53c49 100644 --- a/applications/samples/backend/samples/__main__.py +++ b/applications/samples/backend/samples/__main__.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 -from cloudharness.utils.server import init_flask, main +from cloudharness.utils.flask_server import init_flask, main app = init_flask(title="Cloudharness sample application", webapp=True) diff --git a/applications/volumemanager/server/volumemanager/__main__.py b/applications/volumemanager/server/volumemanager/__main__.py index 24522b72a..356cfbe01 100644 --- a/applications/volumemanager/server/volumemanager/__main__.py +++ b/applications/volumemanager/server/volumemanager/__main__.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 -from cloudharness.utils.server import init_flask, main +from cloudharness.utils.flask_server import init_flask, main app = init_flask(title="Volume manager") diff --git a/applications/workflows/server/workflows_api/__main__.py b/applications/workflows/server/workflows_api/__main__.py index bafa51099..95e93dbd2 100644 --- a/applications/workflows/server/workflows_api/__main__.py +++ b/applications/workflows/server/workflows_api/__main__.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 -from cloudharness.utils.server import init_flask, main +from cloudharness.utils.flask_server import init_flask, main app = init_flask() diff --git a/deployment-configuration/codefresh-template-dev.yaml b/deployment-configuration/codefresh-template-dev.yaml index 68b353801..0512075cb 100644 --- a/deployment-configuration/codefresh-template-dev.yaml +++ b/deployment-configuration/codefresh-template-dev.yaml @@ -77,7 +77,7 @@ steps: kube_context: ${{CLUSTER_NAME}} namespace: ${{NAMESPACE}} chart_version: ${{CF_BUILD_ID}} - cmd_ps: --wait --timeout 600s --create-namespace + cmd_ps: --timeout 200s --create-namespace custom_value_files: - ./deployment/helm/values.yaml build_test_images: @@ -93,6 +93,7 @@ steps: stage: qa title: Wait deployment to be ready image: codefresh/kubectl + timeout: 10m commands: - kubectl config use-context ${{CLUSTER_NAME}} - kubectl config set-context --current --namespace=${{NAMESPACE}} diff --git a/deployment-configuration/codefresh-template-stage.yaml b/deployment-configuration/codefresh-template-stage.yaml index e8df8bb50..d812a79f7 100644 --- a/deployment-configuration/codefresh-template-stage.yaml +++ b/deployment-configuration/codefresh-template-stage.yaml @@ -75,6 +75,7 @@ steps: stage: qa title: Wait deployment to be ready image: codefresh/kubectl + timeout: 10m commands: - kubectl config use-context ${{CLUSTER_NAME}} - kubectl config set-context --current --namespace=${{NAMESPACE}} diff --git a/deployment-configuration/codefresh-template-test.yaml b/deployment-configuration/codefresh-template-test.yaml index 31d16fb78..bc34913cd 100644 --- a/deployment-configuration/codefresh-template-test.yaml +++ b/deployment-configuration/codefresh-template-test.yaml @@ -76,7 +76,7 @@ steps: kube_context: ${{CLUSTER_NAME}} namespace: test-${{NAMESPACE_BASENAME}} chart_version: ${{CF_SHORT_REVISION}} - cmd_ps: "--timeout 600s --create-namespace" + cmd_ps: "--timeout 200s --create-namespace" custom_value_files: - ./deployment/helm/values.yaml build_test_images: @@ -88,6 +88,7 @@ steps: stage: qa title: Wait deployment to be ready image: codefresh/kubectl + timeout: 10m commands: - kubectl config use-context ${{CLUSTER_NAME}} - kubectl config set-context --current --namespace=test-${{NAMESPACE_BASENAME}} diff --git a/deployment/codefresh-test.yaml b/deployment/codefresh-test.yaml index 1763b7da2..c4821d6ae 100644 --- a/deployment/codefresh-test.yaml +++ b/deployment/codefresh-test.yaml @@ -528,6 +528,7 @@ steps: stage: qa title: Wait deployment to be ready image: codefresh/kubectl + timeout: 10m commands: - kubectl config use-context ${{CLUSTER_NAME}} - kubectl config set-context --current --namespace=test-${{NAMESPACE_BASENAME}} diff --git a/tools/deployment-cli-tools/tests/resources_buggy/applications/myapp/myapp_code/__main__.py b/tools/deployment-cli-tools/tests/resources_buggy/applications/myapp/myapp_code/__main__.py index 3118318a7..2eea53c49 100644 --- a/tools/deployment-cli-tools/tests/resources_buggy/applications/myapp/myapp_code/__main__.py +++ b/tools/deployment-cli-tools/tests/resources_buggy/applications/myapp/myapp_code/__main__.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 -from cloudharness.utils.server import init_flask, main +from cloudharness.utils.flask_server import init_flask, main app = init_flask(title="Cloudharness sample application", webapp=True) diff --git a/tools/deployment-cli-tools/tests/resources_buggy/applications/myapp2/myapp_code/__main__.py b/tools/deployment-cli-tools/tests/resources_buggy/applications/myapp2/myapp_code/__main__.py index 3118318a7..2eea53c49 100644 --- a/tools/deployment-cli-tools/tests/resources_buggy/applications/myapp2/myapp_code/__main__.py +++ b/tools/deployment-cli-tools/tests/resources_buggy/applications/myapp2/myapp_code/__main__.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 -from cloudharness.utils.server import init_flask, main +from cloudharness.utils.flask_server import init_flask, main app = init_flask(title="Cloudharness sample application", webapp=True) From e27da91098d98c1ac0e858e474bda8b14acbe9f6 Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Tue, 1 Sep 2026 15:38:24 +0200 Subject: [PATCH 12/15] Fix statefulsets not updating after failed roll --- deployment-configuration/helm/templates/auto-database.yaml | 6 ++++++ .../helm/templates/auto-deployments.yaml | 6 ++++++ tools/deployment-cli-tools/tests/test_helm.py | 4 ++++ 3 files changed, 16 insertions(+) diff --git a/deployment-configuration/helm/templates/auto-database.yaml b/deployment-configuration/helm/templates/auto-database.yaml index 69d2c8c68..40e48ac60 100644 --- a/deployment-configuration/helm/templates/auto-database.yaml +++ b/deployment-configuration/helm/templates/auto-database.yaml @@ -63,6 +63,12 @@ spec: # StatefulSet updates terminate the old pod before creating its replacement, so no # Recreate strategy or node pinning is needed. serviceName: {{ .app.harness.database.name | quote }} + # With the default OrderedReady policy the controller waits for every existing pod + # to be Running and Ready before applying a template update, so a crash-looping pod + # blocks the rollout of its own fix. Parallel lifts that gate; ordered bring-up is + # not needed for a single-replica database. Immutable field: changing it on an + # existing deployment requires deleting the StatefulSet (use --cascade=orphan). + podManagementPolicy: Parallel {{- else }} # The database's ReadWriteOnce volume attaches to a single node and the pod is # pinned to it via podAffinity. Recreate terminates the old pod before starting diff --git a/deployment-configuration/helm/templates/auto-deployments.yaml b/deployment-configuration/helm/templates/auto-deployments.yaml index dd98c4a84..d12201612 100644 --- a/deployment-configuration/helm/templates/auto-deployments.yaml +++ b/deployment-configuration/helm/templates/auto-deployments.yaml @@ -45,6 +45,12 @@ spec: # StatefulSet updates terminate the old pod before creating its replacement, so no # Recreate strategy or node pinning is needed even for ReadWriteOnce volumes. serviceName: {{ .app.harness.service.name | quote }} + # With the default OrderedReady policy the controller waits for every existing pod + # to be Running and Ready before applying a template update, so a crash-looping pod + # blocks the rollout of its own fix. Parallel lifts that gate; ordered bring-up is + # not needed for these single-app workloads. Immutable field: changing it on an + # existing deployment requires deleting the StatefulSet (use --cascade=orphan). + podManagementPolicy: Parallel {{- else if $rwoVolume }} # A ReadWriteOnce volume attaches to a single node and the pod is pinned to the # volume's node via podAffinity (see below). Recreate terminates the old pod diff --git a/tools/deployment-cli-tools/tests/test_helm.py b/tools/deployment-cli-tools/tests/test_helm.py index 0767dc14a..90914b18e 100644 --- a/tools/deployment-cli-tools/tests/test_helm.py +++ b/tools/deployment-cli-tools/tests/test_helm.py @@ -473,6 +473,9 @@ def test_statefulset_option(tmp_path): manifests = render_helm_chart(helm_path) sts = find_manifest(manifests, 'StatefulSet', dep_name) assert sts['spec']['serviceName'] == service_name + # OrderedReady would block template updates while an existing pod is unready, + # so a crash-looping pod could never be replaced by its own fix. + assert sts['spec']['podManagementPolicy'] == 'Parallel' assert 'strategy' not in sts['spec'] assert 'affinity' not in sts['spec']['template']['spec'] assert 'initContainers' not in sts['spec']['template']['spec'] @@ -485,6 +488,7 @@ def test_statefulset_option(tmp_path): db_sts = find_manifest(manifests, 'StatefulSet', db_name) assert db_sts['spec']['serviceName'] == db_name + assert db_sts['spec']['podManagementPolicy'] == 'Parallel' assert 'strategy' not in db_sts['spec'] assert 'affinity' not in db_sts['spec']['template']['spec'] assert 'initContainers' not in db_sts['spec']['template']['spec'] From a9a4ed6a89dc154c576b12121cf09651204f2d4c Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Tue, 1 Sep 2026 15:49:42 +0200 Subject: [PATCH 13/15] Fix sample test issue --- applications/samples/api/openapi.yaml | 4 ++++ applications/samples/backend/samples/openapi/openapi.yaml | 1 + 2 files changed, 5 insertions(+) diff --git a/applications/samples/api/openapi.yaml b/applications/samples/api/openapi.yaml index 96da7ed44..24381a476 100644 --- a/applications/samples/api/openapi.yaml +++ b/applications/samples/api/openapi.yaml @@ -55,6 +55,10 @@ paths: application/json: schema: type: object + # `nullable` is what actually makes the body optional for connexion: + # `required: false` alone still 400s an absent body, because the json + # validator only skips validation when the schema is nullable. + nullable: true properties: content: type: string diff --git a/applications/samples/backend/samples/openapi/openapi.yaml b/applications/samples/backend/samples/openapi/openapi.yaml index 3b4b40593..6bf28c13f 100644 --- a/applications/samples/backend/samples/openapi/openapi.yaml +++ b/applications/samples/backend/samples/openapi/openapi.yaml @@ -369,6 +369,7 @@ components: title: SampleResource type: object write_file_request: + nullable: true properties: content: title: content From bce0f43bc221f880717769bd38b031dba0222f59 Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Tue, 1 Sep 2026 18:52:10 +0200 Subject: [PATCH 14/15] chore: linting fixes --- applications/samples/backend/samples/util.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/applications/samples/backend/samples/util.py b/applications/samples/backend/samples/util.py index 5b241814f..b802fafda 100644 --- a/applications/samples/backend/samples/util.py +++ b/applications/samples/backend/samples/util.py @@ -67,8 +67,8 @@ def deserialize_date(string): :rtype: date """ if string is None: - return None - + return None + try: from dateutil.parser import parse return parse(string).date() @@ -87,8 +87,8 @@ def deserialize_datetime(string): :rtype: datetime """ if string is None: - return None - + return None + try: from dateutil.parser import parse return parse(string) @@ -144,4 +144,4 @@ def _deserialize_dict(data, boxed_type): :rtype: dict """ return {k: _deserialize(v, boxed_type) - for k, v in data.items() } + for k, v in data.items()} From f76e11981ce8bf7f805368cc363d1639958fd5cd Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Tue, 1 Sep 2026 18:54:35 +0200 Subject: [PATCH 15/15] chore: linting fixes --- libraries/cloudharness-common/cloudharness/utils/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/cloudharness-common/cloudharness/utils/__init__.py b/libraries/cloudharness-common/cloudharness/utils/__init__.py index 5aace16ec..ca30f3399 100644 --- a/libraries/cloudharness-common/cloudharness/utils/__init__.py +++ b/libraries/cloudharness-common/cloudharness/utils/__init__.py @@ -10,6 +10,7 @@ def __getattr__(name): return flask_server raise AttributeError(f"module {__name__!r} has no attribute {name!r}") + def dict_merge(dct, merge_dct, add_keys=True, merge_none=True): """ Recursive dict merge. Inspired by :meth:``dict.update()``, instead of updating only top-level keys, dict_merge recurses down into dicts nested @@ -48,4 +49,4 @@ def dict_merge(dct, merge_dct, add_keys=True, merge_none=True): return dct -__all__ = ["dict_merge", "server"] \ No newline at end of file +__all__ = ["dict_merge", "server"]