Debulleting Porch Architecture & Components - Engine - #1089
Conversation
Signed-off-by: Dominika Schweier <dominika.schweier@nokia.com>
✅ Deploy Preview for kpt-porch ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
dosubot
Bot
added
the
size:XL
There was a problem hiding this comment.
Pull request overview
Documentation-focused PR that converts several bulleted “Architecture & Components / Engine” sections into prose and tables to improve readability and reduce future merge conflicts while keeping the technical content aligned with existing engine behavior.
Changes:
- Rewrites multiple “Engine” documentation pages from bulleted lists into narrative paragraphs.
- Introduces/adjusts structured formatting (tables/alerts) to present rules and transitions more clearly.
- Updates/clarifies several workflow and interaction descriptions across engine documentation pages.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/content/en/docs/5_architecture_and_components/engine/interactions.md | Converts cache/task-handler/watch sections from bullets to prose. |
| docs/content/en/docs/5_architecture_and_components/engine/functionality/validation-business-rules.md | Debullets rationales; reformats task/upgrade rule descriptions for readability. |
| docs/content/en/docs/5_architecture_and_components/engine/functionality/task-coordination.md | Debullets responsibilities/parameter docs; adds prose summaries and tables. |
| docs/content/en/docs/5_architecture_and_components/engine/functionality/lifecycle-management.md | Rewrites lifecycle rationale/audit sections and transition presentation. |
| docs/content/en/docs/5_architecture_and_components/engine/functionality/draft-commit-orchestration.md | Rewrites render-failure/rollback/optimization sections; replaces an “Important notes” block with an alert. |
| docs/content/en/docs/5_architecture_and_components/engine/design.md | Rewrites repository abstraction and interface characteristics; updates lifecycle diagram reference. |
Signed-off-by: Dominika Schweier <dominika.schweier@nokia.com>
Catalin-Stratulat-Ericsson
left a comment
Catalin-Stratulat-Ericsson
left a comment
There was a problem hiding this comment.
Very good work. thanks for your effort on this. just a few comments
| #### Draft State | ||
|
|
||
| This state can be created directly, or as default when no lifecycle is specified. It allows full modifications of tasks, resources, | ||
| metadata or lifecycle. From draft state packages can transition to either Proposed or Published. However, they cannot be created with | ||
| Published or DeletionProposed lifecycle. |
There was a problem hiding this comment.
id reword this. not of your rewording that is the problem here its just the original information was not so accurate. draft can only move into proposed state or be directly deleted (which is not a state). the last sentence also of "However, they cannot be created with Published or DeletionProposed lifecycle." makes 0 sense to me. remove it
| #### Proposed State | ||
|
|
||
| This state indicates that the package revision is ready for review. From the proposed state, packages can transition to either Draft | ||
| (for rework) or Published (for approval). This is typically used in approval workflows. |
There was a problem hiding this comment.
maybe we can reword "for approval" to something else? the proposed state is the "check for approval state" the published is it passed and was approved state. just reads a bit off for me
There was a problem hiding this comment.
(last sentence) its not that this is typically used in approval workflows. it is solely used to act as the approval workflow state. just get rid of the last sentence there.
There was a problem hiding this comment.
i think this state is also immutable much like the published state. mutability can only happen in draft state. hence why a user must reject the proposed package to send it back to draft to make changes.
This is also stated here in the docs https://deploy-preview-1089--kpt-porch.netlify.app/docs/4_tutorials_and_how-tos/working_with_package_revisions/creating-packages/#step-5-propose-the-package-revision
| the render status with function results is returned. This does not involve a lifecycle change, as the resources are updated in-place. | ||
|
|
||
| The render is executed by running a configured KRM function pipeline. These functions can validate, transform and generate resources. | ||
| The results are returned in RenderStatus. |
There was a problem hiding this comment.
| The results are returned in RenderStatus. | |
| The results are returned in a `RenderStatus`. |
The RenderStatus is a data type
| ``` | ||
|
|
||
| **Important notes:** | ||
| {{% alert title="Note" color="primary" %}} |
There was a problem hiding this comment.
good change. thanks. makes it more readable
| | Direction | Transition | Action | | ||
| |-----------|------------|--------| | ||
| | Forward | Draft → Proposed | Submit for review | | ||
| | Forward | Proposed → Published | Approve | | ||
| | Forward | Published → DeletionProposed | Mark for deletion | | ||
| | Backward | Proposed → Draft | Return for rework | | ||
| | Backward | DeletionProposed → Published | Reject deletion | | ||
| | Forbidden | Published → Draft | Cannot unpublish | | ||
| | Forbidden | Published → Proposed | Cannot unpublish | | ||
| | Forbidden | Any state → Draft (except from Proposed) | — | |
There was a problem hiding this comment.
nice table. i will say the ASCII diagram above it can go its pointless with the table. + the section above with the visual diagram of the state definitions already makes it quite clear which state can do what
| - Contains task specification in Spec.Tasks | ||
| - First task in list executed | ||
| - Task type determines which implementation runs | ||
| PackageRevision contains task specification in Spec.Tasks. The first task in the list is executed. Task type |
There was a problem hiding this comment.
again im not 100% sure if we should have things like spec.tasks marked with `` as they are types. would incur alot of changes given my reading of just this engine area of the architecture section. up to yourself
| - **upgrade**: Merge changes from new upstream version | ||
| | Task type | Action | | ||
| |-----------|--------| | ||
| | init | Create new package from scratch | |
There was a problem hiding this comment.
not sure if the statement is 100% accurate. from scratch makes it seem like its creating it from nothing. what really happens is porch has a "hollow package template" stored in memory. whenever init is triggered it just uses this template as the packages contents and fills in the areas of the package from user prompt e.g. name, workspace etc where appropriate.
Up to you how to word this given the context. or even if a wording change is needed. im just providing a bit more context.
There was a problem hiding this comment.
| | init | Create new package from scratch | | |
| | init | Initialize new, blank package | |
| which uses runtime for function execution. | ||
|
|
||
| **For details on function runtime implementations, see [Function Runner]({{% relref "/docs/5_architecture_and_components/function-runner/_index.md" %}}).** | ||
| For details on function runtime implementations, see [Function Runner]({{% relref "/docs/5_architecture_and_components/function-runner/_index.md" %}}). |
There was a problem hiding this comment.
i think perhaps this should link to here instead https://deploy-preview-1089--kpt-porch.netlify.app/docs/5_architecture_and_components/function-runner/design/
this is the area that directly tackles the runtime implementations
| Packages must progress through the Draft/Proposed states before being published. This prevents bypassing the review/approval | ||
| workflows and ensures all packages have a draft history. |
There was a problem hiding this comment.
one note to be aware of here is it also ensures all packages which pass to the published stage have a "successful" render of its kpt pipeline. so no failing render package can ever slip through.
| Only allowing one operation simplifies the creation workflow. However, you can add multiple tasks later with updates. The default init | ||
| task provides s sensible starting point. |
There was a problem hiding this comment.
seems there is an extra "s" in the sentence here
| - Copies resources to draft for modification | ||
| - Preserves original revision (immutable) | ||
| In this case, the current package revision of the content is loaded and a mutable draft | ||
| workspace is created. Resources are copied to draft for modification, while the original revision is preserved (immutable) |
There was a problem hiding this comment.
| workspace is created. Resources are copied to draft for modification, while the original revision is preserved (immutable) | |
| workspace is created. Resources are copied to draft for modification, while the original revision is preserved (immutable). |
| **Draft operations:** | ||
| - Same as creation draft | ||
| - Can modify resources, lifecycle, metadata | ||
| - Changes isolated until closed | ||
| **Draft operations:** The same as creation draft, meaning resources, lifecycle and metadata can be modified. | ||
| The changes are isolated until closed. |
There was a problem hiding this comment.
I don't know what "same as creation draft" was meant to mean here but it sounds wrong. Maybe "same as an init draft" or "same as draft creation"?
If anybody has any idea, please chime in.
| Old and new package revision specs are compared and new tasks to apply are identified. These tasks are delegated to the | ||
| task handler for execution after which a modified draft is returned. |
There was a problem hiding this comment.
We should mention that there can be only 1 task now, the "creation source" (init/copy/clone/upgrade). I assume this part of the docs was not updated when we made it a single-element list
There was a problem hiding this comment.
i think this was being discussed at the time of the creation or there was some very odd case where you could have more than one in the task list. i don't remember fully but if its not true anymore it should be altered
| The render is executed by running a configured KRM function pipeline. These functions can validate, transform and generate resources. | ||
| The results are returned in RenderStatus. | ||
|
|
||
| In case of render failure, the default behavior is for render errors to prevent draft closure (no resources persisted). |
There was a problem hiding this comment.
| In case of render failure, the default behavior is for render errors to prevent draft closure (no resources persisted). | |
| In case of render failure, the default behavior is for render errors to prevent draft finalization (no resources persisted). |
| The draft creation cost is relatively lightweight. The workspace is allocated in the repository and | ||
| the resources, for updates, are copied. |
There was a problem hiding this comment.
| The draft creation cost is relatively lightweight. The workspace is allocated in the repository and | |
| the resources, for updates, are copied. | |
| The draft creation cost is relatively lightweight. The workspace is allocated in the repository and | |
| the resources are copied in case of updates. |
| - Clear error messages | ||
| - No side effects on failure | ||
|
|
||
| It is fail fast (before expensive operations), gives clear error messages, and has no side effects on failure. |
There was a problem hiding this comment.
| It is fail fast (before expensive operations), gives clear error messages, and has no side effects on failure. | |
| The benefits of this are that it is fails fast (before expensive operations), gives clear error messages, and has no side effects on failure. |
| - Can check against existing data | ||
| - Rollback mechanism available | ||
|
|
||
| It has access to the repository state, can check against existing data and has rollback mechanism available. |
There was a problem hiding this comment.
| It has access to the repository state, can check against existing data and has rollback mechanism available. | |
| The benefits of this are that it has access to the repository state, can check against existing data and has rollback mechanism available. |
| For update operations, the engine uses **Kubernetes resource versions**. The client must provide current resource version in update | ||
| request. Then the engine compares provided version with actual version and returns a conflict error if the versions do not match. This | ||
| forces the client to re-read and retry with the latest version. This way lost updates are prevented when multiple clients modify the same | ||
| package. |
There was a problem hiding this comment.
| For update operations, the engine uses **Kubernetes resource versions**. The client must provide current resource version in update | |
| request. Then the engine compares provided version with actual version and returns a conflict error if the versions do not match. This | |
| forces the client to re-read and retry with the latest version. This way lost updates are prevented when multiple clients modify the same | |
| package. | |
| For update operations, the engine uses **Kubernetes resource versions**. The client must provide current resource version in update | |
| request. Then, the engine compares provided version with actual version and returns a conflict error if the versions do not match. This | |
| forces the client to re-read and retry with the latest version. This way lost updates are prevented when multiple clients modify the same | |
| package. |
| This state can be created directly, or as default when no lifecycle is specified. It allows full modifications of tasks, resources, | ||
| metadata or lifecycle. From draft state packages can transition to either Proposed or Published. However, they cannot be created with | ||
| Published or DeletionProposed lifecycle. |
There was a problem hiding this comment.
| This state can be created directly, or as default when no lifecycle is specified. It allows full modifications of tasks, resources, | |
| metadata or lifecycle. From draft state packages can transition to either Proposed or Published. However, they cannot be created with | |
| Published or DeletionProposed lifecycle. | |
| This is the starting state of a package when created and the default when no lifecycle is specified. | |
| It allows full modifications of tasks, resources, metadata or lifecycle. | |
| Packages in this state can transition to either Proposed or Published. |
| Packages in this state are immutable, meaning that no resource or task modifications are allowed to them. Only metadata (labels, | ||
| annotations) and lifecycle can be updated. From the published state, packages can transition to DeletionProposed. The published state | ||
| represents the deployed, production-ready state. |
There was a problem hiding this comment.
| Packages in this state are immutable, meaning that no resource or task modifications are allowed to them. Only metadata (labels, | |
| annotations) and lifecycle can be updated. From the published state, packages can transition to DeletionProposed. The published state | |
| represents the deployed, production-ready state. | |
| Packages in this state are immutable, meaning that no resource or task modifications are allowed to them. Only metadata (labels, | |
| annotations) and lifecycle can be updated. From the published state, packages can transition to DeletionProposed. The published state | |
| represents the deployable, production-ready state. |
Title
Debulleting Porch Architecture & Components - Engine
Description
Minor rewrites to make the text more readable. The Architecture & Components page is going to be debulleted in batches to avoid merge conflicts.
Type of Change
Checklist