From f1e04c60edb4b138f31c149b45cbf4a8a6412ea9 Mon Sep 17 00:00:00 2001 From: Antonio Alonso Dominguez Date: Thu, 17 Mar 2022 10:19:21 +0100 Subject: [PATCH] Simplify Helm package build and remove special case for `value_or_default` in `output_path` --- .../pants/backend/helm/goals/package.py | 45 ++++++++-------- .../pants/backend/helm/goals/package_test.py | 52 +++++++------------ src/python/pants/backend/helm/target_types.py | 27 +++++----- .../pants/backend/helm/util_rules/chart.py | 4 -- 4 files changed, 58 insertions(+), 70 deletions(-) diff --git a/src/python/pants/backend/helm/goals/package.py b/src/python/pants/backend/helm/goals/package.py index ae3261d6d66..67f61f30ebc 100644 --- a/src/python/pants/backend/helm/goals/package.py +++ b/src/python/pants/backend/helm/goals/package.py @@ -11,32 +11,25 @@ from pants.backend.helm.util_rules.chart import HelmChart, HelmChartMetadata, HelmChartRequest from pants.backend.helm.util_rules.tool import HelmProcess from pants.core.goals.package import BuiltPackage, BuiltPackageArtifact, PackageFieldSet -from pants.engine.fs import AddPrefix, CreateDigest, Digest, Directory, MergeDigests, RemovePrefix +from pants.engine.fs import ( + AddPrefix, + CreateDigest, + Digest, + Directory, + MergeDigests, + RemovePrefix, + Snapshot, +) from pants.engine.process import ProcessResult from pants.engine.rules import Get, MultiGet, collect_rules, rule +from pants.engine.unions import UnionRule from pants.util.logging import LogLevel logger = logging.getLogger(__name__) -@dataclass(frozen=True) -class BuiltHelmArtifact(BuiltPackageArtifact): - name: str | None = None - metadata: HelmChartMetadata | None = None - - @classmethod - def create(cls, path_to_dir: str, chart_metadata: HelmChartMetadata) -> BuiltHelmArtifact: - path = os.path.join(path_to_dir, _helm_artifact_filename(chart_metadata)) - return cls( - name=chart_metadata.artifact_name, - metadata=chart_metadata, - relpath=path, - extra_log_lines=(f"Built Helm chart artifact: {path}",), - ) - - def _helm_artifact_filename(chart_metadata: HelmChartMetadata) -> str: - return f"{chart_metadata.artifact_name}.tgz" + return f"{chart_metadata.name}-{chart_metadata.version}.tgz" @dataclass(frozen=True) @@ -70,10 +63,18 @@ async def run_helm_package(field_set: HelmPackageFieldSet) -> BuiltPackage: Digest, RemovePrefix(process_result.output_digest, result_dir) ) - artifact_path = field_set.output_path.value_or_default(file_ending=None) - dest_digest = await Get(Digest, AddPrefix(stripped_output_digest, artifact_path)) - return BuiltPackage(dest_digest, (BuiltHelmArtifact.create(artifact_path, chart.metadata),)) + final_snapshot = await Get( + Snapshot, + AddPrefix(stripped_output_digest, field_set.output_path.value_or_default(file_ending=None)), + ) + return BuiltPackage( + final_snapshot.digest, + artifacts=tuple( + BuiltPackageArtifact(file, extra_log_lines=(f"Built Helm chart artifact: {file}",)) + for file in final_snapshot.files + ), + ) def rules(): - return collect_rules() + return [*collect_rules(), UnionRule(PackageFieldSet, HelmPackageFieldSet)] diff --git a/src/python/pants/backend/helm/goals/package_test.py b/src/python/pants/backend/helm/goals/package_test.py index 72f7e303afa..867c8ecc735 100644 --- a/src/python/pants/backend/helm/goals/package_test.py +++ b/src/python/pants/backend/helm/goals/package_test.py @@ -8,7 +8,7 @@ import pytest from pants.backend.helm.goals import package -from pants.backend.helm.goals.package import BuiltHelmArtifact, HelmPackageFieldSet +from pants.backend.helm.goals.package import HelmPackageFieldSet from pants.backend.helm.subsystems.helm import HelmSubsystem from pants.backend.helm.target_types import HelmChartTarget from pants.backend.helm.testutil import ( @@ -18,11 +18,9 @@ gen_chart_file, ) from pants.backend.helm.util_rules import chart, sources, tool -from pants.backend.helm.util_rules.chart import HelmChartMetadata from pants.build_graph.address import Address from pants.core.goals.package import BuiltPackage from pants.core.util_rules import config_files, external_tool, stripped_source_files -from pants.engine.fs import Snapshot from pants.engine.rules import QueryRule, SubsystemRule from pants.source.source_root import rules as source_root_rules from pants.testutil.rule_runner import RuleRunner @@ -47,27 +45,19 @@ def rule_runner() -> RuleRunner: ) -def _assert_build_package( - rule_runner: RuleRunner, *, chart_name: str, chart_version: str, output_path: str = "" -) -> None: - target = rule_runner.get_target(Address("", target_name=chart_name)) - field_set = HelmPackageFieldSet.create(target) +def _assert_build_package(rule_runner: RuleRunner, *, chart_name: str, chart_version: str) -> None: + rule_runner.set_options(["--source-root-patterns=['src/*']"]) - expected_metadata = HelmChartMetadata( - name=chart_name, - version=chart_version, - ) - expected_built_package = BuiltHelmArtifact.create(output_path, expected_metadata) + target = rule_runner.get_target(Address(f"src/{chart_name}", target_name=chart_name)) + field_set = HelmPackageFieldSet.create(target) + dest_dir = field_set.output_path.value_or_default(file_ending=None) result = rule_runner.request(BuiltPackage, [field_set]) - chart_snapshot = rule_runner.request(Snapshot, [result.digest]) assert len(result.artifacts) == 1 - assert result.artifacts[0] == expected_built_package - - if output_path != "": - assert chart_snapshot.dirs == (output_path,) - assert chart_snapshot.files == (os.path.join(output_path, f"{chart_name}-{chart_version}.tgz"),) + assert result.artifacts[0].relpath == os.path.join( + dest_dir, f"{chart_name}-{chart_version}.tgz" + ) def test_helm_package(rule_runner: RuleRunner) -> None: @@ -76,11 +66,11 @@ def test_helm_package(rule_runner: RuleRunner) -> None: rule_runner.write_files( { - "BUILD": f"helm_chart(name='{chart_name}')", - "Chart.yaml": gen_chart_file(chart_name, version=chart_version), - "values.yaml": HELM_VALUES_FILE, - "templates/_helpers.tpl": HELM_TEMPLATE_HELPERS_FILE, - "templates/service.yaml": K8S_SERVICE_FILE, + f"src/{chart_name}/BUILD": f"helm_chart(name='{chart_name}')", + f"src/{chart_name}/Chart.yaml": gen_chart_file(chart_name, version=chart_version), + f"src/{chart_name}/values.yaml": HELM_VALUES_FILE, + f"src/{chart_name}/templates/_helpers.tpl": HELM_TEMPLATE_HELPERS_FILE, + f"src/{chart_name}/templates/service.yaml": K8S_SERVICE_FILE, } ) @@ -95,14 +85,12 @@ def test_helm_package_with_custom_output_path(rule_runner: RuleRunner) -> None: rule_runner.write_files( { - "BUILD": f"""helm_chart(name="{chart_name}", output_path="{output_path}")""", - "Chart.yaml": gen_chart_file(chart_name, version=chart_version), - "values.yaml": HELM_VALUES_FILE, - "templates/_helpers.tpl": HELM_TEMPLATE_HELPERS_FILE, - "templates/service.yaml": K8S_SERVICE_FILE, + f"src/{chart_name}/BUILD": f"""helm_chart(name="{chart_name}", output_path="{output_path}")""", + f"src/{chart_name}/Chart.yaml": gen_chart_file(chart_name, version=chart_version), + f"src/{chart_name}/values.yaml": HELM_VALUES_FILE, + f"src/{chart_name}/templates/_helpers.tpl": HELM_TEMPLATE_HELPERS_FILE, + f"src/{chart_name}/templates/service.yaml": K8S_SERVICE_FILE, } ) - _assert_build_package( - rule_runner, chart_name=chart_name, chart_version=chart_version, output_path=output_path - ) + _assert_build_package(rule_runner, chart_name=chart_name, chart_version=chart_version) diff --git a/src/python/pants/backend/helm/target_types.py b/src/python/pants/backend/helm/target_types.py index 0d6cd34db93..e99294190fa 100644 --- a/src/python/pants/backend/helm/target_types.py +++ b/src/python/pants/backend/helm/target_types.py @@ -3,9 +3,7 @@ from __future__ import annotations -import os from dataclasses import dataclass -from textwrap import dedent from pants.core.goals.package import OutputPathField from pants.engine.target import ( @@ -60,18 +58,23 @@ class HelmChartDependenciesField(Dependencies): class HelmChartOutputPathField(OutputPathField): - help = dedent( - """\ - The destination folder where the final packaged chart will be located.\n - The final package name will still follow Helm convention, this output path will only affect the destination folder where can be found. - """ + help = ( + "Where the built directory tree should be located.\n\n" + "If undefined, this will use the path to the BUILD file, " + "For example, `src/charts/mychart:mychart` would be " + "`src.charts.mychart/mychart/`.\n\n" + "Regardless of whether you use the default or set this field, the path will end with " + "Helms's file format of `-.tgz`, where " + "`chart_name` and `chart_version` are the values extracted from the Chart.yaml file. " + "So, using the default for this field, the target " + "`src/charts/mychart` might have a final path like " + "`src.charts.mychart/mychart-0.1.0.tgz`.\n\n" + f"When running `{bin_name()} package`, this path will be prefixed by `--distdir` (e.g. " + "`dist/`).\n\n" + "Warning: setting this value risks naming collisions with other package targets you may " + "have." ) - def value_or_default(self, *, file_ending: str | None) -> str: - if self.value: - return self.value - return os.path.join(self.address.spec_path.replace(os.sep, ".")) - class HelmChartLintStrictField(TriBoolField): alias = "lint_strict" diff --git a/src/python/pants/backend/helm/util_rules/chart.py b/src/python/pants/backend/helm/util_rules/chart.py index 4f437ba6192..884e958f40b 100644 --- a/src/python/pants/backend/helm/util_rules/chart.py +++ b/src/python/pants/backend/helm/util_rules/chart.py @@ -191,10 +191,6 @@ def from_dict(cls, d: dict[str, Any]) -> HelmChartMetadata: def from_bytes(cls, content: bytes) -> HelmChartMetadata: return cls.from_dict(yaml.safe_load(content)) - @property - def artifact_name(self) -> str: - return f"{self.name}-{self.version}" - def to_dict(self) -> dict[str, Any]: d: dict[str, Any] = { "apiVersion": self.api_version,