GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [pipeline-manager] Check pipeline storage limits in the api-server by xiasongh · Pull Request #7712 · feldera/feldera

Conversation

@xiasongh

@xiasongh xiasongh commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

The API accepted a runtime_config with a storage class, volume attributes class, or storage size that the Kubernetes runner refuses, and the next start failed (for example "Volume attributes class iops-4000 is not supported").

POST, PUT, and PATCH now return 400 InvalidRuntimeConfig for:

Field Limit
resources.storage_class --enable-storage-classes-allowlist, --storage-classes
resources.volume_attributes_class (empty is unset) --enable-volume-attributes-classes-allowlist, --volume-attributes-classes
resources.storage_min, resources.storage_max --upper-limit-storage-megabyte
storage_min > storage_max always

The flags take the same values as the runner; the Helm chart passes them in feldera/cloud. Without them, only storage_min > storage_max is refused. Stored configs still load: only request bodies are checked.

Commits:

  1. Add the api-server flags. Nothing reads them.
  2. Check requests against them. Tests, docs, and changelog.

Not in this PR: PATCH deployment_config (#7656). Whichever PR merges second calls ApiServerConfig::check_storage there.

Tests: check_storage_refuses_what_the_runner_refuses and runtime_config_storage_requests. Each check and each handler call, disabled on its own, fails them.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@xiasongh xiasongh mentioned this pull request Oct 29, 2026
[pipeline-manager] Grow a running pipeline's storage with PATCH #7656
Merged
@xiasongh
xiasongh force-pushed the song/api-storage-validation branch from 32dc194 to 451b7da Compare October 29, 2026 18:57
@xiasongh
xiasongh marked this pull request as ready for review October 30, 2026 10:53
@xiasongh
xiasongh requested a review from gz October 30, 2026 10:53
@xiasongh xiasongh changed the title [pipeline-manager] Refuse runtime_config storage that the runner refuses [pipeline-manager] Check pipeline storage settings against the runner's limits Oct 30, 2026
@xiasongh xiasongh changed the title [pipeline-manager] Check pipeline storage settings against the runner's limits [pipeline-manager] Check pipeline storage limits in the api-server Oct 30, 2026
xiasongh added 2 commits October 30, 2026 11:56
@xiasongh
[pipeline-manager] Add the runner's storage limits to the api-server
6f77a55 
--enable-storage-classes-allowlist, --storage-classes,
--enable-volume-attributes-classes-allowlist, --volume-attributes-classes,
and --upper-limit-storage-megabyte take the same values as the Kubernetes
runner. Nothing reads them yet.
@xiasongh
[pipeline-manager] Check pipeline storage limits in the api-server
5aeda0d 
POST, PUT, and PATCH return 400 InvalidRuntimeConfig for a storage class,
volume attributes class, or storage size outside the limits that the
api-server shares with the runner, and for storage_min above storage_max.
Before, the next start failed. Stored configs still load.
@xiasongh
xiasongh force-pushed the song/api-storage-validation branch from 451b7da to 5aeda0d Compare October 30, 2026 11:06
mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Oct 30, 2026
View reviewed changes
Comment thread crates/pipeline-manager/src/api/endpoints/pipeline_management.rs
}

/// Refuses a `runtime_config` with storage that the runner refuses at start, so that it fails
/// now instead of at the next start. Only new values are checked: stored ones still load.

@mythical-fred-oss mythical-fred-oss Bot Oct 30, 2026

Copy link
Copy Markdown
Contributor

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.

pitfall (medium): The comment says "Only new values are checked", but the request body is checked whether or not it changed. I tested this: after the limits are set, PUT /pipelines/old with only program_code changed and the same stored runtime_config (storage_class: io2) returns 400. The Python SDK's create_or_replace and other PUT-everything clients therefore can't edit the program of such a pipeline. Either skip the check when runtime_config matches the stored value, or fix the comment and docs and add this case to runtime_config_storage_requests.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Oct 30, 2026
View reviewed changes
Comment thread crates/pipeline-manager/src/config.rs
"resources.{field} ({amount}) is more than the maximum of {upper_limit}M on this installation. Lower it, or ask an administrator to raise kubernetesRunner.upperLimitStorageMegabyte."
));
}
if let Some(class) = &resources.storage_class

@mythical-fred-oss mythical-fred-oss Bot Oct 30, 2026

Copy link
Copy Markdown
Contributor

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.

pitfall (medium): An empty volume_attributes_class counts as unset, but an empty storage_class does not. I tested it: {"storage_class": ""} with the allowlist on returns 400 '' is not allowed. If the runner also treats an empty storage class as unset, this refuses something the runner accepts. Please check against the runner and add a test for "" either way.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@mythical-fred-oss

mythical-fred-oss Bot commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

What I ran

Check Result
cargo test --locked -p pipeline-manager --lib -- check_storage runtime_config_storage ✅ 2 passed
pre-commit run on the changed files (clippy, fmt, machete, OpenAPI, docs) ✅ passed
Extra case: re-PUT of an unchanged stored storage_class: io2 with only the program edited returns 400 (see inline comment)
Extra case: storage_class: "" with the allowlist on returns 400, while an empty volume_attributes_class is accepted (see inline comment)

Gates: tests, docs, changelog and actionable error messages are all in place. There is no unsafe, no new dependency and no CI change.

Missing tests (medium):

  • A re-PUT or full PATCH of an unchanged stored config, plus a decision on whether it should pass.
  • An empty storage_class.
  • A PUT or PATCH on a running pipeline with a refused value. The new 400 now comes before the "update restricted to stopped" error, so the user sees the storage error first.

Pitfall (medium): The parity with the Kubernetes runner can't be checked in this repo, so the tests only check that the api-server matches itself. Please confirm in the PR description that the MB units (10^6 bytes, the upper_limit as u64 cast) and the empty-name handling match the runner.

Breaking (low, documented): storage_min > storage_max is now refused on every installation, including OSS. It is in the changelog, so this is fine.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
gz
gz approved these changes Oct 30, 2026
View reviewed changes
@xiasongh
xiasongh added this pull request to the merge queue Oct 30, 2026
Merged via the queue into main with commit 9c79856 Oct 30, 2026
1 check passed

Uh oh!

There was an error while loading. Please reload this page.
@xiasongh
xiasongh deleted the song/api-storage-validation branch October 30, 2026 23:36
This was referenced Nov 2, 2026
[python] Use only real storage and volume attributes classes in tests #7790
Merged
[pipeline-manager] Check memory and storage autoscaling limits in the api-server #7819
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mythical-fred-oss mythical-fred-oss[bot] mythical-fred-oss[bot] left review comments
@gz gz gz approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@xiasongh @gz
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.