daemon: skip nice/ionice for extensions image pull during firstboot#6139
daemon: skip nice/ionice for extensions image pull during firstboot#6139sdodson wants to merge 2 commits into
Conversation
During firstboot no workloads are running on the node yet, so throttling podman to idle I/O and low CPU priority only slows down provisioning. Thread a firstBoot flag from update() down through applyOSChanges/applyLayeredOSChanges/ExtractExtensionsImage to pullExtensionsImage and podmanCopy, switching from RunExtBackground to RunExt when firstBoot is true. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
WalkthroughThreads a background flag through extensions image pull/copy so podman operations can run in background or foreground. Threads a firstBoot boolean through OS update paths to disable background extraction on first boot. Adds jittered rpm-ostree cleanup delay, updates a unit test, and bind-mounts a tmpfs OSTree config during firstboot. ChangesDaemon update and tests
Firstboot service unit
🎯 3 (Moderate) | ⏱️ ~18 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)Command failed Comment |
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sdodson The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
…ring firstboot Two related optimizations to reduce I/O contention during node scale-up: 1. Defer rpm-ostree rollback cleanup by 60s +jitter (removeRollback) On boot 2, MCD calls `rpm-ostree cleanup -r` shortly after starting, which triggers three syncfs() calls totalling ~43s. These run concurrently with CNI image pulls and compete for disk bandwidth during the kubelet-to-ready window. Sleeping before cleanup lets the node reach Ready first. Jitter (up to 30s) spreads cleanup across nodes that scale up in the same burst. 2. Disable ostree fsync during machine-config-daemon-firstboot (from openshift#6122) Bind mount a tmpfs-backed copy of the ostree repo config over the real path before the rebase so that setting core.fsync=false is ephemeral. If the host crashes or reboots, the bind mount disappears and the on-disk config is unchanged automatically. We append a second [core] section (GKeyFile merges duplicate groups) rather than using `ostree config set` because ostree uses an atomic rename which fails with EBUSY on a bind-mounted file. Saves ~3-5s on rebase fetch. Measured at n=18 on AWS m6a.xlarge.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/daemon/update.go (1)
1497-1504: 🏗️ Heavy liftMake the rollback-cleanup deferral cancelable.
This adds an uninterruptible 60–90s sleep in a daemon path. If shutdown/update cancellation happens during that window, the goroutine still blocks until the timer expires. Thread a
context.Context(or the daemon’s existing stop signal) intoremoveRollback()and wait on a timer withselectinstead.♻️ Suggested direction
-func (dn *Daemon) removeRollback() error { +func (dn *Daemon) removeRollback(ctx context.Context) error { if !dn.os.IsCoreOSVariant() { return nil } delay := 60*time.Second + time.Duration(rand.Int63n(int64(30*time.Second))) klog.Infof("Deferring rpm-ostree rollback cleanup by %s to reduce boot I/O contention", delay.Round(time.Second)) - time.Sleep(delay) + + timer := time.NewTimer(delay) + defer timer.Stop() + select { + case <-timer.C: + case <-ctx.Done(): + return ctx.Err() + } + return runRpmOstree("cleanup", "-r") }As per coding guidelines,
**/*.go: context.Context for cancellation and timeouts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/daemon/update.go` around lines 1497 - 1504, The current uninterruptible sleep in removeRollback() (the delay := 60*time.Second + ...; time.Sleep(delay)) must be made cancelable: add a context.Context parameter (or use the daemon's existing stop channel/context) to removeRollback and replace time.Sleep with a timer and a select that waits on timer.C and ctx.Done() so the deferred rpm-ostree cleanup can be aborted on shutdown/update cancellation; ensure the log still reports the intended delay when scheduled and that the function returns promptly on ctx cancellation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pkg/daemon/update.go`:
- Around line 1497-1504: The current uninterruptible sleep in removeRollback()
(the delay := 60*time.Second + ...; time.Sleep(delay)) must be made cancelable:
add a context.Context parameter (or use the daemon's existing stop
channel/context) to removeRollback and replace time.Sleep with a timer and a
select that waits on timer.C and ctx.Done() so the deferred rpm-ostree cleanup
can be aborted on shutdown/update cancellation; ensure the log still reports the
intended delay when scheduled and that the function returns promptly on ctx
cancellation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f74aebfb-a6e1-4ea7-83c7-6cf6de1dc806
📒 Files selected for processing (2)
pkg/daemon/update.gotemplates/common/_base/units/machine-config-daemon-firstboot.service.yaml
1 participant
Two related optimizations to reduce I/O contention during node scale-up, measured on AWS m6a.xlarge (4.22 nightly, n=18 for fsync, n=1 baseline for cleanup contention).
1. Defer rpm-ostree rollback cleanup by 60s + jitter
On boot 2, MCD calls
rpm-ostree cleanup -rshortly after starting. This triggers threesyncfs()calls from the rpmostree daemon totalling ~43s, which run concurrently with CNI image pulls and compete for disk bandwidth during the kubelet-to-ready window. The syncfs calls straddle NodeReady — the first two complete before it, and the third finishes ~11s after.Sleeping 60s + up to 30s jitter in
removeRollback()before callingrpm-ostree cleanup -rdefers the I/O until after the node is Ready. The sleep is insidecheckStateOnFirstRun()which runs beforedn.booting = falseand beforeMachineConfigNodeResumedis posted, so MCO's Done state is delayed by the sleep — but NodeReady itself is kubelet-driven and is unaffected.Jitter spreads cleanup across nodes that scale up in the same burst so they don't all hit the disk simultaneously when the delay expires.
2. Disable ostree fsync during machine-config-daemon-firstboot (includes #6122)
Bind mount a tmpfs-backed copy of the ostree repo config over
/sysroot/ostree/repo/configbefore the rebase so that settingcore.fsync=falseis ephemeral. If the host crashes or reboots, the bind mount disappears and the on-disk config is unchanged automatically.We append a second
[core]section to the tmpfs copy (GKeyFile merges duplicate groups) rather than usingostree config setbecause ostree uses an atomic rename which fails withEBUSYon a bind-mounted file. Saves ~3-5s on rebase fetch.- How to verify
For cleanup deferral: on a new node's boot 2 journal, the
rpm-ostree[N]: Starting syncfs for system repoentries frommachine-config-operatorclient should now appear >60s after kubelet registers, rather than concurrent with CNI pulls.For fsync: the many per-object
Starting/Completed syncfs() for system repoentries from rpm-ostree during the rebase window should be absent;sysroot-ostree-repo-config.mount: Deactivated successfullyconfirms the bind mount was active.- Description for the changelog
Deferred rpm-ostree rollback cleanup to after NodeReady to reduce boot I/O contention; disabled ostree fsync during firstboot rebase to save ~3-5s
Summary by CodeRabbit
Bug Fixes
Tests