feat: add `preChecks`

[0z13]

The idea is to have healthchecks in the pre-activation stage aswell, this is useful if you wanna make sure there are no active connections to the node you are about to switch.

the api/logic is exactly like current healthchecks, except no deployment.healthChecks.http style checks.

example:

  deployment.preChecks.cmd =
    [
      {
        cmd = [
          "false"
        ];
        description = "Node is quarantined, do not touch.";
      }
    ];

[adamtulinius]

Assigned to myself after agreement with @0z13

[adamtulinius]

I wonder if preChecks should be named so it's more clear that it's pre-deploy checks. On the other hand it's also implicit that health checks are post-deploy. Thoughts? When we pick a name it's hard to change down the line. I'm probably just rambling here.

[cafkafk]

I wonder if preChecks should be named so it's more clear that it's pre-deploy checks. On the other hand it's also implicit that health checks are post-deploy. Thoughts? When we pick a name it's hard to change down the line. I'm probably just rambling here.

pre-deploy-checks (or, well preDeployChecks) are better IMO :+1:

I'd also support renaming healthchecks

[adamtulinius]

I wonder if preChecks should be named so it's more clear that it's pre-deploy checks. On the other hand it's also implicit that health checks are post-deploy. Thoughts? When we pick a name it's hard to change down the line. I'm probably just rambling here.

pre-deploy-checks (or, well preDeployChecks) are better IMO 👍

~I'd also support renaming healthchecks~

I wonder if the semantics are actually different enough to call it something completely else. Some CI tools use the concept of deployment gates, and I wonder if we should use that here as well. That way we can also open up the question if morph should wait for a gate to become true/open, or if we would want morph to exit when the requirements for a gate is not met.

[adamtulinius]

What I have done :scream::

1. rebased on master
2. renamed preCheck to preDeployCheck
3. removed all the code duplication and extra nix types

This have caused the following side-effects (maybe I missed something?):

1. preDeployChecks now support both commands and HTTP requests (exactly like health checks)
2. The output of errors from running health checks have had some superfluous text cut from error messages
3. Morph will now print "Running health checks on [...]" instead of "Running healthchecks on [...]"

The only problem I see is if someone is parsing the human readable output from morph, but we never promised that to be stable, so I think it's worth changing the output slightly to avoid copy/pasting code.

I'm not sure that preDeployChecks should keep running forever (technically right now they obey the --timeout flag, which is shared with healthChecks). Maybe we want them to actually just cause morph to fail instantly. Thoughts?

@0z13 - can you kindly test that this is still working, and review my changes? Since we've both touched this MR I suspect we will need a third person to approve, but I want to hear your thoughts. I also resolved my own comments.

[0z13]

beautiful diff stats on that commit @adamtulinius, this is soo much nicer now :D

[srhb]

I understand you need a tie-breaker for the merge. Has this been tested anywhere?

[adamtulinius]

I understand you need a tie-breaker for the merge. Has this been tested anywhere?

I hacked an existing deployment (we really need automated tests, heh) to test the new functionality, and tested that normal health checks still work (albeit with slightly altered output). @0z13 also did testing, but only wrote about it on Slack (I'll DM you).

[srhb]

Oh, ok, works for me. And yes to automated tests, I'll write a story.