Skip to content

Conversation

@kvaps
Copy link
Member

@kvaps kvaps commented Mar 5, 2025

Signed-off-by: Andrei Kvapil kvapss@gmail.com

Summary by CodeRabbit

  • New Features

    • Introduced dynamic IP address management support.
    • Enabled comprehensive lifecycle hooks that trigger during both installation and upgrades.
    • Expanded configuration options with new fields for flexible deployments and customizations.
  • Chores

    • Upgraded the application and chart versions.
    • Improved deployment settings with enhanced health checks, diagnostic endpoints, and service account management.
    • Updated provider versions to enhance overall stability and performance.

Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
@kvaps kvaps requested a review from lllamnyp as a code owner March 5, 2025 13:08
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Mar 5, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 5, 2025

Walkthrough

This pull request updates the Cluster API Operator’s Helm chart by incrementing version numbers and modifying hook annotations to include both post-install and post-upgrade events. It also expands the conditional logic in several templates to support additional configuration via a new manager field and introduces a new IPAM provider template with input validation. Deployment configurations are enhanced with a new service account, revised command-line arguments, and updated health probe settings.

Changes

File(s) Change Summary
packages/.../charts/cluster-api-operator/Chart.yaml,
packages/.../charts/cluster-api-operator/values.yaml,
packages/.../providers.yaml
Updated version fields (e.g., appVersion, version, provider versions) and image tags; added new configuration fields (ipam, fetchConfig, watchConfigSecret); modified health and diagnostics addresses.
packages/.../templates/{addon,bootstrap,core-conditions,infra-conditions}.yaml Modified helm hook annotations from "post-install" to "post-install,post-upgrade" to trigger hooks on both install and upgrade events.
packages/.../templates/{control-plane,core,infra}.yaml Expanded conditional rendering in the spec sections to include $.Values.manager, enabling additional configuration such as manager featureGates and other settings.
packages/.../templates/deployment.yaml,
packages/.../templates/ipam.yaml
Enhanced deployment configuration with a new service account, updated command-line arguments (including --watch-configsecret), and added liveness/readiness probes; introduced a new IPAM provider template with input validation and dynamic spec rendering.

Sequence Diagram(s)

sequenceDiagram
    participant U as User
    participant H as Helm
    participant T as Template Engine
    participant K as Kubernetes API
    participant O as Operator

    U->>H: Execute helm install/upgrade
    H->>T: Process chart templates
    T->>T: Evaluate conditions (e.g., manager, ipam, configSecret)
    T->>H: Render manifests with updated hook annotations
    H->>K: Apply manifests to cluster
    K->>K: Trigger post-install and post-upgrade hooks
    K->>O: Operator monitors and reports health via probes
Loading
sequenceDiagram
    participant U as User
    participant H as Helm
    participant I as IPAM Template Processor
    participant K as Kubernetes API

    U->>H: Initiate helm upgrade with ipam value
    H->>I: Process ipam.yaml template
    I->>I: Validate and parse ipam input (namespace, name, version)
    I->>H: Render Namespace & IPAMProvider resources with annotations
    H->>K: Deploy rendered IPAM resources to cluster
Loading

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • themoriarti

Poem

I’m a little rabbit, fast on my feet,
Hopping through code where changes meet.
Hooks now leap on upgrade and install,
Conditional magic added to enthrall.
Celebrate this release with a joyful beat! 🐇✨


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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@dosubot dosubot bot added the enhancement New feature or request label Mar 5, 2025
Copy link
Contributor

@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: 0

🧹 Nitpick comments (7)
packages/system/capi-operator/charts/cluster-api-operator/templates/ipam.yaml (2)

1-23: IPAM Provider Input Parsing and Validation
This section splits the .Values.ipam string using split for both ";" and ":" to extract the namespace, provider name, and version. Ensure that accessing list elements using dot notation (e.g. $ipamArgs._0) is supported in your templating engine; if not, consider using the index function (e.g. index $ipamArgs 0) for improved robustness.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 2-2: syntax error: expected the node content, but found '-'

(syntax)


[warning] 11-11: wrong indentation: expected 0 but found 2

(indentation)


[warning] 12-12: wrong indentation: expected 0 but found 2

(indentation)


[warning] 13-13: wrong indentation: expected 0 but found 2

(indentation)


[warning] 14-14: too many spaces after hyphen

(hyphens)


[warning] 15-15: wrong indentation: expected 0 but found 2

(indentation)


[warning] 16-16: wrong indentation: expected 0 but found 2

(indentation)


[warning] 17-17: wrong indentation: expected 0 but found 2

(indentation)


[warning] 18-18: too many spaces after hyphen

(hyphens)


[warning] 19-19: wrong indentation: expected 0 but found 2

(indentation)


[warning] 20-20: wrong indentation: expected 0 but found 2

(indentation)


[warning] 22-22: wrong indentation: expected 0 but found 2

(indentation)


49-61: Manager Configuration and Feature Gates
This block conditionally includes a manager section and iterates over feature gates when available. Consider simplifying the logic—for example, by directly indexing into $.Values.manager.featureGates using the IPAM name—to improve clarity and maintainability.

packages/system/capi-operator/charts/cluster-api-operator/templates/addon.yaml (1)

1-23: Addon Provider Input Parsing and Validation
This section splits the .Values.addon string to extract the namespace, name, and version for addon providers. As with the IPAM template, verify that using dot notation (e.g. $addonArgs._0) correctly accesses list elements—consider using the index function if needed for better reliability.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 2-2: syntax error: expected the node content, but found '-'

(syntax)


[warning] 11-11: wrong indentation: expected 0 but found 2

(indentation)


[warning] 12-12: wrong indentation: expected 0 but found 2

(indentation)


[warning] 13-13: wrong indentation: expected 0 but found 2

(indentation)


[warning] 14-14: too many spaces after hyphen

(hyphens)


[warning] 15-15: wrong indentation: expected 0 but found 2

(indentation)


[warning] 16-16: wrong indentation: expected 0 but found 2

(indentation)


[warning] 17-17: wrong indentation: expected 0 but found 2

(indentation)


[warning] 18-18: too many spaces after hyphen

(hyphens)


[warning] 19-19: wrong indentation: expected 0 but found 2

(indentation)


[warning] 20-20: wrong indentation: expected 0 but found 2

(indentation)


[warning] 22-22: wrong indentation: expected 0 but found 2

(indentation)

packages/system/capi-operator/charts/cluster-api-operator/templates/infra-conditions.yaml (1)

45-70: ControlPlaneProvider Resource with Optional Manager Configuration
The ControlPlaneProvider for kubeadm is defined with proper metadata and hook annotations. The spec block conditionally includes a manager configuration with nested feature gates. Please verify the indentation and ensure the conditional manager block renders correctly in all scenarios.

packages/system/capi-operator/charts/cluster-api-operator/values.yaml (1)

8-8: New Field 'ipam' Initialization
The new ipam field is introduced as an empty string. Consider adding inline documentation or comments to describe the expected values and usage for clarity.

packages/system/capi-operator/charts/cluster-api-operator/templates/deployment.yaml (1)

100-105: Diagnostics Port Extraction Logic
The logic extracts the port number from $.Values.diagnosticsAddress if it contains a colon, converting the substring into an integer. Consider adding error handling or validation to ensure that the splitting and conversion behave as expected when the format is nonstandard.

packages/system/capi-operator/charts/cluster-api-operator/templates/control-plane.yaml (1)

47-59: Manager Configuration Block – Consider Simplification
A new block has been added to conditionally include a manager section when $.Values.manager is defined and contains a matching key for the current control plane name. While functionally correct, the nested conditions (first using hasKey and then checking eq within the loop) might be streamlined for improved readability. Consider refactoring to combine the conditional logic if possible, ensuring that the YAML output remains correctly indented in all cases.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ec603bc and e2cab5e.

📒 Files selected for processing (12)
  • packages/system/capi-operator/charts/cluster-api-operator/Chart.yaml (1 hunks)
  • packages/system/capi-operator/charts/cluster-api-operator/templates/addon.yaml (2 hunks)
  • packages/system/capi-operator/charts/cluster-api-operator/templates/bootstrap.yaml (2 hunks)
  • packages/system/capi-operator/charts/cluster-api-operator/templates/control-plane.yaml (2 hunks)
  • packages/system/capi-operator/charts/cluster-api-operator/templates/core-conditions.yaml (2 hunks)
  • packages/system/capi-operator/charts/cluster-api-operator/templates/core.yaml (2 hunks)
  • packages/system/capi-operator/charts/cluster-api-operator/templates/deployment.yaml (4 hunks)
  • packages/system/capi-operator/charts/cluster-api-operator/templates/infra-conditions.yaml (4 hunks)
  • packages/system/capi-operator/charts/cluster-api-operator/templates/infra.yaml (3 hunks)
  • packages/system/capi-operator/charts/cluster-api-operator/templates/ipam.yaml (1 hunks)
  • packages/system/capi-operator/charts/cluster-api-operator/values.yaml (2 hunks)
  • packages/system/capi-providers/templates/providers.yaml (2 hunks)
✅ Files skipped from review due to trivial changes (1)
  • packages/system/capi-operator/charts/cluster-api-operator/Chart.yaml
🧰 Additional context used
🪛 YAMLlint (1.35.1)
packages/system/capi-operator/charts/cluster-api-operator/templates/ipam.yaml

[error] 2-2: syntax error: expected the node content, but found '-'

(syntax)


[warning] 11-11: wrong indentation: expected 0 but found 2

(indentation)


[warning] 12-12: wrong indentation: expected 0 but found 2

(indentation)


[warning] 13-13: wrong indentation: expected 0 but found 2

(indentation)


[warning] 14-14: too many spaces after hyphen

(hyphens)


[warning] 15-15: wrong indentation: expected 0 but found 2

(indentation)


[warning] 16-16: wrong indentation: expected 0 but found 2

(indentation)


[warning] 17-17: wrong indentation: expected 0 but found 2

(indentation)


[warning] 18-18: too many spaces after hyphen

(hyphens)


[warning] 19-19: wrong indentation: expected 0 but found 2

(indentation)


[warning] 20-20: wrong indentation: expected 0 but found 2

(indentation)


[warning] 22-22: wrong indentation: expected 0 but found 2

(indentation)

🔇 Additional comments (39)
packages/system/capi-providers/templates/providers.yaml (3)

8-8: Updated Cluster API Provider Version
The version for the Cluster API provider has been updated to v1.9.5. This revision aligns the provider with the latest releases and should help ensure compatibility with updated operator features.


16-16: Updated Kamaji Provider Version
The Kamaji control plane provider version is now v0.14.1. This change reflects the necessary updates and improvements implemented in the new release.


31-31: Updated Kubeadm Provider Version
The kubeadm bootstrap provider version has been updated to v1.9.5, ensuring consistency with the upgraded Cluster API provider versions across the configuration.

packages/system/capi-operator/charts/cluster-api-operator/templates/ipam.yaml (5)

24-32: Namespace Resource Definition for IPAM Providers
The Namespace resource is defined with appropriate Helm hook annotations (post-install,post-upgrade) and sync-wave settings. While static analysis tools flag indentation issues, these are likely false positives due to templating syntax. Please verify the final rendered YAML with helm template.


34-42: IPAMProvider Resource Definition
The IPAMProvider resource is properly set up with the necessary metadata and hook annotations. The conditional inclusion of a spec block based on the presence of optional fields is a neat approach to maintain flexibility.


43-48: Conditional Spec Block for Version Assignment
The template conditionally adds the version field only if $ipamVersion is provided. This design keeps the resource definition clean when versioning is not specified.


62-71: Optional ConfigSecret and Additional Deployments Blocks
The conditional inclusion of configSecret and additionalDeployments sections is well implemented. The use of toYaml piped with nindent ensures proper formatting of complex values.


1-74: Note on Static Analysis Warnings
Several static analysis warnings (e.g., indentation and syntax issues) are reported by YAMLlint. These are likely false positives due to the Helm templating syntax. Please validate the rendered YAML output using helm template to ensure correctness.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 2-2: syntax error: expected the node content, but found '-'

(syntax)


[warning] 11-11: wrong indentation: expected 0 but found 2

(indentation)


[warning] 12-12: wrong indentation: expected 0 but found 2

(indentation)


[warning] 13-13: wrong indentation: expected 0 but found 2

(indentation)


[warning] 14-14: too many spaces after hyphen

(hyphens)


[warning] 15-15: wrong indentation: expected 0 but found 2

(indentation)


[warning] 16-16: wrong indentation: expected 0 but found 2

(indentation)


[warning] 17-17: wrong indentation: expected 0 but found 2

(indentation)


[warning] 18-18: too many spaces after hyphen

(hyphens)


[warning] 19-19: wrong indentation: expected 0 but found 2

(indentation)


[warning] 20-20: wrong indentation: expected 0 but found 2

(indentation)


[warning] 22-22: wrong indentation: expected 0 but found 2

(indentation)

packages/system/capi-operator/charts/cluster-api-operator/templates/addon.yaml (4)

24-32: Namespace Resource for Addon Provider
The Namespace for the addon provider is defined with the expected Helm hook annotations (post-install,post-upgrade) and sync-wave settings. Confirm that the final YAML output renders with correct indentation despite potential static analysis warnings.


34-42: AddonProvider Resource Definition
The AddonProvider resource is structured correctly with appropriate metadata and hook annotations. The conditional inclusion of a spec block is used effectively to handle optional configurations.


43-48: Conditional Spec Block for Version Assignment
Including the version field only when $addonVersion is provided ensures flexibility. This conditional rendering keeps the resource definition concise when version information is not necessary.


49-54: Optional Secret Configuration for Addon Provider
The template conditionally adds secretName and secretNamespace if defined in the values. This allows for secure customization of the provider’s secret settings.

packages/system/capi-operator/charts/cluster-api-operator/templates/infra-conditions.yaml (2)

4-32: Bootstrap Components Deployment for Infrastructure
This section deploys the bootstrap Namespace and BootstrapProvider resources when .Values.bootstrap is not set. The Helm hook annotations and sync-wave settings are updated to trigger on both installation and upgrade events, and the optional configSecret configuration adds flexibility.


34-44: Control Plane Namespace Definition
A Namespace for the control plane (capi-kubeadm-control-plane-system) is defined with updated hook annotations. This aligns with the multi-environment deployment strategy and ensures consistent handling by Helm.

packages/system/capi-operator/charts/cluster-api-operator/templates/core-conditions.yaml (3)

1-3: Conditional Deployment for Core Components
The template conditionally deploys core components only if they have not been explicitly set via .Values.core. This logic helps prevent resource conflicts in multi-component deployments.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)


5-11: Namespace Definition for Core Components
The Namespace (capi-system) for core components is defined with updated Helm hook annotations (post-install,post-upgrade) and a sync-wave setting. Verify that the final rendered YAML respects the intended indentation despite warnings from static analysis tools.


13-21: CoreProvider Resource Definition
The CoreProvider resource for cluster-api is configured with the latest hook annotations and optionally includes a configSecret block if provided. This configuration is consistent with the overall update strategy in the Helm chart.

packages/system/capi-operator/charts/cluster-api-operator/values.yaml (5)

11-11: Introduction of 'fetchConfig' Configuration
A new fetchConfig field (an empty map) has been added. Please verify that downstream templates properly validate and consume this field, and document its expected schema if necessary.


24-24: Manager Image Tag Update
The tag for the manager image is updated to v0.17.0, which aligns with the overall version bump. Ensure that all related documentation and image references are updated accordingly.


28-28: Diagnostics Address Format Update
The diagnosticsAddress is now set to ":8443", standardizing the format with a colon prefix. Verify that all components expecting this value are compatible with the new format.


29-29: Health Address Port Change
The healthAddr has been updated to ":9440". Ensure that any health probes or network configurations correctly reference this new port.


31-31: New 'watchConfigSecret' Flag Added
The new boolean flag watchConfigSecret is introduced (set to false). Confirm that its conditional usage in deployment templates triggers the intended behavior and that adequate tests cover this functionality.

packages/system/capi-operator/charts/cluster-api-operator/templates/core.yaml (3)

28-29: Hook Annotation Update for Namespace
The annotation "helm.sh/hook": "post-install,post-upgrade" now ensures that the Namespace resource is processed during both installation and upgrade events.


38-39: Hook Annotation Update for CoreProvider
The CoreProvider’s hook annotation is updated to trigger on both post-install and post-upgrade events, ensuring its lifecycle is managed consistently.


41-41: Enhanced Conditional Logic for Spec Rendering
The condition for rendering the spec field now includes $.Values.manager (in addition to $coreVersion and $.Values.configSecret.name). Verify that this inclusion correctly handles scenarios where manager configurations are provided.

packages/system/capi-operator/charts/cluster-api-operator/templates/bootstrap.yaml (2)

29-29: Hook Annotation Update for Namespace in Bootstrap Provider
The Namespace metadata now uses "helm.sh/hook": "post-install,post-upgrade", which will ensure it is reprocessed during upgrades as well as initial installs.


39-39: Hook Annotation Update for BootstrapProvider
The BootstrapProvider's metadata annotation is updated to trigger on both post-install and post-upgrade events, improving lifecycle management.

packages/system/capi-operator/charts/cluster-api-operator/templates/deployment.yaml (4)

50-51: New Service Account Configuration
The deployment now specifies serviceAccountName: capi-operator-manager and enables automountServiceAccountToken: true. This configuration ensures the operator pod runs with a dedicated service account. Please confirm that the corresponding RBAC roles and service account are configured in the cluster.


74-76: Conditional Watch ConfigSecret Argument
The new --watch-configsecret flag is conditionally added based on .Values.watchConfigSecret. Verify that the operator’s command-line parser correctly recognizes and uses this flag.


125-125: Termination Message Policy Set
The addition of terminationMessagePolicy: FallbackToLogsOnError enhances error reporting, ensuring that logs are appropriately captured on container termination.


126-149: Enhanced Health Probes Configuration
New liveness and readiness probes are configured using a dynamically extracted port from healthAddr (with a default of 9440). Please confirm that the endpoints /healthz and /readyz are correctly implemented in the operator and that the probe parameters (delays, timeouts) meet the intended operational requirements.

packages/system/capi-operator/charts/cluster-api-operator/templates/infra.yaml (5)

29-30: Namespace Hook Annotation Update
The Namespace metadata now uses "helm.sh/hook": "post-install,post-upgrade", ensuring that namespace resources are revisited during upgrade events.


40-41: InfrastructureProvider Hook Annotation Update
The InfrastructureProvider’s metadata annotation is updated to trigger on both post-install and post-upgrade events, which improves the resource’s lifecycle management.


43-43: Enhanced Spec Rendering Conditional
The condition to render the spec field now includes $.Values.additionalDeployments along with the existing checks. Please verify that this expanded condition correctly triggers spec rendering in all intended scenarios.


62-71: Dynamic FetchConfig Integration
A new block renders the fetchConfig section if $.Values.fetchConfig is a map containing a key matching the infrastructure name. Confirm that the structure of entries under fetchConfig meets the expected schema for downstream processing.


79-81: Additional Deployments Configuration
The new conditional block for additionalDeployments allows extra deployment settings to be injected. Ensure that the YAML rendered by toYaml is correctly indented and valid within the overall manifest structure.

packages/system/capi-operator/charts/cluster-api-operator/templates/control-plane.yaml (3)

29-30: Update Namespace Hook Annotations
The hook annotation on the Namespace resource has been updated to include both post-install and post-upgrade events. This change ensures that the hook is triggered during both the installation and upgrade phases.


38-40: Update ControlPlaneProvider Hook Annotations
Similarly, the ControlPlaneProvider resource now uses the updated hook annotation (post-install,post-upgrade) with an appropriate hook weight. This update helps maintain consistency across resources for lifecycle events.


41-43: Expanded Conditional for Spec Rendering
The conditional check for rendering the spec section now includes an additional value ($.Values.manager). This accommodates extra configuration options and aligns with changes across other related templates.

@kvaps kvaps merged commit 47dfaaa into main Mar 5, 2025
0 of 2 checks passed
@kvaps kvaps deleted the upd-capi-providers branch March 5, 2025 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants