-
Notifications
You must be signed in to change notification settings - Fork 119
[robotlb] add Hetzner Robotlb balancer #1233
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Warning Rate limit exceeded@IvanHunters has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 26 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 selected for processing (4)
WalkthroughThis change introduces a new Helm chart package for deploying the Hetzner RobotLB operator. It adds chart metadata, default configuration values, Kubernetes manifests for deployment and RBAC, Helm helper templates, a Makefile for chart management, and supporting files for chart packaging and deployment. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm
participant Kubernetes
participant RobotLB Operator
User->>Helm: Install hetzner-robotlb chart
Helm->>Kubernetes: Apply manifests (Deployment, ServiceAccount, RBAC)
Kubernetes->>RobotLB Operator: Deploy pod(s) per configuration
User->>RobotLB Operator: Configure via README instructions
Estimated code review effort2 (~15 minutes) Possibly related PRs
Suggested labels
Suggested reviewers
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @IvanHunters, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces native support for the Hetzner Robotlb load balancer within the platform. It integrates the robotlb
Helm chart into our core distribution bundles, allowing for seamless deployment and management of load-balanced services on Hetzner Cloud infrastructure.
Highlights
- Hetzner Robotlb Integration: I've added the
hetzner-robotlb
component as an optional release to thedistro-full
,distro-hosted
,paas-full
, andpaas-hosted
bundles. This makes the Hetzner Robotlb load balancer available for deployment across various platform configurations. - New Helm Chart Package: A new package,
packages/system/hetzner-robotlb
, has been introduced. This package acts as a wrapper for the upstreamrobotlb
Helm chart, including its necessary Kubernetes Deployment, RBAC (ClusterRole and ClusterRoleBinding), and Service Account configurations. - Automated Chart Management: I've included a Makefile within the new package that automates the process of pulling the
robotlb
Helm chart directly from its OCI registry. This ensures that the platform'shetzner-robotlb
package can easily stay up-to-date with upstream changes.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces the hetzner-robotlb
Helm chart to provide load balancing capabilities via Hetzner Robot. My review identified a critical issue with the resource definition in the values.yaml
that would prevent the deployment from starting. Additionally, there are some high-severity issues regarding missing dependencies in the bundle definitions, which could lead to race conditions during deployment. Some medium-severity suggestions are also provided to improve logging defaults.
cpu: "" | ||
memory: "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The structure for resources
is incorrect. The Kubernetes container resource block expects limits
and/or requests
fields, not cpu
and memory
directly at the top level. This configuration will cause the deployment to be rejected by the Kubernetes API server.
To specify no resources, you should use an empty map ({}
). If you want to define resources, the structure should be:
resources:
requests:
cpu: 100m
memory: 128Mi
limits:
cpu: 100m
memory: 128Mi
{}
fullnameOverride: "" | ||
|
||
envs: | ||
ROBOTLB_LOG_LEVEL: "DEBUG" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default log level is set to DEBUG
. This is typically too verbose for production environments and can generate a large volume of logs, potentially impacting performance and increasing costs. It's recommended to use INFO
as the default level. Users can override this to DEBUG
when they need to troubleshoot issues.
ROBOTLB_LOG_LEVEL: "INFO"
|
||
|
||
envs: | ||
ROBOTLB_LOG_LEVEL: "DEBUG" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this 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
🔭 Outside diff range comments (2)
packages/core/platform/bundles/paas-full.yaml (1)
418-424
: Add explicit dependsOn for hetzner-robotlb to ensure CNI readiness
- In
packages/core/platform/bundles/paas-full.yaml
, update thehetzner-robotlb
entry (around line 419) to declare a dependency on the CNI charts:- optional: true - chart: cozy-hetzner-robotlb - namespace: cozy-hetzner-robotlb + optional: true + dependsOn: [cilium,kubeovn] + chart: cozy-hetzner-robotlb + namespace: cozy-hetzner-robotlb
- Since most network-level components (e.g. Metallb, GPU-operator) already specify
dependsOn: [cilium,kubeovn]
, this ensures Helm waits for the CNI to be ready before installingrobotlb
.- If the upstream
cozy-hetzner-robotlb
chart uses webhooks or installs CRDs, you may also need to includecert-manager
(or other prerequisites) in itsdependsOn
. Please verify the chart’s requirements and adjust accordingly.packages/system/hetzner-robotlb/charts/robotlb/values.yaml (1)
1-74
: Missingreplicas
default breaks the Deployment template
deployment.yaml
expects.Values.replicas
, but the key is absent here. Add it to avoid the invalid<no value>
rendered earlier.@@ fullnameOverride: "" +# Number of robotlb replicas +replicas: 1 + envs:
♻️ Duplicate comments (1)
packages/core/platform/bundles/distro-hosted.yaml (1)
175-179
: SamedependsOn
caveat as noted inpaas-full.yaml
See the earlier comment – the new release should likely depend on
cilium
/kubeovn
to avoid install ordering issues.
🧹 Nitpick comments (6)
packages/system/hetzner-robotlb/Chart.yaml (1)
1-3
: Add minimal recommended chart metadataHelm will happily install the chart as-is, but the chart is missing a
description
andtype
(and optionallyappVersion
/maintainers
). Adding them improveshelm search
output and linting.apiVersion: v2 name: hetzner-robotlb +description: Helm umbrella chart for the Hetzner RobotLB operator +type: application +# The chart version is updated automatically during CI version: 0.1.3 # Placeholder, the actual version will be automatically set during the build processpackages/system/hetzner-robotlb/charts/robotlb/Chart.yaml (1)
1-6
: Polish wording & add optional chart metadataNitpicks:
- “loadbalancer” → “load balancer”.
- Consider adding
home
,sources
, andmaintainers
to meet chart-testing best practices.-description: A Helm chart for robotlb (loadbalancer on hetzner cloud). +description: RobotLB operator – layer-4 load balancer for Hetzner dedicated servers. + +# Optional but recommended +home: https://github.com/intreecom/robotlb +sources: + - https://github.com/intreecom/robotlb +maintainers: + - name: IvanHunters + email: ivan@example.compackages/system/hetzner-robotlb/charts/robotlb/templates/NOTES.txt (1)
1-4
: Fix minor grammar & formatting in post-install notes-The RobotLB Operator was successfully installed. -Please follow the readme to create loadbalanced services. - -README: https://github.com/intreecom/robotlb +RobotLB Operator was successfully installed. + +Follow the README to create load-balanced services: +https://github.com/intreecom/robotlbpackages/system/hetzner-robotlb/charts/robotlb/templates/role.yaml (1)
6-6
: PuttoYaml
on the next line for cleaner rendering & easier diffsAlthough
{{-
will swallow the newline, having the template directive on its own line keeps the generated YAML readable and avoids accidental white-space issues when the surrounding logic changes.-rules: {{- toYaml .Values.serviceAccount.permissions | nindent 2 }} +rules: +{{- toYaml .Values.serviceAccount.permissions | nindent 2 }}packages/system/hetzner-robotlb/charts/robotlb/templates/deployment.yaml (1)
29-35
: Avoid emitting empty securityContext blocksWhen the map is empty the manifest will contain
securityContext: {}
.
While valid YAML, many teams prefer to omit empty fields altogether to keep manifests concise.- securityContext: - {{- toYaml .Values.podSecurityContext | nindent 8 }} + {{- with .Values.podSecurityContext }} + securityContext: + {{- toYaml . | nindent 8 }} + {{- end }}packages/system/hetzner-robotlb/values.yaml (1)
1-81
: Duplicate values files introduce drift – consolidate or document precedenceThis top-level
values.yaml
diverges from the chart-local one (e.g., it containsreplicas
, differentresources
schema). Unless Helmfile/Kustomize deliberately layers this file over the chart, the two may fall out of sync.Consider:
- Deleting the duplicate.
- Or, documenting clearly in the bundle how this outer file is merged so future edits happen in one place.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
packages/core/platform/bundles/distro-full.yaml
(1 hunks)packages/core/platform/bundles/distro-hosted.yaml
(1 hunks)packages/core/platform/bundles/paas-full.yaml
(1 hunks)packages/core/platform/bundles/paas-hosted.yaml
(1 hunks)packages/system/hetzner-robotlb/Chart.yaml
(1 hunks)packages/system/hetzner-robotlb/Makefile
(1 hunks)packages/system/hetzner-robotlb/charts/robotlb/.helmignore
(1 hunks)packages/system/hetzner-robotlb/charts/robotlb/Chart.yaml
(1 hunks)packages/system/hetzner-robotlb/charts/robotlb/templates/NOTES.txt
(1 hunks)packages/system/hetzner-robotlb/charts/robotlb/templates/_helpers.tpl
(1 hunks)packages/system/hetzner-robotlb/charts/robotlb/templates/deployment.yaml
(1 hunks)packages/system/hetzner-robotlb/charts/robotlb/templates/role.yaml
(1 hunks)packages/system/hetzner-robotlb/charts/robotlb/templates/serviceaccount.yaml
(1 hunks)packages/system/hetzner-robotlb/charts/robotlb/values.yaml
(1 hunks)packages/system/hetzner-robotlb/values.yaml
(1 hunks)
🧠 Learnings (2)
packages/system/hetzner-robotlb/charts/robotlb/templates/role.yaml (1)
Learnt from: lllamnyp
PR: #1160
File: packages/system/hetzner-robotlb/charts/robotlb/templates/role.yaml:6-8
Timestamp: 2025-07-11T06:28:13.696Z
Learning: In Helm templates, the {{-
directive chomps all leading whitespace including newlines back to the previous content, so {{- toYaml .Values.something | nindent 2 }}
will render correctly even with apparent indentation issues. However, for better style, it's cleaner to put the template directive on the same line as the parent key (e.g., rules: {{- toYaml .Values.serviceAccount.permissions | nindent 2 }}
).
packages/system/hetzner-robotlb/charts/robotlb/templates/_helpers.tpl (1)
Learnt from: lllamnyp
PR: #1160
File: packages/system/hetzner-robotlb/charts/robotlb/templates/role.yaml:6-8
Timestamp: 2025-07-11T06:28:13.696Z
Learning: In Helm templates, the {{-
directive chomps all leading whitespace including newlines back to the previous content, so {{- toYaml .Values.something | nindent 2 }}
will render correctly even with apparent indentation issues. However, for better style, it's cleaner to put the template directive on the same line as the parent key (e.g., rules: {{- toYaml .Values.serviceAccount.permissions | nindent 2 }}
).
🪛 YAMLlint (1.37.1)
packages/system/hetzner-robotlb/charts/robotlb/templates/serviceaccount.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/hetzner-robotlb/charts/robotlb/templates/role.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/hetzner-robotlb/charts/robotlb/templates/deployment.yaml
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
🪛 checkmake (0.2.2)
packages/system/hetzner-robotlb/Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
🧰 Additional context used
🧠 Learnings (2)
packages/system/hetzner-robotlb/charts/robotlb/templates/role.yaml (1)
Learnt from: lllamnyp
PR: #1160
File: packages/system/hetzner-robotlb/charts/robotlb/templates/role.yaml:6-8
Timestamp: 2025-07-11T06:28:13.696Z
Learning: In Helm templates, the {{-
directive chomps all leading whitespace including newlines back to the previous content, so {{- toYaml .Values.something | nindent 2 }}
will render correctly even with apparent indentation issues. However, for better style, it's cleaner to put the template directive on the same line as the parent key (e.g., rules: {{- toYaml .Values.serviceAccount.permissions | nindent 2 }}
).
packages/system/hetzner-robotlb/charts/robotlb/templates/_helpers.tpl (1)
Learnt from: lllamnyp
PR: #1160
File: packages/system/hetzner-robotlb/charts/robotlb/templates/role.yaml:6-8
Timestamp: 2025-07-11T06:28:13.696Z
Learning: In Helm templates, the {{-
directive chomps all leading whitespace including newlines back to the previous content, so {{- toYaml .Values.something | nindent 2 }}
will render correctly even with apparent indentation issues. However, for better style, it's cleaner to put the template directive on the same line as the parent key (e.g., rules: {{- toYaml .Values.serviceAccount.permissions | nindent 2 }}
).
🪛 YAMLlint (1.37.1)
packages/system/hetzner-robotlb/charts/robotlb/templates/serviceaccount.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/hetzner-robotlb/charts/robotlb/templates/role.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/hetzner-robotlb/charts/robotlb/templates/deployment.yaml
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
🪛 checkmake (0.2.2)
packages/system/hetzner-robotlb/Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
⏰ 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). (1)
- GitHub Check: Build
🔇 Additional comments (5)
packages/core/platform/bundles/paas-hosted.yaml (1)
242-246
: Verify that prerequisites for RobotLB are metThe new release is marked
optional: true
, but there are nodependsOn
entries.
If the operator requires CRDs (e.g.cert-manager
,external-secrets
) or privileged security context, add the corresponding dependencies /privileged: true
flag to avoid ordering issues.Please confirm and update if necessary.
packages/core/platform/bundles/distro-full.yaml (1)
262-266
: Same dependency check as in paas-hostedEnsure RobotLB’s chart installs cleanly in a cold cluster: add
dependsOn
orprivileged
flags if the operator expects CRDs, webhooks, or host-network capabilities.Keeping release blocks consistent across bundles prevents subtle drift.
packages/system/hetzner-robotlb/charts/robotlb/.helmignore (1)
1-24
: 👍 Good use of.helmignore
The ignore list is comprehensive and keeps chart packages lean.
packages/system/hetzner-robotlb/charts/robotlb/templates/serviceaccount.yaml (1)
1-13
: YAMLlint false positive – template front-matter is validThe leading
{{- if ... -}}
control statement produces either a ServiceAccount manifest or nothing, which confuses plain YAML linters. No action required.packages/system/hetzner-robotlb/charts/robotlb/templates/_helpers.tpl (1)
56-61
: ServiceAccount helper LGTMLogic correctly falls back to
default
whencreate
is false.
packages/system/hetzner-robotlb/charts/robotlb/templates/deployment.yaml
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see my comments in #1160. This pull request has many of the same issues that need to be addressed before merging. E.g. ./values.yaml
should not be a copy of ./charts/robotlb/values.yaml
.
There was a problem hiding this 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
🧹 Nitpick comments (1)
packages/system/hetzner-robotlb/charts/robotlb/templates/deployment.yaml (1)
37-40
: Missing liveness / readiness probes
The deployment runs a network-facing binary but lacks health probes. This impedes rollout/auto-healing safety.Example addition (tweak paths/ports to match the app):
command: - /usr/local/bin/robotlb + readinessProbe: + httpGet: + path: /healthz + port: 8080 + initialDelaySeconds: 5 + periodSeconds: 10 + livenessProbe: + httpGet: + path: /healthz + port: 8080 + initialDelaySeconds: 30 + periodSeconds: 10
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/system/hetzner-robotlb/charts/robotlb/templates/deployment.yaml
(1 hunks)packages/system/hetzner-robotlb/charts/robotlb/values.yaml
(1 hunks)packages/system/hetzner-robotlb/values.yaml
(1 hunks)
🧠 Learnings (1)
packages/system/hetzner-robotlb/charts/robotlb/templates/deployment.yaml (1)
Learnt from: lllamnyp
PR: #1160
File: packages/system/hetzner-robotlb/charts/robotlb/templates/role.yaml:6-8
Timestamp: 2025-07-11T06:28:13.696Z
Learning: In Helm templates, the {{-
directive chomps all leading whitespace including newlines back to the previous content, so {{- toYaml .Values.something | nindent 2 }}
will render correctly even with apparent indentation issues. However, for better style, it's cleaner to put the template directive on the same line as the parent key (e.g., rules: {{- toYaml .Values.serviceAccount.permissions | nindent 2 }}
).
🪛 YAMLlint (1.37.1)
packages/system/hetzner-robotlb/charts/robotlb/templates/deployment.yaml
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
✅ Files skipped from review due to trivial changes (1)
- packages/system/hetzner-robotlb/values.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/system/hetzner-robotlb/charts/robotlb/values.yaml
🧰 Additional context used
🧠 Learnings (1)
packages/system/hetzner-robotlb/charts/robotlb/templates/deployment.yaml (1)
Learnt from: lllamnyp
PR: #1160
File: packages/system/hetzner-robotlb/charts/robotlb/templates/role.yaml:6-8
Timestamp: 2025-07-11T06:28:13.696Z
Learning: In Helm templates, the {{-
directive chomps all leading whitespace including newlines back to the previous content, so {{- toYaml .Values.something | nindent 2 }}
will render correctly even with apparent indentation issues. However, for better style, it's cleaner to put the template directive on the same line as the parent key (e.g., rules: {{- toYaml .Values.serviceAccount.permissions | nindent 2 }}
).
🪛 YAMLlint (1.37.1)
packages/system/hetzner-robotlb/charts/robotlb/templates/deployment.yaml
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (1)
packages/system/hetzner-robotlb/charts/robotlb/templates/deployment.yaml (1)
8-8
: Replica default fix looks good
Nice catch addingdefault 1
– this removes thereplicas: <no value>
rendering issue flagged in the previous review.
packages/system/hetzner-robotlb/charts/robotlb/templates/deployment.yaml
Show resolved
Hide resolved
packages/system/hetzner-robotlb/charts/robotlb/templates/deployment.yaml
Show resolved
Hide resolved
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> Co-authored-by: Ahmad Murzahmatov <gwynbleidd2106@yandex.com>
8867a79
to
7c8e3b5
Compare
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
7c8e3b5
to
3612bbd
Compare
What this PR does
Release note
Summary by CodeRabbit
New Features
Chores
Co-authored-by: Ahmad Murzahmatov <gwynbleidd2106@yandex.com>