Skip to content

Conversation

@Ankit152
Copy link
Contributor

Jira: PKO-351

Comment on lines +25 to +44
```mermaid
flowchart LR
subgraph Hive
metrics-sss["metrics-forwarder<br><b>SelectorSyncSet</b>"]
end
subgraph HyperShift Management Cluster
metrics-hsp["metrics-forwarder<br><b>HyperShift Package</b>"]
subgraph Namespace: my-cluster-x
ns-c1["<b>Package</b>"]
end
subgraph Namespace: my-cluster-y
ns-c2["<b>Package</b>"]
end
end
metrics-sss--->metrics-hsp
metrics-hsp--->ns-c1
metrics-hsp--->ns-c2
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove Hive and the SelectorSyncSet from this diagram, too.

My proposal:

flowchart
  subgraph "HyperShift Management Cluster (MC)"
    metrics-hsp["<b>HyperShift Package</b><br>metrics-forwarder"]
    subgraph "HC Namespace: my-cluster-x"
      ns-c1["<b>Package</b><br>metrics-forwarder"]
    end
    subgraph "HC Namespace: my-cluster-y"
      ns-c2["<b>Package</b><br>metrics-forwarder"]
    end
  end
  metrics-hsp--->ns-c1
  metrics-hsp--->ns-c2
Loading

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about adding an image instead of flowchart? I have created this in excalidraw. Let me know your thoughts.
hostedclusterpackage-api

available, the operator proceeds to select the next cluster in the partition.

* **Failure**: If a Package update fails or becomes unavailable, the rollout
pauses for that specific rollout path. This prevents the propagation of errors
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is a rollout path?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rollout path = target hosted cluster 🙂

Comment on lines 154 to 158
In addition to the resource status, the controller exports metrics that
provide a fleet-wide overview of the rollout state. SREs can utilize these
metrics to build dashboards that visualize the number of available versus
unavailable packages over time.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK this is not true.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will remove this.

The following YAML example demonstrates a HostedClusterPackage configured with
risk-based partitioning and a rolling upgrade strategy.

```yaml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example is completely missing .spec.template

@eqrx
Copy link
Member

eqrx commented Jan 29, 2026

@CodeRabbit full review

@coderabbitai
Copy link

coderabbitai bot commented Jan 29, 2026

✅ Actions performed

Full review triggered.

@coderabbitai
Copy link

coderabbitai bot commented Jan 29, 2026

Walkthrough

Adds a new cluster-scoped HostedClusterPackage API and associated types and docs, introduces PackageTemplateSpec, and updates package-operator API fields to include revision/version metadata; includes examples and partitioned rollout documentation. (No code behavior changes beyond API/type additions.)

Changes

Cohort / File(s) Summary
HostedClusterPackage documentation
content/en/docs/advanced_features/hostedclusterpackage-api.md
New documentation describing the HostedClusterPackage API: overview, targeting/partitioning, progression, status/monitoring, examples, and a Mermaid diagram.
Package operator API reference & types
content/en/docs/api_reference/package-operator-api.md
Adds HostedClusterPackage public types (HostedClusterPackage, HostedClusterPackageSpec, HostedClusterPackageStatus, HostedClusterPackagePartitionSpec, HostedClusterPackagePartitionOrderSpec, HostedClusterPackagePartitionStatus, HostedClusterPackageRefStatus) and introduces PackageTemplateSpec. Adds revision fields (ClusterObjectSetSpec.revision, ObjectSetSpec.revision), updates ObjectSetStatus.revision description, and adds version to controller/controlled reference fields; updates examples, tables, and field semantics to reflect partitioned rollout and versioning.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding documentation for the HostedClusterPackage API, which aligns with the changeset that introduces new documentation files.
Description check ✅ Passed The description is related to the changeset, providing a Jira ticket reference that connects to the documented feature work on HostedClusterPackage API documentation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 |
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
| `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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Ankit152 Ankit152 force-pushed the pko-351 branch 2 times, most recently from fac9098 to 853e844 Compare January 29, 2026 13:24
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants