-
-
Notifications
You must be signed in to change notification settings - Fork 644
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
New kubernetes backend #21796
base: main
Are you sure you want to change the base?
New kubernetes backend #21796
Conversation
> fd _category_ docs/docs/ --max-depth 2 --exec rg position '{}' --with-filename --line-number | sort -k 3,3 -n docs/docs/introduction/_category_.json:3: "position": 1 docs/docs/getting-started/_category_.json:3: "position": 2 docs/docs/using-pants/_category_.json:3: "position": 3 docs/docs/python/_category_.json:3: "position": 4 docs/docs/go/_category_.json:3: "position": 5 docs/docs/jvm/_category_.json:3: "position": 6 docs/docs/shell/_category_.json:3: "position": 7 docs/docs/docker/_category_.json:3: "position": 8 docs/docs/kubernetes/_category_.json:3: "position": 9 docs/docs/helm/_category_.json:3: "position": 10 docs/docs/terraform/_category_.json:3: "position": 11 docs/docs/sql/_category_.json:3: "position": 12 docs/docs/ad-hoc-tools/_category_.json:3: "position": 13 docs/docs/javascript/_category_.json:3: "position": 13 docs/docs/writing-plugins/_category_.json:3: "position": 14 docs/docs/releases/_category_.json:3: "position": 15 docs/docs/contributions/_category_.json:3: "position": 16 docs/docs/tutorials/_category_.json:3: "position": 17
cc @tgolsson who I know has done some k8s related work at https://github.com/tgolsson/pants-backends/tree/main/pants-plugins/k8s Broad topic not intended to detail this whole PR: Something we have struggled with internally is that we would like to be able to generate the final reified k8s yaml for inspection or alternative deployments. (I have a PoC plugin that does this for helm) This is along the lines of 'package' but not not quite what helm means by 'package', and I'm unsure what to call it. It would be nice if all the Pant k8s generators could eventually both "spit out the yaml" and "deploy". |
Haha, we have exactly this problem, I've called the goal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
My only real sticking point is about the semantics of the requirement for context
putative_targets = [] | ||
|
||
if k8s.tailor_source_targets: | ||
all_k8s_files_globs = req.path_globs("*.yaml") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
include "*.yml" too? or maybe make this a customisable option? Not necessary for a first pass
if k8s.tailor_source_targets: | ||
all_k8s_files_globs = req.path_globs("*.yaml") | ||
all_k8s_files = await Get(Paths, PathGlobs, all_k8s_files_globs) | ||
unowned_k8s_files = set(all_k8s_files.files) - set(all_owned_sources) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that globbing every yaml file will be too broad, but I don't know that there's a standard way of detecting if a manifest is a k8s manifest (kubectl apply --validate=true --dry-run=client
seems to need a server). Worst case people can turn off tailoring.
|
||
|
||
@dataclass(frozen=True) | ||
class VersionHash: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could use pants.core.util_rules.external_tool.ExternalToolVersion
) | ||
|
||
backward_platform_mapping = {v: k for k, v in platform_mapping.items()} | ||
for result in results: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could have these output in semver order (instead of lexical order) by using from packaging.version import Version
, by collecting the versions and then sorting with something like sorted(versions, key=lambda e: Version(e.version))
|
||
backward_platform_mapping = {v: k for k, v in platform_mapping.items()} | ||
for result in results: | ||
v = result.get(60) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clarify that this is for the timeout
v = result.get(60) | |
v = result.get(timeout=60) |
kubectl_tool = await Get( | ||
DownloadedExternalTool, ExternalToolRequest, kubectl.get_request(platform) | ||
) | ||
digest = await Get(Digest, MergeDigests([kubectl_tool.digest, request.input_digest])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could use Process.immutable_input_digest
for the tool. Here's how it's done in the Helm backend
pants/src/python/pants/backend/helm/util_rules/tool.py
Lines 454 to 457 in 1d1e93e
immutable_input_digests = { | |
**request.extra_immutable_input_digests, | |
**helm_binary.immutable_input_digests, | |
} |
) -> DeployProcess: | ||
context = field_set.context.value | ||
if context is None: | ||
raise ValueError(f"Missing `{K8sBundleContextField.alias}` field") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could make this more helpful by including the FS's address with field_set.address.spec
platform: Platform, | ||
) -> DeployProcess: | ||
context = field_set.context.value | ||
if context is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I've missed something in the logic here. Why do we force the field_set.context
to have a value even when kubectl.pass_context
is False
?
While
helm
backend already exists and works fine for kubernetes, creating a helm chart might be an overkill for a small thing like a single configmap or a secret. New kubernetes backend can deploy single object easily given a yaml file.See docs for more details on usage.
I'm also planning to contribute our integration with
python_format_string
target (I will open a pr) that can handle simple python templating in text files.