[vm] Always expose VMs with a service #1738

lllamnyp · 2025-12-19T11:39:39Z

What this PR does

When VMs are created without a public IP, no service is created for them and they have no in-cluster DNS name. This PR fixes this and resolves #1731.

Release note

[vm] Always expose VMs with at least a ClusterIP service.

Summary by CodeRabbit

Improvements
- Service templates now conditionally enable external-specific annotations and settings when external mode is enabled.
- External LoadBalancer deployments support richer port configuration (including whole-IP fallback), while internal services retain a single default port (65535).
- External traffic policy and node port allocation are applied only in external mode to preserve internal behavior.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

gemini-code-assist · 2025-12-19T11:39:51Z

Summary of Changes

Hello @lllamnyp, I'm Gemini Code Assist¹! 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 addresses a critical issue where Virtual Machines provisioned without a public IP address lacked an associated Kubernetes Service, consequently preventing them from having an in-cluster DNS name. By modifying the service templates, all VMs will now consistently be exposed with at least a ClusterIP service, ensuring proper internal discoverability and DNS resolution within the cluster.

Highlights

Service Creation Logic: The conditional logic that prevented a Kubernetes Service from being created for Virtual Machines (VMs) without a public IP has been removed. Services will now always be created for all VMs.
In-cluster DNS Resolution: This change ensures that all VMs, including those without public IPs, will have an in-cluster DNS name, resolving an issue where such VMs were not discoverable within the cluster.
Default Service Port: For VMs that are not exposed externally (i.e., without a public IP), a default service port of 65535 has been added to ensure the ClusterIP service is properly defined.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

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.

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. ↩

coderabbitai · 2025-12-19T11:39:53Z

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Two Helm Service templates were updated to gate annotations, LoadBalancer settings, and ports behind an .Values.external flag. When external: true the templates emit LoadBalancer-specific fields and dynamic ports (or fallback 65535); when external: false they emit a headless/internal Service with clusterIP: None and a single port 65535.

Changes

Cohort / File(s)	Summary
VM service templates `packages/apps/virtual-machine/templates/service.yaml`, `packages/apps/vm-instance/templates/service.yaml`	Add conditional blocks around annotations and external-related fields. When `.Values.external` is true: set `type: LoadBalancer`, include `externalTrafficPolicy: Local`, optionally `allocateLoadBalancerNodePorts`, and generate ports from `.Values.externalPorts` (or fallback to port 65535 for `externalMethod: "WholeIP"`). When false: set `clusterIP: None` and emit a single port 65535.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Areas to check:
- Correct branching between external: true (LoadBalancer) and external: false (headless ClusterIP)
- Templates that render ports: name/port/targetPort generation and the WholeIP fallback
- That annotations and allocateLoadBalancerNodePorts are only emitted when intended

Poem

🐇 I hopped through templates, tidy and neat,

External gates open, internal ones meet.
Ports now decide to shout or stay small,
ClusterIP None gives names to them all. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title '[vm] Always expose VMs with a service' accurately and concisely describes the main change—ensuring all VMs get a Service for DNS access, whether external or internal.
Linked Issues check	✅ Passed	The changes implement the core requirement from #1731: VMs without external=true now get clusterIP: None services for in-cluster DNS access, achieving FQDN-based naming parity.
Out of Scope Changes check	✅ Passed	All changes are directly focused on the linked objective; conditional logic gates external annotations and port handling while adding default 65535 port for non-external services.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/1731-vm-dns

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c8dc801 and ba50d01.

📒 Files selected for processing (2)

packages/apps/virtual-machine/templates/service.yaml (2 hunks)
packages/apps/vm-instance/templates/service.yaml (2 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-07-11T06:28:13.696Z

Learnt from: lllamnyp
Repo: cozystack/cozystack 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 }}`).

Applied to files:

packages/apps/virtual-machine/templates/service.yaml
packages/apps/vm-instance/templates/service.yaml

🪛 YAMLlint (1.37.1)

packages/apps/virtual-machine/templates/service.yaml

[error] 9-9: syntax error: could not find expected ':'

(syntax)

packages/apps/vm-instance/templates/service.yaml

[error] 9-9: syntax error: could not find expected ':'

(syntax)

🔇 Additional comments (6)

packages/apps/vm-instance/templates/service.yaml (3)

9-12: Conditional annotations look good.

The annotations are correctly gated behind .Values.external, ensuring they only apply to LoadBalancer services.

20-22: Headless service requirement addressed.

The addition of clusterIP: None when .Values.external is false correctly implements the headless service requirement from issue #1731, enabling DNS-based discovery for internal-only VMs.

26-38: Port configuration logic is correct.

The conditional port definitions properly handle both external (with dynamic ports or fallback to 65535) and internal (fixed 65535) scenarios.

packages/apps/virtual-machine/templates/service.yaml (3)

9-12: Conditional annotations look good.

The annotations are correctly gated behind .Values.external, ensuring they only apply to LoadBalancer services.

20-22: Headless service requirement addressed.

The addition of clusterIP: None when .Values.external is false correctly implements the headless service requirement from issue #1731, enabling DNS-based discovery for internal-only VMs.

26-38: Port configuration logic is correct.

The conditional port definitions properly handle both external (with dynamic ports or fallback to 65535) and internal (fixed 65535) scenarios.

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.}

gemini-code-assist

Code Review

This pull request correctly modifies the Helm charts to ensure a Kubernetes Service is always created for a virtual machine, providing a stable in-cluster DNS name even when not exposed externally. This is achieved by creating a ClusterIP service when .Values.external is false. The changes are applied to both the virtual-machine and vm-instance charts.

I've found a critical indentation issue in both service.yaml templates that will likely cause the Helm chart to fail rendering due to invalid YAML. Please see the specific comments for details and a suggested fix.

packages/apps/virtual-machine/templates/service.yaml

packages/apps/vm-instance/templates/service.yaml

coderabbitai

Actionable comments posted: 1

♻️ Duplicate comments (2)

packages/apps/virtual-machine/templates/service.yaml (2)
34-36: Verify that port 65535 is appropriate for internal VM access.

Same concern as in packages/apps/vm-instance/templates/service.yaml - confirm that port 65535 correctly supports internal VM access and service routing.

14-20: Missing headless service specification for internal access.

Same issue as in packages/apps/vm-instance/templates/service.yaml - the implementation should include clusterIP: None when .Values.external is false to create a headless service as specified in issue #1731.
🔎 Proposed fix to add headless service support
 spec:
   type: {{ ternary "LoadBalancer" "ClusterIP" .Values.external }}
+{{- if not .Values.external }}
+  clusterIP: None
+{{- end }}
 {{- if .Values.external }}
   externalTrafficPolicy: Local

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fe7bdcf and c8dc801.

📒 Files selected for processing (2)

packages/apps/virtual-machine/templates/service.yaml (2 hunks)
packages/apps/vm-instance/templates/service.yaml (2 hunks)

🧰 Additional context used

🪛 YAMLlint (1.37.1)

packages/apps/vm-instance/templates/service.yaml

[error] 9-9: syntax error: could not find expected ':'

(syntax)

packages/apps/virtual-machine/templates/service.yaml

[error] 9-9: syntax error: could not find expected ':'

(syntax)

⏰ 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 (3)

packages/apps/vm-instance/templates/service.yaml (2)

9-12: Static analysis false positive can be safely ignored.

The YAMLlint syntax error on line 9 is a false positive. The {{- if .Values.external }} syntax is valid Helm templating that will be processed before YAML validation.

34-36: Port 65535 may be a placeholder and should be verified against actual VM port requirements.

While technically valid in Kubernetes (range 1 to 65535), port 65535 is the maximum allowed port and does not correspond to standard VM service ports. Confirm:

Does this port align with the actual ports exposed by your VMs?

Is this a placeholder that requires proper port mapping from .Values configuration?

Are there corresponding containerPort definitions that expect this specific port number?

packages/apps/virtual-machine/templates/service.yaml (1)

9-12: Static analysis false positive can be safely ignored.

Same as in packages/apps/vm-instance/templates/service.yaml - the YAMLlint error is a false positive for valid Helm template syntax.

packages/apps/vm-instance/templates/service.yaml

## What this PR does When VMs are created without a public IP, no service is created for them and they have no in-cluster DNS name. This PR fixes this and resolves #1731. ### Release note ```release-note [vm] Always expose VMs with at least a ClusterIP service. ``` Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>

nbykov0

LGTM

github-actions · 2025-12-24T13:10:16Z

Successfully created backport PR for release-0.39:

[Backport release-0.39] [vm] Always expose VMs with a service #1751

# Description Backport of #1738 to `release-0.39`.

lllamnyp requested review from kvaps and nbykov0 as code owners December 19, 2025 11:39

dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working labels Dec 19, 2025

gemini-code-assist bot reviewed Dec 19, 2025

View reviewed changes

packages/apps/virtual-machine/templates/service.yaml Show resolved Hide resolved

packages/apps/vm-instance/templates/service.yaml Show resolved Hide resolved

coderabbitai bot reviewed Dec 19, 2025

View reviewed changes

packages/apps/vm-instance/templates/service.yaml Show resolved Hide resolved

lllamnyp force-pushed the fix/1731-vm-dns branch from c8dc801 to ba50d01 Compare December 19, 2025 11:51

lllamnyp enabled auto-merge December 23, 2025 06:29

nbykov0 disabled auto-merge December 24, 2025 13:04

nbykov0 approved these changes Dec 24, 2025

View reviewed changes

dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 24, 2025

lllamnyp added the backport Should change be backported on previus release label Dec 24, 2025

lllamnyp merged commit 09e0b5f into main Dec 24, 2025
26 checks passed

lllamnyp deleted the fix/1731-vm-dns branch December 24, 2025 13:10

github-actions bot mentioned this pull request Dec 24, 2025

[Backport release-0.39] [vm] Always expose VMs with a service #1751

Merged

kvaps added a commit that referenced this pull request Dec 24, 2025

[Backport release-0.39] [vm] Always expose VMs with a service (#1751)

56ed29c

# Description Backport of #1738 to `release-0.39`.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[vm] Always expose VMs with a service #1738

[vm] Always expose VMs with a service #1738

Uh oh!

lllamnyp commented Dec 19, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

gemini-code-assist bot commented Dec 19, 2025

Uh oh!

coderabbitai bot commented Dec 19, 2025 •

edited

Loading

Other AI code review bot(s) detected

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

nbykov0 left a comment

Uh oh!

Uh oh!

github-actions bot commented Dec 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[vm] Always expose VMs with a service #1738

[vm] Always expose VMs with a service #1738

Uh oh!

Conversation

lllamnyp commented Dec 19, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Release note

Summary by CodeRabbit

Uh oh!

gemini-code-assist bot commented Dec 19, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

coderabbitai bot commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Other AI code review bot(s) detected

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nbykov0 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Dec 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lllamnyp commented Dec 19, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 19, 2025 •

edited

Loading