From ef523516ae34cd07637fca951f04c954c99525e1 Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Wed, 16 Sep 2026 15:44:58 +0200 Subject: [PATCH 1/8] CH-293 support for application multi instances --- .../instance1/resources/example.yaml | 3 + .../deploy/instances/instance1/values.yaml | 8 + .../helm/templates/secrets/_secrets.tpl | 2 +- docs/applications/README.md | 71 +++ .../ch_cli_tools/common_types.py | 4 + .../ch_cli_tools/configurationgenerator.py | 271 ++++++------ .../ch_cli_tools/constants.py | 8 + .../ch_cli_tools/dockercompose.py | 8 +- .../deployment-cli-tools/ch_cli_tools/helm.py | 8 +- .../ch_cli_tools/instances.py | 235 ++++++++++ .../ch_cli_tools/utils.py | 15 + .../instances/inst1/resources/aresource.txt | 1 + .../inst1/resources/instanceonly.txt | 1 + .../instances/inst1/templates/mytemplate.yaml | 7 + .../deploy/instances/inst1/values-dev.yaml | 4 + .../myapp/deploy/instances/inst1/values.yaml | 5 + tools/deployment-cli-tools/tests/test_helm.py | 410 ++++++++++++++++++ 17 files changed, 919 insertions(+), 142 deletions(-) create mode 100644 applications/samples/deploy/instances/instance1/resources/example.yaml create mode 100644 applications/samples/deploy/instances/instance1/values.yaml create mode 100644 tools/deployment-cli-tools/ch_cli_tools/constants.py create mode 100644 tools/deployment-cli-tools/ch_cli_tools/instances.py create mode 100644 tools/deployment-cli-tools/tests/resources/applications/myapp/deploy/instances/inst1/resources/aresource.txt create mode 100644 tools/deployment-cli-tools/tests/resources/applications/myapp/deploy/instances/inst1/resources/instanceonly.txt create mode 100644 tools/deployment-cli-tools/tests/resources/applications/myapp/deploy/instances/inst1/templates/mytemplate.yaml create mode 100644 tools/deployment-cli-tools/tests/resources/applications/myapp/deploy/instances/inst1/values-dev.yaml create mode 100644 tools/deployment-cli-tools/tests/resources/applications/myapp/deploy/instances/inst1/values.yaml diff --git a/applications/samples/deploy/instances/instance1/resources/example.yaml b/applications/samples/deploy/instances/instance1/resources/example.yaml new file mode 100644 index 000000000..7c7357c74 --- /dev/null +++ b/applications/samples/deploy/instances/instance1/resources/example.yaml @@ -0,0 +1,3 @@ +hey: "this" +is: "an instance overridden" +yaml: "file" \ No newline at end of file diff --git a/applications/samples/deploy/instances/instance1/values.yaml b/applications/samples/deploy/instances/instance1/values.yaml new file mode 100644 index 000000000..4a4ca6691 --- /dev/null +++ b/applications/samples/deploy/instances/instance1/values.yaml @@ -0,0 +1,8 @@ +harness: + subdomain: samples1 + deployment: + replicas: 1 + database: + connect_string: "instance1 connection string" + envmap: + ENVIRONMENT_TEST_A: "instance1 value" diff --git a/deployment-configuration/helm/templates/secrets/_secrets.tpl b/deployment-configuration/helm/templates/secrets/_secrets.tpl index 9432431b9..edc670c85 100644 --- a/deployment-configuration/helm/templates/secrets/_secrets.tpl +++ b/deployment-configuration/helm/templates/secrets/_secrets.tpl @@ -216,8 +216,8 @@ Usage: {{ include "deploy_utils.secretManagerResources" (dict "root" $root "app" {{- if include "deploy_utils.secretIsExternal" (dict "spec" $spec) }} {{- $manager := include "deploy_utils.secretManager" (dict "spec" $spec) }} {{- $context := dict "root" $root "app" $app "name" $name "spec" $spec "resourceName" (include "deploy_utils.secretResourceName" (dict "app" $app "name" $name)) }} ---- {{ include (printf "deploy_utils.secretmanager.%s.resource" $manager) $context }} +--- {{- end }} {{- end }} {{- end -}} diff --git a/docs/applications/README.md b/docs/applications/README.md index 5a2021672..7c45a5b80 100644 --- a/docs/applications/README.md +++ b/docs/applications/README.md @@ -70,6 +70,77 @@ harness: To customize the helm templates to use, put them inside the *deploy* subdirectory. +## Application instances + +An application can be deployed several times over, each deployment on its own subdomain and with +its own configuration and database. Each of those deployments is an *instance*, declared as a +directory under the application's `deploy/instances`: + +``` +applications/samples/ + Dockerfile + deploy/ + values.yaml # the application's configuration + resources/ + example.yaml + myConfig.json + instances/ + instance1/ # the instance's name is its directory's + values.yaml # what this instance changes + resources/ + example.yaml # overrides the application's resource of the same name + templates/ # optional, overlaid on the application's templates +``` + +The instance above is deployed as the application `samples-instance1`: that key names its service, +deployment, database, volume, gatekeeper and configmaps, and is how it is referenced on the command +line. It runs the image built for `samples` — an instance adds no build, so declare no Dockerfile +in it. The key must not take over one of the application's task images: an instance named `print` +on an application with a `tasks/print-file` is rejected, as `samples-print` would own +`samples-print-file`. + +Everything else is inherited from the application, so an instance's `values.yaml` only carries what +it changes: + +```yaml +harness: + subdomain: samples1 + deployment: + replicas: 1 +``` + +Values are merged over the application's the usual way: mappings key by key, lists as a whole. An +instance overriding `uri_role_mapping` therefore replaces the whole list rather than adding to it. +Resources and templates are overlaid file by file, so an instance inherits every file it does not +override — above, `myConfig.json` comes from the application and `example.yaml` from the instance. + +Environment specific values apply at both levels, the instance's taking precedence over the +application's: + +``` +deploy/instances/instance1/values-[ENV].yaml # wins +deploy/instances/instance1/values.yaml +deploy/values-[ENV].yaml +deploy/values.yaml # loses +``` + +What identifies the application is never inherited, so that an instance never claims the +application's hosts or resources: + +- `subdomain`, `aliases` and `domain`. An instance without a `subdomain` of its own answers on + its directory's name, so `instances/samples1/` alone is served at `samples1.[DOMAIN]`; declare + `subdomain: null` to give an instance no ingress at all +- the names of the service, deployment and database, which are derived from the instance key +- `deployment.volume.name`, prefixed with the instance key, so the instance never mounts the + application's storage +- `database.connect_string`, emptied: an instance of an application using an externally managed + database needs a connection string of its own. Set `database.auto: true` to have CloudHarness + deploy a database of its own for it instead. + +Instances are deployed together with their application: `harness-deployment -i samples` deploys +`samples` and all its instances, and `-e samples-instance1` leaves one out. CI builds and tests the +application only, since an instance runs the same image. + ## Dependency to an existing Helm chart TBD diff --git a/tools/deployment-cli-tools/ch_cli_tools/common_types.py b/tools/deployment-cli-tools/ch_cli_tools/common_types.py index eb1e5feb7..bc860e010 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/common_types.py +++ b/tools/deployment-cli-tools/ch_cli_tools/common_types.py @@ -3,6 +3,10 @@ from typing import Union +class ValuesValidationException(Exception): + """Raised when the values of a deployment do not make a valid configuration.""" + + try: from enum import StrEnum except ImportError: diff --git a/tools/deployment-cli-tools/ch_cli_tools/configurationgenerator.py b/tools/deployment-cli-tools/ch_cli_tools/configurationgenerator.py index 4b30e8604..3e00a83b3 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/configurationgenerator.py +++ b/tools/deployment-cli-tools/ch_cli_tools/configurationgenerator.py @@ -19,16 +19,16 @@ get_template, merge_configuration_directories, dict_merge, app_name_from_path, \ find_dockerfiles_paths, get_git_commit_hash, yaml from .secrets import secret_definition_error +# Re-exported on purpose: the rest of the cli tools and the tests read the deployment values +# vocabulary from here. Keep them importable — see test_values_vocabulary_is_re_exported. +from .constants import KEY_APPS, KEY_DATABASE, KEY_DEPLOYMENT, KEY_HARNESS, \ + KEY_SERVICE, KEY_TASK_IMAGES, KEY_TEST_IMAGES +from .common_types import ValuesValidationException +from .instances import build_instance_values, check_instance_collisions, collect_instances, inherit_parent_image, \ + instance_app_key, instance_directories, instance_names, resolve_instance_includes -KEY_HARNESS = 'harness' -KEY_SERVICE = 'service' -KEY_DATABASE = 'database' -KEY_DEPLOYMENT = 'deployment' -KEY_APPS = 'apps' -KEY_TASK_IMAGES = 'task-images' # KEY_TASK_IMAGES_BUILD = f"{KEY_TASK_IMAGES}-build" -KEY_TEST_IMAGES = 'test-images' DEFAULT_IGNORE = ('/tasks', '.dockerignore', '.hypothesis', "__pycache__", '.node_modules', 'dist', 'build', '.coverage') @@ -56,6 +56,7 @@ def __init__(self, root_paths: List[str], tag: Union[str, int, None] = 'latest', self.env = env or {} self.namespace = namespace self.calculate_hash_tags = calculate_hash_tags + check_instance_collisions(self.root_paths, exclude=self.exclude) # In this tree we will collect the and their parent dependencies self.build_tree: dict[str, list[str]] = {} @@ -147,16 +148,59 @@ def _load_all_app_values(self, helm_values): def _collect_app_values_lightweight(self, app_base_path, helm_values=None): """Collect only YAML-based values for all apps (no image processing).""" + return self._collect_root_app_values( + app_base_path, helm_values, + lambda app_name, app_path: self.load_app_values(app_name, app_path, helm_values=helm_values)) + + def _collect_root_app_values(self, app_base_path, helm_values, load_app_values): + """Values of the applications found under one root path, read with `load_app_values`, + and of the instances they declare. + + An instance is derived from its application's values as merged so far, this root + included, and added to the deployment right away: from then on it is an application + like any other, merged over the root paths that follow the same way its parent is. + """ + merged_apps = (helm_values or {}).get(KEY_APPS, {}) values = {} - for app_path in app_base_path.glob("*/"): + for app_path in app_base_path.glob("*/"): # We get the sub-files that are directories app_name = app_name_from_path(f"{app_path.relative_to(app_base_path)}") if app_name in self.exclude: continue - app_values = self.load_app_values(app_name, app_path, helm_values=helm_values) - values[app_name] = dict_merge( - values[app_name], app_values) if app_name in values else app_values + values[app_name] = load_app_values(app_name, app_path) + values.update(self.collect_instance_values( + app_name, dict_merge(merged_apps.get(app_name, {}), values[app_name]))) return values + def collect_instance_values(self, app_name, app_values): + """The instances declared by an application, as applications of their own derived from + `app_values`, the application's configuration merged so far. + + Instances are deployed together with their application, so `--include` is resolved over + them here: including either side includes the other. + """ + instances = {} + for instance_name, instance_values in collect_instances(app_name, self.root_paths, envs=self.env).items(): + app_key = instance_app_key(app_name, instance_name) + if app_key in self.exclude: + continue + instances[app_key] = build_instance_values(app_values, app_name, instance_name, instance_values) + if self.include: + self.include = resolve_instance_includes(self.include, app_name, instances) + return instances + + def _inherit_instance_images(self, helm_values): + """Give every instance the image of its parent application, once images are known. + + With `--include`, images are only computed when the included applications are finalized, + after their instances have been derived from them. + """ + apps = helm_values[KEY_APPS] + for app_name in list(apps): + for instance_name in instance_names(app_name, self.root_paths): + app_key = instance_app_key(app_name, instance_name) + if app_key in apps: + inherit_parent_image(apps[app_key], apps[app_name]) + def _finalize_included_app_values(self, helm_values, base_image_name=None): """Expensive pass: run Dockerfile discovery and image tagging for included apps only.""" included_apps = set(helm_values[KEY_APPS].keys()) @@ -173,21 +217,10 @@ def _finalize_included_app_values(self, helm_values, base_image_name=None): helm_values[KEY_APPS][app_name], finalized) def collect_app_values(self, app_base_path: Path, base_image_name=None, helm_values=None): - values = {} - - for app_path in app_base_path.glob("*/"): # We get the sub-files that are directories - app_name = app_name_from_path(f"{app_path.relative_to(app_base_path)}") - - if app_name in self.exclude: - continue - app_key = app_name - - app_values = self.create_app_values_spec(app_name, app_path, base_image_name=base_image_name, helm_values=helm_values) - - values[app_key] = dict_merge( - values[app_key], app_values) if app_key in values else app_values - - return values + return self._collect_root_app_values( + app_base_path, helm_values, + lambda app_name, app_path: self.create_app_values_spec( + app_name, app_path, base_image_name=base_image_name, helm_values=helm_values)) def _init_static_images(self, base_image_name): for i in range(len(self.root_paths)): @@ -655,10 +688,6 @@ def hosts_info(values): "\nTo test locally, update your hosts file" + f"\n{ip}\t{domain + ' ' + ' '.join(sd + '.' + domain for sd in subdomains)}") -class ValuesValidationException(Exception): - pass - - def validate_helm_values(values): validate_dependencies(values) validate_secrets(values) @@ -774,119 +803,91 @@ def collect_apps_helm_templates(search_root, dest_helm_chart_path, templates_pat if app_name in exclude or (include and not any(inc in app_name for inc in include)): continue - # Determine which template directory to use - regular_template_dir = app_path / 'deploy' / 'templates' - if templates_path == HELM_PATH: - template_dir = regular_template_dir - else: - template_dir = app_path / 'deploy' / f'templates-{templates_path}' + collect_app_deploy_directories( + app_path, app_name, dest_helm_chart_path, templates_path=templates_path, envs=envs) + + for instance_name, instance_path in instance_directories(app_path).items(): + instance_key = instance_app_key(app_name, instance_name) + if instance_key in exclude or (include and not any(inc in instance_key for inc in include)): + continue + # The instance's own files are collected over the application's, so that it inherits + # every resource and template it does not override. + collect_app_deploy_directories( + app_path, instance_key, dest_helm_chart_path, templates_path=templates_path, envs=envs) + collect_app_deploy_directories( + instance_path, instance_key, dest_helm_chart_path, templates_path=templates_path, + envs=envs, deploy_subpath='.') + + +def collect_app_deploy_directories(app_path, app_name, dest_helm_chart_path, templates_path=HELM_PATH, + envs=(), deploy_subpath='deploy'): + """Collect the templates, resources and sub-charts of an application into the destination + chart, under `app_name`. + + `deploy_subpath` is where those directories live inside `app_path`: applications keep them + in `deploy`, instances directly in their own directory. + """ + deploy_path = Path(app_path) / deploy_subpath - if template_dir.exists(): + # Determine which template directory to use + regular_template_dir = deploy_path / 'templates' + if templates_path == HELM_PATH: + template_dir = regular_template_dir + else: + template_dir = deploy_path / f'templates-{templates_path}' + + if template_dir.exists(): + dest_dir = dest_helm_chart_path / 'templates' / app_name + + logging.info( + "Collecting templates for application %s to %s", app_name, dest_dir) + if dest_dir.exists(): + logging.warning( + "Merging/overriding all files in directory %s", dest_dir) + merge_configuration_directories(f"{template_dir}", f"{dest_dir}", envs) + else: + shutil.copytree(template_dir, dest_dir) + if envs: + merge_configuration_directories(f"{dest_dir}", f"{dest_dir}", envs) + + # For non-helm mode (e.g., compose), also copy helper templates (_*.tpl) from regular + # templates directory. These are needed because resources (e.g., realm.json) may reference + # template helpers defined there. + if templates_path != HELM_PATH and regular_template_dir.exists(): + helper_files = list(regular_template_dir.glob("_*.tpl")) + if helper_files: dest_dir = dest_helm_chart_path / 'templates' / app_name + dest_dir.mkdir(parents=True, exist_ok=True) + logging.info( + "Collecting helper templates for application %s to %s", app_name, dest_dir) + for helper_file in helper_files: + dest_file = dest_dir / helper_file.name + if not dest_file.exists(): # Don't overwrite if templates-{path} provided one + shutil.copy(helper_file, dest_file) + + resources_dir = deploy_path / 'resources' + if resources_dir.exists(): + dest_dir = dest_helm_chart_path / 'resources' / app_name + + logging.info( + "Collecting resources for application %s to %s", app_name, dest_dir) + + merge_configuration_directories(f"{resources_dir}", f"{dest_dir}", envs) + if envs: + merge_configuration_directories(f"{dest_dir}", f"{dest_dir}", envs) + + if templates_path == HELM_PATH: + subchart_dir = deploy_path / 'charts' + if subchart_dir.exists(): + dest_dir = dest_helm_chart_path / 'charts' / app_name logging.info( "Collecting templates for application %s to %s", app_name, dest_dir) if dest_dir.exists(): logging.warning( "Merging/overriding all files in directory %s", dest_dir) - merge_configuration_directories(f"{template_dir}", f"{dest_dir}", envs) + merge_configuration_directories(f"{subchart_dir}", f"{dest_dir}", envs) else: - shutil.copytree(template_dir, dest_dir) + shutil.copytree(subchart_dir, dest_dir) if envs: merge_configuration_directories(f"{dest_dir}", f"{dest_dir}", envs) - - # For non-helm mode (e.g., compose), also copy helper templates (_*.tpl) from regular - # templates directory. These are needed because resources (e.g., realm.json) may reference - # template helpers defined there. - if templates_path != HELM_PATH and regular_template_dir.exists(): - helper_files = list(regular_template_dir.glob("_*.tpl")) - if helper_files: - dest_dir = dest_helm_chart_path / 'templates' / app_name - dest_dir.mkdir(parents=True, exist_ok=True) - logging.info( - "Collecting helper templates for application %s to %s", app_name, dest_dir) - for helper_file in helper_files: - dest_file = dest_dir / helper_file.name - if not dest_file.exists(): # Don't overwrite if templates-{path} provided one - shutil.copy(helper_file, dest_file) - - resources_dir = app_path / 'deploy' / 'resources' - if resources_dir.exists(): - dest_dir = dest_helm_chart_path / 'resources' / app_name - - logging.info( - "Collecting resources for application %s to %s", app_name, dest_dir) - - merge_configuration_directories(f"{resources_dir}", f"{dest_dir}", envs) - if envs: - merge_configuration_directories(f"{dest_dir}", f"{dest_dir}", envs) - - if templates_path == HELM_PATH: - subchart_dir = app_path / 'deploy/charts' - if subchart_dir.exists(): - dest_dir = dest_helm_chart_path / 'charts' / app_name - - logging.info( - "Collecting templates for application %s to %s", app_name, dest_dir) - if dest_dir.exists(): - logging.warning( - "Merging/overriding all files in directory %s", dest_dir) - merge_configuration_directories(f"{subchart_dir}", f"{dest_dir}", envs) - else: - shutil.copytree(subchart_dir, dest_dir) - if envs: - merge_configuration_directories(f"{dest_dir}", f"{dest_dir}", envs) - - -# def collect_apps_helm_templates(search_root, dest_helm_chart_path, templates_path=None, exclude=(), include=None): -# """ -# Searches recursively for helm templates inside the applications and collects the templates in the destination - -# :param search_root: -# :param dest_helm_chart_path: collected helm templates destination folder -# :param exclude: -# :return: -# """ -# app_base_path = os.path.join(search_root, APPS_PATH) - -# import ipdb; ipdb.set_trace() # fmt: skip - -# for app_path in get_sub_paths(app_base_path): -# app_name = app_name_from_path(os.path.relpath(app_path, app_base_path)) -# if app_name in exclude or (include and not any(inc in app_name for inc in include)): -# continue -# template_dir = os.path.join(app_path, 'deploy', 'templates') -# if os.path.exists(template_dir): -# dest_dir = os.path.join( -# dest_helm_chart_path, 'templates', app_name) - -# logging.info( -# "Collecting templates for application %s to %s", app_name, dest_dir) -# if os.path.exists(dest_dir): -# logging.warning( -# "Merging/overriding all files in directory %s", dest_dir) -# merge_configuration_directories(template_dir, dest_dir) -# else: -# shutil.copytree(template_dir, dest_dir) -# resources_dir = os.path.join(app_path, 'deploy/resources') -# if os.path.exists(resources_dir): -# dest_dir = os.path.join( -# dest_helm_chart_path, 'resources', app_name) - -# logging.info( -# "Collecting resources for application %s to %s", app_name, dest_dir) - -# merge_configuration_directories(resources_dir, dest_dir) - -# subchart_dir = os.path.join(app_path, 'deploy/charts') -# if os.path.exists(subchart_dir): -# dest_dir = os.path.join(dest_helm_chart_path, 'charts', app_name) - -# logging.info( -# "Collecting templates for application %s to %s", app_name, dest_dir) -# if os.path.exists(dest_dir): -# logging.warning( -# "Merging/overriding all files in directory %s", dest_dir) -# merge_configuration_directories(subchart_dir, dest_dir) -# else: -# shutil.copytree(subchart_dir, dest_dir) diff --git a/tools/deployment-cli-tools/ch_cli_tools/constants.py b/tools/deployment-cli-tools/ch_cli_tools/constants.py new file mode 100644 index 000000000..e35d035a7 --- /dev/null +++ b/tools/deployment-cli-tools/ch_cli_tools/constants.py @@ -0,0 +1,8 @@ +# Keys of the deployment values structure, shared by everything reading or writing it +KEY_HARNESS = 'harness' +KEY_SERVICE = 'service' +KEY_DATABASE = 'database' +KEY_DEPLOYMENT = 'deployment' +KEY_APPS = 'apps' +KEY_TASK_IMAGES = 'task-images' +KEY_TEST_IMAGES = 'test-images' diff --git a/tools/deployment-cli-tools/ch_cli_tools/dockercompose.py b/tools/deployment-cli-tools/ch_cli_tools/dockercompose.py index 05b0fd269..d9cff2917 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/dockercompose.py +++ b/tools/deployment-cli-tools/ch_cli_tools/dockercompose.py @@ -12,7 +12,7 @@ from cloudharness_utils.constants import VALUES_MANUAL_PATH, COMPOSE from .utils import get_cluster_ip, image_name_from_dockerfile_path, get_template, \ merge_to_yaml_file, dict_merge, app_name_from_path, find_dockerfiles_paths, find_file_paths, \ - yaml, yaml_rt + is_buildable_dockerfile_path, yaml, yaml_rt from .models import HarnessMainConfig @@ -93,6 +93,8 @@ def process_values(self) -> HarnessMainConfig: values, include = self.__finish_helm_values(values=helm_values, defer_task_images=False) + self._inherit_instance_images(helm_values) + # Adjust dependencies from static (common) images self._assign_static_build_dependencies(helm_values) @@ -282,7 +284,7 @@ def create_app_values_spec(self, app_name: str, app_path: Path, base_image_name: values[KEY_HARNESS]['name']) image_paths = [path for path in find_dockerfiles_paths( - app_path) if 'tasks/' not in path and 'subapps' not in path] + app_path) if is_buildable_dockerfile_path(path)] # Inject entry points commands to enable debug if helm_values.get("debug", False): @@ -378,7 +380,7 @@ def finalize_app_values(self, app_name, app_path, app_values, base_image_name=No values = app_values image_paths = [path for path in find_dockerfiles_paths( - app_path) if 'tasks/' not in path and 'subapps' not in path] + app_path) if is_buildable_dockerfile_path(path)] # Inject entry points commands to enable debug if helm_values.get("debug", False): diff --git a/tools/deployment-cli-tools/ch_cli_tools/helm.py b/tools/deployment-cli-tools/ch_cli_tools/helm.py index 858824d76..585920e31 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/helm.py +++ b/tools/deployment-cli-tools/ch_cli_tools/helm.py @@ -11,7 +11,7 @@ from cloudharness_utils.constants import VALUES_MANUAL_PATH, HELM_CHART_PATH from .utils import get_cluster_ip, get_dockerfile_baseimg_args, get_git_commit_hash, get_image_name, image_name_from_dockerfile_path, \ get_template, merge_to_yaml_file, dict_merge, app_name_from_path, \ - find_dockerfiles_paths, yaml + find_dockerfiles_paths, is_buildable_dockerfile_path, yaml from .models import HarnessMainConfig @@ -137,6 +137,8 @@ def process_values(self) -> HarnessMainConfig: # Collect all source_images and move them to the root self._aggregate_source_images(self.base_images, helm_values) + self._inherit_instance_images(helm_values) + self.create_tls_certificate(helm_values) # Adjust dependencies from static (common) images @@ -359,7 +361,7 @@ def create_app_values_spec(self, app_name: str, app_path: Path, base_image_name: values[KEY_HARNESS]['name']) image_paths = [path for path in find_dockerfiles_paths( - f"{app_path}") if 'tasks/' not in path and 'subapps' not in path] + f"{app_path}") if is_buildable_dockerfile_path(path)] if len(image_paths) > 1: logging.warning('Multiple Dockerfiles found in application %s. Picking the first one: %s', app_name, image_paths[0]) @@ -443,7 +445,7 @@ def finalize_app_values(self, app_name: str, app_path: Path, app_values, base_im values = app_values image_paths = [path for path in find_dockerfiles_paths( - f"{app_path}") if 'tasks/' not in path and 'subapps' not in path] + f"{app_path}") if is_buildable_dockerfile_path(path)] if len(image_paths) > 1: logging.warning('Multiple Dockerfiles found in application %s. Picking the first one: %s', app_name, image_paths[0]) diff --git a/tools/deployment-cli-tools/ch_cli_tools/instances.py b/tools/deployment-cli-tools/ch_cli_tools/instances.py new file mode 100644 index 000000000..179ddb6e7 --- /dev/null +++ b/tools/deployment-cli-tools/ch_cli_tools/instances.py @@ -0,0 +1,235 @@ +"""Application instances. + +An instance is a separate deployment of an application, declared as a directory under the +application's `deploy/instances`. It is served on its own subdomain and gets its own service, +deployment, database, volume and secrets, while running the image built for the application and +inheriting its whole configuration, resources and templates. + +An instance is read together with the application declaring it and added to the deployment right +away as the application `[application]-[instance]`, derived from the application's values merged +so far. From then on it is an application like any other: it is merged root path by root path, +named, filtered and rendered the same way. Nothing in the values tells an instance apart from an +application: the directories under `deploy/instances` are the only record of what is an instance. +""" + +import copy +from pathlib import Path + +from cloudharness_utils.constants import APPS_PATH +from .constants import KEY_DATABASE, KEY_DEPLOYMENT, KEY_HARNESS, KEY_SERVICE, KEY_TASK_IMAGES +from .common_types import ValuesValidationException +from .utils import app_name_from_path, dict_merge, get_template, yaml + + +# Directory of an application holding its instances, one sub-directory each +INSTANCES_PATH = 'instances' + +# Everything that maps an application to a host: an instance serves its own subdomain, and +# inheriting these would make it claim the parent's hosts with a different backend. +INSTANCE_HOST_KEYS = ('subdomain', 'aliases', 'domain') + +# Resource names are derived from the application key. Dropping the parent's lets an instance +# get its own service, deployment and database instead of colliding with the parent's. +INSTANCE_NAMED_RESOURCES = (KEY_SERVICE, KEY_DEPLOYMENT, KEY_DATABASE) + + +def instance_app_key(app_name, instance_name): + """Application key an instance is deployed under: instances are applications in their own right. + + The key is what the whole deployment is built around: resources and templates are collected + under it, and the service, deployment, database and gatekeeper are named after it. + """ + return f"{app_name}-{instance_name}" + + +def instances_path(app_path): + """Directory holding the instances of an application.""" + return Path(app_path) / 'deploy' / INSTANCES_PATH + + +def instance_directories(app_path): + """The instance directories of an application, by instance name.""" + return {app_name_from_path(f"{path.name}"): path + for path in sorted(instances_path(app_path).glob("*/")) + if path.is_dir() and not path.name.startswith('.')} + + +def instance_names(app_name, root_paths): + """Names of the instances an application declares in any root path.""" + return {name for root_path in root_paths + for name in instance_directories(Path(root_path) / APPS_PATH / app_name)} + + +def application_names(root_paths): + """Names of the applications found in any root path.""" + return {app_name_from_path(f"{path.name}") for root_path in root_paths + for path in (Path(root_path) / APPS_PATH).glob("*/") if path.is_dir()} + + +def instance_sets(instance_values, *path): + """Whether an instance defines a value of its own at the given path of its configuration. + + Presence, not truthiness: an instance explicitly declaring an empty value (e.g. + `connect_string: ""` as a deliberate opt-out) still counts as declaring it. + """ + node = instance_values + for key in path[:-1]: + if not isinstance(node, dict): + return False + node = node.get(key) + return isinstance(node, dict) and path[-1] in node + + +def build_instance_values(app_values, parent_name, instance_name, instance_values): + """Application values of a single instance: the parent's configuration, stripped of what + identifies the parent, with the instance's own values merged on top. + + The merge follows `dict_merge`: mappings are merged key by key, lists (`env`, + `uri_role_mapping`, `aliases`, ...) are replaced wholesale by the instance's. + """ + app_key = instance_app_key(parent_name, instance_name) + instance_app = copy.deepcopy(app_values) + harness = instance_app.setdefault(KEY_HARNESS, {}) + + for key in INSTANCE_HOST_KEYS: + harness.pop(key, None) + harness.pop('name', None) + for key in INSTANCE_NAMED_RESOURCES: + resource = harness.get(key) + if isinstance(resource, dict): + resource.pop('name', None) + + instance_app = dict_merge(instance_app, instance_values) + harness = instance_app[KEY_HARNESS] + + # An instance is reached on its own subdomain: without one of its own it answers on its + # directory's name, so that creating the directory is enough to deploy it. An instance + # declaring `subdomain: null` opts out and gets no ingress. + if 'subdomain' not in (instance_values.get(KEY_HARNESS) or {}): + harness['subdomain'] = instance_name + + # The volume name is the name of the claim the application mounts: left inherited, the + # instance would mount the parent's storage. Named after the instance, it gets its own. + volume = (harness.get(KEY_DEPLOYMENT) or {}).get('volume') or {} + if volume.get('name') and not instance_sets(instance_values, KEY_HARNESS, KEY_DEPLOYMENT, 'volume', 'name'): + volume['name'] = f"{app_key}-{volume['name']}" + + # A connection string points at one database: inherited, it would connect the instance to the + # parent's. Emptied, it keeps the parent's intent of an externally managed database while + # requiring a value of its own, the way an application declaring `connect_string: ""` does. + database = harness.get(KEY_DATABASE) or {} + if database.get('connect_string') and not instance_sets(instance_values, KEY_HARNESS, KEY_DATABASE, 'connect_string'): + database['connect_string'] = '' + + # The instance runs the parent's image, inherited with the rest of the configuration: it is + # never built on its own, and the task images produced by the parent's build belong to the + # parent alone. + instance_app['build'] = False + instance_app[KEY_TASK_IMAGES] = {} + + return instance_app + + +def inherit_parent_image(instance_app, parent_app): + """Give an instance the image of its parent application, unless it pins one of its own. + + An instance inherits the image with the rest of the parent's values, but with `--include` the + image is only computed when the included applications are finalized, after the instance has + been derived from them. + """ + parent_harness_image = ((parent_app.get(KEY_HARNESS) or {}).get(KEY_DEPLOYMENT) or {}).get('image') + image = instance_app.get('image') or parent_app.get('image') or parent_harness_image + if not image: + return + instance_app['image'] = image + deployment = instance_app.setdefault(KEY_HARNESS, {}).setdefault(KEY_DEPLOYMENT, {}) + if not deployment.get('image'): + deployment['image'] = image + + +def check_instance_collisions(root_paths, exclude=()): + """Check that no instance and application are deployed under the same key, as merging their + values into one would silently deploy neither.""" + applications = application_names(root_paths) - set(exclude) + for app_name in applications: + for instance_name in instance_names(app_name, root_paths): + app_key = instance_app_key(app_name, instance_name) + if app_key in applications: + raise ValuesValidationException( + f"Instance `{instance_name}` of application `{app_name}` is deployed as application " + f"`{app_key}`, which already exists. Rename the instance or the application.") + + +def resolve_instance_includes(include, app_name, instance_keys): + """Resolve `--include` over the instances of one application. + + An instance is deployed together with the application it belongs to: including an + application includes its instances, and including an instance alone pulls in the parent it + inherits its configuration and image from. Single instances are left out with `--exclude`. + """ + resolved = set(include) + if app_name in resolved or resolved & set(instance_keys): + resolved.add(app_name) + resolved.update(instance_keys) + return resolved + + +def task_image_collision(app_key, task_images): + """The task image an instance application key would take ownership of, if any. + + Task images are resolved to the application that builds them by longest name prefix + (`resolve_task_image_owner`), so an instance is in the way of the image named after it and + of every image whose name it prefixes: `samples-print` would own `samples-print-file`. + """ + for task_image in sorted(task_images): + if task_image == app_key or task_image.startswith(f"{app_key}-"): + return task_image + return None + + +def load_instance_values(instance_path, envs=()): + """The override values declared in one instance directory: `values.yaml`, overridden by + `values-[env].yaml`.""" + values = get_template(instance_path / 'values.yaml') + for env in envs: + env_values_path = instance_path / f'values-{env}.yaml' + if env_values_path.exists(): + with env_values_path.open() as f: + values = dict_merge(values, yaml.load(f)) + return values + + +def collect_instances(app_name, root_paths, envs=()): + """Collect the override values of the instances an application declares in any root path. + + An instance is a directory under the application's `deploy/instances`, holding a + `values.yaml` (and optionally `values-[env].yaml`) with the values overriding the + application's, plus the `resources` and `templates` overriding the application's own. The + same instance declared in several root paths is merged, a later root overriding an earlier one. + + Returns a mapping of instance name -> override values, empty when the application declares + no instance. + """ + instances = {} + # Task images are named `[application]-[task directory]`, the same way an instance + # application is: an instance whose key prefixes one would take ownership of it. + task_images = set() + + for root_path in root_paths: + app_path = Path(root_path) / APPS_PATH / app_name + task_images.update(app_name_from_path(f"{app_name}/{task_path.name}") + for task_path in (app_path / 'tasks').glob("*/") if task_path.is_dir()) + + for instance_name, instance_path in instance_directories(app_path).items(): + instances[instance_name] = dict_merge( + instances.get(instance_name, {}), load_instance_values(instance_path, envs)) + + for instance_name in instances: + collision = task_image_collision(instance_app_key(app_name, instance_name), task_images) + if collision: + raise ValuesValidationException( + f"Instance `{instance_name}` of application `{app_name}` is deployed as application " + f"`{instance_app_key(app_name, instance_name)}`, which takes over the task image " + f"`{collision}`. Rename the instance.") + + return instances diff --git a/tools/deployment-cli-tools/ch_cli_tools/utils.py b/tools/deployment-cli-tools/ch_cli_tools/utils.py index aa40fdc5d..162014538 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/utils.py +++ b/tools/deployment-cli-tools/ch_cli_tools/utils.py @@ -95,6 +95,21 @@ def find_dockerfiles_paths(base_directory: str) -> tuple[str, ...]: return tuple(p for p in dockerfiles_without_git if not re.search(r'(^|/).*dependencies.*/', p + '/')) +# Directory names that hold Dockerfiles belonging to something other than the application's own +# buildable image: task images, deprecated subapps, and instance overrides. +NON_BUILDABLE_DOCKERFILE_SEGMENTS = frozenset({'tasks', 'subapps', 'instances'}) + + +def is_buildable_dockerfile_path(path: str) -> bool: + """Whether a Dockerfile path belongs to the application's own buildable image, i.e. is not + nested under a `tasks`, `subapps` or `instances` directory. + + Matches by path segment rather than substring, so an application or resource directory whose + name merely contains one of these words (e.g. `myinstances`) is not excluded by mistake. + """ + return not (set(Path(path).parts) & NON_BUILDABLE_DOCKERFILE_SEGMENTS) + + def get_parent_app_name(app_relative_path): return app_relative_path.split("/")[0] if "/" in app_relative_path else "" diff --git a/tools/deployment-cli-tools/tests/resources/applications/myapp/deploy/instances/inst1/resources/aresource.txt b/tools/deployment-cli-tools/tests/resources/applications/myapp/deploy/instances/inst1/resources/aresource.txt new file mode 100644 index 000000000..34572e846 --- /dev/null +++ b/tools/deployment-cli-tools/tests/resources/applications/myapp/deploy/instances/inst1/resources/aresource.txt @@ -0,0 +1 @@ +instance resource diff --git a/tools/deployment-cli-tools/tests/resources/applications/myapp/deploy/instances/inst1/resources/instanceonly.txt b/tools/deployment-cli-tools/tests/resources/applications/myapp/deploy/instances/inst1/resources/instanceonly.txt new file mode 100644 index 000000000..4352413c2 --- /dev/null +++ b/tools/deployment-cli-tools/tests/resources/applications/myapp/deploy/instances/inst1/resources/instanceonly.txt @@ -0,0 +1 @@ +only in the instance diff --git a/tools/deployment-cli-tools/tests/resources/applications/myapp/deploy/instances/inst1/templates/mytemplate.yaml b/tools/deployment-cli-tools/tests/resources/applications/myapp/deploy/instances/inst1/templates/mytemplate.yaml new file mode 100644 index 000000000..8a74a21bc --- /dev/null +++ b/tools/deployment-cli-tools/tests/resources/applications/myapp/deploy/instances/inst1/templates/mytemplate.yaml @@ -0,0 +1,7 @@ +{{- $app := index .Values.apps "myapp-inst1" }} +kind: ConfigMap +apiVersion: v1 +metadata: + name: "{{ $app.harness.deployment.name }}-instance-template" +data: + subdomain: "{{ $app.harness.subdomain }}" diff --git a/tools/deployment-cli-tools/tests/resources/applications/myapp/deploy/instances/inst1/values-dev.yaml b/tools/deployment-cli-tools/tests/resources/applications/myapp/deploy/instances/inst1/values-dev.yaml new file mode 100644 index 000000000..177e6bdd6 --- /dev/null +++ b/tools/deployment-cli-tools/tests/resources/applications/myapp/deploy/instances/inst1/values-dev.yaml @@ -0,0 +1,4 @@ +harness: + deployment: + replicas: 4 +a: instance-dev-b diff --git a/tools/deployment-cli-tools/tests/resources/applications/myapp/deploy/instances/inst1/values.yaml b/tools/deployment-cli-tools/tests/resources/applications/myapp/deploy/instances/inst1/values.yaml new file mode 100644 index 000000000..09a65a8c6 --- /dev/null +++ b/tools/deployment-cli-tools/tests/resources/applications/myapp/deploy/instances/inst1/values.yaml @@ -0,0 +1,5 @@ +harness: + subdomain: myinstance + deployment: + replicas: 3 +a: instance-b diff --git a/tools/deployment-cli-tools/tests/test_helm.py b/tools/deployment-cli-tools/tests/test_helm.py index 81d6362c1..386d46a61 100644 --- a/tools/deployment-cli-tools/tests/test_helm.py +++ b/tools/deployment-cli-tools/tests/test_helm.py @@ -15,6 +15,7 @@ generate_hash_based_image_tags, preprocess_build_overrides, ) +from ch_cli_tools.instances import * HERE = os.path.dirname(os.path.realpath(__file__)) RESOURCES = os.path.join(HERE, 'resources') @@ -1428,3 +1429,412 @@ def test_collect_helm_values_source_images_merge_no_include(tmp_path): source_images = values.get("source_images") assert source_images["KEYCLOAK"] == "myregistry.myapp:15.3" assert "NODE" in source_images + + +def test_instances_expand_into_applications(tmp_path): + """An instance is deployed as an application of its own, inheriting the parent's configuration.""" + out_folder = tmp_path / 'test_instances_expand_into_applications' + values = create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, include=['samples'], + domain="my.local", namespace='test', env='dev', local=False, tag=1, registry='reg') + + apps = values[KEY_APPS] + assert 'samples-instance1' in apps, 'including an application includes its instances' + instance = apps['samples-instance1'] + parent = apps['samples'] + + # Its own host: the parent's subdomain and aliases are never inherited + assert instance[KEY_HARNESS]['subdomain'] == 'samples1' + assert not instance[KEY_HARNESS]['aliases'] + assert parent[KEY_HARNESS]['subdomain'] == 'www' + assert parent[KEY_HARNESS]['aliases'] == ['samples'] + + # Its own resources + assert instance[KEY_HARNESS]['name'] == 'samples-instance1' + assert instance[KEY_HARNESS]['service']['name'] == 'samples-instance1' + assert instance[KEY_HARNESS][KEY_DEPLOYMENT]['name'] == 'samples-instance1' + assert instance[KEY_HARNESS][KEY_DATABASE]['name'] == 'samples-instance1-db' + assert instance[KEY_HARNESS][KEY_DEPLOYMENT]['volume']['name'] == \ + 'samples-instance1-my-shared-volume', 'an instance must not mount the parent claim' + assert parent[KEY_HARNESS][KEY_DEPLOYMENT]['volume']['name'] == 'my-shared-volume' + + # The parent's image, built once + assert instance['build'] is False + assert instance['image'] == parent['image'] + assert instance[KEY_HARNESS][KEY_DEPLOYMENT]['image'] == parent[KEY_HARNESS][KEY_DEPLOYMENT]['image'] + assert not instance[KEY_TASK_IMAGES] + + # Inherited configuration, with the instance's own values merged over it + assert instance[KEY_HARNESS]['secured'] == parent[KEY_HARNESS]['secured'] + assert instance[KEY_HARNESS][KEY_DEPLOYMENT]['replicas'] == 1 + assert parent[KEY_HARNESS][KEY_DEPLOYMENT]['replicas'] == 2 + assert instance[KEY_HARNESS]['envmap']['ENVIRONMENT_TEST_B'] == 123 + + +def test_instances_expand_without_include(tmp_path): + out_folder = tmp_path / 'test_instances_expand_without_include' + values = create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, domain="my.local", + namespace='test', env='dev', local=False, tag=1, registry='reg') + + instance = values[KEY_APPS]['samples-instance1'] + assert instance[KEY_HARNESS]['subdomain'] == 'samples1' + assert instance['build'] is False + assert instance['image'] == values[KEY_APPS]['samples']['image'] + + +def test_instance_excluded_individually(tmp_path): + """A single instance is left out with --exclude, without affecting its parent.""" + out_folder = tmp_path / 'test_instance_excluded_individually' + values = create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, include=['samples'], + exclude=['samples-instance1'], domain="my.local", namespace='test', env='dev', + local=False, tag=1, registry='reg') + + assert 'samples-instance1' not in values[KEY_APPS] + assert 'samples' in values[KEY_APPS] + # an excluded instance leaves no collected files behind either + assert not (out_folder / HELM_CHART_PATH / 'resources' / 'samples-instance1').exists() + + +def test_instance_include_pulls_in_its_parent(tmp_path): + """Including an instance alone deploys the application it inherits its image from too.""" + out_folder = tmp_path / 'test_instance_include_pulls_in_its_parent' + values = create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, + include=['samples-instance1'], domain="my.local", namespace='test', env='dev', + local=False, tag=1, registry='reg') + + assert 'samples-instance1' in values[KEY_APPS] + assert 'samples' in values[KEY_APPS] + assert values[KEY_APPS]['samples-instance1']['image'] == values[KEY_APPS]['samples']['image'] + + +def test_instance_resources_are_overlaid_on_the_application(tmp_path): + """An instance's resources override the application's file by file, and it inherits the rest.""" + out_folder = tmp_path / 'test_instance_resources' + create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, include=['samples'], + domain="my.local", namespace='test', env='dev', local=False, tag=1, registry='reg') + + helm_path = out_folder / HELM_CHART_PATH + instance_resources = helm_path / 'resources' / 'samples-instance1' + assert (instance_resources / 'example.yaml').exists() + assert (instance_resources / 'myConfig.json').exists(), \ + 'a resource the instance does not override is inherited' + + shutil.rmtree(helm_path / 'charts', ignore_errors=True) + manifests = render_helm_chart(helm_path) + + # the overridden resource carries the instance's content... + overridden = find_manifest(manifests, 'ConfigMap', 'samples-instance1-example') + assert 'an instance overridden' in overridden['data']['important_config.yaml'] + # ...the inherited one the application's, and it is not empty (the configmap is looked up + # by deployment name, so a missing instance directory renders an empty value) + inherited = find_manifest(manifests, 'ConfigMap', 'samples-instance1-my-config') + parent_config = find_manifest(manifests, 'ConfigMap', 'samples-my-config') + assert inherited['data']['myConfig.json'].strip() + assert inherited['data'] == parent_config['data'] + # the application keeps its own + parent = find_manifest(manifests, 'ConfigMap', 'samples-example') + assert 'an instance overridden' not in parent['data']['important_config.yaml'] + + +def test_instance_templates_are_overlaid_on_the_application(tmp_path): + """An instance's helm templates are collected over the application's.""" + out_folder = tmp_path / 'test_instance_templates' + create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, include=['myapp'], + domain="my.local", namespace='test', env='dev', local=False, tag=1, registry='reg') + + helm_path = out_folder / HELM_CHART_PATH + assert (helm_path / 'templates' / 'myapp-inst1' / 'mytemplate.yaml').exists() + assert (helm_path / 'resources' / 'myapp-inst1' / 'instanceonly.txt').exists() + assert (helm_path / 'resources' / 'myapp-inst1' / 'aresource.txt').exists() + + shutil.rmtree(helm_path / 'charts', ignore_errors=True) + manifests = render_helm_chart(helm_path) + rendered = find_manifest(manifests, 'ConfigMap', 'myapp-inst1-instance-template') + assert rendered['data']['subdomain'] == 'myinstance' + + +def test_instance_does_not_inherit_the_parent_connect_string(tmp_path): + """A connection string points at one database: an instance never inherits the parent's.""" + out_folder = tmp_path / 'test_instance_connect_string' + values = create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, include=['samples'], + domain="my.local", namespace='test', env='dev', local=False, tag=1, registry='reg') + + instance_db = values[KEY_APPS]['samples-instance1'][KEY_HARNESS][KEY_DATABASE] + parent_db = values[KEY_APPS]['samples'][KEY_HARNESS][KEY_DATABASE] + assert instance_db['connect_string'] != parent_db['connect_string'] + + # an instance that declares none is left with an empty one, to be supplied per instance at + # deploy time, rather than silently connecting to the parent's database + parent = {KEY_HARNESS: {KEY_DATABASE: {'type': 'postgres', 'connect_string': 'parent connection'}}} + instance = build_instance_values(parent, 'myapp', 'i1', {KEY_HARNESS: {'subdomain': 'myapp1'}}) + assert instance[KEY_HARNESS][KEY_DATABASE]['connect_string'] == '' + assert parent[KEY_HARNESS][KEY_DATABASE]['connect_string'] == 'parent connection' + + +def test_instance_renders_its_own_manifests(tmp_path): + """The instance gets the full set of manifests on its own subdomain, backed by its own workload.""" + out_folder = tmp_path / 'test_instance_renders_its_own_manifests' + create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, include=['samples'], + domain="my.local", namespace='test', env='dev', local=False, tag=1, registry='reg') + + helm_path = out_folder / HELM_CHART_PATH + shutil.rmtree(helm_path / 'charts', ignore_errors=True) + manifests = render_helm_chart(helm_path) + + statefulset = find_manifest(manifests, 'StatefulSet', 'samples-instance1') + find_manifest(manifests, 'Service', 'samples-instance1') + + containers = statefulset['spec']['template']['spec']['containers'] + app_container = next(c for c in containers if c['name'] == 'samples-instance1') + env = {e['name']: e.get('value') for e in app_container['env']} + assert env['CH_CURRENT_APP_NAME'] == 'samples-instance1', \ + 'the instance must read its own configuration, not the parent one' + + # Its own claim: sharing the parent's would give the instance the parent's data + claims = [c['metadata']['name'] for c in statefulset['spec'].get('volumeClaimTemplates', [])] + parent_claims = [c['metadata']['name'] + for c in find_manifest(manifests, 'StatefulSet', 'samples')['spec'].get('volumeClaimTemplates', [])] + assert claims and not set(claims) & set(parent_claims) + + # Its own database secret + instance_secret = find_manifest(manifests, 'Secret', 'samples-instance1-db') + parent_secret = find_manifest(manifests, 'Secret', 'samples-db') + assert instance_secret['stringData']['connect_string'] != parent_secret['stringData']['connect_string'] + + # Its own host, served by its own gatekeeper + hosts = {} + for manifest in manifests: + if manifest.get('kind') != 'Ingress': + continue + for rule in manifest['spec']['rules']: + for path in rule['http']['paths']: + hosts.setdefault(rule['host'], set()).add(path['backend']['service']['name']) + + assert 'samples1.my.local' in hosts + assert 'samples-instance1' in hosts['samples1.my.local'] + assert 'samples1-gk' in hosts['samples1.my.local'] + # the parent keeps its own hosts, and the instance never serves them + assert 'samples-instance1' not in hosts['samples.my.local'] + assert 'samples-instance1' not in hosts['www.my.local'] + + gatekeeper = find_manifest(manifests, 'ConfigMap', 'samples1-gk') + proxy_config = yaml.load(gatekeeper['data']['proxy.yml']) + assert proxy_config['redirection-url'] == 'https://samples1.my.local' + assert proxy_config['upstream-url'].startswith('http://samples-instance1.') + + +def test_instance_colliding_with_an_application_is_rejected(tmp_path): + """An instance and an application under the same key would be merged into one, whichever + root path declares each of them.""" + first, second = tmp_path / 'first', tmp_path / 'second' + (first / APPS_PATH / 'myapp' / 'deploy' / INSTANCES_PATH / 'other').mkdir(parents=True) + (second / APPS_PATH / 'myapp-other' / 'deploy').mkdir(parents=True) + + check_instance_collisions([first]) + check_instance_collisions([second]) + with pytest.raises(ValuesValidationException, match='myapp-other'): + check_instance_collisions([first, second]) + with pytest.raises(ValuesValidationException, match='myapp-other'): + check_instance_collisions([second, first]) + check_instance_collisions([first, second], exclude=['myapp-other']) + + # the check is made by the generator before anything is read + with pytest.raises(ValuesValidationException, match='myapp-other'): + create_helm_chart([CLOUDHARNESS_ROOT, first, second], output_path=tmp_path / 'out', include=['myapp'], + domain="my.local", namespace='test', local=False, tag=1, registry='reg') + + +def test_instance_directory_colliding_with_a_task_is_rejected(tmp_path): + """Task image ownership is resolved by longest name prefix, so an instance named `print` on + an application owning `myapp-print-file` would take the image over and it would never build. + The collision is caught when instances are collected, before anything reads task images.""" + app_path = tmp_path / APPS_PATH / 'myapp' + (app_path / 'tasks' / 'print-file').mkdir(parents=True) + instance_path = app_path / 'deploy' / INSTANCES_PATH / 'print' + instance_path.mkdir(parents=True) + (instance_path / 'values.yaml').write_text('harness:\n subdomain: myprint\n') + + with pytest.raises(ValuesValidationException, match='myapp-print-file'): + collect_instances('myapp', [tmp_path]) + + assert resolve_task_image_owner('myapp-print-file', {'myapp', 'myapp-print'}) == 'myapp-print', \ + 'the prefix match this guards against' + + +def test_collect_instances_skips_hidden_directories(tmp_path): + instances_dir = tmp_path / APPS_PATH / 'myapp' / 'deploy' / INSTANCES_PATH + (instances_dir / '.hidden').mkdir(parents=True) + (instances_dir / 'real').mkdir(parents=True) + (instances_dir / 'real' / 'values.yaml').write_text('harness:\n subdomain: real\n') + + assert set(collect_instances('myapp', [tmp_path])) == {'real'} + + +def test_collect_instances_of_an_application_without_any(tmp_path): + (tmp_path / APPS_PATH / 'myapp' / 'deploy').mkdir(parents=True) + assert collect_instances('myapp', [tmp_path]) == {} + assert collect_instances('missing', [tmp_path]) == {}, 'an application absent from a root declares nothing there' + + +def test_collect_instances_merges_the_roots_declaring_the_same_instance(tmp_path): + for root, replicas in (('first', 1), ('second', 2)): + instance_path = tmp_path / root / APPS_PATH / 'myapp' / 'deploy' / INSTANCES_PATH / 'inst' + instance_path.mkdir(parents=True) + (instance_path / 'values.yaml').write_text(f'harness:\n deployment:\n replicas: {replicas}\n{root}: true\n') + + instances = collect_instances('myapp', [tmp_path / 'first', tmp_path / 'second']) + assert instances['inst'][KEY_HARNESS][KEY_DEPLOYMENT]['replicas'] == 2, 'a later root wins' + assert instances['inst']['first'] and instances['inst']['second'], 'what a root does not override is kept' + + +def test_instance_values_replace_parent_lists(): + """An instance's lists replace the parent's rather than adding to them. + + `uri_role_mapping` is what the gatekeeper whitelists, so the two semantics differ in + what an instance leaves open: pinned here because the merge rule is documented. + """ + parent_mapping = [ + {'uri': '/', 'white-listed': True}, + {'uri': '/api/ping', 'white-listed': True}, + ] + parent_app = { + KEY_HARNESS: { + 'uri_role_mapping': parent_mapping, + 'env': [{'name': 'WORKERS', 'value': '3'}], + 'envmap': {'A': 'parent', 'B': 'kept'}, + } + } + instance = build_instance_values(parent_app, 'myapp', 'i1', { + KEY_HARNESS: { + 'subdomain': 'myapp1', + 'uri_role_mapping': [{'uri': '/*', 'roles': ['administrator']}], + 'envmap': {'A': 'instance'}, + } + })[KEY_HARNESS] + parent = parent_app[KEY_HARNESS] + + # lists are replaced as a whole + assert instance['uri_role_mapping'] == [{'uri': '/*', 'roles': ['administrator']}] + assert parent['uri_role_mapping'] == parent_mapping + # a list the instance does not override is inherited + assert instance['env'] == [{'name': 'WORKERS', 'value': '3'}] + # mappings are merged key by key + assert instance['envmap'] == {'A': 'instance', 'B': 'kept'} + assert parent['envmap'] == {'A': 'parent', 'B': 'kept'} + + +def test_instances_are_collected_with_the_application_they_belong_to(tmp_path): + """An instance is added to the deployment as soon as its application is read: the values + collected from a root path hold the instances next to the applications, derived from the + application's values as merged so far.""" + generator = CloudHarnessHelm([CLOUDHARNESS_ROOT, RESOURCES], + output_path=tmp_path / 'test_instances_collected', + domain="my.local", namespace='test', env=['dev'], + local=False, tag=1, registry='reg') + merged_so_far = {KEY_APPS: {'myapp': {KEY_HARNESS: {'secured': True, 'subdomain': 'earlier'}}}} + + values = generator.collect_app_values(Path(RESOURCES) / APPS_PATH, helm_values=merged_so_far) + + instance = values['myapp-inst1'] + assert instance[KEY_HARNESS]['subdomain'] == 'myinstance' + assert instance[KEY_HARNESS]['secured'] is True, 'derived from the application merged so far' + assert instance['a'] == 'instance-dev-b', 'with the instance values on top' + assert instance['build'] is False + assert 'accounts-inst1' not in values, 'applications declaring no instance get none' + # nothing in the values tells the instance apart: the directories are the only record + assert instance_names('myapp', [RESOURCES]) == {'inst1'} + assert instance_names('accounts', [RESOURCES]) == set() + + +def test_values_vocabulary_is_re_exported(): + """The values keys live in `constants` and the validation exception lives in `common_types`, + and both are read from `configurationgenerator` by the other cli tools and by these tests: + dropping the re-export breaks them with an ImportError far from where it was caused.""" + from ch_cli_tools import common_types, constants + + for name in ('KEY_HARNESS', 'KEY_SERVICE', 'KEY_DATABASE', 'KEY_DEPLOYMENT', 'KEY_APPS', + 'KEY_TASK_IMAGES', 'KEY_TEST_IMAGES'): + assert getattr(configurationgenerator, name) is getattr(constants, name), \ + f"{name} must stay importable from configurationgenerator" + + assert configurationgenerator.ValuesValidationException is common_types.ValuesValidationException, \ + "ValuesValidationException must stay importable from configurationgenerator" + + +def test_instance_subdomain_defaults_to_its_directory_name(): + """Creating the directory is enough to reach an instance: without a subdomain of its own it + answers on its name. An explicit null opts out of the ingress.""" + def subdomain_of(instance_values): + parent = {KEY_HARNESS: {'subdomain': 'www', 'aliases': ['samples']}} + return build_instance_values(parent, 'samples', 'instance1', instance_values)[KEY_HARNESS].get('subdomain', '') + + assert subdomain_of({}) == 'instance1' + assert subdomain_of({KEY_HARNESS: {'replicas': 2}}) == 'instance1' + assert subdomain_of({KEY_HARNESS: {'subdomain': 'samples1'}}) == 'samples1' + assert subdomain_of({KEY_HARNESS: {'subdomain': None}}) is None + + +def test_instance_values_precedence(tmp_path): + """Values are layered instance env > instance > application env > application. + + The `myapp` fixture sets `a` in all four files and `dev` only in the application's + environment values, so the winner and the inheritance are both visible. + """ + out_folder = tmp_path / 'test_instance_values_precedence' + values = create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, include=['myapp'], + domain="my.local", namespace='test', env='dev', local=False, tag=1, registry='reg') + + instance = values[KEY_APPS]['myapp-inst1'] + parent = values[KEY_APPS]['myapp'] + + # the instance's environment values win over everything + assert instance['a'] == 'instance-dev-b' + assert instance[KEY_HARNESS][KEY_DEPLOYMENT]['replicas'] == 4, \ + 'the instance values-dev.yaml overrides its values.yaml' + # what no instance file sets is inherited, environment values included + assert instance['dev'] is True + # the application keeps its own + assert parent['a'] == 'b' + assert parent[KEY_HARNESS]['subdomain'] == 'mysubdomain' + + +def test_instance_without_env_values_still_layers_over_the_application(tmp_path): + """Without the environment, the instance's values.yaml is the top of the chain.""" + out_folder = tmp_path / 'test_instance_values_precedence_noenv' + values = create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, include=['myapp'], + domain="my.local", namespace='test', local=False, tag=1, registry='reg') + + instance = values[KEY_APPS]['myapp-inst1'] + assert instance['a'] == 'instance-b' + assert instance[KEY_HARNESS][KEY_DEPLOYMENT]['replicas'] == 3 + assert 'dev' not in instance + + +def test_instance_inherits_the_application_merged_across_roots(tmp_path): + """An instance inherits the application's final configuration, not the one of the root its + directory happens to live in. + + An instance is derived again in every root path that reads its application, from the + application as merged up to that root and the instance's values from every root: a + scaffolding overriding an application reaches its instances, and the instance's own values + still win over it. + """ + overriding_root = tmp_path / 'overriding_root' + app_deploy = overriding_root / APPS_PATH / 'myapp' / 'deploy' + app_deploy.mkdir(parents=True) + (app_deploy / 'values.yaml').write_text( + 'harness:\n secured: true\n deployment:\n port: 9999\n replicas: 1\n') + + out_folder = tmp_path / 'test_instance_inherits_merged' + values = create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES, overriding_root], output_path=out_folder, + include=['myapp'], domain="my.local", namespace='test', env='dev', + local=False, tag=1, registry='reg') + + # the instance is declared in RESOURCES, the override comes from a later root + instance = values[KEY_APPS]['myapp-inst1'][KEY_HARNESS] + assert values[KEY_APPS]['myapp'][KEY_HARNESS][KEY_DEPLOYMENT]['port'] == 9999 + assert instance[KEY_DEPLOYMENT]['port'] == 9999 + assert instance['secured'] is True + # and what the instance sets for itself still wins over the later root's application values + assert instance['subdomain'] == 'myinstance' + assert instance[KEY_DEPLOYMENT]['replicas'] == 4 + assert values[KEY_APPS]['myapp'][KEY_HARNESS][KEY_DEPLOYMENT]['replicas'] == 1 From 7d1b8b3e204488c69665f9c3328c0ca9e935e18f Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Wed, 16 Sep 2026 16:44:39 +0200 Subject: [PATCH 2/8] CH-293 refactor: move configuration generation utilities in one package --- tools/cloudharness-test/cloudharness_test/api.py | 2 +- tools/cloudharness-test/cloudharness_test/e2e.py | 2 +- .../deployment-cli-tools/ch_cli_tools/codefresh.py | 4 ++-- .../ch_cli_tools/configuration/__init__.py | 0 .../{ => configuration}/configurationgenerator.py | 8 ++++---- .../ch_cli_tools/{ => configuration}/instances.py | 6 +++--- .../{ => configuration}/preprocessing.py | 4 ++-- .../ch_cli_tools/{ => configuration}/secrets.py | 0 .../ch_cli_tools/dockercompose.py | 2 +- tools/deployment-cli-tools/ch_cli_tools/helm.py | 2 +- tools/deployment-cli-tools/harness-deployment | 4 ++-- tools/deployment-cli-tools/tests/test_codefresh.py | 6 +++--- .../tests/test_dockercompose.py | 4 ++-- tools/deployment-cli-tools/tests/test_helm.py | 14 +++++++------- .../tests/test_preprocessing.py | 2 +- tools/deployment-cli-tools/tests/test_skaffold.py | 2 +- tools/deployment-cli-tools/tests/test_tilt.py | 2 +- 17 files changed, 32 insertions(+), 32 deletions(-) create mode 100644 tools/deployment-cli-tools/ch_cli_tools/configuration/__init__.py rename tools/deployment-cli-tools/ch_cli_tools/{ => configuration}/configurationgenerator.py (99%) rename tools/deployment-cli-tools/ch_cli_tools/{ => configuration}/instances.py (98%) rename tools/deployment-cli-tools/ch_cli_tools/{ => configuration}/preprocessing.py (97%) rename tools/deployment-cli-tools/ch_cli_tools/{ => configuration}/secrets.py (100%) diff --git a/tools/cloudharness-test/cloudharness_test/api.py b/tools/cloudharness-test/cloudharness_test/api.py index af80f6bd3..f9f172ef5 100644 --- a/tools/cloudharness-test/cloudharness_test/api.py +++ b/tools/cloudharness-test/cloudharness_test/api.py @@ -3,7 +3,7 @@ import logging import subprocess -from ch_cli_tools.preprocessing import get_build_paths +from ch_cli_tools.configuration.preprocessing import get_build_paths from cloudharness_model.models import HarnessMainConfig, ApiTestsConfig, ApplicationHarnessConfig diff --git a/tools/cloudharness-test/cloudharness_test/e2e.py b/tools/cloudharness-test/cloudharness_test/e2e.py index 077c264f3..d9db8a909 100644 --- a/tools/cloudharness-test/cloudharness_test/e2e.py +++ b/tools/cloudharness-test/cloudharness_test/e2e.py @@ -7,7 +7,7 @@ from cloudharness_model.models import ApplicationHarnessConfig from cloudharness_utils.constants import E2E_TESTS_PROJECT_PATH, E2E_TESTS_DIRNAME -from ch_cli_tools.preprocessing import get_build_paths +from ch_cli_tools.configuration.preprocessing import get_build_paths from cloudharness_utils.testing.util import get_app_environment HERE = os.path.dirname(os.path.realpath(__file__)).replace(os.path.sep, '/') diff --git a/tools/deployment-cli-tools/ch_cli_tools/codefresh.py b/tools/deployment-cli-tools/ch_cli_tools/codefresh.py index c04014757..8ffbe4dc8 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/codefresh.py +++ b/tools/deployment-cli-tools/ch_cli_tools/codefresh.py @@ -11,8 +11,8 @@ from cloudharness_utils.testing.util import get_app_environment from .models import HarnessMainConfig, ApplicationTestConfig, ApplicationHarnessConfig from cloudharness_utils.constants import * -from .configurationgenerator import KEY_APPS, KEY_TASK_IMAGES, KEY_TEST_IMAGES -from .secrets import is_cloudharness_managed, is_secret_config, secret_value +from .constants import KEY_APPS, KEY_TASK_IMAGES, KEY_TEST_IMAGES +from .configuration.secrets import is_cloudharness_managed, is_secret_config, secret_value from .utils import check_image_exists_in_registry, find_dockerfiles_paths, get_app_relative_to_base_path, guess_build_dependencies_from_dockerfile, \ get_template, dict_merge, app_name_from_path, clean_path, strip_registry_tag, get_image_source, yaml, yaml_rt from cloudharness_utils.testing.api import get_api_filename, get_schemathesis_command, get_urls_from_api_file diff --git a/tools/deployment-cli-tools/ch_cli_tools/configuration/__init__.py b/tools/deployment-cli-tools/ch_cli_tools/configuration/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tools/deployment-cli-tools/ch_cli_tools/configurationgenerator.py b/tools/deployment-cli-tools/ch_cli_tools/configuration/configurationgenerator.py similarity index 99% rename from tools/deployment-cli-tools/ch_cli_tools/configurationgenerator.py rename to tools/deployment-cli-tools/ch_cli_tools/configuration/configurationgenerator.py index 3e00a83b3..12c9243b2 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/configurationgenerator.py +++ b/tools/deployment-cli-tools/ch_cli_tools/configuration/configurationgenerator.py @@ -12,18 +12,18 @@ from pathlib import Path import abc -from . import HERE, CH_ROOT +from .. import HERE, CH_ROOT from cloudharness_utils.constants import TEST_IMAGES_PATH, HELM_CHART_PATH, APPS_PATH, HELM_PATH, \ DEPLOYMENT_CONFIGURATION_PATH, BASE_IMAGES_PATH, STATIC_IMAGES_PATH -from .utils import get_cluster_ip, env_variable, get_dockerfile_baseimg_args, get_sub_paths, guess_build_dependencies_from_dockerfile, image_name_from_dockerfile_path, \ +from ..utils import get_cluster_ip, env_variable, get_dockerfile_baseimg_args, get_sub_paths, guess_build_dependencies_from_dockerfile, image_name_from_dockerfile_path, \ get_template, merge_configuration_directories, dict_merge, app_name_from_path, \ find_dockerfiles_paths, get_git_commit_hash, yaml from .secrets import secret_definition_error # Re-exported on purpose: the rest of the cli tools and the tests read the deployment values # vocabulary from here. Keep them importable — see test_values_vocabulary_is_re_exported. -from .constants import KEY_APPS, KEY_DATABASE, KEY_DEPLOYMENT, KEY_HARNESS, \ +from ..constants import KEY_APPS, KEY_DATABASE, KEY_DEPLOYMENT, KEY_HARNESS, \ KEY_SERVICE, KEY_TASK_IMAGES, KEY_TEST_IMAGES -from .common_types import ValuesValidationException +from ..common_types import ValuesValidationException from .instances import build_instance_values, check_instance_collisions, collect_instances, inherit_parent_image, \ instance_app_key, instance_directories, instance_names, resolve_instance_includes diff --git a/tools/deployment-cli-tools/ch_cli_tools/instances.py b/tools/deployment-cli-tools/ch_cli_tools/configuration/instances.py similarity index 98% rename from tools/deployment-cli-tools/ch_cli_tools/instances.py rename to tools/deployment-cli-tools/ch_cli_tools/configuration/instances.py index 179ddb6e7..f058c9804 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/instances.py +++ b/tools/deployment-cli-tools/ch_cli_tools/configuration/instances.py @@ -16,9 +16,9 @@ from pathlib import Path from cloudharness_utils.constants import APPS_PATH -from .constants import KEY_DATABASE, KEY_DEPLOYMENT, KEY_HARNESS, KEY_SERVICE, KEY_TASK_IMAGES -from .common_types import ValuesValidationException -from .utils import app_name_from_path, dict_merge, get_template, yaml +from ..constants import KEY_DATABASE, KEY_DEPLOYMENT, KEY_HARNESS, KEY_SERVICE, KEY_TASK_IMAGES +from ..common_types import ValuesValidationException +from ..utils import app_name_from_path, dict_merge, get_template, yaml # Directory of an application holding its instances, one sub-directory each diff --git a/tools/deployment-cli-tools/ch_cli_tools/preprocessing.py b/tools/deployment-cli-tools/ch_cli_tools/configuration/preprocessing.py similarity index 97% rename from tools/deployment-cli-tools/ch_cli_tools/preprocessing.py rename to tools/deployment-cli-tools/ch_cli_tools/configuration/preprocessing.py index 9d2eb3aff..15a086844 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/preprocessing.py +++ b/tools/deployment-cli-tools/ch_cli_tools/configuration/preprocessing.py @@ -7,10 +7,10 @@ from glob import glob from os.path import join, basename, dirname, isabs, relpath -from .helm import KEY_APPS, KEY_TASK_IMAGES, KEY_HARNESS, KEY_DEPLOYMENT +from ..constants import KEY_APPS, KEY_TASK_IMAGES, KEY_HARNESS, KEY_DEPLOYMENT from .configurationgenerator import DEFAULT_IGNORE, generate_tag_from_content -from .utils import app_name_from_path, merge_app_directories, merge_configuration_directories, find_subdirs, read_dockerignore, guess_build_dependencies_from_dockerfile +from ..utils import app_name_from_path, merge_app_directories, merge_configuration_directories, find_subdirs, read_dockerignore, guess_build_dependencies_from_dockerfile from cloudharness_utils.constants import APPS_PATH, BASE_IMAGES_PATH, STATIC_IMAGES_PATH, DEFAULT_MERGE_PATH, EXCLUDE_PATHS diff --git a/tools/deployment-cli-tools/ch_cli_tools/secrets.py b/tools/deployment-cli-tools/ch_cli_tools/configuration/secrets.py similarity index 100% rename from tools/deployment-cli-tools/ch_cli_tools/secrets.py rename to tools/deployment-cli-tools/ch_cli_tools/configuration/secrets.py diff --git a/tools/deployment-cli-tools/ch_cli_tools/dockercompose.py b/tools/deployment-cli-tools/ch_cli_tools/dockercompose.py index d9cff2917..432222f6e 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/dockercompose.py +++ b/tools/deployment-cli-tools/ch_cli_tools/dockercompose.py @@ -16,7 +16,7 @@ from .models import HarnessMainConfig -from .configurationgenerator import ConfigurationGenerator, \ +from .configuration.configurationgenerator import ConfigurationGenerator, \ 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 diff --git a/tools/deployment-cli-tools/ch_cli_tools/helm.py b/tools/deployment-cli-tools/ch_cli_tools/helm.py index 585920e31..4b4f3254e 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/helm.py +++ b/tools/deployment-cli-tools/ch_cli_tools/helm.py @@ -15,7 +15,7 @@ from .models import HarnessMainConfig -from .configurationgenerator import ConfigurationGenerator, get_included_builds, validate_helm_values, resolve_task_image_owner, \ +from .configuration.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 diff --git a/tools/deployment-cli-tools/harness-deployment b/tools/deployment-cli-tools/harness-deployment index a5aaf1b0a..cac65a61f 100644 --- a/tools/deployment-cli-tools/harness-deployment +++ b/tools/deployment-cli-tools/harness-deployment @@ -6,11 +6,11 @@ import os from ch_cli_tools.dockercompose import create_docker_compose_configuration from ch_cli_tools.helm import create_helm_chart, deploy -from ch_cli_tools.configurationgenerator import hosts_info +from ch_cli_tools.configuration.configurationgenerator import hosts_info from ch_cli_tools.skaffold import create_skaffold_configuration, create_vscode_debug_configuration from ch_cli_tools.tilt import create_tilt_configuration from ch_cli_tools.codefresh import create_codefresh_deployment_scripts, write_env_file -from ch_cli_tools.preprocessing import preprocess_build_overrides, generate_hash_based_image_tags +from ch_cli_tools.configuration.preprocessing import preprocess_build_overrides, generate_hash_based_image_tags from ch_cli_tools.utils import merge_app_directories, merge_to_yaml_file from ch_cli_tools.migration import perform_migration from cloudharness_utils.constants import DEPLOYMENT_PATH, COMPOSE_ENGINE, HELM_ENGINE, VALUES_MANUAL_PATH, HELM_PATH, COMPOSE diff --git a/tools/deployment-cli-tools/tests/test_codefresh.py b/tools/deployment-cli-tools/tests/test_codefresh.py index 7db9fe4ca..cbd30a20f 100644 --- a/tools/deployment-cli-tools/tests/test_codefresh.py +++ b/tools/deployment-cli-tools/tests/test_codefresh.py @@ -1,9 +1,9 @@ -from ch_cli_tools.preprocessing import preprocess_build_overrides +from ch_cli_tools.configuration.preprocessing import preprocess_build_overrides from ch_cli_tools.helm import * -from ch_cli_tools.configurationgenerator import * +from ch_cli_tools.configuration.configurationgenerator import * from ch_cli_tools.codefresh import * -from ch_cli_tools.secrets import is_cloudharness_managed, is_secret_config, secret_manager, secret_value +from ch_cli_tools.configuration.secrets import is_cloudharness_managed, is_secret_config, secret_manager, secret_value HERE = os.path.dirname(os.path.realpath(__file__)) RESOURCES = os.path.join(HERE, 'resources') diff --git a/tools/deployment-cli-tools/tests/test_dockercompose.py b/tools/deployment-cli-tools/tests/test_dockercompose.py index 6b3cbd3b5..cac57a287 100644 --- a/tools/deployment-cli-tools/tests/test_dockercompose.py +++ b/tools/deployment-cli-tools/tests/test_dockercompose.py @@ -1,6 +1,6 @@ from ch_cli_tools.dockercompose import * -from ch_cli_tools.configurationgenerator import * -from ch_cli_tools.preprocessing import preprocess_build_overrides, generate_hash_based_image_tags +from ch_cli_tools.configuration.configurationgenerator import * +from ch_cli_tools.configuration.preprocessing import preprocess_build_overrides, generate_hash_based_image_tags import pytest import shutil import subprocess diff --git a/tools/deployment-cli-tools/tests/test_helm.py b/tools/deployment-cli-tools/tests/test_helm.py index 386d46a61..8a74ede7e 100644 --- a/tools/deployment-cli-tools/tests/test_helm.py +++ b/tools/deployment-cli-tools/tests/test_helm.py @@ -1,21 +1,21 @@ from ch_cli_tools.helm import * -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 +from ch_cli_tools.configuration.configurationgenerator import * +from ch_cli_tools.configuration import configurationgenerator +from ch_cli_tools.configuration.preprocessing import preprocess_build_overrides, generate_hash_based_image_tags import logging import pytest import shutil import subprocess import pytest -from ch_cli_tools import configurationgenerator -from ch_cli_tools.configurationgenerator import * +from ch_cli_tools.configuration import configurationgenerator +from ch_cli_tools.configuration.configurationgenerator import * from ch_cli_tools.helm import * -from ch_cli_tools.preprocessing import ( +from ch_cli_tools.configuration.preprocessing import ( generate_hash_based_image_tags, preprocess_build_overrides, ) -from ch_cli_tools.instances import * +from ch_cli_tools.configuration.instances import * HERE = os.path.dirname(os.path.realpath(__file__)) RESOURCES = os.path.join(HERE, 'resources') diff --git a/tools/deployment-cli-tools/tests/test_preprocessing.py b/tools/deployment-cli-tools/tests/test_preprocessing.py index 96b124c67..8c254cc6a 100644 --- a/tools/deployment-cli-tools/tests/test_preprocessing.py +++ b/tools/deployment-cli-tools/tests/test_preprocessing.py @@ -4,7 +4,7 @@ import tempfile from ch_cli_tools.helm import * -from ch_cli_tools.preprocessing import * +from ch_cli_tools.configuration.preprocessing import * HERE = os.path.dirname(os.path.realpath(__file__)) RESOURCES = os.path.join(HERE, 'resources') diff --git a/tools/deployment-cli-tools/tests/test_skaffold.py b/tools/deployment-cli-tools/tests/test_skaffold.py index c3006bf0b..0c97c650e 100644 --- a/tools/deployment-cli-tools/tests/test_skaffold.py +++ b/tools/deployment-cli-tools/tests/test_skaffold.py @@ -2,7 +2,7 @@ import shutil from ch_cli_tools.helm import * -from ch_cli_tools.preprocessing import preprocess_build_overrides +from ch_cli_tools.configuration.preprocessing import preprocess_build_overrides from ch_cli_tools.skaffold import * HERE = os.path.dirname(os.path.realpath(__file__)) diff --git a/tools/deployment-cli-tools/tests/test_tilt.py b/tools/deployment-cli-tools/tests/test_tilt.py index 9ed09817c..eeed6d848 100644 --- a/tools/deployment-cli-tools/tests/test_tilt.py +++ b/tools/deployment-cli-tools/tests/test_tilt.py @@ -4,7 +4,7 @@ import shutil from ch_cli_tools.helm import * -from ch_cli_tools.preprocessing import preprocess_build_overrides +from ch_cli_tools.configuration.preprocessing import preprocess_build_overrides from ch_cli_tools.tilt import create_tilt_configuration HERE = os.path.dirname(os.path.realpath(__file__)) From 8c45b30f9f78d67218c0753ecd4210518455df3d Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Wed, 16 Sep 2026 17:28:05 +0200 Subject: [PATCH 3/8] CH-293 handle volume and database name conflicts --- .../{values.yaml => values-test.yaml} | 7 ++ deployment/codefresh-test.yaml | 44 ++++----- docs/applications/README.md | 19 +++- .../configuration/configurationgenerator.py | 6 +- .../ch_cli_tools/configuration/instances.py | 80 +++++++++++----- tools/deployment-cli-tools/tests/test_helm.py | 94 +++++++++++++++++-- 6 files changed, 187 insertions(+), 63 deletions(-) rename applications/samples/deploy/instances/instance1/{values.yaml => values-test.yaml} (56%) diff --git a/applications/samples/deploy/instances/instance1/values.yaml b/applications/samples/deploy/instances/instance1/values-test.yaml similarity index 56% rename from applications/samples/deploy/instances/instance1/values.yaml rename to applications/samples/deploy/instances/instance1/values-test.yaml index 4a4ca6691..58ff90182 100644 --- a/applications/samples/deploy/instances/instance1/values.yaml +++ b/applications/samples/deploy/instances/instance1/values-test.yaml @@ -2,6 +2,13 @@ harness: subdomain: samples1 deployment: replicas: 1 + statefulset: false + resources: + limits: + memory: "160Mi" + requests: + cpu: "1m" + memory: "16Mi" database: connect_string: "instance1 connection string" envmap: diff --git a/deployment/codefresh-test.yaml b/deployment/codefresh-test.yaml index 5fa047db3..2315355e1 100644 --- a/deployment/codefresh-test.yaml +++ b/deployment/codefresh-test.yaml @@ -55,9 +55,9 @@ steps: buildkit: true build_arguments: - NOCACHE=${{CF_BUILD_ID}} - - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - GOLANG=golang:1.26 - ROCKYLINUX=rockylinux/rockylinux:10.1-minimal + - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - NODE=node:22-alpine - PYTHON=python:3.12-slim-trixie image_name: cloud-harness/accounts @@ -82,9 +82,9 @@ steps: buildkit: true build_arguments: - NOCACHE=${{CF_BUILD_ID}} - - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - GOLANG=golang:1.26 - ROCKYLINUX=rockylinux/rockylinux:10.1-minimal + - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - NODE=node:22-alpine - PYTHON=python:3.12-slim-trixie image_name: cloud-harness/cloudharness-base @@ -109,9 +109,9 @@ steps: buildkit: true build_arguments: - NOCACHE=${{CF_BUILD_ID}} - - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - GOLANG=golang:1.26 - ROCKYLINUX=rockylinux/rockylinux:10.1-minimal + - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - NODE=node:22-alpine - PYTHON=python:3.12-slim-trixie image_name: cloud-harness/cloudharness-frontend-build @@ -136,9 +136,9 @@ steps: buildkit: true build_arguments: - NOCACHE=${{CF_BUILD_ID}} - - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - GOLANG=golang:1.26 - ROCKYLINUX=rockylinux/rockylinux:10.1-minimal + - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - NODE=node:22-alpine - PYTHON=python:3.12-slim-trixie image_name: cloud-harness/nfsserver @@ -163,9 +163,9 @@ steps: buildkit: true build_arguments: - NOCACHE=${{CF_BUILD_ID}} - - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - GOLANG=golang:1.26 - ROCKYLINUX=rockylinux/rockylinux:10.1-minimal + - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - NODE=node:22-alpine - PYTHON=python:3.12-slim-trixie image_name: cloud-harness/test-e2e @@ -197,9 +197,9 @@ steps: build_arguments: - NOCACHE=${{CF_BUILD_ID}} - CLOUDHARNESS_BASE=${{REGISTRY}}/cloud-harness/cloudharness-base:${{CLOUDHARNESS_BASE_TAG}} - - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - GOLANG=golang:1.26 - ROCKYLINUX=rockylinux/rockylinux:10.1-minimal + - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - NODE=node:22-alpine - PYTHON=python:3.12-slim-trixie image_name: cloud-harness/cloudharness-django @@ -225,9 +225,9 @@ steps: build_arguments: - NOCACHE=${{CF_BUILD_ID}} - CLOUDHARNESS_BASE=${{REGISTRY}}/cloud-harness/cloudharness-base:${{CLOUDHARNESS_BASE_TAG}} - - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - GOLANG=golang:1.26 - ROCKYLINUX=rockylinux/rockylinux:10.1-minimal + - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - NODE=node:22-alpine - PYTHON=python:3.12-slim-trixie image_name: cloud-harness/cloudharness-flask @@ -253,9 +253,9 @@ steps: build_arguments: - NOCACHE=${{CF_BUILD_ID}} - CLOUDHARNESS_BASE=${{REGISTRY}}/cloud-harness/cloudharness-base:${{CLOUDHARNESS_BASE_TAG}} - - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - GOLANG=golang:1.26 - ROCKYLINUX=rockylinux/rockylinux:10.1-minimal + - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - NODE=node:22-alpine - PYTHON=python:3.12-slim-trixie image_name: cloud-harness/jupyterhub @@ -281,9 +281,9 @@ steps: build_arguments: - NOCACHE=${{CF_BUILD_ID}} - CLOUDHARNESS_BASE=${{REGISTRY}}/cloud-harness/cloudharness-base:${{CLOUDHARNESS_BASE_TAG}} - - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - GOLANG=golang:1.26 - ROCKYLINUX=rockylinux/rockylinux:10.1-minimal + - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - NODE=node:22-alpine - PYTHON=python:3.12-slim-trixie image_name: cloud-harness/sampleapp-print-file @@ -309,9 +309,9 @@ steps: build_arguments: - NOCACHE=${{CF_BUILD_ID}} - CLOUDHARNESS_BASE=${{REGISTRY}}/cloud-harness/cloudharness-base:${{CLOUDHARNESS_BASE_TAG}} - - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - GOLANG=golang:1.26 - ROCKYLINUX=rockylinux/rockylinux:10.1-minimal + - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - NODE=node:22-alpine - PYTHON=python:3.12-slim-trixie image_name: cloud-harness/sampleapp-secret @@ -337,9 +337,9 @@ steps: build_arguments: - NOCACHE=${{CF_BUILD_ID}} - CLOUDHARNESS_BASE=${{REGISTRY}}/cloud-harness/cloudharness-base:${{CLOUDHARNESS_BASE_TAG}} - - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - GOLANG=golang:1.26 - ROCKYLINUX=rockylinux/rockylinux:10.1-minimal + - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - NODE=node:22-alpine - PYTHON=python:3.12-slim-trixie image_name: cloud-harness/test-api @@ -366,9 +366,9 @@ steps: build_arguments: - NOCACHE=${{CF_BUILD_ID}} - CLOUDHARNESS_BASE=${{REGISTRY}}/cloud-harness/cloudharness-base:${{CLOUDHARNESS_BASE_TAG}} - - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - GOLANG=golang:1.26 - ROCKYLINUX=rockylinux/rockylinux:10.1-minimal + - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - NODE=node:22-alpine - PYTHON=python:3.12-slim-trixie image_name: cloud-harness/workflows-extract-download @@ -394,9 +394,9 @@ steps: build_arguments: - NOCACHE=${{CF_BUILD_ID}} - CLOUDHARNESS_BASE=${{REGISTRY}}/cloud-harness/cloudharness-base:${{CLOUDHARNESS_BASE_TAG}} - - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - GOLANG=golang:1.26 - ROCKYLINUX=rockylinux/rockylinux:10.1-minimal + - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - NODE=node:22-alpine - PYTHON=python:3.12-slim-trixie image_name: cloud-harness/workflows-notify-queue @@ -422,9 +422,9 @@ steps: build_arguments: - NOCACHE=${{CF_BUILD_ID}} - CLOUDHARNESS_BASE=${{REGISTRY}}/cloud-harness/cloudharness-base:${{CLOUDHARNESS_BASE_TAG}} - - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - GOLANG=golang:1.26 - ROCKYLINUX=rockylinux/rockylinux:10.1-minimal + - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - NODE=node:22-alpine - PYTHON=python:3.12-slim-trixie image_name: cloud-harness/workflows-send-result-event @@ -455,9 +455,9 @@ steps: build_arguments: - NOCACHE=${{CF_BUILD_ID}} - CLOUDHARNESS_FLASK=${{REGISTRY}}/cloud-harness/cloudharness-flask:${{CLOUDHARNESS_FLASK_TAG}} - - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - GOLANG=golang:1.26 - ROCKYLINUX=rockylinux/rockylinux:10.1-minimal + - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - NODE=node:22-alpine - PYTHON=python:3.12-slim-trixie image_name: cloud-harness/common @@ -484,9 +484,9 @@ steps: - NOCACHE=${{CF_BUILD_ID}} - CLOUDHARNESS_FRONTEND_BUILD=${{REGISTRY}}/cloud-harness/cloudharness-frontend-build:${{CLOUDHARNESS_FRONTEND_BUILD_TAG}} - CLOUDHARNESS_FLASK=${{REGISTRY}}/cloud-harness/cloudharness-flask:${{CLOUDHARNESS_FLASK_TAG}} - - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - GOLANG=golang:1.26 - ROCKYLINUX=rockylinux/rockylinux:10.1-minimal + - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - NODE=node:22-alpine - PYTHON=python:3.12-slim-trixie image_name: cloud-harness/sampleapp @@ -512,9 +512,9 @@ steps: build_arguments: - NOCACHE=${{CF_BUILD_ID}} - CLOUDHARNESS_FLASK=${{REGISTRY}}/cloud-harness/cloudharness-flask:${{CLOUDHARNESS_FLASK_TAG}} - - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - GOLANG=golang:1.26 - ROCKYLINUX=rockylinux/rockylinux:10.1-minimal + - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - NODE=node:22-alpine - PYTHON=python:3.12-slim-trixie image_name: cloud-harness/volumemanager @@ -540,9 +540,9 @@ steps: build_arguments: - NOCACHE=${{CF_BUILD_ID}} - CLOUDHARNESS_FLASK=${{REGISTRY}}/cloud-harness/cloudharness-flask:${{CLOUDHARNESS_FLASK_TAG}} - - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - GOLANG=golang:1.26 - ROCKYLINUX=rockylinux/rockylinux:10.1-minimal + - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - NODE=node:22-alpine - PYTHON=python:3.12-slim-trixie image_name: cloud-harness/workflows @@ -573,9 +573,9 @@ steps: build_arguments: - NOCACHE=${{CF_BUILD_ID}} - SAMPLES=${{REGISTRY}}/cloud-harness/sampleapp:${{SAMPLES_TAG}} - - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - GOLANG=golang:1.26 - ROCKYLINUX=rockylinux/rockylinux:10.1-minimal + - KEYCLOAK=quay.io/keycloak/keycloak:26.5 - NODE=node:22-alpine - PYTHON=python:3.12-slim-trixie image_name: cloud-harness/sampleapp-sum @@ -618,6 +618,7 @@ steps: custom_value_files: - ./deployment/helm/values.yaml custom_values: + - 'apps_samples-instance1_harness_secrets_asecret="${{ASECRET}}"' - 'apps_samples_harness_secrets_asecret="${{ASECRET}}"' wait_deployment: stage: qa @@ -627,10 +628,11 @@ steps: commands: - kubectl config use-context ${{CLUSTER_NAME}} - kubectl config set-context --current --namespace=test-${{NAMESPACE_BASENAME}} - - kubectl rollout status statefulset/samples - - kubectl rollout status deployment/common - kubectl rollout status deployment/volumemanager + - kubectl rollout status deployment/samples-instance1 - kubectl rollout status deployment/accounts + - kubectl rollout status statefulset/samples + - kubectl rollout status deployment/common - kubectl rollout status deployment/workflows - sleep 60 tests_nfs_failover: diff --git a/docs/applications/README.md b/docs/applications/README.md index 7c45a5b80..77cdeefc6 100644 --- a/docs/applications/README.md +++ b/docs/applications/README.md @@ -124,19 +124,30 @@ deploy/values-[ENV].yaml deploy/values.yaml # loses ``` +An instance is declared by its values files: with a `values.yaml` it is deployed in every +environment, with only a `values-[ENV].yaml` it is deployed in that environment alone (with +`harness-deployment -e ENV`). A directory with neither is ignored. + What identifies the application is never inherited, so that an instance never claims the application's hosts or resources: - `subdomain`, `aliases` and `domain`. An instance without a `subdomain` of its own answers on - its directory's name, so `instances/samples1/` alone is served at `samples1.[DOMAIN]`; declare - `subdomain: null` to give an instance no ingress at all + its directory's name, so `instances/samples1/` with an empty `values.yaml` is served at + `samples1.[DOMAIN]`; declare `subdomain: null` to give an instance no ingress at all - the names of the service, deployment and database, which are derived from the instance key -- `deployment.volume.name`, prefixed with the instance key, so the instance never mounts the - application's storage +- `deployment.volume.name` when the volume is automatic (`auto` unset or `true`): prefixed with + the instance name (`instance1-my-shared-volume`), so the instance gets a claim of its own instead + of mounting the application's storage. A non-automatic volume is a pre-existing claim and stays + shared - `database.connect_string`, emptied: an instance of an application using an externally managed database needs a connection string of its own. Set `database.auto: true` to have CloudHarness deploy a database of its own for it instead. +An instance may share the application's database server by declaring its `database.name` +(`samples-db` for `samples`). Its initial database is then named after the instance application, +hyphens turned to underscores (`samples_instance1`), so the two never share data. Declare +`postgres.initialdb` on the instance to pick the name yourself. + Instances are deployed together with their application: `harness-deployment -i samples` deploys `samples` and all its instances, and `-e samples-instance1` leaves one out. CI builds and tests the application only, since an instance runs the same image. diff --git a/tools/deployment-cli-tools/ch_cli_tools/configuration/configurationgenerator.py b/tools/deployment-cli-tools/ch_cli_tools/configuration/configurationgenerator.py index 12c9243b2..a75f909c8 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/configuration/configurationgenerator.py +++ b/tools/deployment-cli-tools/ch_cli_tools/configuration/configurationgenerator.py @@ -56,7 +56,7 @@ def __init__(self, root_paths: List[str], tag: Union[str, int, None] = 'latest', self.env = env or {} self.namespace = namespace self.calculate_hash_tags = calculate_hash_tags - check_instance_collisions(self.root_paths, exclude=self.exclude) + check_instance_collisions(self.root_paths, exclude=self.exclude, envs=self.env) # In this tree we will collect the and their parent dependencies self.build_tree: dict[str, list[str]] = {} @@ -196,7 +196,7 @@ def _inherit_instance_images(self, helm_values): """ apps = helm_values[KEY_APPS] for app_name in list(apps): - for instance_name in instance_names(app_name, self.root_paths): + for instance_name in instance_names(app_name, self.root_paths, self.env): app_key = instance_app_key(app_name, instance_name) if app_key in apps: inherit_parent_image(apps[app_key], apps[app_name]) @@ -806,7 +806,7 @@ def collect_apps_helm_templates(search_root, dest_helm_chart_path, templates_pat collect_app_deploy_directories( app_path, app_name, dest_helm_chart_path, templates_path=templates_path, envs=envs) - for instance_name, instance_path in instance_directories(app_path).items(): + for instance_name, instance_path in instance_directories(app_path, envs).items(): instance_key = instance_app_key(app_name, instance_name) if instance_key in exclude or (include and not any(inc in instance_key for inc in include)): continue diff --git a/tools/deployment-cli-tools/ch_cli_tools/configuration/instances.py b/tools/deployment-cli-tools/ch_cli_tools/configuration/instances.py index f058c9804..f4b53635c 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/configuration/instances.py +++ b/tools/deployment-cli-tools/ch_cli_tools/configuration/instances.py @@ -13,12 +13,13 @@ """ import copy +import logging from pathlib import Path from cloudharness_utils.constants import APPS_PATH from ..constants import KEY_DATABASE, KEY_DEPLOYMENT, KEY_HARNESS, KEY_SERVICE, KEY_TASK_IMAGES from ..common_types import ValuesValidationException -from ..utils import app_name_from_path, dict_merge, get_template, yaml +from ..utils import app_name_from_path, dict_merge, yaml # Directory of an application holding its instances, one sub-directory each @@ -47,17 +48,34 @@ def instances_path(app_path): return Path(app_path) / 'deploy' / INSTANCES_PATH -def instance_directories(app_path): - """The instance directories of an application, by instance name.""" - return {app_name_from_path(f"{path.name}"): path - for path in sorted(instances_path(app_path).glob("*/")) - if path.is_dir() and not path.name.startswith('.')} +def instance_values_files(instance_path, envs=()): + """The values files declaring an instance for the given environments, in merge order: + `values.yaml`, then `values-[env].yaml` for each environment.""" + return [path for path in [instance_path / 'values.yaml', *(instance_path / f'values-{env}.yaml' for env in envs)] + if path.exists()] -def instance_names(app_name, root_paths): +def instance_directories(app_path, envs=()): + """The instance directories of an application declared for the given environments, by name. + + An instance is declared by its values files: `values.yaml` deploys it in every environment, + `values-[env].yaml` alone only in that environment. A directory declaring neither is skipped. + """ + directories = {} + for path in sorted(instances_path(app_path).glob("*/")): + if not path.is_dir() or path.name.startswith('.'): + continue + if not instance_values_files(path, envs): + logging.info("Instance %s declares no values for the current environments, skipping", path) + continue + directories[app_name_from_path(f"{path.name}")] = path + return directories + + +def instance_names(app_name, root_paths, envs=()): """Names of the instances an application declares in any root path.""" return {name for root_path in root_paths - for name in instance_directories(Path(root_path) / APPS_PATH / app_name)} + for name in instance_directories(Path(root_path) / APPS_PATH / app_name, envs)} def application_names(root_paths): @@ -87,7 +105,6 @@ def build_instance_values(app_values, parent_name, instance_name, instance_value The merge follows `dict_merge`: mappings are merged key by key, lists (`env`, `uri_role_mapping`, `aliases`, ...) are replaced wholesale by the instance's. """ - app_key = instance_app_key(parent_name, instance_name) instance_app = copy.deepcopy(app_values) harness = instance_app.setdefault(KEY_HARNESS, {}) @@ -103,16 +120,18 @@ def build_instance_values(app_values, parent_name, instance_name, instance_value harness = instance_app[KEY_HARNESS] # An instance is reached on its own subdomain: without one of its own it answers on its - # directory's name, so that creating the directory is enough to deploy it. An instance + # directory's name, so that an empty values file is enough to deploy it. An instance # declaring `subdomain: null` opts out and gets no ingress. if 'subdomain' not in (instance_values.get(KEY_HARNESS) or {}): harness['subdomain'] = instance_name - # The volume name is the name of the claim the application mounts: left inherited, the - # instance would mount the parent's storage. Named after the instance, it gets its own. + # An automatic volume is a claim created for the application: left inherited, the instance + # would mount the parent's storage. Named after the instance, it gets a claim of its own. A + # volume that is not automatic is a pre-existing claim, shared like between any applications. volume = (harness.get(KEY_DEPLOYMENT) or {}).get('volume') or {} - if volume.get('name') and not instance_sets(instance_values, KEY_HARNESS, KEY_DEPLOYMENT, 'volume', 'name'): - volume['name'] = f"{app_key}-{volume['name']}" + if volume.get('name') and volume.get('auto', True) \ + and not instance_sets(instance_values, KEY_HARNESS, KEY_DEPLOYMENT, 'volume', 'name'): + volume['name'] = f"{instance_name}-{volume['name']}" # A connection string points at one database: inherited, it would connect the instance to the # parent's. Emptied, it keeps the parent's intent of an externally managed database while @@ -121,6 +140,16 @@ def build_instance_values(app_values, parent_name, instance_name, instance_value if database.get('connect_string') and not instance_sets(instance_values, KEY_HARNESS, KEY_DATABASE, 'connect_string'): database['connect_string'] = '' + # An instance declaring the parent's database name shares its database server. Its initial + # database is then named after the instance application, so that it never gets the parent's + # data. Hyphens become underscores: database identifiers with hyphens need quoting in SQL. + parent_database = (app_values.get(KEY_HARNESS) or {}).get(KEY_DATABASE) or {} + if database.get('name') and database['name'] == (parent_database.get('name') or f"{parent_name}-db"): + for database_type, type_config in database.items(): + if isinstance(type_config, dict) and type_config.get('initialdb') \ + and not instance_sets(instance_values, KEY_HARNESS, KEY_DATABASE, database_type, 'initialdb'): + type_config['initialdb'] = instance_app_key(parent_name, instance_name).replace('-', '_') + # The instance runs the parent's image, inherited with the rest of the configuration: it is # never built on its own, and the task images produced by the parent's build belong to the # parent alone. @@ -147,12 +176,12 @@ def inherit_parent_image(instance_app, parent_app): deployment['image'] = image -def check_instance_collisions(root_paths, exclude=()): +def check_instance_collisions(root_paths, exclude=(), envs=()): """Check that no instance and application are deployed under the same key, as merging their values into one would silently deploy neither.""" applications = application_names(root_paths) - set(exclude) for app_name in applications: - for instance_name in instance_names(app_name, root_paths): + for instance_name in instance_names(app_name, root_paths, envs): app_key = instance_app_key(app_name, instance_name) if app_key in applications: raise ValuesValidationException( @@ -190,12 +219,10 @@ def task_image_collision(app_key, task_images): def load_instance_values(instance_path, envs=()): """The override values declared in one instance directory: `values.yaml`, overridden by `values-[env].yaml`.""" - values = get_template(instance_path / 'values.yaml') - for env in envs: - env_values_path = instance_path / f'values-{env}.yaml' - if env_values_path.exists(): - with env_values_path.open() as f: - values = dict_merge(values, yaml.load(f)) + values = {} + for values_path in instance_values_files(instance_path, envs): + with values_path.open() as f: + values = dict_merge(values, yaml.load(f) or {}) return values @@ -203,9 +230,10 @@ def collect_instances(app_name, root_paths, envs=()): """Collect the override values of the instances an application declares in any root path. An instance is a directory under the application's `deploy/instances`, holding a - `values.yaml` (and optionally `values-[env].yaml`) with the values overriding the - application's, plus the `resources` and `templates` overriding the application's own. The - same instance declared in several root paths is merged, a later root overriding an earlier one. + `values.yaml` (or a `values-[env].yaml`, to deploy it in that environment only) with the + values overriding the application's, plus the `resources` and `templates` overriding the + application's own. The same instance declared in several root paths is merged, a later root + overriding an earlier one. Returns a mapping of instance name -> override values, empty when the application declares no instance. @@ -220,7 +248,7 @@ def collect_instances(app_name, root_paths, envs=()): task_images.update(app_name_from_path(f"{app_name}/{task_path.name}") for task_path in (app_path / 'tasks').glob("*/") if task_path.is_dir()) - for instance_name, instance_path in instance_directories(app_path).items(): + for instance_name, instance_path in instance_directories(app_path, envs).items(): instances[instance_name] = dict_merge( instances.get(instance_name, {}), load_instance_values(instance_path, envs)) diff --git a/tools/deployment-cli-tools/tests/test_helm.py b/tools/deployment-cli-tools/tests/test_helm.py index 8a74ede7e..afee3582a 100644 --- a/tools/deployment-cli-tools/tests/test_helm.py +++ b/tools/deployment-cli-tools/tests/test_helm.py @@ -1454,7 +1454,7 @@ def test_instances_expand_into_applications(tmp_path): assert instance[KEY_HARNESS][KEY_DEPLOYMENT]['name'] == 'samples-instance1' assert instance[KEY_HARNESS][KEY_DATABASE]['name'] == 'samples-instance1-db' assert instance[KEY_HARNESS][KEY_DEPLOYMENT]['volume']['name'] == \ - 'samples-instance1-my-shared-volume', 'an instance must not mount the parent claim' + 'instance1-my-shared-volume', 'an instance must not mount the parent claim' assert parent[KEY_HARNESS][KEY_DEPLOYMENT]['volume']['name'] == 'my-shared-volume' # The parent's image, built once @@ -1552,6 +1552,49 @@ def test_instance_templates_are_overlaid_on_the_application(tmp_path): assert rendered['data']['subdomain'] == 'myinstance' +def test_instance_gets_its_own_automatic_volume(): + """An automatic volume is a claim created for the application: the instance gets one of its + own, named after the instance. A non-automatic volume is a pre-existing claim, left shared.""" + def volume_of(parent_volume, instance_values={}): + parent = {KEY_HARNESS: {KEY_DEPLOYMENT: {'volume': parent_volume}}} + return build_instance_values(parent, 'samples', 'instance1', instance_values)[KEY_HARNESS][KEY_DEPLOYMENT]['volume'] + + assert volume_of({'name': 'shared', 'auto': True})['name'] == 'instance1-shared' + assert volume_of({'name': 'shared'})['name'] == 'instance1-shared', 'volumes are automatic by default' + assert volume_of({'name': 'existing-claim', 'auto': False})['name'] == 'existing-claim' + assert volume_of({'name': 'shared', 'auto': True}, + {KEY_HARNESS: {KEY_DEPLOYMENT: {'volume': {'name': 'mine'}}}})['name'] == 'mine' + + +def test_instance_sharing_the_parent_database_server_gets_its_own_database(): + """By default an instance gets a database server of its own. Declaring the parent's database + name shares the server, and the initial database is then named after the instance application + so the data is not shared. Underscores, as hyphens need quoting in SQL identifiers.""" + parent = {KEY_HARNESS: {KEY_DATABASE: { + 'type': 'postgres', 'auto': True, 'postgres': {'initialdb': 'cloudharness'}, 'mongo': {'image': 'mongo:5'}}}} + + def database_of(instance_database): + return build_instance_values(parent, 'samples', 'instance1', + {KEY_HARNESS: {KEY_DATABASE: instance_database}})[KEY_HARNESS][KEY_DATABASE] + + own_server = database_of({}) + assert 'name' not in own_server, 'named after the instance when the deployment is finalized' + assert own_server['postgres']['initialdb'] == 'cloudharness' + + shared_server = database_of({'name': 'samples-db'}) + assert shared_server['postgres']['initialdb'] == 'samples_instance1' + assert shared_server['mongo'] == {'image': 'mongo:5'} + assert parent[KEY_HARNESS][KEY_DATABASE]['postgres']['initialdb'] == 'cloudharness' + + assert database_of({'name': 'samples-db', 'postgres': {'initialdb': 'mine'}})['postgres']['initialdb'] == 'mine' + assert database_of({'name': 'other-db'})['postgres']['initialdb'] == 'cloudharness' + + # a parent naming its database explicitly is matched on that name + parent[KEY_HARNESS][KEY_DATABASE]['name'] = 'central-db' + assert database_of({'name': 'central-db'})['postgres']['initialdb'] == 'samples_instance1' + assert database_of({'name': 'samples-db'})['postgres']['initialdb'] == 'cloudharness' + + def test_instance_does_not_inherit_the_parent_connect_string(tmp_path): """A connection string points at one database: an instance never inherits the parent's.""" out_folder = tmp_path / 'test_instance_connect_string' @@ -1580,20 +1623,23 @@ def test_instance_renders_its_own_manifests(tmp_path): shutil.rmtree(helm_path / 'charts', ignore_errors=True) manifests = render_helm_chart(helm_path) - statefulset = find_manifest(manifests, 'StatefulSet', 'samples-instance1') + # the fixture instance opts out of the parent's statefulset, so it is a plain Deployment + workload = find_manifest(manifests, 'Deployment', 'samples-instance1') find_manifest(manifests, 'Service', 'samples-instance1') - containers = statefulset['spec']['template']['spec']['containers'] + containers = workload['spec']['template']['spec']['containers'] app_container = next(c for c in containers if c['name'] == 'samples-instance1') env = {e['name']: e.get('value') for e in app_container['env']} assert env['CH_CURRENT_APP_NAME'] == 'samples-instance1', \ 'the instance must read its own configuration, not the parent one' - # Its own claim: sharing the parent's would give the instance the parent's data - claims = [c['metadata']['name'] for c in statefulset['spec'].get('volumeClaimTemplates', [])] - parent_claims = [c['metadata']['name'] - for c in find_manifest(manifests, 'StatefulSet', 'samples')['spec'].get('volumeClaimTemplates', [])] - assert claims and not set(claims) & set(parent_claims) + # Its own claim, named after the instance: sharing the parent's would give it the parent's data + find_manifest(manifests, 'PersistentVolumeClaim', 'instance1-my-shared-volume') + claims = {v['persistentVolumeClaim']['claimName'] + for v in workload['spec']['template']['spec'].get('volumes', []) if 'persistentVolumeClaim' in v} + parent_claims = {c['metadata']['name'] + for c in find_manifest(manifests, 'StatefulSet', 'samples')['spec'].get('volumeClaimTemplates', [])} + assert claims == {'instance1-my-shared-volume'} and not claims & parent_claims # Its own database secret instance_secret = find_manifest(manifests, 'Secret', 'samples-instance1-db') @@ -1626,7 +1672,9 @@ def test_instance_colliding_with_an_application_is_rejected(tmp_path): """An instance and an application under the same key would be merged into one, whichever root path declares each of them.""" first, second = tmp_path / 'first', tmp_path / 'second' - (first / APPS_PATH / 'myapp' / 'deploy' / INSTANCES_PATH / 'other').mkdir(parents=True) + instance_path = first / APPS_PATH / 'myapp' / 'deploy' / INSTANCES_PATH / 'other' + instance_path.mkdir(parents=True) + (instance_path / 'values.yaml').write_text('') (second / APPS_PATH / 'myapp-other' / 'deploy').mkdir(parents=True) check_instance_collisions([first]) @@ -1669,6 +1717,34 @@ def test_collect_instances_skips_hidden_directories(tmp_path): assert set(collect_instances('myapp', [tmp_path])) == {'real'} +def test_instance_declared_for_an_environment_only(tmp_path): + """An instance with a `values-[env].yaml` alone is deployed in that environment only; the + `samples` fixture declares `instance1` for `dev`.""" + instances_dir = Path(CLOUDHARNESS_ROOT) / APPS_PATH / 'samples' / 'deploy' / INSTANCES_PATH / 'instance1' + assert (instances_dir / 'values-dev.yaml').exists() and not (instances_dir / 'values.yaml').exists() + + assert instance_names('samples', [CLOUDHARNESS_ROOT]) == set() + assert instance_names('samples', [CLOUDHARNESS_ROOT], envs=['dev']) == {'instance1'} + assert collect_instances('samples', [CLOUDHARNESS_ROOT]) == {} + assert collect_instances('samples', [CLOUDHARNESS_ROOT], envs=['dev'])['instance1'][KEY_HARNESS]['subdomain'] == 'samples1' + + out_folder = tmp_path / 'test_instance_env_only' + values = create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, include=['samples'], + domain="my.local", namespace='test', local=False, tag=1, registry='reg') + assert 'samples-instance1' not in values[KEY_APPS] + assert not (out_folder / HELM_CHART_PATH / 'resources' / 'samples-instance1').exists(), \ + 'an instance not deployed leaves no collected files behind' + + +def test_instance_directory_without_values_is_ignored(tmp_path): + instance_path = tmp_path / APPS_PATH / 'myapp' / 'deploy' / INSTANCES_PATH / 'inst' + (instance_path / 'resources').mkdir(parents=True) + assert collect_instances('myapp', [tmp_path], envs=['dev']) == {} + + (instance_path / 'values.yaml').write_text('') + assert collect_instances('myapp', [tmp_path]) == {'inst': {}}, 'an empty values file declares it' + + def test_collect_instances_of_an_application_without_any(tmp_path): (tmp_path / APPS_PATH / 'myapp' / 'deploy').mkdir(parents=True) assert collect_instances('myapp', [tmp_path]) == {} From 743a58ec5e002b3cc33ae2e5dc6efa23fd57c98e Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Wed, 16 Sep 2026 17:56:08 +0200 Subject: [PATCH 4/8] CH-293 Collision handling improvements and clarifications --- docs/applications/README.md | 39 ++++++++++--------- .../ch_cli_tools/configuration/instances.py | 29 -------------- tools/deployment-cli-tools/tests/test_helm.py | 39 ++++++------------- 3 files changed, 32 insertions(+), 75 deletions(-) diff --git a/docs/applications/README.md b/docs/applications/README.md index 77cdeefc6..0a28373e4 100644 --- a/docs/applications/README.md +++ b/docs/applications/README.md @@ -70,14 +70,14 @@ harness: To customize the helm templates to use, put them inside the *deploy* subdirectory. -## Application instances +## Multiple application instances An application can be deployed several times over, each deployment on its own subdomain and with its own configuration and database. Each of those deployments is an *instance*, declared as a -directory under the application's `deploy/instances`: +directory under the application's `deploy/instances`, such as: ``` -applications/samples/ +applications/myapp/ Dockerfile deploy/ values.yaml # the application's configuration @@ -92,19 +92,17 @@ applications/samples/ templates/ # optional, overlaid on the application's templates ``` -The instance above is deployed as the application `samples-instance1`: that key names its service, +The instance above is deployed as the application `myapp-instance1`: that key names its service, deployment, database, volume, gatekeeper and configmaps, and is how it is referenced on the command -line. It runs the image built for `samples` — an instance adds no build, so declare no Dockerfile -in it. The key must not take over one of the application's task images: an instance named `print` -on an application with a `tasks/print-file` is rejected, as `samples-print` would own -`samples-print-file`. +line. It runs the image built for `myapp` — an instance adds no build, so declare no Dockerfile +in it. Everything else is inherited from the application, so an instance's `values.yaml` only carries what it changes: ```yaml harness: - subdomain: samples1 + subdomain: myapp1 deployment: replicas: 1 ``` @@ -114,6 +112,7 @@ instance overriding `uri_role_mapping` therefore replaces the whole list rather Resources and templates are overlaid file by file, so an instance inherits every file it does not override — above, `myConfig.json` comes from the application and `example.yaml` from the instance. +### Envs and values precedence resolution Environment specific values apply at both levels, the instance's taking precedence over the application's: @@ -128,12 +127,13 @@ An instance is declared by its values files: with a `values.yaml` it is deployed environment, with only a `values-[ENV].yaml` it is deployed in that environment alone (with `harness-deployment -e ENV`). A directory with neither is ignored. -What identifies the application is never inherited, so that an instance never claims the +### Inheritance exceptions +What identifies the application is never inherited, so to avoid collisions across application's hosts or resources: - `subdomain`, `aliases` and `domain`. An instance without a `subdomain` of its own answers on - its directory's name, so `instances/samples1/` with an empty `values.yaml` is served at - `samples1.[DOMAIN]`; declare `subdomain: null` to give an instance no ingress at all + its directory's name, so `instances/myapp1/` with an empty `values.yaml` is served at + `myapp1.[DOMAIN]`; declare `subdomain: null` to give an instance no ingress at all - the names of the service, deployment and database, which are derived from the instance key - `deployment.volume.name` when the volume is automatic (`auto` unset or `true`): prefixed with the instance name (`instance1-my-shared-volume`), so the instance gets a claim of its own instead @@ -143,13 +143,16 @@ application's hosts or resources: database needs a connection string of its own. Set `database.auto: true` to have CloudHarness deploy a database of its own for it instead. -An instance may share the application's database server by declaring its `database.name` -(`samples-db` for `samples`). Its initial database is then named after the instance application, -hyphens turned to underscores (`samples_instance1`), so the two never share data. Declare +### Database sharing +An instance may share the application's database server by explitly declaring its `database.name` +(For instance, `myapp-db` for `myapp`), and not overriding the name in the instance (or using the same). +When the database instance is shared, its initial database is then named after the instance application, +hyphens turned to underscores (`myapp_instance1`), so the two never share data. Declare `postgres.initialdb` on the instance to pick the name yourself. -Instances are deployed together with their application: `harness-deployment -i samples` deploys -`samples` and all its instances, and `-e samples-instance1` leaves one out. CI builds and tests the +### Deployment +Instances are deployed together with their application: `harness-deployment -i myapp` deploys +`myapp` and all its instances, and `-ex myapp-instance1` leaves one out. CI builds and tests the application only, since an instance runs the same image. ## Dependency to an existing Helm chart @@ -210,6 +213,6 @@ The most important configuration entries are the following: - `buildArgs`: a map of build arguments to provide to the dockerfile when building with Skaffold # Example code -- [Sample application](../../applications/samples) is a sample web application providing working examples of deployment configuration, backend and frontend code. +- [Sample application](../../applications/myapp) is a sample web application providing working examples of deployment configuration, backend and frontend code. diff --git a/tools/deployment-cli-tools/ch_cli_tools/configuration/instances.py b/tools/deployment-cli-tools/ch_cli_tools/configuration/instances.py index f4b53635c..babd529f0 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/configuration/instances.py +++ b/tools/deployment-cli-tools/ch_cli_tools/configuration/instances.py @@ -203,19 +203,6 @@ def resolve_instance_includes(include, app_name, instance_keys): return resolved -def task_image_collision(app_key, task_images): - """The task image an instance application key would take ownership of, if any. - - Task images are resolved to the application that builds them by longest name prefix - (`resolve_task_image_owner`), so an instance is in the way of the image named after it and - of every image whose name it prefixes: `samples-print` would own `samples-print-file`. - """ - for task_image in sorted(task_images): - if task_image == app_key or task_image.startswith(f"{app_key}-"): - return task_image - return None - - def load_instance_values(instance_path, envs=()): """The override values declared in one instance directory: `values.yaml`, overridden by `values-[env].yaml`.""" @@ -239,25 +226,9 @@ def collect_instances(app_name, root_paths, envs=()): no instance. """ instances = {} - # Task images are named `[application]-[task directory]`, the same way an instance - # application is: an instance whose key prefixes one would take ownership of it. - task_images = set() - for root_path in root_paths: app_path = Path(root_path) / APPS_PATH / app_name - task_images.update(app_name_from_path(f"{app_name}/{task_path.name}") - for task_path in (app_path / 'tasks').glob("*/") if task_path.is_dir()) - for instance_name, instance_path in instance_directories(app_path, envs).items(): instances[instance_name] = dict_merge( instances.get(instance_name, {}), load_instance_values(instance_path, envs)) - - for instance_name in instances: - collision = task_image_collision(instance_app_key(app_name, instance_name), task_images) - if collision: - raise ValuesValidationException( - f"Instance `{instance_name}` of application `{app_name}` is deployed as application " - f"`{instance_app_key(app_name, instance_name)}`, which takes over the task image " - f"`{collision}`. Rename the instance.") - return instances diff --git a/tools/deployment-cli-tools/tests/test_helm.py b/tools/deployment-cli-tools/tests/test_helm.py index afee3582a..4402084ec 100644 --- a/tools/deployment-cli-tools/tests/test_helm.py +++ b/tools/deployment-cli-tools/tests/test_helm.py @@ -1435,7 +1435,7 @@ def test_instances_expand_into_applications(tmp_path): """An instance is deployed as an application of its own, inheriting the parent's configuration.""" out_folder = tmp_path / 'test_instances_expand_into_applications' values = create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, include=['samples'], - domain="my.local", namespace='test', env='dev', local=False, tag=1, registry='reg') + domain="my.local", namespace='test', env='test', local=False, tag=1, registry='reg') apps = values[KEY_APPS] assert 'samples-instance1' in apps, 'including an application includes its instances' @@ -1473,7 +1473,7 @@ def test_instances_expand_into_applications(tmp_path): def test_instances_expand_without_include(tmp_path): out_folder = tmp_path / 'test_instances_expand_without_include' values = create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, domain="my.local", - namespace='test', env='dev', local=False, tag=1, registry='reg') + namespace='test', env='test', local=False, tag=1, registry='reg') instance = values[KEY_APPS]['samples-instance1'] assert instance[KEY_HARNESS]['subdomain'] == 'samples1' @@ -1485,7 +1485,7 @@ def test_instance_excluded_individually(tmp_path): """A single instance is left out with --exclude, without affecting its parent.""" out_folder = tmp_path / 'test_instance_excluded_individually' values = create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, include=['samples'], - exclude=['samples-instance1'], domain="my.local", namespace='test', env='dev', + exclude=['samples-instance1'], domain="my.local", namespace='test', env='test', local=False, tag=1, registry='reg') assert 'samples-instance1' not in values[KEY_APPS] @@ -1498,7 +1498,7 @@ def test_instance_include_pulls_in_its_parent(tmp_path): """Including an instance alone deploys the application it inherits its image from too.""" out_folder = tmp_path / 'test_instance_include_pulls_in_its_parent' values = create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, - include=['samples-instance1'], domain="my.local", namespace='test', env='dev', + include=['samples-instance1'], domain="my.local", namespace='test', env='test', local=False, tag=1, registry='reg') assert 'samples-instance1' in values[KEY_APPS] @@ -1510,7 +1510,7 @@ def test_instance_resources_are_overlaid_on_the_application(tmp_path): """An instance's resources override the application's file by file, and it inherits the rest.""" out_folder = tmp_path / 'test_instance_resources' create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, include=['samples'], - domain="my.local", namespace='test', env='dev', local=False, tag=1, registry='reg') + domain="my.local", namespace='test', env='test', local=False, tag=1, registry='reg') helm_path = out_folder / HELM_CHART_PATH instance_resources = helm_path / 'resources' / 'samples-instance1' @@ -1599,7 +1599,7 @@ def test_instance_does_not_inherit_the_parent_connect_string(tmp_path): """A connection string points at one database: an instance never inherits the parent's.""" out_folder = tmp_path / 'test_instance_connect_string' values = create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, include=['samples'], - domain="my.local", namespace='test', env='dev', local=False, tag=1, registry='reg') + domain="my.local", namespace='test', env='test', local=False, tag=1, registry='reg') instance_db = values[KEY_APPS]['samples-instance1'][KEY_HARNESS][KEY_DATABASE] parent_db = values[KEY_APPS]['samples'][KEY_HARNESS][KEY_DATABASE] @@ -1617,7 +1617,7 @@ def test_instance_renders_its_own_manifests(tmp_path): """The instance gets the full set of manifests on its own subdomain, backed by its own workload.""" out_folder = tmp_path / 'test_instance_renders_its_own_manifests' create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, include=['samples'], - domain="my.local", namespace='test', env='dev', local=False, tag=1, registry='reg') + domain="my.local", namespace='test', env='test', local=False, tag=1, registry='reg') helm_path = out_folder / HELM_CHART_PATH shutil.rmtree(helm_path / 'charts', ignore_errors=True) @@ -1691,23 +1691,6 @@ def test_instance_colliding_with_an_application_is_rejected(tmp_path): domain="my.local", namespace='test', local=False, tag=1, registry='reg') -def test_instance_directory_colliding_with_a_task_is_rejected(tmp_path): - """Task image ownership is resolved by longest name prefix, so an instance named `print` on - an application owning `myapp-print-file` would take the image over and it would never build. - The collision is caught when instances are collected, before anything reads task images.""" - app_path = tmp_path / APPS_PATH / 'myapp' - (app_path / 'tasks' / 'print-file').mkdir(parents=True) - instance_path = app_path / 'deploy' / INSTANCES_PATH / 'print' - instance_path.mkdir(parents=True) - (instance_path / 'values.yaml').write_text('harness:\n subdomain: myprint\n') - - with pytest.raises(ValuesValidationException, match='myapp-print-file'): - collect_instances('myapp', [tmp_path]) - - assert resolve_task_image_owner('myapp-print-file', {'myapp', 'myapp-print'}) == 'myapp-print', \ - 'the prefix match this guards against' - - def test_collect_instances_skips_hidden_directories(tmp_path): instances_dir = tmp_path / APPS_PATH / 'myapp' / 'deploy' / INSTANCES_PATH (instances_dir / '.hidden').mkdir(parents=True) @@ -1719,14 +1702,14 @@ def test_collect_instances_skips_hidden_directories(tmp_path): def test_instance_declared_for_an_environment_only(tmp_path): """An instance with a `values-[env].yaml` alone is deployed in that environment only; the - `samples` fixture declares `instance1` for `dev`.""" + `samples` fixture declares `instance1` for `test`.""" instances_dir = Path(CLOUDHARNESS_ROOT) / APPS_PATH / 'samples' / 'deploy' / INSTANCES_PATH / 'instance1' - assert (instances_dir / 'values-dev.yaml').exists() and not (instances_dir / 'values.yaml').exists() + assert (instances_dir / 'values-test.yaml').exists() and not (instances_dir / 'values.yaml').exists() assert instance_names('samples', [CLOUDHARNESS_ROOT]) == set() - assert instance_names('samples', [CLOUDHARNESS_ROOT], envs=['dev']) == {'instance1'} + assert instance_names('samples', [CLOUDHARNESS_ROOT], envs=['test']) == {'instance1'} assert collect_instances('samples', [CLOUDHARNESS_ROOT]) == {} - assert collect_instances('samples', [CLOUDHARNESS_ROOT], envs=['dev'])['instance1'][KEY_HARNESS]['subdomain'] == 'samples1' + assert collect_instances('samples', [CLOUDHARNESS_ROOT], envs=['test'])['instance1'][KEY_HARNESS]['subdomain'] == 'samples1' out_folder = tmp_path / 'test_instance_env_only' values = create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, include=['samples'], From bafe6055da32f5ea86092e1d0bf94d1734b3cd3a Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Thu, 17 Sep 2026 10:38:53 +0200 Subject: [PATCH 5/8] CH-293 test fix --- tools/deployment-cli-tools/tests/test_dockercompose.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/deployment-cli-tools/tests/test_dockercompose.py b/tools/deployment-cli-tools/tests/test_dockercompose.py index cac57a287..ab08b9395 100644 --- a/tools/deployment-cli-tools/tests/test_dockercompose.py +++ b/tools/deployment-cli-tools/tests/test_dockercompose.py @@ -134,7 +134,8 @@ def render_proxy_config(): stderr=subprocess.PIPE, text=True, ) - for document in yaml.load_all(completed.stdout): + documents = list(yaml.load_all(completed.stdout)) + for document in documents: metadata = (document or {}).get('cloudharness-metadata', {}) if metadata.get('path') == 'resources/generated/samples-gk/proxy.yml': return yaml.load(document['data']) From 3ccb2dfbbecd6e8ef5c13a2a0840dcc1ed6b3de0 Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Thu, 17 Sep 2026 10:39:18 +0200 Subject: [PATCH 6/8] CH-293 fix auto tagging not being propagated to the instance deployment --- .../configuration/preprocessing.py | 23 ++++++++ tools/deployment-cli-tools/tests/test_helm.py | 54 ++++++++++++++++++- 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/tools/deployment-cli-tools/ch_cli_tools/configuration/preprocessing.py b/tools/deployment-cli-tools/ch_cli_tools/configuration/preprocessing.py index 15a086844..d4134a714 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/configuration/preprocessing.py +++ b/tools/deployment-cli-tools/ch_cli_tools/configuration/preprocessing.py @@ -257,9 +257,11 @@ def generate_hash_based_image_tags(root_paths, helm_values, merge_build_path=DEF calculated_tags[image_key] = sha1(content_hash.encode('utf-8')).hexdigest() pending.remove(image_key) + retagged_images = {} for image_key, image_tag in calculated_tags.items(): image_spec = image_specs[image_key] retagged = _set_image_tag(image_spec['image'], image_tag) + retagged_images[image_spec['image']] = retagged if image_spec['kind'] == KEY_APPS and image_key in helm_values[KEY_APPS]: app_values = helm_values[KEY_APPS][image_key] @@ -269,4 +271,25 @@ def generate_hash_based_image_tags(root_paths, helm_values, merge_build_path=DEF elif image_spec['kind'] == KEY_TASK_IMAGES and image_key in helm_values[KEY_TASK_IMAGES]: helm_values[KEY_TASK_IMAGES][image_key] = retagged + apply_retagged_images(helm_values, retagged_images) + return helm_values + + +def apply_retagged_images(helm_values, retagged_images): + """Carry the recomputed tags over to the applications running an image they do not build. + + An instance declares no build of its own, so no tag is computed for it: it is left holding the + untagged image name the chart was generated with, and would be deployed without a tag. An + application still referencing the bare name of an image that was just tagged runs that very + image, so it takes its tag. One pinning a tag of its own does not match, and keeps it. + """ + if not retagged_images: + return + + for app_values in helm_values.get(KEY_APPS, {}).values(): + holders = [app_values, (app_values.get(KEY_HARNESS) or {}).get(KEY_DEPLOYMENT) or {}] + for holder in holders: + retagged = retagged_images.get(holder.get('image')) + if retagged: + holder['image'] = retagged diff --git a/tools/deployment-cli-tools/tests/test_helm.py b/tools/deployment-cli-tools/tests/test_helm.py index 4402084ec..2dd1c0c98 100644 --- a/tools/deployment-cli-tools/tests/test_helm.py +++ b/tools/deployment-cli-tools/tests/test_helm.py @@ -1,7 +1,8 @@ from ch_cli_tools.helm import * from ch_cli_tools.configuration.configurationgenerator import * from ch_cli_tools.configuration import configurationgenerator -from ch_cli_tools.configuration.preprocessing import preprocess_build_overrides, generate_hash_based_image_tags +from ch_cli_tools.configuration.preprocessing import preprocess_build_overrides, generate_hash_based_image_tags, \ + apply_retagged_images import logging import pytest import shutil @@ -1897,3 +1898,54 @@ def test_instance_inherits_the_application_merged_across_roots(tmp_path): assert instance['subdomain'] == 'myinstance' assert instance[KEY_DEPLOYMENT]['replicas'] == 4 assert values[KEY_APPS]['myapp'][KEY_HARNESS][KEY_DEPLOYMENT]['replicas'] == 1 + + +def test_auto_tag_reaches_instance_images(tmp_path): + """An instance runs the image built for its application, so it carries that image's tag. + + Instances declare no build, so no hash tag is computed for them: without carrying the + application's over they would be deployed with a bare, untagged image name. + """ + out_folder = str(tmp_path / 'test_auto_tag_reaches_instance_images') + merge_build_path = str(tmp_path / '.overrides') + + values = create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, + include=['samples', 'myapp'], exclude=['events'], domain="my.local", + namespace='test', env='test', local=False, tag=None, registry='reg') + # tag omitted: the chart is generated with bare image names, tags come from the content hash + assert values[KEY_APPS]['samples-instance1'][KEY_HARNESS][KEY_DEPLOYMENT]['image'] == \ + values[KEY_APPS]['samples'][KEY_HARNESS][KEY_DEPLOYMENT]['image'] + + preprocess_build_overrides([CLOUDHARNESS_ROOT, RESOURCES], values, merge_build_path=merge_build_path) + generate_hash_based_image_tags([CLOUDHARNESS_ROOT, RESOURCES], values, merge_build_path=merge_build_path) + + for app_name, instance_key in (('samples', 'samples-instance1'), ('myapp', 'myapp-inst1')): + parent_image = values[KEY_APPS][app_name][KEY_HARNESS][KEY_DEPLOYMENT]['image'] + instance = values[KEY_APPS][instance_key] + assert ':' in parent_image, f'{app_name} should be tagged with its content hash' + assert instance['image'] == parent_image + assert instance[KEY_HARNESS][KEY_DEPLOYMENT]['image'] == parent_image + + +def test_auto_tag_leaves_a_pinned_image_alone(tmp_path): + """An application running a prebuilt image keeps it: only bare names of images that were + just tagged are carried over.""" + values = { + KEY_APPS: { + 'builder': {'image': 'reg/app:abc123', + KEY_HARNESS: {KEY_DEPLOYMENT: {'image': 'reg/app:abc123'}}}, + 'inherits': {'image': 'reg/app', + KEY_HARNESS: {KEY_DEPLOYMENT: {'image': 'reg/app'}}}, + 'pinned': {'image': 'reg/app:v1.0', + KEY_HARNESS: {KEY_DEPLOYMENT: {'image': 'reg/app:v1.0'}}}, + 'unrelated': {'image': 'nginx:1.0', + KEY_HARNESS: {KEY_DEPLOYMENT: {'image': 'nginx:1.0'}}}, + } + } + apply_retagged_images(values, {'reg/app': 'reg/app:abc123'}) + + assert values[KEY_APPS]['inherits']['image'] == 'reg/app:abc123' + assert values[KEY_APPS]['inherits'][KEY_HARNESS][KEY_DEPLOYMENT]['image'] == 'reg/app:abc123' + assert values[KEY_APPS]['pinned']['image'] == 'reg/app:v1.0' + assert values[KEY_APPS]['unrelated']['image'] == 'nginx:1.0' + assert values[KEY_APPS]['builder']['image'] == 'reg/app:abc123' From dc27b85086c02f535d496ec819b659abae1334f5 Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Thu, 17 Sep 2026 11:07:40 +0200 Subject: [PATCH 7/8] CH-293 fix missed instances within dependencies --- .../configuration/configurationgenerator.py | 27 +++++++++ .../ch_cli_tools/dockercompose.py | 1 + .../deployment-cli-tools/ch_cli_tools/helm.py | 3 + tools/deployment-cli-tools/tests/test_helm.py | 57 +++++++++++++++++++ 4 files changed, 88 insertions(+) diff --git a/tools/deployment-cli-tools/ch_cli_tools/configuration/configurationgenerator.py b/tools/deployment-cli-tools/ch_cli_tools/configuration/configurationgenerator.py index a75f909c8..7589d4e45 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/configuration/configurationgenerator.py +++ b/tools/deployment-cli-tools/ch_cli_tools/configuration/configurationgenerator.py @@ -188,6 +188,33 @@ def collect_instance_values(self, app_name, app_values): self.include = resolve_instance_includes(self.include, app_name, instances) return instances + def _include_application_instances(self, helm_values): + """Include the instances of every included application. + + Resolved once `--include` has been expanded over dependencies: an application is more + often pulled in as another's dependency than named on the command line, and its instances + are deployed with it either way. Instances left out with `--exclude` were never derived, + so they cannot come back here. + """ + apps = helm_values[KEY_APPS] + included = set(self.include) + for app_name in self.include: + for instance_name in instance_names(app_name, self.root_paths, self.env): + app_key = instance_app_key(app_name, instance_name) + if app_key in apps: + included.add(app_key) + return included + + def _keep_included_instances(self, apps, included_apps): + """Keep the instances of the included applications in the deployment. + + Applications are selected by walking the build closure, which instances are never part of: + they build nothing. They are deployed with the application they belong to all the same. + """ + for app_key in self.include: + if app_key in apps and app_key not in included_apps: + included_apps[app_key] = apps[app_key] + def _inherit_instance_images(self, helm_values): """Give every instance the image of its parent application, once images are known. diff --git a/tools/deployment-cli-tools/ch_cli_tools/dockercompose.py b/tools/deployment-cli-tools/ch_cli_tools/dockercompose.py index 432222f6e..284657442 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/dockercompose.py +++ b/tools/deployment-cli-tools/ch_cli_tools/dockercompose.py @@ -235,6 +235,7 @@ def __finish_helm_values(self, values, defer_task_images=False): included_builds = get_included_builds(values, set(self.include)) self.include = get_included_applications( values, set(self.include)) + self.include = self._include_application_instances(values) logging.info('Selecting included applications') keep = set(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 4b4f3254e..b32ac4268 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/helm.py +++ b/tools/deployment-cli-tools/ch_cli_tools/helm.py @@ -279,6 +279,7 @@ def __finish_helm_values(self, values, defer_task_images=False): # Only include applications that are specified in the include list and their dependencies self.include = get_included_applications( values, set(self.include)) + self.include = self._include_application_instances(values) self.include -= set(self.exclude) @@ -302,6 +303,7 @@ def __finish_helm_values(self, values, defer_task_images=False): owner = resolve_task_image_owner(dep_name, set(apps)) if owner and owner in apps: included_apps[owner] = apps[owner] + self._keep_included_instances(apps, included_apps) values[KEY_APPS] = included_apps else: # Original single-pass mode: filter apps and aggregate task images @@ -328,6 +330,7 @@ def __finish_helm_values(self, values, defer_task_images=False): if key in included_builds or app_name in self.include: values[KEY_TASK_IMAGES][key] = apps[app_name][KEY_TASK_IMAGES][key] + self._keep_included_instances(apps, included_apps) values[KEY_APPS] = included_apps elif not defer_task_images: for v in apps: diff --git a/tools/deployment-cli-tools/tests/test_helm.py b/tools/deployment-cli-tools/tests/test_helm.py index 2dd1c0c98..6d9024c96 100644 --- a/tools/deployment-cli-tools/tests/test_helm.py +++ b/tools/deployment-cli-tools/tests/test_helm.py @@ -1949,3 +1949,60 @@ def test_auto_tag_leaves_a_pinned_image_alone(tmp_path): assert values[KEY_APPS]['pinned']['image'] == 'reg/app:v1.0' assert values[KEY_APPS]['unrelated']['image'] == 'nginx:1.0' assert values[KEY_APPS]['builder']['image'] == 'reg/app:abc123' + + +def test_instances_of_a_dependency_are_included(tmp_path): + """An application is more often pulled in as another's dependency than named on the command + line, and its instances are deployed with it either way. + + Instances build nothing, so they are absent from the build closure the applications are + selected from: resolved too early, or filtered by that closure alone, they silently vanish + while their templates and resources are still collected into the chart. + """ + dependent_root = tmp_path / 'dependent_root' + app_deploy = dependent_root / APPS_PATH / 'needsmyapp' / 'deploy' + app_deploy.mkdir(parents=True) + (app_deploy / 'values.yaml').write_text( + 'harness:\n' + ' subdomain: needsmyapp\n' + ' dependencies:\n' + ' soft: [myapp]\n' + ' deployment:\n' + ' auto: true\n' + ' image: reg/needsmyapp:1\n' + ) + + out_folder = tmp_path / 'test_instances_of_a_dependency' + values = create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES, dependent_root], output_path=out_folder, + include=['needsmyapp'], domain="my.local", namespace='test', env='dev', + local=False, tag=1, registry='reg') + + apps = values[KEY_APPS] + assert 'myapp' in apps, 'the dependency itself is included' + assert 'myapp-inst1' in apps, 'and so are its instances' + assert apps['myapp-inst1'][KEY_HARNESS]['subdomain'] == 'myinstance' + assert apps['myapp-inst1']['image'] == apps['myapp']['image'] + + +def test_instance_of_a_dependency_can_still_be_excluded(tmp_path): + dependent_root = tmp_path / 'dependent_root' + app_deploy = dependent_root / APPS_PATH / 'needsmyapp' / 'deploy' + app_deploy.mkdir(parents=True) + (app_deploy / 'values.yaml').write_text( + 'harness:\n' + ' subdomain: needsmyapp\n' + ' dependencies:\n' + ' soft: [myapp]\n' + ' deployment:\n' + ' auto: true\n' + ' image: reg/needsmyapp:1\n' + ) + + out_folder = tmp_path / 'test_instance_of_a_dependency_excluded' + values = create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES, dependent_root], output_path=out_folder, + include=['needsmyapp'], exclude=['myapp-inst1'], domain="my.local", + namespace='test', env='dev', local=False, tag=1, registry='reg') + + assert 'myapp' in values[KEY_APPS] + assert 'myapp-inst1' not in values[KEY_APPS] + assert not (out_folder / HELM_CHART_PATH / 'resources' / 'myapp-inst1').exists() From 0f93a068032b6415672a0f93f2dcbbdac1e101e8 Mon Sep 17 00:00:00 2001 From: Filippo Ledda Date: Thu, 17 Sep 2026 11:30:45 +0200 Subject: [PATCH 8/8] CH-293 fix test deployment stability --- applications/samples/Dockerfile | 2 +- .../samples/deploy/instances/instance1/values-test.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/applications/samples/Dockerfile b/applications/samples/Dockerfile index a0b9f8c86..8e16f2415 100644 --- a/applications/samples/Dockerfile +++ b/applications/samples/Dockerfile @@ -20,7 +20,7 @@ RUN yarn build FROM $CLOUDHARNESS_FLASK ENV MODULE_NAME=samples -ENV WORKERS=2 +ENV WORKERS=1 ENV PORT=8080 COPY backend/requirements.txt /usr/src/app/ diff --git a/applications/samples/deploy/instances/instance1/values-test.yaml b/applications/samples/deploy/instances/instance1/values-test.yaml index 58ff90182..ef38abdcc 100644 --- a/applications/samples/deploy/instances/instance1/values-test.yaml +++ b/applications/samples/deploy/instances/instance1/values-test.yaml @@ -5,7 +5,7 @@ harness: statefulset: false resources: limits: - memory: "160Mi" + memory: "300Mi" requests: cpu: "1m" memory: "16Mi"