-
Notifications
You must be signed in to change notification settings - Fork 0
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
ADR for deployment resource #46
base: main
Are you sure you want to change the base?
Conversation
34697a5
to
86b4fef
Compare
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.
Great ADR overall. I do have some questions but for this early on this is natural. Will especially take over the "yamlsubst" naming.
* Deciders: Gergely Brautigam, Fabian Burth, Jakob Moeller, Uwe Krueger | ||
* Date: 2024-10-22 | ||
|
||
## Technical Story |
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.
Nice technical story, well written 💯
docs/adr/deployments/README.md
Outdated
- all *technical resources* to be referenced from the technical deployment (for example OCI images) | ||
- A *deployment template* for the technical deployer used by the deploymentr pipeline to finally feed the target environment (for example a Helm Chart or a Kustomize template) | ||
|
||
- It describes the rules how location of the described technical resources as been found in a target repository landscape have to be injected into the deployment template. |
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.
Is this always the case or is there a potential for this to be optional. Im thinking of a helper chart with a component thats referencable everywhere?
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 rewrote this to:
- It describes the rules how location of the resources have to be injected into the deployment template.
That's still accurate, right?
docs/adr/deployments/README.md
Outdated
On the side of the operational environment, the deployment side, | ||
the operator describes a deployment resource (matching the type of the deployment artifact in the OCM component version). | ||
|
||
It refers to a config map with the deployment specific configuration values, and to a target, which describes the deployment target. |
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.
Side note: Currently Im planning a separate CRD but youre completely right that this should better be a ConfigMap. Currently though, because the engine needs to understand the instructions in the ConfigMap, the format has to be standardized.
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 simply renamed config map
here to object
. So whatever it will be it should be okay.
docs/adr/deployments/README.md
Outdated
|
||
## Improvements for the OCM controllers | ||
|
||
 |
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.
Nice overview
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.
Agreed, but at this point, I think it would be great if it would resemble our actual current architecture / resources.
docs/adr/deployments/README.md
Outdated
|
||
The rules engine itself depends only on the kind of snapshot and the modification technology (for example field-based substitution of yaml documents). | ||
|
||
Therefore, it seems to be useful to separate the creation of the effective modification rules from the execution engine. This allows to use the same technical rules engine (and the related K8S resource types). It should replace the separated configuration and localization resources so far provided by the OCM controllers. |
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.
This assumes that people would want to reuse our substitution engine but not our value resolution (i.e. Localization) process.
I would assume (but we would have to ask potential endusers for this) that most people wanting to customize would like to customize our entire substitution process and then reuse some of our logic where applicable.
The reason I think this is because If I think of approaches not covered in our standard case (say an MTA deployment or some other arbitrary description method), then not only will the value resolution be touched, but especially the substitution engine (the modification engine, i.e. field-based yaml changes) .
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.
That said I am for separation of the creation of rules from the execution. I reflected this in the current approach of LocalizedResource (that I have yet to update in the ADR as of writing this).
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 would assume (but we would have to ask potential endusers for this) that most people wanting to customize would like to customize our entire substitution process and then reuse some of our logic where applicable.
You mean something like a custom substitution engine? Like, not yaml based but toml or some other arbitrary configuration mechanism?
|
||
In any case the K8S deployment resource denotes the OCM resource for the deployment description. | ||
Similar to the current low-level approach, the deployment template is provided by the controller via a K8s `Resource` resource, directly configured according to the content of the deployment description. If described it extracts the rules for the localization and configuration and manages the appropriately typed rules and modification resources. In a first step there will only be two modification engines: | ||
- *yamlsubst* for file based substitution of yaml paths |
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.
good name, will blatantly copy&paste that for the current PRs for localization. I used "mapped" before which is... bad.
docs/adr/deployments/README.md
Outdated
|
||
The final deployer resource depends on the artifacts type of the deployment template as described by the OCM component version. | ||
It can explicitly be defined as template in the deployment description or defaulted by taking this artifact type into account. | ||
It must template the Flux source resource and the target. Therefore, |
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.
The various templating references in the Deployment are my only contention point right now:
I am a little unsure if this is something that people truly want/need, However I do understand that we would like to "bundle" everything into one. We need to make sure that we keep in mind that the flux ecosystem is also evolving based on its original resources (i.e. https://github.com/controlplaneio-fluxcd/flux-operator) and so to opinionate too much into this direction may not be wise.
My argument is that templating such a resource is not having significant benefits over referencing the resource if its simply a "one-shot" creation. The only reason we should template imo is if we have a need to constantly patch/update the Flux source resource/target.
However since the setup is once per component version (i.e. one-shot Component CRD and one-shot Resources that automatically update based on semver or not) and then unlikely to change in the Spec, Im unsure about the various templates involved here.
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, this is not really meant as a templating step but rather similar to the pod template
in a k8s deployment resource.
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 agree with Fabian. I think this is meant to say, provide a template that we create the object based on or provide the type and we default something?
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.
Some of the images have text on transparent background which is impossible to read if one uses dark-themed GitHub.
docs/adr/deployments/README.md
Outdated
On the provider side the development team has to provide a component version with all the artifacts required for a deployment. Additionally, | ||
a separate declarative content has to be provided describing the intended deployment. | ||
|
||
All those artifacts are types. This is especially true for this additional deployment resource. Its type is the used to match the deployment resource of the consumption side and it represents a dedicated description methodology understandable by the deployment controller. |
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.
All those artifacts are types. This is especially true for this additional deployment resource. Its type is the used to match the deployment resource of the consumption side and it represents a dedicated description methodology understandable by the deployment controller. | |
All those artifacts are types. This is especially true for this additional deployment resource. Its type is used to match the deployment resource of the consumption side and it represents a dedicated description methodology understandable by the deployment controller. |
|
||
The controller responsible for the K8S deployment resource has then the task to manage the complete deployment pipeline, consisting of the OCM-related K8S resources and the Flux resources used for the technical deployment. This pipeline is then responsible to provide the final technical deployment in the intended target environment. | ||
|
||
## Improvements for the OCM controllers |
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.
A lot has changed since this was written. Several of the improvements were already considered in the design of the other controllers. Thus, the section has to be revisited.
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.
Should I re-write this section with the that in mind that we want ocm to localize objects?
Co-authored-by: Fabian Burth <fabian.burth@sap.com> Co-authored-by: Jakob Möller <jakob.moeller@sap.com>
What this PR does / why we need it:
Which issue(s) this PR fixes: