From 3a1467cf766e99d18bb186ce0809173825e6b875 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Wed, 22 Nov 2023 17:36:40 -0800 Subject: [PATCH] Remove support for multiple clusters in a single path (#398) Remove support for multiple clusters in a single path This is a breaking change that removes the `name` and `namespace` fields from the `Cluster` object so they no longer act like fake kustomizations. Issue #390 --- flux_local/git_repo.py | 55 +++------ flux_local/manifest.py | 27 ----- flux_local/tool/get.py | 4 +- flux_local/tool/test.py | 2 +- tests/__snapshots__/test_git_repo.ambr | 106 ++++++++---------- tests/test_manifest.py | 4 - tests/tool/__snapshots__/test_diff_ks.ambr | 8 +- .../tool/__snapshots__/test_get_cluster.ambr | 40 ++++--- 8 files changed, 89 insertions(+), 157 deletions(-) diff --git a/flux_local/git_repo.py b/flux_local/git_repo.py index e10fbc87..26c870a5 100644 --- a/flux_local/git_repo.py +++ b/flux_local/git_repo.py @@ -243,13 +243,11 @@ class MetadataSelector: @property def predicate( self, - ) -> Callable[ - [Kustomization | HelmRelease | Cluster | HelmRepository | ClusterPolicy], bool - ]: + ) -> Callable[[Kustomization | HelmRelease | HelmRepository | ClusterPolicy], bool]: """A predicate that selects Kustomization objects.""" def predicate( - obj: Kustomization | HelmRelease | Cluster | HelmRepository | ClusterPolicy, + obj: Kustomization | HelmRelease | HelmRepository | ClusterPolicy, ) -> bool: if not self.enabled: return False @@ -472,41 +470,24 @@ async def get_clusters( f"Try specifying another path within the git repo?" ) _LOGGER.debug("roots=%s", roots) - clusters = [ - Cluster(name=ks.name, namespace=ks.namespace or "", path=ks.path) - for ks in roots - if cluster_selector.predicate(ks) - ] - build = True - if not clusters: - # There are no flux-system Kustomizations within this path. Fall back to - # assuming everything in the current directory is part of a cluster. - _LOGGER.debug( - "No clusters found; Processing as a Kustomization: %s", - path_selector.relative_path, + names = {ks.namespaced_name for ks in roots} + if len(names) != len(roots): + raise FluxException( + "Detected multiple Fluxtomizations with the same name indicating a multi-cluster setup. Please run with a more strict path" ) - clusters = [ - Cluster(name="cluster", namespace="", path=str(path_selector.relative_path)) - ] - build = False - - tasks = [] - for cluster in clusters: - _LOGGER.debug("Building cluster %s %s", cluster.name, cluster.path) - tasks.append( - kustomization_traversal( - path_selector, - PathSelector(path=Path(cluster.path), sources=path_selector.sources), - build=build, - ) + results = await kustomization_traversal( + path_selector, + PathSelector(path=path_selector.relative_path, sources=path_selector.sources), + build=False, + ) + return [ + Cluster( + path=str(path_selector.relative_path), + kustomizations=[ + ks for ks in results if kustomization_selector.predicate(ks) + ], ) - finished = await asyncio.gather(*tasks) - for cluster, results in zip(clusters, finished): - cluster.kustomizations = [ - ks for ks in results if kustomization_selector.predicate(ks) - ] - clusters.sort(key=lambda x: (x.path, x.namespace, x.name)) - return clusters + ] async def build_kustomization( diff --git a/flux_local/manifest.py b/flux_local/manifest.py index 9f0cf4c7..bb375b65 100644 --- a/flux_local/manifest.py +++ b/flux_local/manifest.py @@ -348,39 +348,12 @@ class Cluster(BaseManifest): a repo may also contain multiple (e.g. dev an prod). """ - name: str - """The name of the cluster.""" - - namespace: str - """The namespace of the cluster.""" - path: str """The local git repo path to the Kustomization objects for the cluster.""" kustomizations: list[Kustomization] = Field(default_factory=list) """A list of flux Kustomizations for the cluster.""" - @classmethod - def parse_doc(cls, doc: dict[str, Any]) -> "Cluster": - """Parse a partial Kustomization from a kubernetes resource.""" - _check_version(doc, FLUXTOMIZE_DOMAIN) - if not (metadata := doc.get("metadata")): - raise InputException(f"Invalid {cls} missing metadata: {doc}") - if not (name := metadata.get("name")): - raise InputException(f"Invalid {cls} missing metadata.name: {doc}") - if not (namespace := metadata.get("namespace")): - raise InputException(f"Invalid {cls} missing metadata.namespace: {doc}") - if not (spec := doc.get("spec")): - raise InputException(f"Invalid {cls} missing spec: {doc}") - if not (path := spec.get("path")): - raise InputException(f"Invalid {cls} missing spec.path: {doc}") - return Cluster(name=name, namespace=namespace, path=path) - - @property - def namespaced_name(self, sep: str = "/") -> str: - """Return the namespace and name concatenated as an id.""" - return f"{self.namespace}{sep}{self.name}" - @property def id_name(self) -> str: """Identifier for the Cluster in tests.""" diff --git a/flux_local/tool/get.py b/flux_local/tool/get.py index 85b94818..3ab8843c 100644 --- a/flux_local/tool/get.py +++ b/flux_local/tool/get.py @@ -174,9 +174,7 @@ async def run( # type: ignore[no-untyped-def] YamlFormatter().print([manifest.compact_dict()]) return - cols = ["name", "path", "kustomizations"] - if query.cluster.namespace is None: - cols.insert(0, "namespace") + cols = ["path", "kustomizations"] results: list[dict[str, Any]] = [] for cluster in manifest.clusters: value: dict[str, Any] = cluster.dict(include=set(cols)) diff --git a/flux_local/tool/test.py b/flux_local/tool/test.py index 920d1733..923905a3 100644 --- a/flux_local/tool/test.py +++ b/flux_local/tool/test.py @@ -205,7 +205,7 @@ def from_parent( # type: ignore[override] """The public constructor.""" item: ClusterCollector = super().from_parent( parent=parent, - name=cluster.name, + name=cluster.path, path=Path(cluster.path), nodeid=str(Path(cluster.path)), ) diff --git a/tests/__snapshots__/test_git_repo.ambr b/tests/__snapshots__/test_git_repo.ambr index 6ac5e1f1..691cc88e 100644 --- a/tests/__snapshots__/test_git_repo.ambr +++ b/tests/__snapshots__/test_git_repo.ambr @@ -98,9 +98,7 @@ 'path': 'tests/testdata/cluster/infrastructure/controllers', }), ]), - 'name': 'flux-system', - 'namespace': 'flux-system', - 'path': './tests/testdata/cluster/clusters/prod', + 'path': 'tests/testdata/cluster', }), ]), }) @@ -183,9 +181,7 @@ 'path': 'tests/testdata/cluster/infrastructure/controllers', }), ]), - 'name': 'flux-system', - 'namespace': 'flux-system', - 'path': './tests/testdata/cluster/clusters/prod', + 'path': 'tests/testdata/cluster', }), ]), }) @@ -289,9 +285,7 @@ 'path': 'tests/testdata/cluster/infrastructure/controllers', }), ]), - 'name': 'flux-system', - 'namespace': 'flux-system', - 'path': './tests/testdata/cluster/clusters/prod', + 'path': 'tests/testdata/cluster', }), ]), }) @@ -299,19 +293,19 @@ # name: test_helm_release_visitor.1 list([ tuple( - 'tests/testdata/cluster/clusters/prod', + 'tests/testdata/cluster', 'tests/testdata/cluster/apps/prod', 'podinfo', 'podinfo', ), tuple( - 'tests/testdata/cluster/clusters/prod', + 'tests/testdata/cluster', 'tests/testdata/cluster/infrastructure/controllers', 'flux-system', 'weave-gitops', ), tuple( - 'tests/testdata/cluster/clusters/prod', + 'tests/testdata/cluster', 'tests/testdata/cluster/infrastructure/controllers', 'metallb', 'metallb', @@ -399,9 +393,7 @@ 'path': 'tests/testdata/cluster/infrastructure/controllers', }), ]), - 'name': 'flux-system', - 'namespace': 'flux-system', - 'path': './tests/testdata/cluster/clusters/prod', + 'path': 'tests/testdata/cluster', }), ]), }) @@ -505,9 +497,7 @@ 'path': 'tests/testdata/cluster/infrastructure/controllers', }), ]), - 'name': 'flux-system', - 'namespace': 'flux-system', - 'path': './tests/testdata/cluster/clusters/prod', + 'path': 'tests/testdata/cluster', }), ]), }) @@ -515,19 +505,19 @@ # name: test_helm_repo_visitor.1 list([ tuple( - 'tests/testdata/cluster/clusters/prod', + 'tests/testdata/cluster', 'tests/testdata/cluster/infrastructure/configs', 'flux-system', 'bitnami', ), tuple( - 'tests/testdata/cluster/clusters/prod', + 'tests/testdata/cluster', 'tests/testdata/cluster/infrastructure/configs', 'flux-system', 'podinfo', ), tuple( - 'tests/testdata/cluster/clusters/prod', + 'tests/testdata/cluster', 'tests/testdata/cluster/infrastructure/configs', 'flux-system', 'weave-charts', @@ -732,17 +722,14 @@ }), "Traversing Kustomization 'tests/testdata/cluster5'": dict({ 'cmds': list([ - 'kustomize build tests/testdata/cluster5 (abs)', - 'kustomize cfg grep kind=Kustomization', + '(tests/testdata/cluster5 (abs)) kustomize cfg grep kind=Kustomization .', "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", ]), }), "Traversing Kustomization 'tests/testdata/cluster5/clusters/prod'": dict({ 'cmds': list([ - 'kustomize build tests/testdata/cluster5/clusters/prod (abs)', - 'kustomize cfg grep kind=Kustomization', + '(tests/testdata/cluster5/clusters/prod (abs)) kustomize cfg grep kind=Kustomization .', "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", - 'kustomize build tests/testdata/cluster5/clusters/prod/flux-system (abs)', ]), }), 'cmds': list([ @@ -771,20 +758,22 @@ "kustomize cfg grep 'kind=^(HelmRepository|HelmRelease|ClusterPolicy)$'", ]), }), + "Traversing Kustomization 'tests/testdata/cluster6'": dict({ + 'cmds': list([ + '(tests/testdata/cluster6 (abs)) kustomize cfg grep kind=Kustomization .', + "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", + ]), + }), "Traversing Kustomization 'tests/testdata/cluster6/apps'": dict({ 'cmds': list([ - 'kustomize build tests/testdata/cluster6/apps (abs)', - 'kustomize cfg grep kind=Kustomization', + '(tests/testdata/cluster6/apps (abs)) kustomize cfg grep kind=Kustomization .', "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", - 'kustomize build tests/testdata/cluster6/apps/renovate (abs)', ]), }), "Traversing Kustomization 'tests/testdata/cluster6/cluster'": dict({ 'cmds': list([ - 'kustomize build tests/testdata/cluster6/cluster (abs)', - 'kustomize cfg grep kind=Kustomization', + '(tests/testdata/cluster6/cluster (abs)) kustomize cfg grep kind=Kustomization .', "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", - 'kustomize build tests/testdata/cluster6/cluster/flux-system (abs)', ]), }), 'cmds': list([ @@ -821,27 +810,27 @@ "kustomize cfg grep 'kind=^(HelmRepository|HelmRelease|ClusterPolicy)$'", ]), }), + "Traversing Kustomization 'tests/testdata/cluster7'": dict({ + 'cmds': list([ + '(tests/testdata/cluster7 (abs)) kustomize cfg grep kind=Kustomization .', + "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", + ]), + }), "Traversing Kustomization 'tests/testdata/cluster7/clusters/home'": dict({ 'cmds': list([ - 'kustomize build tests/testdata/cluster7/clusters/home (abs)', - 'kustomize cfg grep kind=Kustomization', + '(tests/testdata/cluster7/clusters/home (abs)) kustomize cfg grep kind=Kustomization .', "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", - 'kustomize build tests/testdata/cluster7/clusters/home/flux-system (abs)', ]), }), "Traversing Kustomization 'tests/testdata/cluster7/flux/apps'": dict({ 'cmds': list([ - 'kustomize build tests/testdata/cluster7/flux/apps (abs)', - 'kustomize cfg grep kind=Kustomization', + '(tests/testdata/cluster7/flux/apps (abs)) kustomize cfg grep kind=Kustomization .', "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", - 'kustomize build tests/testdata/cluster7/flux/apps/database (abs)', - 'kustomize build tests/testdata/cluster7/flux/apps/database/postgresql (abs)', ]), }), "Traversing Kustomization 'tests/testdata/cluster7/flux/charts'": dict({ 'cmds': list([ - 'kustomize build tests/testdata/cluster7/flux/charts (abs)', - 'kustomize cfg grep kind=Kustomization', + '(tests/testdata/cluster7/flux/charts (abs)) kustomize cfg grep kind=Kustomization .', "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", ]), }), @@ -884,32 +873,33 @@ "kustomize cfg grep 'kind=^(HelmRepository|HelmRelease|ClusterPolicy)$'", ]), }), + "Traversing Kustomization 'tests/testdata/cluster'": dict({ + 'cmds': list([ + '(tests/testdata/cluster (abs)) kustomize cfg grep kind=Kustomization .', + "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", + ]), + }), "Traversing Kustomization 'tests/testdata/cluster/apps/prod'": dict({ 'cmds': list([ - 'kustomize build tests/testdata/cluster/apps/prod (abs)', - 'kustomize cfg grep kind=Kustomization', + '(tests/testdata/cluster/apps/prod (abs)) kustomize cfg grep kind=Kustomization .', "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", ]), }), "Traversing Kustomization 'tests/testdata/cluster/clusters/prod'": dict({ 'cmds': list([ - 'kustomize build tests/testdata/cluster/clusters/prod (abs)', - 'kustomize cfg grep kind=Kustomization', + '(tests/testdata/cluster/clusters/prod (abs)) kustomize cfg grep kind=Kustomization .', "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", - 'kustomize build tests/testdata/cluster/clusters/prod/flux-system (abs)', ]), }), "Traversing Kustomization 'tests/testdata/cluster/infrastructure/configs'": dict({ 'cmds': list([ - 'kustomize build tests/testdata/cluster/infrastructure/configs (abs)', - 'kustomize cfg grep kind=Kustomization', + '(tests/testdata/cluster/infrastructure/configs (abs)) kustomize cfg grep kind=Kustomization .', "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", ]), }), "Traversing Kustomization 'tests/testdata/cluster/infrastructure/controllers'": dict({ 'cmds': list([ - 'kustomize build tests/testdata/cluster/infrastructure/controllers (abs)', - 'kustomize cfg grep kind=Kustomization', + '(tests/testdata/cluster/infrastructure/controllers (abs)) kustomize cfg grep kind=Kustomization .', "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", ]), }), @@ -926,9 +916,7 @@ dict({ 'kustomizations': list([ ]), - 'name': 'flux-system', - 'namespace': 'flux-system', - 'path': './tests/testdata/cluster/clusters/prod', + 'path': 'tests/testdata/cluster', }), ]), }) @@ -1032,9 +1020,7 @@ 'path': 'tests/testdata/cluster/infrastructure/controllers', }), ]), - 'name': 'flux-system', - 'namespace': 'flux-system', - 'path': './tests/testdata/cluster/clusters/prod', + 'path': 'tests/testdata/cluster', }), ]), }) @@ -1042,25 +1028,25 @@ # name: test_kustomization_visitor.1 list([ tuple( - 'tests/testdata/cluster/clusters/prod', + 'tests/testdata/cluster', 'tests/testdata/cluster/apps/prod', 'flux-system', 'apps', ), tuple( - 'tests/testdata/cluster/clusters/prod', + 'tests/testdata/cluster', 'tests/testdata/cluster/clusters/prod', 'flux-system', 'flux-system', ), tuple( - 'tests/testdata/cluster/clusters/prod', + 'tests/testdata/cluster', 'tests/testdata/cluster/infrastructure/configs', 'flux-system', 'infra-configs', ), tuple( - 'tests/testdata/cluster/clusters/prod', + 'tests/testdata/cluster', 'tests/testdata/cluster/infrastructure/controllers', 'flux-system', 'infra-controllers', diff --git a/tests/test_manifest.py b/tests/test_manifest.py index d47fdcd3..289cc0cd 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -102,8 +102,6 @@ async def test_serializing_manifest(tmp_path: Path) -> None: manifest = Manifest( clusters=[ Cluster( - name="cluster", - namespace="flux-system", path="./example", kustomizations=[], ) @@ -114,8 +112,6 @@ async def test_serializing_manifest(tmp_path: Path) -> None: assert new_manifest.dict() == { "clusters": [ { - "name": "cluster", - "namespace": "flux-system", "path": "./example", "kustomizations": [], }, diff --git a/tests/tool/__snapshots__/test_diff_ks.ambr b/tests/tool/__snapshots__/test_diff_ks.ambr index 1e4f8510..05be2b83 100644 --- a/tests/tool/__snapshots__/test_diff_ks.ambr +++ b/tests/tool/__snapshots__/test_diff_ks.ambr @@ -8,13 +8,13 @@ # name: test_diff_ks[yaml-empty-sources] ''' --- - - cluster_path: tests/testdata/cluster/clusters/prod + - cluster_path: tests/testdata/cluster kustomization_path: tests/testdata/cluster/apps/prod kind: Kustomization namespace: flux-system name: apps diffs: - - cluster_path: tests/testdata/cluster/clusters/prod + - cluster_path: tests/testdata/cluster kustomization_path: tests/testdata/cluster/apps/prod kind: Namespace namespace: flux-system @@ -32,7 +32,7 @@ -metadata: - name: podinfo - - - cluster_path: tests/testdata/cluster/clusters/prod + - cluster_path: tests/testdata/cluster kustomization_path: tests/testdata/cluster/apps/prod kind: ConfigMap namespace: podinfo @@ -53,7 +53,7 @@ - name: podinfo-config - namespace: podinfo - - - cluster_path: tests/testdata/cluster/clusters/prod + - cluster_path: tests/testdata/cluster kustomization_path: tests/testdata/cluster/apps/prod kind: HelmRelease namespace: podinfo diff --git a/tests/tool/__snapshots__/test_get_cluster.ambr b/tests/tool/__snapshots__/test_get_cluster.ambr index bab16b6d..93932ac1 100644 --- a/tests/tool/__snapshots__/test_get_cluster.ambr +++ b/tests/tool/__snapshots__/test_get_cluster.ambr @@ -1,64 +1,64 @@ # serializer version: 1 # name: test_get_cluster[all-namespaces] ''' - NAMESPACE NAME PATH KUSTOMIZATIONS - flux-system flux-system ./tests/testdata/cluster/clusters/prod 4 + PATH KUSTOMIZATIONS + tests/testdata/cluster 4 ''' # --- # name: test_get_cluster[cluster2] ''' - NAME PATH KUSTOMIZATIONS - cluster tests/testdata/cluster2 5 + PATH KUSTOMIZATIONS + tests/testdata/cluster2 5 ''' # --- # name: test_get_cluster[cluster3-no-source] ''' - NAME PATH KUSTOMIZATIONS - flux-system ./tests/testdata/cluster3/clusters/cluster3 1 + PATH KUSTOMIZATIONS + tests/testdata/cluster3 1 ''' # --- # name: test_get_cluster[cluster3] ''' - NAME PATH KUSTOMIZATIONS - cluster tests/testdata/cluster3 2 + PATH KUSTOMIZATIONS + tests/testdata/cluster3 2 ''' # --- # name: test_get_cluster[cluster4] ''' - NAME PATH KUSTOMIZATIONS - cluster tests/testdata/cluster4 3 + PATH KUSTOMIZATIONS + tests/testdata/cluster4 3 ''' # --- # name: test_get_cluster[cluster5] ''' - NAME PATH KUSTOMIZATIONS - flux-system ./tests/testdata/cluster5/clusters/prod 1 + PATH KUSTOMIZATIONS + tests/testdata/cluster5 1 ''' # --- # name: test_get_cluster[cluster6] ''' - NAME PATH KUSTOMIZATIONS - flux-system ./tests/testdata/cluster6/cluster 2 + PATH KUSTOMIZATIONS + tests/testdata/cluster6 2 ''' # --- # name: test_get_cluster[cluster7] ''' - NAME PATH KUSTOMIZATIONS - flux-system ./tests/testdata/cluster7/clusters/home 3 + PATH KUSTOMIZATIONS + tests/testdata/cluster7 3 ''' # --- # name: test_get_cluster[cluster] ''' - NAME PATH KUSTOMIZATIONS - flux-system ./tests/testdata/cluster/clusters/prod 4 + PATH KUSTOMIZATIONS + tests/testdata/cluster 4 ''' # --- @@ -66,9 +66,7 @@ ''' --- clusters: - - name: flux-system - namespace: flux-system - path: ./tests/testdata/cluster/clusters/prod + - path: tests/testdata/cluster kustomizations: - name: apps namespace: flux-system