Reuse networking configuration from the node if exists #53
Conversation
|
Warning Rate limit exceeded@kvaps has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 43 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
WalkthroughThis update modifies Helm chart helper templates for network interface configuration. It introduces conditional logic to include either an existing interfaces configuration or a default setup, depending on whether an existing configuration is detected. The floating IP assignment is now more restrictive, applying only when both a floating IP is set and the machine type is "controlplane." Additionally, a new helper is added to the Changes
Sequence Diagram(s)sequenceDiagram
participant HelmTemplate
participant Cluster
participant MachineConfig
HelmTemplate->>Cluster: Lookup machineconfig (v1alpha1)
Cluster-->>HelmTemplate: Return resource (if exists)
HelmTemplate->>HelmTemplate: Extract .spec.machine.network.interfaces
alt interfaces found
HelmTemplate->>HelmTemplate: Render existing interfaces configuration
else
HelmTemplate->>HelmTemplate: Render default interface configuration
end
alt floatingIP set and type == "controlplane"
HelmTemplate->>HelmTemplate: Add VIP block with floating IP
end
Possibly related PRs
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pkg/generated/presets.gois excluded by!**/generated/**
📒 Files selected for processing (3)
charts/cozystack/templates/_helpers.tpl(1 hunks)charts/generic/templates/_helpers.tpl(1 hunks)charts/talm/templates/_helpers.tpl(1 hunks)
🔇 Additional comments (2)
8000charts/talm/templates/_helpers.tpl (1)
145-149: Well-implemented helper for discovering existing network interfacesThis new helper function correctly retrieves the existing network interfaces configuration from a machineconfig resource if it exists. The implementation properly handles the case when the resource or nested fields don't exist by defaulting to an empty list.
This is a good approach for reusing existing network configuration rather than always generating a new one, which aligns with the PR objective.
charts/cozystack/templates/_helpers.tpl (1)
50-62: Good conditional logic for network interfaces configurationThe implementation correctly uses the new helper to conditionally include either the existing interfaces configuration or the default setup, matching the implementation in the generic chart.
The floating IP assignment is now more restrictive, requiring both a floating IP value and the machine type to be "controlplane". This is a security improvement over the previous implementation.
Similar to the generic chart, I notice that the
$existingInterfacesConfigurationvariable is not explicitly defined in this file.Do both
charts/generic/templates/_helpers.tplandcharts/cozystack/templates/_helpers.tplget the$existingInterfacesConfigurationvariable from the same source? Helm variables are typically scoped to their template, so this variable should be defined or imported in both templates.
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Signed-off-by: Andrei Kvapil kvapss@gmail.com
Summary by CodeRabbit
New Features
Improvements