feat: add dry-run mode - #197
Open
vadossam wants to merge 3 commits into
Open
Conversation
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
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.
Motivation
Enabling zeropod on an existing workload is currently an all-or-nothing decision. To find out how a pod would actually behave - how often it would scale down, whether
scaledown-durationis tuned right, how restore latency looks on first connection - you have to let it really checkpoint and restore, which is risky to try on production or production-like workloads.This PR adds a dry-run mode so you can observe zeropod's scale-down/restore decisions on a real workload without it ever actually happening.
What this adds
zeropod.ctrox.dev/dry-run: "true"With it set, the shim runs its scale-down timer exactly as normal - same activity tracking, same
scaledown-durationlogic - but when the timer fires it does not checkpoint/kill the process and does not enable the eBPF traffic redirect. The real process keeps running and serving traffic the entire time. Instead:would have scaled down/would have restored,zeropod_dry_run_scale_downs_total/zeropod_dry_run_would_restores_total(same label set as the existing checkpoint/restore metrics).Restore detection covers both of the triggers real zeropod supports:
lastActivity()/socket_tracker) - polled once a second while in the simulated scaled-down state.kubectl exec, mirrored from the real restore-on-exec path inwrapper.Exec, including the same suppression semantics: an active exec session keeps blocking the simulated scale-down for its whole duration, not just at the momentexecwas called (rescheduling is deferred towrapper.Delete's existingrunningExecs == 0check, exactly like the real path).Cannot be combined with
migrate/live-migrate(rejected at config-parse time) - the two features are contradictory by definition, and node-drain eviction (shim/evac.go) checkpoints independently of the normal timer path this feature guards.Design notes
The core constraint driving the implementation: dry-run must never mutate real container state.
Container.scaledDownstaysfalsefor the container's entire life while dry-run is active, tracked instead via a separatedryRunScaledDownflag. This matters because several other things key offScaledDown():manager/pod_scaler.gowould shrink cpu/memory requests on a container that's actually still fully running.manager/pod_labeller.gowould flip the pod'sstatus.zeropod.ctrox.dev/<container>label toSCALED_DOWN, misleading anything reading it as ground truth.Pids/Stats/Killin the task service branch on it to synthesize scaled-down responses.So instead of adding a new
ContainerPhase, the newContainerStatus.dry_runfield is a side-channel:event_creator.gochecks it before the phase-based switch and creates a distinctDry runevent, while phase itself never changes -pod_scaler/pod_labellerneeded no changes at all.Skipping the real scale-down is a single early branch in
scaleDown(), beforeactivator.Reset()(the eBPF redirect enable) is ever called - confirmed via the eBPF source thattrack_activity()runs on ingress independently of and before the redirect-enable check, so the activity tracker keeps working correctly with the redirect never enabled.Files touched
api/shim/v1/{config.go,shim.proto}- annotation + two new fields (ContainerStatus.dry_run,ContainerMetrics.dry_run_scale_downs/dry_run_would_restores), regenerated viamake ttrpcshim/dryrun.go(new) - the whole mechanismshim/checkpoint.go,shim/container.go,shim/task/service_zeropod.go- wiringmanager/event_creator.go,manager/metrics_collector.go- Events and metricsdocs/configuration/README.md,docs/metrics.mdTesting
Unit tests for config parsing (including the migrate-conflict rejection) and event creation, plus an e2e case asserting the pod's status label never leaves
RUNNINGunder dry-run.Manually verified end-to-end on a real RKE2 cluster (in addition to local kind testing) - actual
kubectl describe podoutput:The container never actually restarted throughout (0 restarts, status label pinned at
RUNNING) while these events were produced from real, unmodified traffic.