Use cgroup CPU limit to set concurrency by premnathnenavath · Pull Request #40997 · envoyproxy/envoy

premnathnenavath · 2025-09-05T11:47:49Z

Commit Message:
This addresses container CPU limit issues reported in #40914
Implement CPU detection to improve behavior in containerized environments where cgroup limits are more restrictive than affinity/hardware thread counts.

Additional Description:
Envoy currently ignores cgroup CPU limits when determining worker thread count, leading to overprovisioning in containers:

Hardware/Affinity reports 64 CPUs but container has resources.limits.cpu: "2"
Envoy creates 64 worker threads instead of 2, causing CPU throttling

This adds cgroup-aware CPU detection following Go's proven algorithm:

effective_cpu = min(hardware_threads, affinity_count, cgroup_limit)
worker_threads = max(1, effective_cpu)

Risk Level:
High - This changes CPU detection behavior in containerized environments, potentially affecting worker thread count and performance characteristics.

Testing:
Docs Changes:
Release Notes:
Platform Specific Features:

This adds Go-style GOMAXPROCS algorithm to Envoy's CPU detection: - Combines hardware threads, CPU affinity, and cgroup limits - Supports both cgroup v1 (cpu.cfs_quota_us) and v2 (cpu.max) - Uses leaf-only cgroup detection matching Go runtime behavior - Changes minimum CPU count from 2 to 1 for single-core containers - Includes comprehensive test suite with filesystem mocking The implementation improves Envoy's behavior in Kubernetes and Docker environments where cgroup limits are more restrictive than hardware thread counts. Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

Fix formatting issues identified by clang-format to ensure compliance with Envoy's coding standards. Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

- Remove Go-specific references from comments and documentation - Replace 'hw_threads' with 'hardware_threads' for better readability - Rename test from 'GoMaxprocsAlgorithm' to 'CpuCountIntegration' - Add missing technical terms to spelling dictionary (CFS, CPUSET, TASKSET, hw) - Apply code formatting fixes for long lines - Simplify comments to focus on container-aware CPU detection functionality - Keep algorithm behavior unchanged, only improve documentation and naming Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

- Add CgroupCpuUtil for cgroup v1/v2 CPU limit parsing - Enhance getCpuCount() with 5-step container-aware algorithm - Add comprehensive test coverage with filesystem mocking - Respect hardware threads, CPU affinity, and cgroup constraints Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

source/server/options_impl_platform_linux.cc

kyessenov · 2025-09-05T23:29:09Z

@ggreenway as a senior approver (please feel free to delegate for first pass, while we recover repokitteh).

ggreenway

I haven't looked at the code yet, but I agree that changing the default from hw-cpus to container-allowed-cpus makes sense. We should add a runtime guard for this, and a clear release note in "incompatible behavior". But I can't think of any situations where the current behavior is better than the new behavior.

- Add extensive comments explaining cgroup v1/v2 precedence and behavior - Improve algorithm documentation with step-by-step explanations - Add detailed examples for CPU limit calculations (1.5 CPUs → 2 CPUs) - Enhance error handling comments for safety and clarity - Fix formatting and code style consistency - Reference Go runtime cgroup implementation for context - Add two-pass cgroup path parsing with proper v1/v2 precedence - Fix missing words in comments and improve readability Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

premnathnenavath · 2025-09-11T05:20:56Z

/retest

- Enable container-aware CPU detection by default with runtime guard - Add envoy.reloadable_features.enable_cgroup_cpu_detection flag (enabled by default) - Update behavior_changes release note to reflect new default behavior - Maintain backward compatibility with runtime flag override option - Position runtime guard in correct alphabetical order in runtime_features.cc This addresses reviewer feedback to make container-aware CPU detection the default behavior while providing an escape hatch for rollback. Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

repokitteh-read-only · 2025-09-11T05:55:41Z

CC @envoyproxy/runtime-guard-changes: FYI only for changes made to (source/common/runtime/runtime_features.cc).

🐱

Caused by: #40997 was synchronize by premnathnenavath.

see: more, trace.

premnathnenavath · 2025-09-11T07:15:30Z

/retest

ggreenway

I like this change a lot. Thanks for clearly documenting the cgroup behavior and expected content of various files.

Ideally I'd like there to be an integration test for this, but I need to do some research to see if that's possible in our CI environment or not.

/wait

changelogs/current.yaml

source/server/options_impl_platform_linux.cc

source/common/runtime/runtime_features.cc

source/server/cgroup_cpu_util.h

source/server/cgroup_cpu_util.cc

source/server/cgroup_cpu_util.h

source/server/cgroup_cpu_util.cc

- Change runtime guard to restart feature: envoy_restart_features_enable_cgroup_cpu_detection - Update API to return absl::optional<uint32_t> without hw_threads parameter - Remove step comments as requested by reviewer - Add Linux-only and --concurrency behavior notes to changelog - Add proper ENVOY_LOG_MISC warning logs for malformed cgroup files - Refactor to use optional throughout, remove UINT32_MAX sentinels - Change string constants to constexpr absl::string_view - Add Go source permalink with specific line number - Remove isV1Available/isV2Available functions for simpler API - Apply clang-format improvements for consistent code style - Update all tests to use explicit absl::optional<uint32_t> types - Simplify test mocks to match new direct file reading approach All reviewer feedback has been addressed and code passes formatting checks. Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

nezdolik · 2025-09-26T10:05:14Z

looks like ci is still failing

premnathnenavath · 2025-09-29T15:20:56Z

Im working on it and the linux documentation. Will get back by this week

This commit introduces a robust cgroup CPU limit detection system that works across both cgroup v1 and v2 hierarchies with comprehensive error handling. Key features: - Cross-platform CPU limit detection (cgroup v1/v2, Docker, Kubernetes, systemd) - Modular architecture with separate helper functions for each cgroup version - Optional-based API design using absl::optional<CpuLimitResult> - Comprehensive test suite with 81 test cases covering edge cases - Performance optimizations with file content caching - Detailed logging for debugging container environments Changes: - Add new cgroup_cpu_util.h/.cc with getCpuLimit() and helper functions - Implement accessCgroupV1Files/accessCgroupV2Files for file access - Implement readActualLimitsV1/readActualLimitsV2 for parsing - Add comprehensive unit tests covering all scenarios - Update BUILD files for new test dependencies - Enhance options_impl_test.cc integration This enables Envoy to automatically detect CPU limits in containerized environments, improving resource management and performance tuning. Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

- Resolved conflicts in changelogs/current.yaml by keeping both behavior changes - Resolved conflicts in source/common/runtime/runtime_features.cc by including both runtime guards - Successfully merged 176 commits from upstream main - Maintained our cgroup CPU detection functionality alongside new upstream features Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

premnathnenavath · 2025-10-01T11:29:27Z

The implementation almost perfectly matches go behaviour

Optimizations of zero heap, BSS allocation and in place parsing for cgroup detection were skipped for simplicity since pathological escaped and huge paths are rare. The impact is however no GC, efficient stack memory allocation & performance in such scenarios. example netblue30/firejail#6450

Let me get back on this if the complication actually makes sense for performance. Meanwhile please review the PR

- Simplified API by removing CpuLimitResult struct wrapper - Changed return type from absl::optional<CpuLimitResult> to absl::optional<double> - Removed 4 unused functions and 40+ unused test cases - Added missing words to spelling dictionary - Fixed formatting issues - Net reduction: 334 lines of cleaner code Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

premnathnenavath · 2025-10-01T12:38:08Z

/retest

premnathnenavath · 2025-10-01T17:27:01Z

/retest

.bazelrc

phlax

to be precise this is disabling all tests atm

https://github.com/envoyproxy/envoy/actions/runs/18527448792/job/52801495867#step:17:643

.bazelrc

Replace empty --test_tag_filters= with --test_tag_filters=- in CI configs to address reviewer concerns about broad filter overrides. The empty filter (--test_tag_filters=) completely clears ALL tag filtering, potentially breaking other CI filtering logic. The new approach using --test_tag_filters=- means 'exclude nothing' and is more targeted, allowing engflow_only tests to run in CI while preserving other tag filtering behavior. This implements the positive filtering approach suggested in code review to make cgroup CPU detection tests run only in appropriate CI environments. Signed-off-by: Premnath Nenavath <premnath.nenavath@gmail.com> Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

.bazelrc

Remove broad CI config tag filter overrides and only keep the override for the EngFlow RBE configuration (rbe-envoy-engflow). Based on code review feedback, we should only run cgroup CPU detection tests in environments where we know ahead of time that the necessary cgroup resources are available. EngFlow RBE is the only such environment we can trust. This ensures: - Local builds exclude engflow_only tests (for development) - Most CI environments exclude engflow_only tests (unknown resources) - Only EngFlow RBE includes engflow_only tests (known good resources) Addresses reviewer concerns about making assumptions about resource availability in different CI environments. Signed-off-by: Premnath Nenavath <premnath.nenavath@gmail.com> Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

premnathnenavath · 2025-10-16T09:29:12Z

/retest

Remove redundant CI environment check from cgroup test since tag filtering already controls when these tests run. The engflow_only tag ensures tests only execute in appropriate EngFlow RBE environments. Signed-off-by: Premnath Nenavath <premnath.nenavath@samsara.com> Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

This change adds CPU detection from cgroup limits to enable container-aware concurrency configuration. When running in containers with CPU limits (e.g., Docker --cpus flag), Envoy can now detect and use those limits instead of the host's CPU count. Implementation: - Added CgroupCpuUtil to read CPU limits from cgroup v1 and v2 filesystems - Whitelisted /proc/self/mountinfo, /proc/self/cgroup, and /sys/fs/cgroup/* paths in filesystem security checks to allow reading cgroup files - Added integration test requiring Docker with CPU limits - Created bazel-test-cgroup.sh wrapper to run tests with Docker --cpus=2 - Added GitHub Actions workflow for automated testing Testing: - Integration test validates CPU limit detection in Docker containers - Filesystem whitelist tests verify cgroup path access - Tests run in privileged Docker containers with CPU limits set Fixes envoyproxy/envoy#issue-number Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

Update actions/cache from v3 to v4 and actions/upload-artifact from v3 to v4 to fix deprecation warning. The v3 versions were deprecated on April 16, 2024. Signed-off-by: Premnath Nenavath <premnath.nenavath@example.com> Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

Changes: - Add --test_tag_filters=containerized to bazel-test-cgroup.sh to explicitly run only containerized tests (overrides global -engflow_only filter) - Quote DOCKER_EXTRA_ARGS in tools/docker_wrapper.sh to fix shellcheck warnings - Remove trailing whitespace from .github/workflows/cgroup-tests.yml This fixes the GitHub Actions error 'No test targets were found' while maintaining the expected behavior that containerized tests are excluded from regular local builds. Tested-on: EC2 (working) Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

The bazel-test-docker.sh was using 'envoyproxy/envoy-build' but the correct image name from .bazelrc is 'envoyproxy/envoy-build-ubuntu'. This was causing GitHub Actions to fail with: docker: Error response from daemon: manifest for envoyproxy/envoy-build:f4a881a1205e8e6db1a57162faf3df7aed88eae8 not found: manifest unknown Fixed by updating IMAGE variable to use envoy-build-ubuntu. Tested-on: EC2 (test passes) Fixes: GitHub Actions Docker image not found error Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

…ix YAML formatting - Add -containerized to test_tag_filters for asan, tsan, msan, test-coverage, and fuzz-coverage configs in .bazelrc - Fix YAML indentation in .github/workflows/cgroup-tests.yml to comply with GitHub Actions formatting standards - Ensures containerized tests only run when explicitly requested via bazel-test-cgroup.sh - Prevents Docker-in-Docker issues during sanitizer and coverage test runs Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

The compile_time_options CI target was overriding the global test_tag_filters with its own -nofips filter, which allowed containerized tests to run. Added -containerized to both test_tag_filters in the compile_time_options target to prevent cgroup integration tests from running in those builds. Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

premnathnenavath · 2025-10-21T20:00:48Z

/retest

Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

- Changed test tag from 'containerized' to 'runtime-cpu' to match upstream - Updated all test_tag_filters in .bazelrc to use -runtime-cpu - Removed .github/workflows/cgroup-tests.yml (upstream uses main CI) - Removed tools/bazel-test-cgroup.sh (upstream uses ci/do_ci.sh cpu-detection) - Reverted tools/docker_wrapper.sh and tools/bazel-test-docker.sh to upstream - Updated test comments to reflect Docker CPU limits execution - Added 'no-remote' tag to prevent RBE execution - Added quotes around 'cgroups' in comments for clarity This aligns with maintainer's approach in PR envoyproxy#41646. Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

tools/bazel-test-docker.sh

tools/docker_wrapper.sh

The compile_time_options CI target was still using -containerized instead of -runtime-cpu, causing the cgroup test to run in compile-time-options builds where it shouldn't. Updated both test_tag_filters in the compile_time_options target to use -runtime-cpu instead of -containerized. Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

premnathnenavath · 2025-10-22T12:47:58Z

Thanks @phlax for adding the IT in main. @ggreenway @phlax please review again

phlax · 2025-10-22T13:11:59Z

/retest flakey tsan

phlax

ci lgtm, thanks @premnathnenavath for working on this and iterating

ggreenway

This looks great! Thanks for all the iterating on the integration test.

- Update the ENVOY_COMMIT and ENVOY_SHA in bazel/repositories.bzl to the latest Envoy's commit. - Update .bazelrc to envoyproxy/envoy#41705, envoyproxy/envoy#40997 - Update .bazelversion to envoyproxy/envoy#41661 - Update ci/docker-compose.yml to envoyproxy/envoy#41646 - Modifications to python test files to accommodate bazelbuild/bazel#18128 - Adjustments to docker image creation to avoid copying symlinks from virtualenv Signed-off-by: tomjzzhang <4367421+tomjzzhang@users.noreply.github.com>

premnathnenavath added 4 commits September 5, 2025 16:22

format: apply code formatting fixes

9825a68

Fix formatting issues identified by clang-format to ensure compliance with Envoy's coding standards. Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

kyessenov reviewed Sep 5, 2025

View reviewed changes

source/server/options_impl_platform_linux.cc Show resolved Hide resolved

kyessenov assigned ggreenway Sep 5, 2025

ggreenway reviewed Sep 8, 2025

View reviewed changes

KBaichoo mentioned this pull request Sep 10, 2025

Set number of workers to allocated cgroup CPUs? #36988

Open

ggreenway requested changes Sep 11, 2025

View reviewed changes

repokitteh-read-only bot added the waiting label Sep 11, 2025

repokitteh-read-only bot removed the waiting label Sep 16, 2025

nezdolik added the waiting label Sep 26, 2025

repokitteh-read-only bot removed the waiting label Oct 1, 2025

premnathnenavath force-pushed the cgroup-cpu-detection branch from 69d654a to 20883ef Compare October 1, 2025 12:07

premnathnenavath requested a review from ggreenway October 1, 2025 17:48

phlax reviewed Oct 15, 2025

View reviewed changes

.bazelrc Outdated Show resolved Hide resolved

premnathnenavath requested a review from ggreenway October 15, 2025 12:25

phlax requested changes Oct 15, 2025

View reviewed changes

phlax reviewed Oct 15, 2025

View reviewed changes

.bazelrc Outdated Show resolved Hide resolved

phlax reviewed Oct 15, 2025

View reviewed changes

.bazelrc Outdated Show resolved Hide resolved

premnathnenavath added 2 commits October 16, 2025 17:00

premnathnenavath requested a review from agrawroh as a code owner October 21, 2025 15:05

repokitteh-read-only bot added the workflows:untested label Oct 21, 2025

premnathnenavath added 5 commits October 21, 2025 20:53

premnathnenavath added 2 commits October 22, 2025 17:28

Merge remote-tracking branch 'upstream/main' into cgroup-cpu-detection

f364496

Signed-off-by: premnath.nenavath <premnath.nenavath@gomotive.com>

phlax reviewed Oct 22, 2025

View reviewed changes

tools/bazel-test-docker.sh Show resolved Hide resolved

phlax reviewed Oct 22, 2025

View reviewed changes

tools/docker_wrapper.sh Show resolved Hide resolved

phlax approved these changes Oct 22, 2025

View reviewed changes

ggreenway approved these changes Oct 22, 2025

View reviewed changes

ggreenway merged commit 2ed4ed0 into envoyproxy:main Oct 22, 2025
29 checks passed

tomjzzhang mentioned this pull request Oct 28, 2025

Updating Envoy version to 2a33d4e (Oct 27, 2025) envoyproxy/nighthawk#1441

Merged

Comments

Conversation

premnathnenavath commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

kyessenov commented Sep 5, 2025

Uh oh!

ggreenway left a comment

Choose a reason for hiding this comment

Uh oh!

premnathnenavath commented Sep 11, 2025

Uh oh!

repokitteh-read-only bot commented Sep 11, 2025

Uh oh!

premnathnenavath commented Sep 11, 2025

Uh oh!

ggreenway left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nezdolik commented Sep 26, 2025

Uh oh!

premnathnenavath commented Sep 29, 2025

Uh oh!

premnathnenavath commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

premnathnenavath commented Oct 1, 2025

Uh oh!

premnathnenavath commented Oct 1, 2025

Uh oh!

Uh oh!

phlax left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

premnathnenavath commented Oct 16, 2025

Uh oh!

premnathnenavath commented Oct 21, 2025

Uh oh!

Uh oh!

Uh oh!

premnathnenavath commented Oct 22, 2025

Uh oh!

phlax commented Oct 22, 2025

Uh oh!

phlax left a comment

Choose a reason for hiding this comment

Uh oh!

ggreenway left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

premnathnenavath commented Sep 5, 2025 •

edited

Loading

premnathnenavath commented Oct 1, 2025 •

edited

Loading