Skip to content

feat: virtual nodes advertise ephemeral storage - #585

Merged
dciangot merged 2 commits into
interlink-hq:mainfrom
matbun:feat/node-ephemeral-storage
Oct 23, 2026
Merged

dciangot merged 2 commits into
interlink-hq:mainfrom
matbun:feat/node-ephemeral-storage

Conversation

@matbun

@matbun matbun commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

A virtual node advertises CPU, memory, pods and accelerators but no ephemeral-storage, so the scheduler never places a pod that requests some (Insufficient ephemeral-storage). Sidecars injected by other controllers often do: Katib's metrics collector requests 500Mi, so every Katib trial routed to a virtual node stayed Pending.

Resources.EphemeralStorage sets the amount the node advertises, 1Ti by default: the remote side has its own disks, and the node only must not refuse such pods.

TestGetResources_EphemeralStorage covers the default and an explicit value. Checked on a cluster: a Katib trial that stayed Pending was scheduled once the node advertised ephemeral-storage: 1Ti.


Related issue :

A virtual node advertised CPU, memory, pods and accelerators but no
ephemeral-storage, so the scheduler never placed a pod that requests some:
"Insufficient ephemeral-storage". Sidecars injected by other controllers
often do; Katib's metrics collector requests 500Mi, which made every Katib
trial routed to a virtual node stay Pending.

Resources.EphemeralStorage sets the amount, 1Ti by default: the remote side
has its own disks, the node only must not refuse such pods.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: Matteo Bunino <[email protected]>
@netlify

netlify Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for interlink-dev canceled.

Name Link
🔨 Latest commit bee981d
🔍 Latest deploy log https://app.netlify.com/projects/interlink-dev/deploys/6adb30727ade55a7bb4280fe

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Invalid ephemeral-storage configuration bypasses validation and causes a panic during provider initialization.

1 open issue
What changed in this PR

This PR lets virtual nodes accept pods that request ephemeral storage, including workloads with injected sidecars such as Katib’s metrics collector.

Changes:

  • Adds configurable Resources.EphemeralStorage, defaulting to 1Ti.
  • Advertises ephemeral storage in node capacity and allocatable resources.
  • Tests the default and an explicit configured value.
File Description
pkg/​virtualkubelet/​virtualkubelet.go Adds the default and advertises ephemeral storage.
pkg/​virtualkubelet/​config.go Defines and documents the configuration field.
pkg/​virtualkubelet/​config_test.go Tests default and explicit storage quantities.

Review effort: Balanced

Comment thread pkg/virtualkubelet/virtualkubelet.go
LoadConfig validated CPU, memory, pods and accelerators but not the new
ephemeral storage value, so a value such as "50GB" passed and then made
resource.MustParse panic while the provider started. It is now parsed in
LoadConfig like the others; an empty value stays valid and keeps the
default.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: Matteo Bunino <[email protected]>

@dciangot dciangot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dciangot

Copy link
Copy Markdown
Member

vktestset/basic_test.py::test_manifest[virtual-kubelet-095-default-entrypoint.yaml] FAILED [ 92%]

this test is failing @matbun

@matbun

matbun commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

@dciangot looks flaky to me: only the alpine container in 095 exited 255, the same test passed on this PR one commit earlier and on #583 and #584 in the same hour, and the last commit only adds config validation. Could you rerun it? I cannot rerun it from the fork

@dciangot dciangot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dciangot
dciangot merged commit fdfb3e6 into interlink-hq:main Oct 23, 2026
11 of 12 checks passed
@matbun
matbun deleted the feat/node-ephemeral-storage branch October 23, 2026 11:14
matbun added a commit to matbun/interLink that referenced this pull request Oct 23, 2026
…terlink-hq#577

The branch carried early copies of interlink-hq#580, interlink-hq#583, interlink-hq#584 and interlink-hq#585 folded into
one commit. It now holds main as of interlink-hq#571 (interlink-hq#580, interlink-hq#581, interlink-hq#582, interlink-hq#583, interlink-hq#585 as
merged), interlink-hq#584 as reviewed and the rebased reverse tunnel, with the demo's
own commits re-applied: TenantNamespace, RefuseForeignPods, kept init
containers on confined nodes and their logs.

Fixes found while porting:
- interlink-hq#571 refreshed the pod IP through computeShadowResourceIdentity, which
  ignores TenantNamespace; on a confined node it looked for the shadow in
  <ns>-shadow and never moved the IP. It uses the provider's identity now.
- The completed kept-init wait saves the init containers' logs before it
  takes the wait over, which cancels its context.
- RefuseForeignPods handles the errors of SetWatchErrorHandler and
  AddEventHandler (errcheck).
- The node ConfigMap patch is a struct, so interlink-hq#584 and interlink-hq#577 together do not
  trip goconst on "metadata".

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: Matteo Bunino <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants