Skip to content

Conversation

@TylerGillson
Copy link
Collaborator

@TylerGillson TylerGillson commented Nov 3, 2025

Summary by CodeRabbit

  • Chores
    • Updated fleetconfig-controller image to version v0.1.3 to pick up the latest release.
    • Configured key namespaces to persist across Helm upgrades and repairs, preventing inadvertent deletion or loss of configuration during maintenance.

Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@openshift-ci
Copy link

openshift-ci bot commented Nov 3, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: TylerGillson

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved label Nov 3, 2025
@coderabbitai
Copy link

coderabbitai bot commented Nov 3, 2025

Walkthrough

Adds helm.sh/resource-policy: keep annotations to three Namespace resources in the chart and updates the fleetconfig-controller image tag from v0.1.2 to v0.1.3 in chart values and README.

Changes

Cohort / File(s) Change Summary
Namespace annotations
fleetconfig-controller/charts/fleetconfig-controller/templates/ocm/topology-resources.yaml
Added helm.sh/resource-policy: keep annotation to three Namespace resources (global, default, spokes). No other resource structure or logic changed.
Image tag bump
fleetconfig-controller/charts/fleetconfig-controller/values.yaml, fleetconfig-controller/charts/fleetconfig-controller/README.md
Bumped fleetconfig-controller image tag from v0.1.2 to v0.1.3 (README updated to match). No other config changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Small, localized changes: identical annotation additions and a straightforward image tag bump.
  • Files to pay extra attention to:
    • templates/ocm/topology-resources.yaml — ensure annotations are on the intended Namespace resources and formatting matches Helm expectations.
    • values.yaml / README.md — verify image tag consistency.

Possibly related PRs

Suggested labels

lgtm

Suggested reviewers

  • ahmad-ibra

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "fix: set 'resource-policy: keep' on managed cluster set namespaces" accurately describes the primary change in the changeset: adding the helm.sh/resource-policy: keep annotation to three Namespace resources (global, default, and spokes) in the topology-resources.yaml file. This is the main objective of the PR as evidenced by the branch name (fix/keep-all-topology-resources) and the detailed summary. The title is specific, clear, and concise—a teammate scanning the repository history would immediately understand the core fix. The secondary image tag updates in values.yaml and README.md (v0.1.2 to v0.1.3) are incidental version bumps that appropriately don't require inclusion in the title.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 41ac8f7 and fa5ff81.

📒 Files selected for processing (1)
  • fleetconfig-controller/charts/fleetconfig-controller/README.md (1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: TylerGillson
Repo: open-cluster-management-io/lab PR: 51
File: fleetconfig-controller/charts/fleetconfig-controller/README.md:155-155
Timestamp: 2025-08-22T17:55:52.159Z
Learning: In the open-cluster-management-io/lab repository, chart versioning for fleetconfig-controller is handled automatically via GitHub release workflows, not through manual version bumps in Chart.yaml during regular PRs.
Learnt from: arturshadnik
Repo: open-cluster-management-io/lab PR: 58
File: fleetconfig-controller/charts/fleetconfig-controller/README.md:155-155
Timestamp: 2025-08-27T21:58:32.141Z
Learning: In the open-cluster-management-io/lab repository, the fleetconfig-controller follows a workflow where chart version bumps (in README.md and values.yaml) are included in PRs before the corresponding Docker image exists. The Docker image is built and pushed automatically via GitHub release workflows after the PR is merged and tagged, making the referenced version available.
Learnt from: arturshadnik
Repo: open-cluster-management-io/lab PR: 69
File: fleetconfig-controller/charts/fleetconfig-controller/templates/ocm/fcc-addon/addon-template.yaml:110-112
Timestamp: 2025-09-25T23:31:11.630Z
Learning: The fleetconfig-controller-manager spoke agent requires create/update/patch/delete permissions on CustomResourceDefinitions because `clusteradm upgrade klusterlet` operations need create/update permissions and cleanup operations require delete permissions for proper lifecycle management.
📚 Learning: 2025-08-22T17:55:52.159Z
Learnt from: TylerGillson
Repo: open-cluster-management-io/lab PR: 51
File: fleetconfig-controller/charts/fleetconfig-controller/README.md:155-155
Timestamp: 2025-08-22T17:55:52.159Z
Learning: In the open-cluster-management-io/lab repository, chart versioning for fleetconfig-controller is handled automatically via GitHub release workflows, not through manual version bumps in Chart.yaml during regular PRs.

Applied to files:

  • fleetconfig-controller/charts/fleetconfig-controller/README.md
📚 Learning: 2025-08-27T21:58:32.141Z
Learnt from: arturshadnik
Repo: open-cluster-management-io/lab PR: 58
File: fleetconfig-controller/charts/fleetconfig-controller/README.md:155-155
Timestamp: 2025-08-27T21:58:32.141Z
Learning: In the open-cluster-management-io/lab repository, the fleetconfig-controller follows a workflow where chart version bumps (in README.md and values.yaml) are included in PRs before the corresponding Docker image exists. The Docker image is built and pushed automatically via GitHub release workflows after the PR is merged and tagged, making the referenced version available.

Applied to files:

  • fleetconfig-controller/charts/fleetconfig-controller/README.md
📚 Learning: 2025-09-22T19:26:11.020Z
Learnt from: arturshadnik
Repo: open-cluster-management-io/lab PR: 59
File: fleetconfig-controller/test/data/fleetconfig-v1alpha1.yaml:47-53
Timestamp: 2025-09-22T19:26:11.020Z
Learning: In the open-cluster-management-io/lab repository's fleetconfig-controller tests, the kubeconfigKey is intentionally set to "value" in test fixtures (fleetconfig-v1alpha1.yaml, fleetconfig-values.yaml) because that's how the test harness provisions the kubeconfig secret during test setup. This differs from the chart default of "kubeconfig" but is correct for the test environment.

Applied to files:

  • fleetconfig-controller/charts/fleetconfig-controller/README.md
📚 Learning: 2025-09-22T19:16:34.109Z
Learnt from: arturshadnik
Repo: open-cluster-management-io/lab PR: 59
File: fleetconfig-controller/internal/webhook/v1beta1/validation.go:103-121
Timestamp: 2025-09-22T19:16:34.109Z
Learning: In the open-cluster-management-io/lab repository's fleetconfig-controller v1beta1 API, the Klusterlet field in SpokeSpec is defined as a struct value (Klusterlet Klusterlet), not a pointer (*Klusterlet), so direct field access like Klusterlet.Annotations is safe without nil checks. The Klusterlet struct does not contain a Source field.

Applied to files:

  • fleetconfig-controller/charts/fleetconfig-controller/README.md
📚 Learning: 2025-09-25T23:31:11.630Z
Learnt from: arturshadnik
Repo: open-cluster-management-io/lab PR: 69
File: fleetconfig-controller/charts/fleetconfig-controller/templates/ocm/fcc-addon/addon-template.yaml:110-112
Timestamp: 2025-09-25T23:31:11.630Z
Learning: The fleetconfig-controller-manager spoke agent requires create/update/patch/delete permissions on CustomResourceDefinitions because `clusteradm upgrade klusterlet` operations need create/update permissions and cleanup operations require delete permissions for proper lifecycle management.

Applied to files:

  • fleetconfig-controller/charts/fleetconfig-controller/README.md
📚 Learning: 2025-09-22T18:42:03.404Z
Learnt from: arturshadnik
Repo: open-cluster-management-io/lab PR: 59
File: fleetconfig-controller/PROJECT:28-31
Timestamp: 2025-09-22T18:42:03.404Z
Learning: In the open-cluster-management-io/lab repository's fleetconfig-controller, the PROJECT file defines multiple API resources with different webhook configurations: FleetConfig v1alpha1 has defaulting: true (requiring MutatingWebhookConfiguration), while Hub and Spoke v1beta1 resources have defaulting: false. MutatingWebhookConfiguration resources in the manifests serve the v1alpha1 FleetConfig, not the v1beta1 Hub/Spoke resources.

Applied to files:

  • fleetconfig-controller/charts/fleetconfig-controller/README.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: e2e (fleetconfig-controller) / e2e
  • GitHub Check: e2e (fleetconfig-controller) / e2e
  • GitHub Check: test (fleetconfig-controller) / Ensure PR is reviewable & run tests
  • GitHub Check: test (fleetconfig-controller) / Run Helm Chart Tests
  • GitHub Check: test (fleetconfig-controller) / Ensure PR is reviewable & run tests
🔇 Additional comments (1)
fleetconfig-controller/charts/fleetconfig-controller/README.md (1)

161-161: Documentation update for image tag is correct.

Line 161 correctly updates the image tag parameter documentation value from v0.1.2 to v0.1.3, consistent with the controller version bump.

However, per the PR objectives, this change should include adding helm.sh/resource-policy: keep annotations to managed cluster set Namespaces in topology-resources.yaml and updating the image tag in values.yaml. Please confirm that all modified files from this PR have been provided for review.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@TylerGillson TylerGillson merged commit 4a6b610 into open-cluster-management-io:main Nov 3, 2025
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant