Skip to content

Add --cpu-quota flag to set systemd's CPUQuota for fleetd on Linux - #53069

Open
lucasmrod wants to merge 1 commit into
mainfrom
49925-cpu-quota-linux-orbit
Open

Add --cpu-quota flag to set systemd's CPUQuota for fleetd on Linux#53069
lucasmrod wants to merge 1 commit into
mainfrom
49925-cpu-quota-linux-orbit

Conversation

@lucasmrod

@lucasmrod lucasmrod commented Sep 11, 2026

Copy link
Copy Markdown
Member

Resolves #49925.

  • Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.

Testing

  • Added/updated automated tests
  • QA'd all new/changed functionality manually

Tested on arm64 (Ubuntu 25.04) and amd64 (Omarchy Quattro) without Fleet Desktop (feature planned to run on servers)

fleetd/orbit/Fleet Desktop

  • Verified compatibility with the latest released version of Fleet (see Must rule)
  • If the change applies to only one platform, confirmed that runtime.GOOS is used as needed to isolate changes
  • Verified auto-update works from the released version of component to the new version (see tools/tuf/test)

Summary by CodeRabbit

  • New Features

    • Added a --cpu-quota option to fleetctl package for configuring the CPU limit applied to the Linux service.
    • Supports Debian, RPM, and Arch Linux package formats.
    • Defaults to a 20% CPU quota and can be configured through the environment.
  • Bug Fixes

    • Added validation to reject unsupported package types and non-positive quota values.

Copilot AI lite review requested due to automatic review settings September 11, 2026 18:19
@lucasmrod
lucasmrod requested a review from a team as a code owner September 11, 2026 18:19

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.

🔵 Needs a closer look

Add a successful Linux package test verifying that a non-default quota reaches the generated systemd unit.

Warning

  • Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview

Adds --cpu-quota support for configuring systemd CPU limits in Linux fleetd packages, preserving the 20% default.

Changes:

  • Adds the packaging option and CLI validation.
  • Renders configurable CPUQuota in the systemd unit.
  • Adds unit and integration tests.
File summaries
File Description
orbit/pkg/packaging/packaging.go Adds the CPU quota packaging option.
orbit/pkg/packaging/linux_shared.go Renders configurable systemd CPU quotas.
orbit/pkg/packaging/linux_shared_test.go Tests default and custom quotas.
cmd/fleetctl/integrationtest/package/package_test.go Tests package types and invalid flag usage.
cmd/fleetctl/fleetctl/package.go Defines and validates the CLI flag.
Review details

Files excluded by content exclusion policy (1)

  • changes/fleetctl-package-cpu-quota

Suppressed comments (1)

cmd/fleetctl/fleetctl/package.go:343

  • The new CLI path is only exercised by validation failures, so no test verifies that an accepted non-default value reaches the generated Linux package. A regression in the flag destination or in passing opt to buildFunc would still leave these tests green; add a successful Linux package test that inspects the systemd unit and asserts, for example, CPUQuota=1%.
			if c.IsSet("cpu-quota") {
				switch c.String("type") {
				case "deb", "rpm", "pkg.tar.zst":
				default:
					return errors.New("--cpu-quota is only supported for deb/rpm/pkg.tar.zst packages")
				}
				if opt.CPUQuota == 0 {
					return errors.New("--cpu-quota must be greater than 0")
				}
			}
  • Files reviewed: 5/6 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: afc1c600-544e-4269-9e74-d9afdf5f8bcd

📥 Commits

Reviewing files that changed from the base of the PR and between 25e3cb2 and b0caaef.

📒 Files selected for processing (6)
  • changes/fleetctl-package-cpu-quota
  • cmd/fleetctl/fleetctl/package.go
  • cmd/fleetctl/integrationtest/package/package_test.go
  • orbit/pkg/packaging/linux_shared.go
  • orbit/pkg/packaging/linux_shared_test.go
  • orbit/pkg/packaging/packaging.go

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


Walkthrough

The fleetctl package command adds a --cpu-quota flag and FLEETCTL_CPU_QUOTA environment variable. Validation restricts the flag to deb, rpm, and pkg.tar.zst packages and requires a value greater than zero. Linux package generation renders the quota in the systemd unit, using 20% when unset. Tests cover validation and quota values of 0, 1, and 150.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to b0caa

The Linux package CPU quota option is validated and rendered consistently, with default and custom values covered by tests.

🚥 Pre-merge checks | ✅ 2 | ❌ 5

❌ Failed checks (5 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR claims to resolve issue #49925, but the linked issue objective concerns denylisting queries that trigger the watchdog. The changes instead add a Linux CPU quota flag and do not address that obj… Link this PR to the issue that requests the CPU quota feature, or modify the implementation to address issue #49925. Do not mark #49925 as resolved unless its denylisting objective is implemented.
Linked Issues check ⚠️ Warning Issue #49925 requires every query that triggers the watchdog to be denylisted, regardless of watchdog_forced_shutdown_delay. The whole diff only adds --cpu-quota, systemd CPUQuota templating, an… Implement the watchdog change required by #49925. Ensure a query that triggers the watchdog is denylisted for every configured watchdog_forced_shutdown_delay, and add an automated regression test that verifies this behavior.
Out of Scope Changes check ⚠️ Warning The changed files implement Linux package CPU quota configuration through fleetctl package --cpu-quota. This behavior does not support the watchdog denylist requirement in issue #49925. The added ch… Remove the CPU quota changes from this pull request, or link them to an issue that requires this functionality. Keep this pull request focused on the watchdog denylist fix.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 5 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The description includes the related issue, changes file, automated testing, manual testing, platform details, and fleetd compatibility checks. However, it does not explain how this CPU quota change a… Explain the connection between the implementation and issue #49925, or reference the correct issue. Add the required ## AI section with the tool name and exact model ID, or state unknown if the model ID is unavailable.
✅ Passed checks (2 passed)
Check name Status Explanation
Out of Scope Changes check ✅ Passed The changed files and tests are directly related to the stated CPU quota feature. No unrelated implementation changes are evident; the linked-issue mismatch is assessed separately.
Title check ✅ Passed The title clearly identifies the primary change: adding the Linux-only --cpu-quota flag to configure systemd CPUQuota for fleetd.
Full details: Linked Issues check

Explanation

The PR claims to resolve issue #49925, but the linked issue objective concerns denylisting queries that trigger the watchdog. The changes instead add a Linux CPU quota flag and do not address that objective.

Full details: Linked Issues check

Explanation

Issue #49925 requires every query that triggers the watchdog to be denylisted, regardless of watchdog_forced_shutdown_delay. The whole diff only adds --cpu-quota, systemd CPUQuota templating, and tests for package validation and unit generation. It contains no watchdog or denylist implementation and no regression test for the reported behavior.

Full details: Out of Scope Changes check

Explanation

The changed files implement Linux package CPU quota configuration through fleetctl package --cpu-quota. This behavior does not support the watchdog denylist requirement in issue #49925. The added changelog, CLI validation, systemd unit changes, and related tests are unrelated to the linked issue.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 5 files. (1 skipped: 1 unsupported.)

Full details: Description check

Explanation

The description includes the related issue, changes file, automated testing, manual testing, platform details, and fleetd compatibility checks. However, it does not explain how this CPU quota change addresses issue #49925, whose stated objective concerns watchdog denylisting, and it omits the required AI section.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 49925-cpu-quota-linux-orbit

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.

@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.07%. Comparing base (3847721) to head (b0caaef).
⚠️ Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
orbit/pkg/packaging/linux_shared.go 83.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #53069      +/-   ##
==========================================
+ Coverage   76.06%   76.07%   +0.01%     
==========================================
  Files        4123     4126       +3     
  Lines      249892   249984      +92     
  Branches    14247    14247              
==========================================
+ Hits       190076   190177     +101     
+ Misses      59640    59631       -9     
  Partials      176      176              
Flag Coverage Δ
backend 77.70% <92.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

Watchdog unreliably triggering denylist

3 participants