@BrunoQuaresma pinged me about the healthcheck section schema. Here is the first draft:

{
    "provisionerd": [
        {
            "name": "provisioner-0",
            "version": "0.23.4",
            "provisioners": ["echo", "terraform"],
            "tags": ["ml-dev", "qa"],
            "created_at": "<timestamp>",
            "last_seen_at": "<timestamp>"
        },
        {
            "name": "provisioner-1",
            ...
        }
    ]
}

Let me know your thoughts!

@BrunoQuaresma pinged me about the healthcheck section schema. Here is the first draft:

{
    "provisionerd": [
        {
            "name": "provisioner-0",
            "version": "0.23.4",
            "provisioners": ["echo", "terraform"],
            "tags": ["ml-dev", "qa"],
            "created_at": "<timestamp>",
            "last_seen_at": "<timestamp>"
        },
        {
            "name": "provisioner-1",
            ...
        }
    ]
}

Let me know your thoughts!

I think that's a reasonable place to start.

I will see what I can do here:

Prune inactive provisioner daemons from database.

@johnstcn Do you think we should rename updated_at to last_seen_at as it is exactly what this field means?

EDIT:

I'm happy to do this, just asking about your thoughts.

@mtojek Yeah I think that makes sense.

@spikecurtis raised questions around the vision, so let me elaborate more on MVP referring to healthcheck requirements

1. We'd like to include a basic section for provisioners in the healthcheck that signals if the provisioner didn't report status in time. The property last_seen_at (and the corresponding field in the database), will be refreshed every few minutes.
2. Healthcheck section will evaluate if the provisioner is up or unresponsive. Let's say it does not report its state in 15 minutes. For now, we can consider only provisioners which checked in during the last 24h.
3. Older entries would be swept with DeleteOld... job.

We'd like to include a basic section for provisioners in the healthcheck that signals if the provisioner didn't report status in time. The property last_seen_at (and the corresponding field in the database), will be refreshed every few minutes.

My interpretation of this is some kind of heartbeat. Is that correct? Are we just having Coderd do the heartbeats as long as there is a connection to the provisionerd, or does it need to be originated at the Provisionerd (e.g. to detect provisioners that are deadlocked even tho the connection is up)?

Healthcheck section will evaluate if the provisioner is up or unresponsive. Let's say it does not report its state in 15 minutes. For now, we can consider only provisioners which checked in during the last 24h.

In a deployment where external provisioner daemons are scaled up and down you'll see a bunch of provisioner daemons that haven't reported in because they were stopped in a scale-down event. I think we need to be careful in the design to not be "alarmist" about these: just note that they were last connected within 24 hours and are now not connected. Don't call this a "warning."

My interpretation of this is some kind of heartbeat. Is that correct? Are we just having Coderd do the heartbeats as long as there is a connection to the provisionerd, or does it need to be originated at the Provisionerd (e.g. to detect provisioners that are deadlocked even tho the connection is up)?

My original thought was to have provisionerdserver bump last_seen_at whenever AcquireJob or AcquireJobWithCancel is called. In the latter case, we may need to spawn a separate goroutine to bump last_seen_at while the in-memory channel is waiting for a job.

Having provisionerd ping a separate endpoint that bumps last_seen_at is a bit further removed than I'd like.

My original thought was to have provisionerdserver bump last_seen_at whenever AcquireJob or AcquireJobWithCancel is called. In the latter case, we may need to spawn a separate goroutine to bump last_seen_at while the in-memory channel is waiting for a job.

Tying the last_seen_at bump explicitly to jobs is going to create annoying edge cases. What if a job takes 30 minutes to run? We should just create a goroutine that bumps it periodically as long as the provisioner is connected.

I know that Cian wrote the mini-design doc (thanks!), but it would be nice to close the conversation.

Are we just having Coderd do the heartbeats as long as there is a connection to the provisionerd, or does it need to be originated at the Provisionerd (e.g. to detect provisioners that are deadlocked even tho the connection is up)?

I defer the final decision to the person responsible for implementation, but my recommendation is to use the best efforts. If a deadlock detection mechanism is hard to implement, I'm fine with scheduling it as a follow-up. Provisionerd heartbeat would be already a great improvement.

because they were stopped in a scale-down event.

Do you know, @spikecurtis, if existing provisionerd could handle the scale-down signal somehow? Even responding to OS signal, and firing an HTTP request to coderd could be a nice starter.

What if a job takes 30 minutes to run? We should just create a goroutine that bumps it periodically as long as the provisioner is connected.

This or and artificial, hidden ALIVE packet streamed similarly to job log entries.

I'm going to say that surfacing provisioner lock-ups is outside of the scope of this specific issue ("add health check for out-of-date provisioner daemon"). We can look deeper into this as a follow-up enhancement.

@stirby current status: this week I've been finishing off the final pieces of this issue. Please see issue description for task list / updates.

Closing based on completed to-do list in issue description.