fix: make cleaner-agent mid-build usage thresholds configurable - #165
Draft
openswe-shared-services[bot] wants to merge 2 commits into
Draft
openswe-shared-services[bot] wants to merge 2 commits into
openswe-shared-services[bot] wants to merge 2 commits into
Conversation
cleaner-agent.sh hardcoded its mid-build disk/inode thresholds to 0.9, ignoring any DISK_USAGE_THRESHOLD/INODES_USAGE_THRESHOLD override set via the Runtime Environment spec, unlike the pre-build docker-clean.sh check. Introduce separate CLEANER_AGENT_DISK_USAGE_THRESHOLD/ CLEANER_AGENT_INODES_USAGE_THRESHOLD config entries (default 0.9, kept higher than the pre-build 0.8 default) so accounts can tune when the continuous mid-build cleaner steps in. Also fix check_inodes_usage_threshold()'s no-arg fallback, which defaulted to DISK_USAGE_THRESHOLD instead of INODES_USAGE_THRESHOLD.
|
|
||
| check_inodes_usage_threshold(){ | ||
| local THRESHOLD=${1:-${DISK_USAGE_THRESHOLD}} | ||
| local THRESHOLD=${1:-${INODES_USAGE_THRESHOLD}} |
Contributor
There was a problem hiding this comment.
this is a bad naming
|
|
||
| check_inodes_usage_threshold(){ | ||
| local THRESHOLD=${1:-${DISK_USAGE_THRESHOLD}} | ||
| local THRESHOLD=${1:-${INODES_USAGE_THRESHOLD}} |
check_disk_usage_threshold() and check_inodes_usage_threshold() both declared a local variable named THRESHOLD, which review flagged as confusing/bad naming given the two functions sit side by side and handle different units. Rename to DISK_THRESHOLD/INODES_THRESHOLD. Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Why
Triage of CCS-61 / CF-2350 (justpark-inc build failures from inode exhaustion on reused dind volumes) found that
cleaner/cleaner-agent.sh— the loop that runs continuously during a build to reclaim disk/inodes — hardcodes its trigger thresholds to0.9and never looks atDISK_USAGE_THRESHOLD/INODES_USAGE_THRESHOLD, even though those are documented, chart-configurable env vars and the pre-builddocker-clean.shcheck honors them correctly. So an account/pipeline that needs the mid-build cleaner to be more aggressive (lower threshold) has no way to configure that — it always waits for 90%.Separately,
functions.sh'scheck_inodes_usage_threshold()falls back toDISK_USAGE_THRESHOLD(notINODES_USAGE_THRESHOLD) when called without an explicit threshold argument — a latent copy-paste bug, currently masked because every caller passes an explicit arg.What changed
cleaner/config: addedCLEANER_AGENT_DISK_USAGE_THRESHOLD/CLEANER_AGENT_INODES_USAGE_THRESHOLD(default0.9, same as before) as the dedicated, overridable knobs forcleaner-agent.sh's mid-build loop, distinct from the pre-buildDISK_USAGE_THRESHOLD/INODES_USAGE_THRESHOLD(default0.8).cleaner/cleaner-agent.sh: removed the hardcodedSTART_DISK_USAGE_THRESHOLD/START_INODES_USAGE_THRESHOLDlocals and reads the new config values instead.cleaner/functions.sh: fixedcheck_inodes_usage_threshold()'s no-arg fallback to useINODES_USAGE_THRESHOLD.cleaner/README.md: documented the new/existing threshold env vars and their defaults.Behavior for every account is unchanged unless someone sets
CLEANER_AGENT_DISK_USAGE_THRESHOLD/CLEANER_AGENT_INODES_USAGE_THRESHOLDexplicitly (defaults keep the previous0.9).Testing
bash -non all three modified scripts.config+functions.shand calledcheck_inodes_usage_thresholdwith no arg to confirm it now readsINODES_USAGE_THRESHOLD(0.8) instead ofDISK_USAGE_THRESHOLD.Related
Made by Open SWE · anthropic:claude-sonnet-5 (medium)
Security Report — codefresh/dind
Note
Compared security scans:
Current image:
quay.io/codefresh/dev/dind:open-swecleaner-agent-inode-threshold@sha256:cf1531270ba69bcc555fa8a3026dcfb4829e6405eb4b72f7bc4c91d24894c57e
Baseline:
quay.io/codefresh/dind:master@sha256:b2230c0889370fa297b7e9643ee43e730897f8befdb6d23b007b4b011764c62f
Fixed CVEs: 0
Fixed issues: 0