Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip unnecessary kustomize cfg step when scanning the cluster #399

Merged
merged 2 commits into from
Nov 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 41 additions & 98 deletions flux_local/git_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand All @@ -379,34 +371,23 @@ 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] = []
visited_paths: set[Path] = set() # Relative paths within the cluster
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})

Expand All @@ -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:
Expand All @@ -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,
Expand Down Expand Up @@ -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 = []
Expand Down
28 changes: 0 additions & 28 deletions tests/__snapshots__/test_git_repo.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -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'",
]),
}),
})
# ---
Expand Down Expand Up @@ -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'",
]),
}),
})
# ---
Expand Down Expand Up @@ -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'",
]),
}),
})
# ---
Expand All @@ -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'",
]),
}),
})
# ---
Expand Down Expand Up @@ -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'",
]),
}),
})
# ---
Expand Down Expand Up @@ -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'",
]),
}),
})
# ---
Expand Down Expand Up @@ -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'",
]),
}),
})
# ---
Expand Down
12 changes: 3 additions & 9 deletions tests/test_git_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down