about summary refs log tree commit diff
diff options
context:
space:
mode:
authorVincent Ambo <mail@tazj.in>2022-06-02T16·59+0000
committertazjin <tazjin@tvl.su>2022-06-03T17·36+0000
commit56a97a0337a3f3eedc859b5f380a6f62adcb0368 (patch)
tree84dd40a45db0643d0f931f7115b6096930b969bb
parenta027ee9f03c33ecb859139d08823da9bdf438b64 (diff)
refactor(nix/buildkite): Explicit support for build phases r/4203
Previously the extra steps were roughly divided into steps that run
"at build time" (i.e. before we publish results to Gerrit), and
"post-build" (i.e. later on).

In practice, these are something like a build/release pairing, where
steps running after the build results are returned are mostly run for
side-effects (e.g. publishing git subtrees to external repos).

This refactoring makes this distinction explicit in //nix/buildkite
and changes the extraSteps API with an explicit `phases` attribute
instead of the previous `postStep` attribute.

In practice the previous API is still supported, but will throw
evaluation warnings until an arbitrarily chosen cutoff date of
2022-10-01 at which point we will change using it into a hard error.

This uncovered a few strange behaviours which we only accidentally
avoided, most of which I have left TODOs about and will clean up in
subsequent commits.

The purpose of this commit is to allow for separate evaluations of
only build or only release steps, for example if release steps are
evaluated in a slightly different context (e.g. with overridden
versioning that is not relevant to standard CI functionality).

Change-Id: I0b0186e3824273c15a774260708702d4a5974dac
Reviewed-on: https://cl.tvl.fyi/c/depot/+/5825
Reviewed-by: ezemtsov <eugene.zemtsov@gmail.com>
Tested-by: BuildkiteCI
-rw-r--r--nix/buildkite/default.nix171
1 files changed, 108 insertions, 63 deletions
diff --git a/nix/buildkite/default.nix b/nix/buildkite/default.nix
index f071b88f9c..abce35b459 100644
--- a/nix/buildkite/default.nix
+++ b/nix/buildkite/default.nix
@@ -11,11 +11,9 @@
 let
   inherit (builtins)
     attrValues
-    concatMap
+    concatLists
     concatStringsSep
-    filter
     foldl'
-    getEnv
     hasAttr
     hashString
     isNull
@@ -23,8 +21,6 @@ let
     length
     listToAttrs
     mapAttrs
-    partition
-    pathExists
     toJSON
     unsafeDiscardStringContext;
 
@@ -147,8 +143,15 @@ rec {
       postBuildSteps ? [ ]
     }:
     let
-      # Convert a target into all of its build and post-build steps,
-      # treated separately as they need to be in different chunks.
+      # List of phases to include. Currently the only phases are 'build'
+      # (Nix builds and extra steps that are not post-build steps) and
+      # 'post' (all post-build steps).
+      #
+      # TODO(tazjin): Configurable set of phases.
+      phases = [ "build" "release" ];
+
+      # Convert a target into all of its steps, separated by build
+      # phase (as phases end up in different chunks).
       targetToSteps = target:
         let
           step = mkStep headBranch parentTargetMap target;
@@ -160,51 +163,38 @@ rec {
           # Note that this will never affect the label.
           overridable = f: mkStep headBranch parentTargetMap (f target);
 
-          # Split build/post-build steps
-          splitExtraSteps = partition ({ postStep, ... }: postStep)
-            (attrValues (mapAttrs
-              (name: value: {
-                inherit name value;
-                postStep = (value ? prompt) || (value.postBuild or false);
-              })
+          # Split extra steps by phase.
+          splitExtraSteps = lib.groupBy ({ phase, ... }: phase)
+            (attrValues (mapAttrs (normaliseExtraStep overridable)
               (target.meta.ci.extraSteps or { })));
 
-          mkExtraStep' = { name, value, ... }: mkExtraStep overridable name value;
-          extraBuildSteps = map mkExtraStep' splitExtraSteps.wrong; # 'wrong' -> no prompt
-          extraPostSteps = map mkExtraStep' splitExtraSteps.right; # 'right' -> has prompt
+          extraSteps = mapAttrs (_: steps: map mkExtraStep steps) splitExtraSteps;
         in
-        {
-          buildSteps = [ step ] ++ extraBuildSteps;
-          postSteps = extraPostSteps;
+        extraSteps // {
+          build = [ step ] ++ (extraSteps.build or [ ]);
         };
 
-      # Combine all target steps into separate build and post-build step lists.
-      steps = foldl'
-        (acc: t: {
-          buildSteps = acc.buildSteps ++ t.buildSteps;
-          postSteps = acc.postSteps ++ t.postSteps;
-        })
-        { buildSteps = [ ]; postSteps = [ ]; }
-        (map targetToSteps drvTargets);
-
-      buildSteps =
-        # Add build steps for each derivation target and their extra
-        # steps.
-        steps.buildSteps
-
-        # Add additional steps (if set).
-        ++ additionalSteps;
-
-      postSteps =
-        # Add post-build steps for each derivation target.
-        steps.postSteps
-
-        # Add any globally defined post-build steps.
-        ++ postBuildSteps;
-
-      buildChunks = pipelineChunks "build" buildSteps;
-      postBuildChunks = pipelineChunks "release" postSteps;
-      chunks = buildChunks ++ postBuildChunks;
+      # Combine all target steps into step lists per phase.
+      #
+      # TODO(tazjin): Refactor when configurable phases show up.
+      globalSteps = {
+        build = additionalSteps;
+        release = postBuildSteps;
+      };
+
+      phasesWithSteps = lib.zipAttrsWithNames phases (_: concatLists)
+        ((map targetToSteps drvTargets) ++ [ globalSteps ]);
+
+      # Generate pipeline chunks for each phase.
+      chunks = foldl'
+        (acc: phase:
+          let phaseSteps = phasesWithSteps.${phase} or [ ]; in
+          if phaseSteps == [ ]
+          then acc
+          else acc ++ (pipelineChunks phase phaseSteps))
+        [ ]
+        phases;
+
     in
     runCommandNoCC "buildkite-pipeline" { } ''
       mkdir $out
@@ -294,42 +284,97 @@ rec {
     ];
   };
 
-  # Create the Buildkite configuration for an extra step, optionally
-  # wrapping it in a gate group.
-  mkExtraStep = overridableParent: key:
+  # Validate and normalise extra step configuration before actually
+  # generating build steps, in order to use user-provided metadata
+  # during the pipeline generation.
+  normaliseExtraStep = overridableParent: key:
     { command
     , label ? key
-    , prompt ? false
     , needsOutput ? false
     , parentOverride ? (x: x)
     , branches ? null
     , alwaysRun ? false
-    , postBuild ? false
-    }@cfg:
+
+      # TODO(tazjin): Default to 'build' after 2022-10-01.
+    , phase ? if (isNull postBuild || !postBuild) then "build" else "release"
+
+      # TODO(tazjin): Forbid prompt steps in 'build' phase.
+    , prompt ? false
+
+      # TODO(tazjin): Turn into hard-failure after 2022-10-01.
+    , postBuild ? null
+    }:
     let
       parent = overridableParent parentOverride;
       parentLabel = parent.env.READTREE_TARGET;
+    in
+    {
+      inherit
+        alwaysRun
+        branches
+        command
+        key
+        label
+        needsOutput
+        parent
+        parentLabel
+        prompt;
+
+      # //nix/buildkite is growing a new feature for adding different
+      # "build phases" which supersedes the previous `postBuild`
+      # boolean API.
+      #
+      # To help users transition, emit warnings if the old API is used.
+      #
+      # TODO(tazjin): Validate available phases.
+      phase = lib.warnIfNot (isNull postBuild) ''
+        In step '${label}' (from ${parentLabel}):
 
+        Please note: The CI system is introducing support for running
+        steps in different build phases.
+
+        The currently supported phases are 'build' (all Nix targets,
+        extra steps such as tests that feed into the build results,
+        etc.) and 'release' (steps that run after builds and tests
+        have already completed).
+
+        This replaces the previous boolean `postBuild` API in extra
+        step definitions. Please remove the `postBuild` parameter from
+        this step and instead set `phase = ${phase};`.
+      ''
+        phase;
+    };
+
+  # Create the Buildkite configuration for an extra step, optionally
+  # wrapping it in a gate group.
+  mkExtraStep = cfg:
+    let
       step = {
-        label = ":gear: ${label} (from ${parentLabel})";
-        skip = if alwaysRun then false else parent.skip or false;
-        depends_on = lib.optional (!alwaysRun && !needsOutput) parent.key;
-        branches = if branches != null then lib.concatStringsSep " " branches else null;
+        label = ":gear: ${cfg.label} (from ${cfg.parentLabel})";
+        skip = if cfg.alwaysRun then false else cfg.parent.skip or false;
+        # TODO(tazjin): Remember to gate this behaviour with active phases.
+        depends_on = lib.optional (!cfg.alwaysRun && !cfg.needsOutput) cfg.parent.key;
+        branches =
+          if cfg.branches != null
+          then lib.concatStringsSep " " cfg.branches else null;
 
-        command = pkgs.writeShellScript "${key}-script" ''
+        command = pkgs.writeShellScript "${cfg.key}-script" ''
           set -ueo pipefail
-          ${lib.optionalString needsOutput "echo '~~~ Preparing build output of ${parentLabel}'"}
-          ${lib.optionalString needsOutput parent.command}
+          ${lib.optionalString cfg.needsOutput
+            "echo '~~~ Preparing build output of ${cfg.parentLabel}'"
+          }
+          ${lib.optionalString cfg.needsOutput cfg.parent.command}
           echo '+++ Running extra step command'
-          exec ${command}
+          exec ${cfg.command}
         '';
       };
     in
-    if (isString prompt)
+    if (isString cfg.prompt)
     then
       mkGatedStep
         {
-          inherit step label parent prompt;
+          inherit step;
+          inherit (cfg) label parent prompt;
         }
     else step;
 }