From 8704eb612e28bf8d140dce193960f11fc336f0cb Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Thu, 6 Jul 2023 09:55:22 -0700 Subject: [PATCH] Improve kustomization handling for more complex repo setups (#264) - Add an explicit simplification for flux-system as the root of the cluster - Don't add circular dependencies when doing automatic root identification - Fix kustomization namespace output - Address issues with not using relative paths --- flux_local/git_repo.py | 23 +++++++++++++++++++---- flux_local/tool/diff.py | 2 +- flux_local/tool/get.py | 2 ++ flux_local/tool/selector.py | 3 ++- flux_local/tool/visitor.py | 10 ++++++++++ tests/tool/testdata/get_cluster3.yaml | 4 +++- tests/tool/testdata/get_ks5_all.yaml | 10 ++++++++++ tests/tool/testdata/get_ks_path_ks.yaml | 4 ++-- 8 files changed, 49 insertions(+), 9 deletions(-) create mode 100644 tests/tool/testdata/get_ks5_all.yaml diff --git a/flux_local/git_repo.py b/flux_local/git_repo.py index 48afa8d3..4cf3de5c 100644 --- a/flux_local/git_repo.py +++ b/flux_local/git_repo.py @@ -76,6 +76,7 @@ GIT_REPO_KIND = "GitRepository" OCI_REPO_KIND = "OCIRepository" DEFAULT_NAMESPACE = "flux-system" +ROOT_KUSTOMIZATION_NAME = "flux-system" @dataclass @@ -381,7 +382,8 @@ async def kustomization_traversal(path_selector: PathSelector) -> list[Kustomiza # full prefix relative to the root. for kustomization in docs: if not kustomization.path: - kustomization.path = str(root / path) + _LOGGER.debug("Assigning implicit path %s", path_selector.relative_path) + kustomization.path = str(path_selector.relative_path) if not kustomization.source_path: continue kustomization.source_path = str( @@ -467,6 +469,9 @@ def make_clusters( ) graph.add_node(node_name(ks), ks=ks) + if ks.name == ROOT_KUSTOMIZATION_NAME and ks.namespace == DEFAULT_NAMESPACE: + # Do not attempt parent search below + continue # Find the parent Kustomization that produced this based on the # matching the kustomize source parent paths with a Kustomization @@ -491,7 +496,10 @@ def make_clusters( ks.namespaced_name, candidate.namespaced_name, ) - graph.add_edge(node_name(candidate), node_name(ks)) + if graph.has_edge(node_name(ks), node_name(candidate)): + _LOGGER.debug("Already has opposite edge; Skipping cycle") + else: + graph.add_edge(node_name(candidate), node_name(ks)) else: _LOGGER.debug( "Skipping candidate source %s", candidate.namespaced_name @@ -502,8 +510,11 @@ def make_clusters( # Clusters are subgraphs within the graph that are connected, with the root # node being the cluster itself. All children Kustomizations are flattended. _LOGGER.debug("Creating clusters based on connectivity") + for node, degree in graph.in_degree(): + _LOGGER.debug("Node: %s, degree: %s", node, degree) roots = [node for node, degree in graph.in_degree() if degree == 0] roots.sort() + clusters: list[Cluster] = [] _LOGGER.debug("roots=%s", roots) for root in roots: @@ -662,11 +673,15 @@ async def build_manifest( _LOGGER.debug("No clusters found; Processing as a Kustomization: %s", selector) # The argument path may be a Kustomization inside a cluster. Create a synthetic # cluster with any found Kustomizations - cluster = Cluster(name="cluster", namespace="", path=str(selector.path.path)) + cluster = Cluster( + name="cluster", namespace="", path=str(selector.path.relative_path) + ) objects = await get_kustomizations(selector.path.path) if objects: cluster.kustomizations = [ - Kustomization(name="kustomization", path=str(selector.path.path)) + Kustomization( + name="kustomization", path=str(selector.path.relative_path) + ) ] clusters.append(cluster) diff --git a/flux_local/tool/diff.py b/flux_local/tool/diff.py index 8c64aee7..fa508bc7 100644 --- a/flux_local/tool/diff.py +++ b/flux_local/tool/diff.py @@ -214,7 +214,7 @@ def create_diff_path( which is useful when run from CI. """ if path_orig := kwargs.get("path_orig"): - yield git_repo.PathSelector(path_orig) + yield git_repo.PathSelector(path_orig, sources=kwargs.get("sources")) return with git_repo.create_worktree(selector.repo) as worktree: diff --git a/flux_local/tool/get.py b/flux_local/tool/get.py index f5abcff1..85b94818 100644 --- a/flux_local/tool/get.py +++ b/flux_local/tool/get.py @@ -61,6 +61,8 @@ async def run( # type: ignore[no-untyped-def] cols = ["name", "path"] if output == "wide": cols.extend(["helmrepos", "releases"]) + if query.kustomization.namespace is None: + cols.insert(0, "namespace") if len(manifest.clusters) > 1: cols.insert(0, "cluster") for cluster in manifest.clusters: diff --git a/flux_local/tool/selector.py b/flux_local/tool/selector.py index 63fed00d..a77f74e9 100644 --- a/flux_local/tool/selector.py +++ b/flux_local/tool/selector.py @@ -208,9 +208,10 @@ def build_cluster_selector( # type: ignore[no-untyped-def] selector.path = git_repo.PathSelector( kwargs.get("path"), sources=kwargs.get("sources") ) - selector.cluster.namespace = kwargs.get("namespace") + selector.kustomization.namespace = kwargs.get("namespace") if kwargs.get("all_namespaces"): selector.cluster.namespace = None + selector.kustomization.namespace = None return selector diff --git a/flux_local/tool/visitor.py b/flux_local/tool/visitor.py index 27781a6a..9dc01f4b 100644 --- a/flux_local/tool/visitor.py +++ b/flux_local/tool/visitor.py @@ -44,6 +44,16 @@ class ResourceKey: namespace: str name: str + def __post_init__(self) -> None: + if self.cluster_path.startswith("/"): + raise AssertionError( + f"Expected cluster_path as relative: {self.cluster_path}" + ) + if self.kustomization_path.startswith("/"): + raise AssertionError( + f"Expected kustomization_path as relative: {self.kustomization_path}" + ) + @property def label(self) -> str: parts = [] diff --git a/tests/tool/testdata/get_cluster3.yaml b/tests/tool/testdata/get_cluster3.yaml index 5838ad40..eea06a25 100644 --- a/tests/tool/testdata/get_cluster3.yaml +++ b/tests/tool/testdata/get_cluster3.yaml @@ -3,6 +3,8 @@ args: - cluster - --path - tests/testdata/cluster3/ +- --sources +- cluster=tests/testdata/cluster3 stdout: | NAME PATH KUSTOMIZATIONS - flux-system ./tests/testdata/cluster3/ 1 + flux-system ./tests/testdata/cluster3/ 3 diff --git a/tests/tool/testdata/get_ks5_all.yaml b/tests/tool/testdata/get_ks5_all.yaml new file mode 100644 index 00000000..e4206267 --- /dev/null +++ b/tests/tool/testdata/get_ks5_all.yaml @@ -0,0 +1,10 @@ +args: +- get +- ks +- -A +- --path +- tests/testdata/cluster5 +stdout: | + NAMESPACE NAME PATH + controllers infra-controllers tests/testdata/cluster5 + flux-system flux-system ./tests/testdata/cluster5/clusters/prod diff --git a/tests/tool/testdata/get_ks_path_ks.yaml b/tests/tool/testdata/get_ks_path_ks.yaml index 95146b12..051393a5 100644 --- a/tests/tool/testdata/get_ks_path_ks.yaml +++ b/tests/tool/testdata/get_ks_path_ks.yaml @@ -5,5 +5,5 @@ args: - --path - ./tests/testdata/cluster/apps/prod stdout: | - NAME PATH - kustomization tests/testdata/cluster/apps/prod + NAMESPACE NAME PATH + None kustomization tests/testdata/cluster/apps/prod