Merge pull request #1028 from zmrow/pubsys-ami

Address feedback about `cargo make ami`

Makefile.toml

@@ -181,7 +181,13 @@ cargo install \
   --root tools \
   --force \
   --quiet
+'''
+]
 
+[tasks.publish-tools]
+dependencies = ["setup", "fetch-sources"]
+script = [
+'''
 cargo install \
   ${CARGO_MAKE_CARGO_ARGS} \
   --path tools/pubsys \
@@ -281,7 +287,10 @@ alias = "build"
 # to create a repo under /build/repos, named after the arch/variant/version,
 # containing subdirectories for the repo metadata and targets.
 [tasks.repo]
-dependencies = ["build"]
+# Rather than depend on "build", which currently rebuilds images each run, we
+# check for the image files below to save time.  This does mean that `cargo
+# make` must be run before `cargo make repo`.
+dependencies = ["publish-tools"]
 script_runner = "bash"
 script = [
 '''
@@ -294,6 +303,14 @@ trap 'cleanup' EXIT
 
 export PATH="${BUILDSYS_TOOLS_DIR}/bin:${PATH}"
 
+bootlz4="${BUILDSYS_OUTPUT_DIR}/${BUILDSYS_NAME_FULL}-boot.ext4.lz4"
+rootlz4="${BUILDSYS_OUTPUT_DIR}/${BUILDSYS_NAME_FULL}-root.ext4.lz4"
+hashlz4="${BUILDSYS_OUTPUT_DIR}/${BUILDSYS_NAME_FULL}-root.verity.lz4"
+if [ ! -s "${bootlz4}" ] || [ ! -s "${rootlz4}" ] || [ ! -s "${hashlz4}" ]; then
+   echo "Image files don't exist for the current version/commit - ${BUILDSYS_VERSION_FULL} - please run 'cargo make'" >&2
+   exit 1
+fi
+
 # TODO: only add migrations from Release.toml, not all
 MIGRATIONS_DIR="$(mktemp -d)"
 tar xpf "${BUILDSYS_OUTPUT_DIR}/${BUILDSYS_NAME_FULL}-migrations.tar" -C "${MIGRATIONS_DIR}"
@@ -313,9 +330,9 @@ pubsys \
    --version "${BUILDSYS_VERSION_IMAGE}" \
    --variant "${BUILDSYS_VARIANT}" \
    \
-   --boot-image "${BUILDSYS_OUTPUT_DIR}/${BUILDSYS_NAME_FULL}-boot.ext4.lz4" \
-   --root-image "${BUILDSYS_OUTPUT_DIR}/${BUILDSYS_NAME_FULL}-root.ext4.lz4" \
-   --hash-image "${BUILDSYS_OUTPUT_DIR}/${BUILDSYS_NAME_FULL}-root.verity.lz4" \
+   --boot-image "${bootlz4}" \
+   --root-image "${rootlz4}" \
+   --hash-image "${hashlz4}" \
    ${ADD_MIGRATION_TARGETS[*]} \
    \
    --repo-expiration-policy-path "${PUBLISH_EXPIRATION_POLICY_PATH}" \
@@ -332,19 +349,34 @@ ln -sfn "${PUBLISH_REPO_OUTPUT_DIR##*/}" "${PUBLISH_REPO_BASE_DIR}/latest"
 ]
 
 [tasks.ami]
-dependencies = ["build"]
+# Rather than depend on "build", which currently rebuilds images each run, we
+# depend on publish-tools and check for the image files below to save time.
+# This does mean that `cargo make` must be run before `cargo make ami`.
+dependencies = ["publish-tools"]
 script_runner = "bash"
 script = [
 '''
 set -e
 
+export PATH="${BUILDSYS_TOOLS_DIR}/bin:${PATH}"
+
 cleanup() {
    [ -f "${root_image}" ] && rm -f "${root_image}"
    [ -f "${data_image}" ] && rm -f "${data_image}"
 }
 trap 'cleanup' EXIT
 
-export PATH="${BUILDSYS_TOOLS_DIR}/bin:${PATH}"
+# Unlz4 the root / data images
+rootlz4="${BUILDSYS_OUTPUT_DIR}/${BUILDSYS_NAME_FULL}.img.lz4"
+root_image="${rootlz4%.lz4}"
+datalz4="${BUILDSYS_OUTPUT_DIR}/${BUILDSYS_NAME_FULL}-data.img.lz4"
+data_image="${datalz4%.lz4}"
+if [ ! -s "${rootlz4}" ] || [ ! -s "${datalz4}" ]; then
+   echo "Image files don't exist for the current version/commit - ${BUILDSYS_VERSION_FULL} - please run 'cargo make'" >&2
+   exit 1
+fi
+lz4 -df "${rootlz4}" "${root_image}"
+lz4 -df "${datalz4}" "${data_image}"
 
 # Canonicalize architecture identifier to the value recognized by EC2.
 case "${BUILDSYS_ARCH,,}" in
@@ -356,15 +388,6 @@ case "${BUILDSYS_ARCH,,}" in
       arch="${BUILDSYS_ARCH}" ;;
 esac
 
-# Unlz4 the root / data images
-rootlz4="${BUILDSYS_OUTPUT_DIR}/${BUILDSYS_NAME_FULL}.img.lz4"
-root_image="${rootlz4%.lz4}"
-lz4 -df "${rootlz4}" "${root_image}"
-
-datalz4="${BUILDSYS_OUTPUT_DIR}/${BUILDSYS_NAME_FULL}-data.img.lz4"
-data_image="${datalz4%.lz4}"
-lz4 -df "${datalz4}" "${data_image}"
-
 pubsys \
    --infra-config-path "${PUBLISH_INFRA_CONFIG_PATH}" \
    \

tools/pubsys/src/aws/ami/mod.rs

@@ -71,9 +71,7 @@ pub(crate) async fn run(args: &Args, ami_args: &AmiArgs) -> Result<()> {
     let infra_config = InfraConfig::from_path(&args.infra_config_path).context(error::Config)?;
     trace!("Parsed infra config: {:?}", infra_config);
 
-    let aws = infra_config.aws.context(error::MissingConfig {
-        missing: "aws section",
-    })?;
+    let aws = infra_config.aws.unwrap_or_else(|| Default::default());
 
     // If the user gave an override list of regions, use that, otherwise use what's in the config.
     let mut regions = if !ami_args.regions.is_empty() {

tools/pubsys/src/config.rs

@@ -34,11 +34,13 @@ impl InfraConfig {
 }
 
 /// AWS-specific infrastructure configuration
-#[derive(Debug, Deserialize)]
+#[derive(Debug, Default, Deserialize)]
 pub(crate) struct AwsConfig {
+    #[serde(default)]
     pub(crate) regions: VecDeque<String>,
     pub(crate) role: Option<String>,
     pub(crate) profile: Option<String>,
+    #[serde(default)]
     pub(crate) region: HashMap<String, AwsRegionConfig>,
 }