From 83d000beea4476ae4f37bb4d911c1ec0c195b303 Mon Sep 17 00:00:00 2001 From: Jan Gilla Date: Thu, 8 Oct 2026 10:39:43 +0200 Subject: [PATCH 1/2] F #-: VRouter: run hooks on VRRP role changes Run the executables in /etc/one-appliance/ha-hooks.d/ in name order with "up" (the node became MASTER) or "down" (it became BACKUP) once the stock failover services have been switched. Other services (a routing daemon, for example) can follow the VRRP role without patching the Failover module. A hook never breaks the failover: every failure is only logged. Each hook runs synchronously but bounded: its output goes to a file (not a pipe, so leftover children cannot block the failover) and a hook past 30s is killed together with its whole process group. Add the first unit tests for the Failover module (event handling and the hooks: order, arguments, non-executable files, failing, hanging and raising hooks). Signed-off-by: Jan Gilla --- appliances/VRouter/Failover/execute.rb | 60 +++++++++ appliances/VRouter/Failover/tests.rb | 175 +++++++++++++++++++++++++ 2 files changed, 235 insertions(+) create mode 100644 appliances/VRouter/Failover/tests.rb diff --git a/appliances/VRouter/Failover/execute.rb b/appliances/VRouter/Failover/execute.rb index 1437eba2..94b46185 100644 --- a/appliances/VRouter/Failover/execute.rb +++ b/appliances/VRouter/Failover/execute.rb @@ -1,6 +1,7 @@ # frozen_string_literal: false require 'json' +require 'tempfile' module Service module Failover @@ -31,6 +32,9 @@ module Failover FIFO_PATH = '/run/keepalived/vrrp_notify_fifo.sock' STATE_PATH = '/run/one-failover.state' + HOOKS_DIR = '/etc/one-appliance/ha-hooks.d' + HOOK_TIMEOUT = 30 + HOOK_LOG_TAIL = 4096 STATE_TO_DIRECTION = { 'BACKUP' => :down, @@ -143,6 +147,58 @@ def execute process_events end + # Runs /etc/one-appliance/ha-hooks.d/* with "up" (MASTER) or "down" (BACKUP). + # Hooks must never break the failover itself, so every failure is only logged. + # Each hook runs synchronously but bounded: output goes to a file (not a pipe, + # so leftover children cannot block us) and a hook past the timeout is killed + # together with its whole process group. + def run_hooks(direction, dir: HOOKS_DIR, timeout: HOOK_TIMEOUT) + Dir.glob(File.join(dir, '*')).sort.each do |hook| + next unless File.file?(hook) && File.executable?(hook) + + run_hook hook, direction, timeout + end + end + + def run_hook(hook, direction, timeout) + Tempfile.create('one-ha-hook') do |out| + pid = Process.spawn hook, direction.to_s, pgroup: true, [:out, :err] => out + status = wait_hook pid, Process.clock_gettime(Process::CLOCK_MONOTONIC) + timeout + + if status.nil? + msg :error, "hook #{hook} #{direction} timed out after #{timeout}s, killed" + elsif !status.success? + msg :error, "hook #{hook} #{direction} failed (#{status.exitstatus || status}): #{hook_output out}" + end + end + rescue StandardError => e + msg :error, "hook #{hook} #{direction} raised: #{e.message}" + end + + # Returns the exit status, or nil after killing the process group on timeout. + def wait_hook(pid, deadline) + loop do + _, status = Process.wait2 pid, Process::WNOHANG + return status if status + break if Process.clock_gettime(Process::CLOCK_MONOTONIC) >= deadline + + sleep 0.05 + end + begin + Process.kill 'KILL', -pid + rescue Errno::ESRCH, Errno::EPERM + nil + end + Process.wait pid + nil + end + + def hook_output(file) + file.flush + file.seek [file.size - HOOK_LOG_TAIL, 0].max + file.read.to_s + end + def stay msg :debug, ":STAY (pid = #{Process.pid})" end @@ -169,11 +225,15 @@ def up end puts bash 'rc-update -v -u ||:', terminate: false + + run_hooks :up # after the stock services: forwarding must not wait for hooks end def down msg :debug, ":DOWN (pid = #{Process.pid})" + run_hooks :down + wait_ready :standby 12.times do |attempt| diff --git a/appliances/VRouter/Failover/tests.rb b/appliances/VRouter/Failover/tests.rb new file mode 100644 index 00000000..922edceb --- /dev/null +++ b/appliances/VRouter/Failover/tests.rb @@ -0,0 +1,175 @@ +# frozen_string_literal: true + +require 'rspec' +require 'tmpdir' +require_relative '../../lib/helpers.rb' +require_relative 'execute.rb' + +RSpec.describe Service::Failover do + let(:dir) { Dir.mktmpdir } + let(:out) { Dir.mktmpdir } + let(:log) { File.join(out, 'log') } + let(:errors) { [] } + + before do + allow(described_class).to receive(:msg) { |level, text| errors << text if level == :error } + end + + after do + FileUtils.remove_entry(dir) + FileUtils.remove_entry(out) + end + + def alive?(pid) + Process.kill(0, pid) + true + rescue Errno::ESRCH + false + end + + def hook(name, body, mode: 0o755) + path = File.join(dir, name) + File.write path, "#!/bin/sh\n#{body}\n" + File.chmod mode, path + end + + it 'runs the hooks in name order with the direction as argument' do + hook '20-b', %(echo "b $1" >> #{log}) + hook '10-a', %(echo "a $1" >> #{log}) + + described_class.run_hooks :up, dir: dir + + expect(File.read(log)).to eq "a up\nb up\n" + end + + it 'skips files that are not executable' do + hook '10-a', %(echo a >> #{log}), mode: 0o644 + + described_class.run_hooks :down, dir: dir + + expect(File.exist?(log)).to be false + end + + it 'keeps going and does not raise when a hook fails' do + hook '10-fail', 'exit 3' + hook '20-ok', %(echo ok >> #{log}) + + expect { described_class.run_hooks :down, dir: dir }.not_to raise_error + expect(File.read(log)).to eq "ok\n" + end + + it 'does nothing when the hook directory does not exist' do + expect { described_class.run_hooks :up, dir: File.join(dir, 'missing') }.not_to raise_error + end + + it 'logs a failing hook and runs the next one' do + hook '10-fail', 'echo boom; exit 3' + hook '20-ok', %(echo ok >> #{log}) + + described_class.run_hooks :down, dir: dir + + expect(errors.join).to match(/10-fail.*failed \(3\).*boom/m) + expect(File.read(log)).to eq "ok\n" + end + + it 'logs a signal-killed hook without an empty exit status' do + hook '10-sig', 'kill -9 $$' + + described_class.run_hooks :down, dir: dir + + expect(errors.join).to match(/10-sig/) + expect(errors.join).not_to match(/\(\)/) + end + + it 'kills a hanging hook after the timeout and runs the next one' do + pidfile = File.join(out, 'pid') + hook '10-hang', %(echo $$ > #{pidfile}\nsleep 60) + hook '20-ok', %(echo ok >> #{log}) + + started = Process.clock_gettime(Process::CLOCK_MONOTONIC) + described_class.run_hooks :up, dir: dir, timeout: 1 + elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - started + + expect(elapsed).to be < 5 + expect(alive?(File.read(pidfile).to_i)).to be false + expect(errors.join).to match(/10-hang.*timed out/) + expect(File.read(log)).to eq "ok\n" + end + + it 'does not wait for a background child that outlives the hook' do + pidfile = File.join(out, 'pid') + hook '10-bg', %(sleep 30 &\necho $! > #{pidfile}) + + started = Process.clock_gettime(Process::CLOCK_MONOTONIC) + described_class.run_hooks :up, dir: dir, timeout: 10 + elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - started + + expect(elapsed).to be < 5 + ensure + Process.kill('KILL', File.read(pidfile).to_i) if pidfile && File.exist?(pidfile) + end + + it 'logs a hook that cannot be executed and does not raise' do + path = File.join(dir, '10-noshebang') + File.write path, "not a program\n" + File.chmod 0o755, path + hook '20-ok', %(echo ok >> #{log}) + + expect { described_class.run_hooks :up, dir: dir }.not_to raise_error + expect(errors.join).to match(/10-noshebang/) + expect(File.read(log)).to eq "ok\n" + end + + describe 'hook ordering' do + let(:calls) { [] } + + before do + allow(described_class).to receive(:wait_ready) { |role| calls << [:wait_ready, role] } + allow(described_class).to receive(:load_env) + allow(described_class).to receive(:puts) + allow(described_class).to receive(:sleep) + allow(described_class).to receive(:bash) { |script, **| calls << [:bash, script]; '' } + allow(described_class).to receive(:run_hooks) { |direction| calls << [:hooks, direction] } + end + + it 'runs the up hooks only after the stock services were restarted' do + ENV['ONEAPP_VNF_ROUTER4_ENABLED'] = 'YES' + + described_class.up + + restarts = calls.each_index.select { |i| calls[i].first == :bash && calls[i].last.include?('restart') } + expect(restarts).not_to be_empty + expect(calls.index([:hooks, :up])).to be > restarts.max + expect(calls.last).to eq [:hooks, :up] + ensure + ENV.delete 'ONEAPP_VNF_ROUTER4_ENABLED' + end + + it 'runs the down hooks first, before waiting for keepalived and stopping services' do + described_class.down + + expect(calls.first).to eq [:hooks, :down] + expect(calls.index([:wait_ready, :standby])).to eq 1 + end + end + + describe 'VRRP instance events' do + it 'ignores the events of a single instance' do + allow(described_class).to receive(:load_state).and_return(state: +'BACKUP') + expect(described_class).not_to receive(:save_state) + + task = described_class.to_task(described_class.to_event('INSTANCE "ETH1" MASTER 100')) + + expect(task).to include(ignored: true, direction: :stay) + end + + it 'reacts to the group event only' do + allow(described_class).to receive(:load_state).and_return(state: +'BACKUP') + allow(described_class).to receive(:save_state) + + task = described_class.to_task(described_class.to_event('GROUP "VRouter" MASTER 100')) + + expect(task).to include(ignored: false, direction: :up) + end + end +end From 7658ce62509c3df140d10767e2cec3d6bd00935f Mon Sep 17 00:00:00 2001 From: Jan Gilla Date: Tue, 20 Oct 2026 23:46:49 +0200 Subject: [PATCH 2/2] F #-: VRouter: document the failover service and its hooks Describe what the service does on a role change and the contract of the hooks in ha-hooks.d: arguments, order, environment, time limit, failure handling and that a hook has to be safe to run twice. Signed-off-by: Jan Gilla --- appliances/VRouter/Failover/README.md | 45 +++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 appliances/VRouter/Failover/README.md diff --git a/appliances/VRouter/Failover/README.md b/appliances/VRouter/Failover/README.md new file mode 100644 index 00000000..3a218ea7 --- /dev/null +++ b/appliances/VRouter/Failover/README.md @@ -0,0 +1,45 @@ +# Failover service for the VRouter appliance + +## What it does + +The `one-failover` OpenRC service follows the VRRP role of the router. Keepalived writes its state changes to a FIFO (`/run/keepalived/vrrp_notify_fifo.sock`), and the service reacts to the event of the VRRP sync group (`GROUP`) only. Events of single instances are ignored. + +- **MASTER** (`up`): waits until Keepalived is ready, loads the VM context, then restarts the services that are enabled in the context and stops the others. The services are the ones in the `SERVICES` table of `execute.rb` (Router4, NAT4, LVS, HAProxy, SDNAT4, DNS, DHCP4, WireGuard, each controlled by its `ONEAPP_VNF_*_ENABLED` attribute). +- **BACKUP** (`down`): waits until Keepalived is on standby and stops all of those services. +- `FAULT`, `STOP` and `DELETED` change nothing. If the event loop fails, all services are stopped to avoid a conflict between the nodes. + +The last role is kept in `/run/one-failover.state` (`{"state":"MASTER"}`, `BACKUP`, ...). It is written before the `up` or `down` step runs. + +## Hooks + +Other services can follow the VRRP role without a change to this module. Put an executable file into `/etc/one-appliance/ha-hooks.d/`. The service runs every executable file of that directory, in name order, with one argument: + +| Argument | When | Order | +|---|---|---| +| `up` | the node became MASTER | after the stock services were restarted, so forwarding does not wait for a hook | +| `down` | the node became BACKUP | first, before the stock services are stopped | + +Use a numeric prefix (`10-bgp`, `20-conntrack`) to fix the order. Files that are not executable, and directories, are skipped. A missing directory is not an error. + +Rules for a hook: + +- **Run again.** `down` runs for every BACKUP event, not only for a change of role (a node that boots as BACKUP, a repeated event, and after an error of the event loop). `up` runs only when the role changed to MASTER. A hook has to be safe to run twice in a row. +- **Environment.** `up` hooks see the VM context in their environment (the service loads it before it runs the hooks). `down` hooks do not: a hook that needs a context value reads it itself, for example from `/run/one-context/one_env`, without sourcing it in a shell. +- **Time.** A hook is killed after 30 seconds, together with its whole process group. Hooks run one after the other, so N hooks can delay the failover by up to N times 30 seconds. Keep them short and start long-running work in the background through the init system. +- **Failure.** A hook never breaks the failover. A non-zero exit, a timeout, a signal or an error to start the hook is logged as an error in `/var/log/one-appliance/one-failover.log` (with the tail of its output, at most 4096 bytes) and the next hook runs. +- **Output.** The output of a hook goes to a temporary file, not to a pipe, so a child that outlives the hook cannot block the failover. It is only logged when the hook fails. +- **Role.** A hook can read the current role from `/run/one-failover.state`; the argument is what to do now. + +Example (`/etc/one-appliance/ha-hooks.d/50-example`, mode `0755`): + +```sh +#!/bin/sh +case "$1" in +up) logger -t ha-hook "became MASTER" ;; +down) logger -t ha-hook "became BACKUP" ;; +esac +``` + +## Tests + +`tests.rb` (RSpec) covers the hooks (order, argument, non-executable files, a missing directory, failing, signal-killed, hanging and unstartable hooks, and a background child that outlives its hook), the position of the hooks relative to the stock steps in `up` and `down`, and the handling of the VRRP events (single instances are ignored, the group event is acted on).