DEV Community

Cover image for Unknown Is Not Failed: Four Bugs Where My Control Plane Claimed to Know Something It Didn't
Nasrul Hazim
Nasrul Hazim

Posted on

Unknown Is Not Failed: Four Bugs Where My Control Plane Claimed to Know Something It Didn't

TL;DR — Nineteen commits into a deployment control plane today. Reading them back, four of them are the same bug wearing different clothes: the system took "I could not find out" and stored it as "I found out, and the answer is bad." A timed-out health probe became a failed workload. An IP address appearing became a machine being reachable. A cached object became a class that would still resolve later. A record being retired became a machine that had stopped billing. Different subsystems, same missing distinction — absence of evidence recorded as evidence of absence.


The setup

I work on a control plane: software whose entire job is to hold an opinion about the state of machines and workloads it doesn't run in-process. Everything it knows, it knows by asking something else over a network — SSH, a vendor API, a container runtime — and every one of those questions can fail to produce an answer.

Which means a control plane has three possible outcomes per question, not two:

  1. It's healthy.
  2. It's broken.
  3. I don't know.

Every bug below is what happens when option three gets folded into option two — or, worse, into option one.


1. A timed-out probe is not a failed workload

The one that made me sit up. The control plane's own application was showing Failed on a page it was, at that moment, successfully serving to me. Its service unit was active running continuously. A root shell on the box couldn't reproduce a single failing read. And yet roughly one health sample in three was recording a 100% error rate.

Here's the shape. Each runtime driver — the systemd one, the Docker one — implements a WorkloadRuntime contract and probes the workload's state. Each one wrapped the probe like this:

public function state(Workload $workload): WorkloadState
{
    try {
        return $this->probe($workload);
    } catch (Throwable) {
        return WorkloadState::Pending;   // ← the lie starts here
    }
}
Enter fullscreen mode Exit fullscreen mode

And the only consumer — a scheduled sampler — did this:

$state = $runtime->state($workload);

$this->record($workload, $state === WorkloadState::Running
    ? Outcome::Healthy
    : Outcome::Failed);          // ← and gets laundered into truth here
Enter fullscreen mode Exit fullscreen mode

Read those two together. An SSH connection that took one second too long returns Pending. Pending is not Running. Not Running maps to Failed. So a slow minute on the network becomes a red badge next to a site answering 200, plus a row in the health log claiming a 100% error rate.

That's already bad as a dashboard bug. It got genuinely dangerous one layer up, because automatic rollback reads those same rows. A healthy release could be rolled back on the strength of a connection that timed out. The safety mechanism was being fed fiction.

The fix is a third channel, not a third enum case:

final class WorkloadStateUnreadable extends RuntimeException
{
    public static function for(Workload $workload, Throwable $previous): self
    {
        return new self(
            "Could not read state for workload {$workload->uuid}.",
            previous: $previous,
        );
    }
}
Enter fullscreen mode Exit fullscreen mode

The driver now throws it where the reading failed, and never where the runtime answered:

public function state(Workload $workload): WorkloadState
{
    try {
        $unit = $this->ssh->run("systemctl show {$workload->unit} --property=ActiveState,LoadState");
    } catch (Throwable $e) {
        throw WorkloadStateUnreadable::for($workload, $e);   // I don't know
    }

    return match ($this->activeState($unit)) {
        'active', 'reloading' => WorkloadState::Running,
        'failed'              => WorkloadState::Failed,
        ''                    => throw WorkloadStateUnreadable::for($workload, ...),
        default               => WorkloadState::Pending,
    };
}
Enter fullscreen mode Exit fullscreen mode

Two details in that match are worth more than the exception itself:

  • reloading is Running. A unit taking new config is serving traffic the whole time. It was falling through to the default arm and turning into a false red.
  • An empty ActiveState is unreadable, not a state. Empty string is output that never arrived. Treating it as "some state I don't recognise" is exactly the category error the whole commit is about.

And the flip side, which is the part people get wrong when they first reach for this pattern: LoadState=not-found stays Pending. A missing systemd unit, a missing Docker container, a missing Kubernetes deployment — those are answers. The runtime replied. The reply was "it isn't here." That's knowledge, and it belongs in the state enum, not in the exception.

The sampler needed no new branch, because it already had the right instinct in a catch that was never being reached:

try {
    $state = $runtime->state($workload);
    $this->record($workload, $state);
} catch (WorkloadStateUnreadable) {
    // Record nothing. Not enough evidence.
}
Enter fullscreen mode Exit fullscreen mode

A gap in the sample history is honest. A row saying "100% errors" is not.

it('records nothing when the runtime cannot be read', function () {
    $runtime = Mockery::mock(WorkloadRuntime::class);
    $runtime->shouldReceive('state')
        ->andThrow(WorkloadStateUnreadable::for($this->workload, new RuntimeException('timeout')));

    app(DeploymentHealthSampler::class)->sample($this->workload);

    expect(DeploymentHealthLog::count())->toBe(0);
});

it('treats a missing unit as pending, not unreadable', function () {
    $this->ssh->respondsWith('ActiveState=inactive'.PHP_EOL.'LoadState=not-found');

    expect($this->runtime->state($this->workload))->toBe(WorkloadState::Pending);
});
Enter fullscreen mode Exit fullscreen mode

One deliberate non-fix: I left the Kubernetes driver alone. Its probe surfaces a missing deployment through the same failure path as a broken kubectl, and telling those two apart means matching on stderr strings I can't verify against a live cluster right now. Fixing it by guessing would replace a known-wrong classifier with an unknown-wrong one. Leaving a documented gap beats shipping confidence I don't have — which, you'll notice, is the same principle as the bug.


2. An IP address is not a reachable machine

Every first-time setup of a freshly provisioned machine failed on Connection refused, and only a manual Retry ever got past it.

The wait job's completion condition was: the cloud provider has reported an address for this machine. Reasonable-sounding. Wrong. The address is published tens of seconds before sshd actually binds to the port. The provider is telling you the truth about the allocation; you're reading it as a claim about the daemon.

So the wait now waits for the thing it actually depends on:

final class SshPortProbe
{
    public function accepts(string $host, int $port = 22, float $timeout = 3.0): bool
    {
        $socket = @fsockopen($host, $port, $errno, $errstr, $timeout);

        if ($socket === false) {
            return false;
        }

        fclose($socket);

        return true;
    }
}
Enter fullscreen mode Exit fullscreen mode

Released every 10 seconds against the same overall deadline the job already had, so this tightens the condition without inventing a new timeout budget.

The important line in that class is @fsockopen, not for the suppression but for what it doesn't do: it does not authenticate. That was a real fork in the road. It's tempting to make the readiness check a full SSH handshake — surely a better signal? No. If the credential is wrong, that's not a race, it's a misconfiguration, and it must fail loudly at the bootstrap step in five seconds rather than get retried politely for ten minutes. A readiness probe that also validates config will happily spend your entire deadline on a problem that will never resolve.

Readiness probes answer "can I start yet?". They should never be answering "will it work?".


3. A cached object is not a class that will still resolve

This one cost me the most time, because the stack trace pointed three frames away from the cause.

Picking a region on the New Server page threw:

Argument #1 ($s) must be of type CloudSize, __PHP_Incomplete_Class given
Enter fullscreen mode Exit fullscreen mode

__PHP_Incomplete_Class is what unserialize() hands back when the class named in the payload cannot be resolved at the moment the payload is read. It doesn't throw. It doesn't warn. It returns an object that satisfies no type declaration anywhere in your codebase, and lets that object travel until something eventually type-hints against it. So the explosion happens in a fn (CloudSize $s) => ... callback, and the cache write that caused it is nowhere in the trace.

Why couldn't the class resolve? Because on this platform it genuinely might not, and I'd never thought about it:

  • Every release installs into its own directory tree.
  • composer install --optimize-autoloader writes a classmap of absolute paths into that tree.
  • Old releases get pruned.
  • A long-lived PHP-FPM worker can be holding a classmap pointing into a directory that no longer exists — while the shared cache still holds a payload naming a class in it.

So this isn't a Laravel bug or a cache-driver bug. It's a structural consequence of atomic-release deploys: the process that writes a cache entry and the process that reads it are not guaranteed to agree on what classes exist.

I could have fixed the symptom — flush on deploy, version the cache key, guard with instanceof. I went structural instead, because the rule is easy to hold in your head and impossible to half-apply:

Caches hold arrays of scalars. Never objects.

Which, with promoted constructor properties, is about six lines per DTO:

final class CloudSize
{
    public function __construct(
        public string $slug,
        public int $vcpus,
        public int $memoryMb,
        public CloudFamily $family,
    ) {}

    public function toCache(): array
    {
        return [...get_object_vars($this), 'family' => $this->family->value];
    }

    public static function fromCache(array $data): self
    {
        return new self(...[
            ...$data,
            'family' => CloudFamily::tryFrom($data['family']) ?? CloudFamily::Unknown,
        ]);
    }
}
Enter fullscreen mode Exit fullscreen mode

Two things I'd point at in a review:

get_object_vars() works here only because every constructor parameter is promoted and public. Spreading a string-keyed array back into the constructor passes them as named arguments, so the round trip is symmetric for free. Add one non-promoted property and this silently drops it. That's a real fragility — it's bought deliberately, in exchange for not hand-writing four mapping methods per DTO, and it's the kind of thing that deserves a comment rather than a shrug.

tryFrom, not from. A backed enum is an object too, so it has to travel as its scalar value. But the vendor can add a new machine family tomorrow, and from() would turn "a distribution I haven't heard of" into a fatal on a cached read — which would be this exact bug class again, one level down. An unknown value is not an invalid one.


4. A retired record is not a stopped bill

The last one isn't a state-reading bug, but it's the same family, and it's the one with an invoice attached.

Two machines were retired from the fleet and went on running — and on billing — because the cloud provider contract shipped with no destroy path at all. That absence was deliberate, and I want to be fair to the reasoning, because it was mine: destroy was sequenced last in the plan, and "last" was enforced by the interface simply not having the method, rather than by anyone remembering.

That's usually a good technique. Make the unsafe thing unrepresentable and you can't do it by accident. But here it produced a platform that can spend a customer's money and cannot stop spending it — and it hid that, because "retire" removing a record looks like the operation finished. The record moving is not the machine stopping.

So destroy landed, and the safety a missing method used to buy got re-bought explicitly:

Provenance decides. Only a machine the platform itself created at a connected account is eligible for deletion. A machine someone adopted by typing in an address is refused — the platform is not what made it exist, and that address might be somebody's employer's VM. The option is absent on the page for an adopted machine, not disabled. Disabled invites a support ticket asking how to enable it.

final class RetireManagedNodeAction
{
    public function __invoke(ManagedNode $node, bool $destroyMachine = false): NodeRetirement
    {
        if ($destroyMachine && ! $node->provider->isCloudCreated()) {
            throw NodeNotDestroyable::notCreatedHere($node);
        }

        if ($destroyMachine) {
            $this->cloud->destroy($node);   // vendor call FIRST
        }

        return $this->retireRecords($node);
    }
}
Enter fullscreen mode Exit fullscreen mode

Opt-in everywhere, defaulting to the old behaviour. The UI requires the machine's address retyped. The equivalent MCP tool takes destroy_machine, defaulting to false, on top of the scope and confirmation it already required.

The vendor call runs before any record moves. This ordering is the whole design. If the API token is missing the delete scope, a 403 retires nothing, and the operator retries against records that still describe reality. Move the records first and a failed vendor call leaves you with a machine nobody is tracking and everybody is paying for.

A vendor 404 is success. Already-gone is the desired end state. A retry has to be able to finish, not die on the absence of the thing it was trying to remove. Idempotency here isn't a nicety; without it the only way out of a partial failure is manual database surgery.

it('refuses to destroy a machine it did not create', function () {
    $node = ManagedNode::factory()->adopted()->create();

    expect(fn () => app(RetireManagedNodeAction::class)($node, destroyMachine: true))
        ->toThrow(NodeNotDestroyable::class);

    expect($node->fresh()->retired_at)->toBeNull();   // nothing moved
});

it('treats an already-deleted machine as destroyed', function () {
    $this->cloud->respondsWith(404);

    $result = app(RetireManagedNodeAction::class)($this->node, destroyMachine: true);

    expect($result->destroyed)->toBeTrue();
});
Enter fullscreen mode Exit fullscreen mode

The takeaway

If your system's job is to hold an opinion about something it can only reach over a network, then "I don't know" is a first-class outcome and needs somewhere to live. Not a nullable field you forget to check. Not a catch that returns a plausible-looking default. Somewhere the type system makes you handle.

A useful audit, and it took me about twenty minutes across this codebase: grep for catch blocks that return instead of throw. Every one is a place where a failure to find out is being converted into a finding. Some of those are correct and considered. Some of them are quietly feeding your alerting, your dashboards, and — in my case — your automatic rollback.

The general shape, stated once:

What actually happened What the system recorded Cost
The probe timed out The workload failed Rollback of a healthy release
An address was allocated The machine was reachable Every first setup failed
An object was cached The class will resolve on read Intermittent 500s, no trace
A record was retired The machine stopped An invoice that never stops

Three of those four were reported to me as flaky. None of them were flaky. They were all deterministic consequences of a missing third case.

What's next: the Kubernetes probe still can't tell "no such deployment" from "kubectl is broken", and I'd rather fix that against a real cluster than against my assumptions about stderr formatting. Same principle — don't ship certainty you haven't earned.

Top comments (0)