From 55aaa2b2776dcd83d11d08b335a66e280856adf4 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Wed, 22 Nov 2023 19:32:47 -0800 Subject: [PATCH] Skip unnecessary kustomize cfg step when scanning the cluster (#399) Issue #390 --- flux_local/git_repo.py | 139 ++++++++----------------- tests/__snapshots__/test_git_repo.ambr | 28 ----- tests/test_git_repo.py | 12 +-- 3 files changed, 44 insertions(+), 135 deletions(-) diff --git a/flux_local/git_repo.py b/flux_local/git_repo.py index 26c870a5..d7660c4d 100644 --- a/flux_local/git_repo.py +++ b/flux_local/git_repo.py @@ -305,37 +305,31 @@ class ResourceSelector: """ClusterPolicy objects to return.""" -async def get_fluxtomizations( +async def grep_fluxtomizations( root: Path, relative_path: Path, - build: bool, - sources: list[Source], ) -> list[Kustomization]: - """Find all flux Kustomizations in the specified path. - - This may be called repeatedly with different paths to repeatedly collect - Kustomizations from the repo. Assumes that any flux Kustomization - for a GitRepository is pointed at this cluster, following normal conventions. - """ - cmd: kustomize.Kustomize - if build: - cmd = ( - kustomize.build(root / relative_path) - .grep(f"kind={CLUSTER_KUSTOMIZE_KIND}") + """Find all flux Kustomizations in the specified path.""" + try: + docs = ( + await kustomize.grep(f"kind={CLUSTER_KUSTOMIZE_KIND}", root / relative_path) .grep(GREP_SOURCE_REF_KIND) + .objects() ) - else: - cmd = kustomize.grep( - f"kind={CLUSTER_KUSTOMIZE_KIND}", root / relative_path - ).grep(GREP_SOURCE_REF_KIND) - docs = await cmd.objects() - results = [] - for doc in filter(FLUXTOMIZE_DOMAIN_FILTER, docs): - ks = Kustomization.parse_doc(doc) - if not is_allowed_source(ks, sources): - continue - results.append(ks) - return results + except FluxException as err: + raise FluxException( + f"Error building Fluxtomization in '{root}' " + f"path '{relative_path}': {err} - {ERROR_DETAIL_BAD_PATH}" + ) + kustomizations = [ + Kustomization.parse_doc(doc) for doc in filter(FLUXTOMIZE_DOMAIN_FILTER, docs) + ] + unique = {ks.namespaced_name for ks in kustomizations} + if len(unique) != len(kustomizations): + raise FluxException( + "Detected multiple Fluxtomizations with the same name indicating a multi-cluster setup. Please run with a more strict path" + ) + return kustomizations def is_allowed_source(doc: Kustomization, sources: list[Source]) -> bool: @@ -350,18 +344,16 @@ def is_allowed_source(doc: Kustomization, sources: list[Source]) -> bool: return False -def adjust_ks_path( - doc: Kustomization, default_path: Path, sources: list[Source] -) -> Path | None: +def adjust_ks_path(doc: Kustomization, selector: PathSelector) -> Path | None: """Make adjustments to the Kustomizations path.""" # Source path is relative to the search path. Update to have the # full prefix relative to the root. if not doc.path: - _LOGGER.debug("Assigning implicit path %s", default_path) - return default_path + _LOGGER.debug("Assigning implicit path %s", selector.relative_path) + return selector.relative_path if doc.source_kind == OCI_REPO_KIND: - for source in sources: + for source in selector.sources or []: if source.name == doc.source_name: _LOGGER.debug( "Updated Source for OCIRepository %s: %s", doc.name, doc.path @@ -379,9 +371,7 @@ def adjust_ks_path( return Path(doc.path) -async def kustomization_traversal( - root_path_selector: PathSelector, path_selector: PathSelector, build: bool -) -> list[Kustomization]: +async def kustomization_traversal(selector: PathSelector) -> list[Kustomization]: """Search for kustomizations in the specified path.""" kustomizations: list[Kustomization] = [] @@ -389,24 +379,15 @@ async def kustomization_traversal( visited_ks: set[str] = set() path_queue: queue.Queue[Path] = queue.Queue() - path_queue.put(path_selector.relative_path) + path_queue.put(selector.relative_path) while not path_queue.empty(): path = path_queue.get() - _LOGGER.debug("Visiting path (%s) %s", root_path_selector.path, path) + _LOGGER.debug("Visiting path (%s) %s", selector.path, path) with trace_context(f"Traversing Kustomization '{str(path)}'"): - try: - docs = await get_fluxtomizations( - root_path_selector.root, - path, - build=build, - sources=path_selector.sources or [], - ) - except FluxException as err: - detail = ERROR_DETAIL_BAD_KS if visited_paths else ERROR_DETAIL_BAD_PATH - raise FluxException( - f"Error building Fluxtomization in '{root_path_selector.root}' " - f"path '{path}': {err} - {detail}" - ) + docs = await grep_fluxtomizations(selector.root, path) + docs = [ + doc for doc in docs if is_allowed_source(doc, selector.sources or []) + ] visited_paths |= set({path}) @@ -424,11 +405,7 @@ async def kustomization_traversal( doc.source_kind, doc.source_name, ) - if not ( - doc_path := adjust_ks_path( - doc, root_path_selector.relative_path, path_selector.sources or [] - ) - ): + if not (doc_path := adjust_ks_path(doc, selector)): continue doc.path = str(doc_path) if doc_path not in visited_paths: @@ -450,46 +427,6 @@ def node_name(ks: Kustomization) -> str: return f"{ks.namespaced_name} @ {ks.id_name}" -async def get_clusters( - path_selector: PathSelector, - cluster_selector: MetadataSelector, - kustomization_selector: MetadataSelector, -) -> list[Cluster]: - """Load Cluster objects from the specified path.""" - try: - roots = await get_fluxtomizations( - path_selector.root, - path_selector.relative_path, - build=False, - sources=path_selector.sources or [], - ) - except FluxException as err: - raise FluxException( - f"Error building Fluxtomization in '{path_selector.root}' path " - f"'{path_selector.relative_path}': {err}" - f"Try specifying another path within the git repo?" - ) - _LOGGER.debug("roots=%s", roots) - 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" - ) - 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) - ], - ) - ] - - async def build_kustomization( kustomization: Kustomization, cluster_path: Path, @@ -593,9 +530,15 @@ async def build_manifest( return Manifest(clusters=[]) with trace_context(f"Traversing Cluster '{str(selector.path.path)}'"): - clusters = await get_clusters( - selector.path, selector.cluster, selector.kustomization - ) + results = await kustomization_traversal(selector.path) + clusters = [ + Cluster( + path=str(selector.path.relative_path), + kustomizations=[ + ks for ks in results if selector.kustomization.predicate(ks) + ], + ) + ] async def update_kustomization(cluster: Cluster) -> None: build_tasks = [] diff --git a/tests/__snapshots__/test_git_repo.ambr b/tests/__snapshots__/test_git_repo.ambr index 691cc88e..26760d9b 100644 --- a/tests/__snapshots__/test_git_repo.ambr +++ b/tests/__snapshots__/test_git_repo.ambr @@ -602,10 +602,6 @@ "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", ]), }), - 'cmds': list([ - '(tests/testdata/cluster2 (abs)) kustomize cfg grep kind=Kustomization .', - "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", - ]), }), }) # --- @@ -644,10 +640,6 @@ "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", ]), }), - 'cmds': list([ - '(tests/testdata/cluster3 (abs)) kustomize cfg grep kind=Kustomization .', - "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", - ]), }), }) # --- @@ -702,10 +694,6 @@ "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", ]), }), - 'cmds': list([ - '(tests/testdata/cluster4 (abs)) kustomize cfg grep kind=Kustomization .', - "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", - ]), }), }) # --- @@ -732,10 +720,6 @@ "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", ]), }), - 'cmds': list([ - '(tests/testdata/cluster5 (abs)) kustomize cfg grep kind=Kustomization .', - "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", - ]), }), }) # --- @@ -776,10 +760,6 @@ "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", ]), }), - 'cmds': list([ - '(tests/testdata/cluster6 (abs)) kustomize cfg grep kind=Kustomization .', - "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", - ]), }), }) # --- @@ -834,10 +814,6 @@ "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", ]), }), - 'cmds': list([ - '(tests/testdata/cluster7 (abs)) kustomize cfg grep kind=Kustomization .', - "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", - ]), }), }) # --- @@ -903,10 +879,6 @@ "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", ]), }), - 'cmds': list([ - '(tests/testdata/cluster (abs)) kustomize cfg grep kind=Kustomization .', - "kustomize cfg grep 'spec.sourceRef.kind=GitRepository|OCIRepository'", - ]), }), }) # --- diff --git a/tests/test_git_repo.py b/tests/test_git_repo.py index 92faa42b..1935429c 100644 --- a/tests/test_git_repo.py +++ b/tests/test_git_repo.py @@ -200,21 +200,15 @@ async def test_kustomization_traversal(path: str) -> None: ] paths = [] - async def fetch( - root: Path, p: Path, build: bool, sources: list[Source] - ) -> list[Kustomization]: + async def grep(root: Path, p: Path) -> list[Kustomization]: nonlocal paths, results paths.append((str(root), str(p))) return results.pop(0) with patch("flux_local.git_repo.PathSelector.root", Path("/home/example")), patch( - "flux_local.git_repo.get_fluxtomizations", fetch + "flux_local.git_repo.grep_fluxtomizations", grep ): - kustomizations = await kustomization_traversal( - root_path_selector=PathSelector(path=Path(path)), - path_selector=PathSelector(path=Path(path)), - build=True, - ) + kustomizations = await kustomization_traversal(PathSelector(path=Path(path))) assert len(kustomizations) == 5 assert paths == [ ("/home/example", "kubernetes/flux"),