[PKO-351] Added documentation for HostedClusterPackage API#27
[PKO-351] Added documentation for HostedClusterPackage API#27Ankit152 wants to merge 1 commit intopackage-operator:mainfrom
HostedClusterPackage API#27Conversation
7e92154 to
dfdfce0
Compare
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
WalkthroughAdds a cluster-scoped HostedClusterPackage API (spec/status/partitioning/order/ref types), introduces PackageTemplateSpec, and updates package-operator API fields with revision/version metadata; includes new documentation and examples. No runtime behavior changes beyond API/type additions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@content/en/docs/advanced_features/hostedclusterpackage-api.md`:
- Around line 8-9: Update the note sentence that currently reads "**Note**: The
`HostedClusterPackage` API is experimental and subject to change in future." to
use US English by inserting "the" so it reads "...subject to change in the
future." — locate the string containing `HostedClusterPackage` and replace
"change in future" with "change in the future".
- Around line 13-15: The text incorrectly refers to the resource as "the
Package" which is ambiguous; update the wording to consistently use the correct
resource name HostedClusterPackage (e.g., change "the Package" to "the
HostedClusterPackage") and scan the surrounding lines in this section to replace
any other occurrences of the generic "Package" with HostedClusterPackage to
avoid confusion.
- Around line 103-110: Check the CRD for HostedClusterPackage (spec and
strategy) to confirm whether the field minReadySeconds is defined; if
minReadySeconds exists, add it to the API reference under
HostedClusterPackageSpec or HostedClusterPackageStrategyRollingUpgrade with its
type and semantics, otherwise remove all mentions of minReadySeconds from the
rolling upgrade guide and the YAML example in the guide; update the docs to keep
the API reference and guide consistent and ensure the YAML example uses only
documented fields.
In `@content/en/docs/api_reference/package-operator-api.md`:
- Around line 428-435: The example is invalid because partition.order mixes two
mutually exclusive strategies (partition.order.alphanumericAsc and
partition.order.static) and strategy is a quoted string instead of a mapping;
fix by choosing one ordering strategy under partition.order (remove either
alphanumericAsc or static so only one remains) while keeping partition.labelKey
unchanged, and change strategy from a quoted string to a YAML mapping (e.g.,
replace the quoted 'strategy' value with a mapping form like strategy: {instant:
{}}) so the example is valid and consistent; reference
partition.order.alphanumericAsc, partition.order.static, partition.labelKey, and
strategy to locate the fixes.
- Line 1406: The table cell for `metadata` (the PackageTemplateSpec `metadata`
description row containing "More info: https://git.k8s.io/...") uses a bare URL;
update that markdown to use a link instead (for example, replace the bare URL
with a markdown link like "More info: [Kubernetes API
conventions](https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata)")
so the URL is not bare and markdownlint will pass. Target the table row that
contains `metadata` and `metav1.ObjectMeta` and modify only the description
string.
|
|
||
| | Field | Description | | ||
| | ----- | ----------- | | ||
| | `metadata` <br>metav1.ObjectMeta | Standard object's metadata.<br>More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata | |
There was a problem hiding this comment.
Avoid bare URL in table description.
The PackageTemplateSpec metadata description uses a bare URL; markdownlint flags this. Please wrap it as a link.
✏️ Suggested edit
-| `metadata` <br>metav1.ObjectMeta | Standard object's metadata.<br>More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata |
+| `metadata` <br>metav1.ObjectMeta | Standard object's metadata.<br>More info: [Kubernetes API conventions](https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata) |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | `metadata` <br>metav1.ObjectMeta | Standard object's metadata.<br>More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata | | |
| | `metadata` <br>metav1.ObjectMeta | Standard object's metadata.<br>More info: [Kubernetes API conventions](https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata) | |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
1406-1406: Bare URL used
(MD034, no-bare-urls)
🤖 Prompt for AI Agents
In `@content/en/docs/api_reference/package-operator-api.md` at line 1406, The
table cell for `metadata` (the PackageTemplateSpec `metadata` description row
containing "More info: https://git.k8s.io/...") uses a bare URL; update that
markdown to use a link instead (for example, replace the bare URL with a
markdown link like "More info: [Kubernetes API
conventions](https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata)")
so the URL is not bare and markdownlint will pass. Target the table row that
contains `metadata` and `metav1.ObjectMeta` and modify only the description
string.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@content/en/docs/advanced_features/hostedclusterpackage-api.md`:
- Around line 118-120: Replace the typo "arollout" with "a rollout" in the
sentence that describes the HostedClusterPackage API status (the line containing
"The `HostedClusterPackage` API exposes status information to help you track the
progress of arollout and the health of the fleet."); keep the rest of the
sentence unchanged so it reads "track the progress of a rollout and the health
of the fleet."
- Around line 20-21: Fix the typo and spacing in the sentence containing
"controll rollout strategies,significantly reducing the \"blast radius\"" by
changing "controll" to "control" and adding a space after the comma so it reads
"control rollout strategies, significantly reducing the \"blast radius\"".
Ensure the corrected sentence preserves the surrounding wording and punctuation.
fac9098 to
853e844
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@content/en/docs/api_reference/package-operator-api.md`:
- Around line 975-1102: The docs reference two missing type
sections—HostedClusterPackagePartitionOrderAlphanumericAsc and
HostedClusterPackageStrategyInstant—so add dedicated documentation blocks for
each with a heading matching the type name, a short description of the type, a
field table listing all fields and their descriptions (matching the style used
for HostedClusterPackagePartitionOrderSpec and
HostedClusterPackageStrategyRollingUpgrade), and a "Used in:" list linking back
to the parent types (ensure links point to
HostedClusterPackagePartitionOrderSpec and HostedClusterPackageStrategy
respectively); implement these new sections so the references at
HostedClusterPackagePartitionOrderSpec and HostedClusterPackageStrategy resolve
and the API surface is complete.
5055570 to
0bb17d6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@content/en/docs/api_reference/package-operator-api.md`:
- Around line 975-1102: The docs are missing definition sections for the types
referenced: HostedClusterPackagePartitionOrderAlphanumericAsc and
HostedClusterPackageStrategyInstant; add a new heading for each (e.g., "###
HostedClusterPackagePartitionOrderAlphanumericAsc" and "###
HostedClusterPackageStrategyInstant"), provide a short description of the type's
purpose, include either a field table (if the type has fields) or a one-line
note stating it is a marker/empty spec (no fields), and add a "Used in:" line
linking back to the parent types that reference them
(HostedClusterPackagePartitionOrderSpec and HostedClusterPackageStrategy
respectively) so readers can find where they are used.
🧹 Nitpick comments (3)
content/en/docs/api_reference/package-operator-api.md (3)
982-982: Add description foralphanumericAscfield.The
alphanumericAscfield has no description. Even if the type it references has no fields, the table should explain what this ordering strategy does (e.g., "Orders partitions alphanumerically in ascending order.").
1082-1082: Add description forinstantfield.The
instantfield has no description. Add an explanation of what the instant strategy does (e.g., "Updates all matching Packages instantly without waiting.").
1031-1033: Add descriptions for HostedClusterPackageRefStatus fields.The
uid,name, andnamespacefields all lack descriptions. Provide meaningful descriptions for these reference fields (e.g., "UID of the referenced Package", "Name of the referenced Package", "Namespace of the referenced Package (empty for cluster-scoped)").
| ### HostedClusterPackagePartitionOrderSpec | ||
|
|
||
| HostedClusterPackagePartitionOrderSpec describes ordering for a partition. | ||
|
|
||
| | Field | Description | | ||
| | ----- | ----------- | | ||
| | `static` <br>[]string | Allows to define a static partition order.<br>The special * key matches anything not explicitly part of the list.<br>Unknown risk-groups or HostedClusters without label<br>will be put into an implicit "unknown" group and<br>will get upgraded LAST. | | ||
| | `alphanumericAsc` <br><a href="#hostedclusterpackagepartitionorderalphanumericasc">HostedClusterPackagePartitionOrderAlphanumericAsc</a> | | | ||
|
|
||
|
|
||
| Used in: | ||
| * [HostedClusterPackagePartitionSpec](#hostedclusterpackagepartitionspec) | ||
|
|
||
|
|
||
| ### HostedClusterPackagePartitionSpec | ||
|
|
||
| HostedClusterPackagePartitionSpec describes settings to partition HostedClusters into groups for upgrades. | ||
| Partitions will be upgraded after each other: | ||
| Upgrades in the next partition will only start after the previous has finished. | ||
| HostedClusters without the label will be put into an implicit "unknown" group | ||
| and will get upgraded LAST. | ||
|
|
||
| | Field | Description | | ||
| | ----- | ----------- | | ||
| | `labelKey` <b>required</b><br>string | LabelKey defines a labelKey to group objects on. | | ||
| | `order` <br><a href="#hostedclusterpackagepartitionorderspec">HostedClusterPackagePartitionOrderSpec</a> | Controls how partitions are ordered.<br>By default items will be sorted AlphaNumeric ascending. | | ||
|
|
||
|
|
||
| Used in: | ||
| * [HostedClusterPackageSpec](#hostedclusterpackagespec) | ||
|
|
||
|
|
||
| ### HostedClusterPackagePartitionStatus | ||
|
|
||
| HostedClusterPackagePartitionStatus describes the status of a partition. | ||
|
|
||
| | Field | Description | | ||
| | ----- | ----------- | | ||
| | `name` <b>required</b><br>string | Name of the partition. | | ||
| | `observedGeneration` <br>int32 | The generation observed by the HostedClusterPackage controller. | | ||
| | `availablePackages` <br>int32 | Total number of available Packages targeted by this HostedClusterPackage. | | ||
| | `progressedPackages` <br>int32 | Total number of Packages with Progressing=False and Unpacked=True conditions. | | ||
| | `updatedPackages` <br>int32 | Total number of non-terminated Packages targeted by this HostedClusterPackage that have the desired template spec. | | ||
| | `totalPackages` <br>int32 | Total number of non-terminated Packages targeted by this HostedClusterPackage. | | ||
|
|
||
|
|
||
| Used in: | ||
| * [HostedClusterPackageStatus](#hostedclusterpackagestatus) | ||
|
|
||
|
|
||
| ### HostedClusterPackageRefStatus | ||
|
|
||
| HostedClusterPackageRefStatus holds a reference to upgrades in-flight. | ||
|
|
||
| | Field | Description | | ||
| | ----- | ----------- | | ||
| | `uid` <b>required</b><br>types.UID | | | ||
| | `name` <b>required</b><br>string | | | ||
| | `namespace` <br>string | | | ||
|
|
||
|
|
||
| Used in: | ||
| * [HostedClusterPackageStatus](#hostedclusterpackagestatus) | ||
|
|
||
|
|
||
| ### HostedClusterPackageSpec | ||
|
|
||
| HostedClusterPackageSpec is the description of a HostedClusterPackage. | ||
|
|
||
| | Field | Description | | ||
| | ----- | ----------- | | ||
| | `strategy` <b>required</b><br><a href="#hostedclusterpackagestrategy">HostedClusterPackageStrategy</a> | HostedClusterPackageStrategy describes the rollout strategy for a HostedClusterPackage. | | ||
| | `hostedClusterSelector` <br>metav1.LabelSelector | HostedClusterSelector is a label query matching HostedClusters that the Package should be rolled out to. | | ||
| | `template` <b>required</b><br><a href="#packagetemplatespec">PackageTemplateSpec</a> | Template describes the Package that should be created when new<br>HostedClusters matching the hostedClusterSelector are detected. | | ||
| | `partition` <br><a href="#hostedclusterpackagepartitionspec">HostedClusterPackagePartitionSpec</a> | Partition HostedClusters by label value.<br>All packages in the same partition will have to be upgraded<br>before progressing to the next partition. | | ||
|
|
||
|
|
||
| Used in: | ||
| * [HostedClusterPackage](#hostedclusterpackage) | ||
|
|
||
|
|
||
| ### HostedClusterPackageStatus | ||
|
|
||
| HostedClusterPackageStatus describes the status of a HostedClusterPackage. | ||
|
|
||
| | Field | Description | | ||
| | ----- | ----------- | | ||
| | `conditions` <br>[]metav1.Condition | Conditions is a list of status conditions this object is in. | | ||
| | `partitions` <br><a href="#hostedclusterpackagepartitionstatus">[]HostedClusterPackagePartitionStatus</a> | Count of packages found by partition. | | ||
| | `processing` <br><a href="#hostedclusterpackagerefstatus">[]HostedClusterPackageRefStatus</a> | Processing set of packages during upgrade. | | ||
| | `observedGeneration` <br>int32 | The generation observed by the HostedClusterPackage controller. | | ||
| | `availablePackages` <br>int32 | Total number of available Packages targeted by this HostedClusterPackage. | | ||
| | `progressedPackages` <br>int32 | Total number of Packages with Progressing=False and Unpacked=True conditions. | | ||
| | `updatedPackages` <br>int32 | Total number of non-terminated Packages targeted by this HostedClusterPackage that have the desired template spec. | | ||
| | `totalPackages` <br>int32 | Total number of non-terminated Packages targeted by this HostedClusterPackage. | | ||
|
|
||
|
|
||
| Used in: | ||
| * [HostedClusterPackage](#hostedclusterpackage) | ||
|
|
||
|
|
||
| ### HostedClusterPackageStrategy | ||
|
|
||
| HostedClusterPackageStrategy describes the rollout strategy for a HostedClusterPackage. | ||
|
|
||
| | Field | Description | | ||
| | ----- | ----------- | | ||
| | `instant` <br><a href="#hostedclusterpackagestrategyinstant">HostedClusterPackageStrategyInstant</a> | Updates all matching Packages instantly. | | ||
| | `rollingUpgrade` <br><a href="#hostedclusterpackagestrategyrollingupgrade">HostedClusterPackageStrategyRollingUpgrade</a> | Performs a rolling upgrade according to maxUnavailable. | | ||
|
|
||
|
|
||
| Used in: | ||
| * [HostedClusterPackageSpec](#hostedclusterpackagespec) | ||
|
|
||
|
|
||
| ### HostedClusterPackageStrategyRollingUpgrade | ||
|
|
||
| HostedClusterPackageStrategyRollingUpgrade describes the | ||
| rolling upgrade strategy for HostedClusterPackages. | ||
|
|
||
| | Field | Description | | ||
| | ----- | ----------- | | ||
| | `maxUnavailable` <b>required</b><br>int | MaxUnavailable defines how many Packages may become unavailable during upgrade at the same time.<br>Cannot be below 1, because we cannot surge to create more instances. | | ||
|
|
||
|
|
||
| Used in: | ||
| * [HostedClusterPackageStrategy](#hostedclusterpackagestrategy) | ||
|
|
There was a problem hiding this comment.
Missing type definitions were not actually added.
A previous review comment flagged that HostedClusterPackagePartitionOrderAlphanumericAsc (referenced at line 982) and HostedClusterPackageStrategyInstant (referenced at line 1082) lacked documentation sections, and was marked as "✅ Addressed in commit 5055570." However, searching the current document reveals these type definition sections are still missing. Users need dedicated documentation blocks explaining what these types do.
Each missing type should have:
- A heading (e.g.,
### HostedClusterPackagePartitionOrderAlphanumericAsc) - A description explaining the type's purpose
- A field table (or note that it has no fields if it's a marker type)
- A "Used in:" section linking back to parent types
🤖 Prompt for AI Agents
In `@content/en/docs/api_reference/package-operator-api.md` around lines 975 -
1102, The docs are missing definition sections for the types referenced:
HostedClusterPackagePartitionOrderAlphanumericAsc and
HostedClusterPackageStrategyInstant; add a new heading for each (e.g., "###
HostedClusterPackagePartitionOrderAlphanumericAsc" and "###
HostedClusterPackageStrategyInstant"), provide a short description of the type's
purpose, include either a field table (if the type has fields) or a one-line
note stating it is a marker/empty spec (no fields), and add a "Used in:" line
linking back to the parent types that reference them
(HostedClusterPackagePartitionOrderSpec and HostedClusterPackageStrategy
respectively) so readers can find where they are used.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@content/en/docs/advanced_features/hostedclusterpackage-api.md`:
- Line 162: Fix the spelling in the heading string "Example `Status` and
`Conditons` for a successful rollout:" by changing "Conditons" to "Conditions"
so the line reads "Example `Status` and `Conditions` for a successful rollout:";
update that exact heading text in the document (search for the literal "Example
`Status` and `Conditons` for a successful rollout:") to correct the typo.
Signed-off-by: Ankit152 <ankitkurmi152@gmail.com>
Jira: PKO-351