Skip to content

fix(core): detect user/container and middleware label changes in job hash - #22

Open
ma-04 wants to merge 1 commit into
masterfrom
fix/job-hash-detect-label-changes
Open

ma-04 wants to merge 1 commit into
masterfrom
fix/job-hash-detect-label-changes

Conversation

@ma-04

@ma-04 ma-04 commented Sep 27, 2026

Copy link
Copy Markdown
Member

Fixes flywp/flywp-internal-tasks#82 (upstream: mcuadros#471).

Problem

A running Ofelia ignores changes to a job's user, container, tty, environment, etc. labels and keeps running the job with the old values until it is restarted. BareJob.Hash() is the only Hash(); every job type embeds BareJob, so the promoted method only hashes Schedule, Name and Command. dockerLabelsUpdate only replaces a job when that hash changes.

This fork is more exposed than upstream because the User default is "" (container's user), so removing the user label is the natural way to switch users, and that change was never picked up.

Changes

  • core/: ExecJob, RunJob, RunServiceJob and LocalJob each get their own Hash(). RunJob.Client and RunServiceJob.Client are tagged hash:"-".
  • cli/config.go: each *JobConfig gets a Hash() over the whole config. dockerLabelsUpdate hashes these config types, so without this, middleware label changes (no-overlap, save-*, slack-*, mail-*) would still be missed. The fix proposed in the issue doesn't cover that.

Tests

  • core/jobhash_test.go: field changes change the hash; the Docker client and runtime state don't.
  • cli/docker_handler_test.go: loads jobs through InitializeApp, then checks that 20 updates with unchanged labels (including environment/volume lists) keep the same jobs, and that removing user and adding no-overlap are applied.
  • Both fail without the fix. core, cli and middlewares suites pass.

Live verification

On two servers (amd64 and arm64), I ran a throwaway container with job id -u every 5 s and recreated it with docker rm -f + docker run. I ran three Ofelia builds side by side:

Step 2e33877 (deployed) ff894a8 (master) this PR
user=nobody label 65534 65534 65534
recreated, no user label, --user 1000:1000 65534 65534 1000
recreated, user=nobody again 65534 65534 65534

With this PR the job was replaced in place (deregister + register in the same millisecond).

Notes

  • If two containers define an exec job with the same name, the job is now replaced on every reconcile, because container is part of the hash. That's a misconfiguration; FlyWP wpcron-<id> names are unique.
  • Existing servers run meghsh/ofelia:latest but never re-pull it, so they need a pull + recreate of ofelia after this is published.

…hash

BareJob.Hash was the only Hash implementation. Every job type embeds
BareJob, so the promoted method only hashed Schedule, Name and Command,
and dockerLabelsUpdate never replaced a job when labels such as user,
container, tty or environment changed. The job kept running with stale
values until Ofelia was restarted.

Give each job type and each job config its own Hash so both job fields
and middleware settings (no-overlap, save-*, slack-*, mail-*) are
covered, and exclude the Docker client from RunJob/RunServiceJob hashes.

Refs flywp/flywp-internal-tasks#82, mcuadros#471
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.

1 participant