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). 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