Merge pull request #34 from boozallen/8-docker-python-dep-install-fix

#8 Require docker builds to fail if Python installation fails + migration

extensions/extensions-docker/aissemble-model-training-api-containers/aissemble-model-training-api-sagemaker/src/main/resources/docker/Dockerfile

@@ -10,6 +10,7 @@ ENV PATH="/opt/venv/bin:$PATH"
 COPY ./target/wheels/* /tmp/wheels/
 # Re-install any dependencies defined in the base image into the virtual environment, then install new requirements
 RUN pip install -r /tmp/requirements.txt && \
+    set -e && \
     cd /tmp/wheels/; for x in *.whl; do pip install $x --no-cache-dir; done
 
 

extensions/extensions-docker/aissemble-model-training-api-containers/aissemble-model-training-api/src/main/resources/docker/Dockerfile

@@ -9,6 +9,7 @@ ENV PATH="/opt/venv/bin:$PATH"
 COPY ./target/wheels/* /tmp/wheels/
 # Re-install any dependencies defined in the base image into the virtual environment, then install new requirements
 RUN pip install -r /tmp/requirements.txt && \
+    set -e && \
     cd /tmp/wheels/; for x in *.whl; do pip install $x --no-cache-dir; done
 
 # Start a new build stage based on a slim Python image, which will have the minimal amount of code, FastAPI

extensions/extensions-docker/aissemble-versioning/src/main/resources/docker/Dockerfile

@@ -16,6 +16,7 @@ RUN wget https://archive.apache.org/dist/maven/maven-3/${MAVEN_VERSION}/binaries
 COPY ./target/wheels/* /tmp/wheels/
 # Re-install any dependencies defined in the base image into the virtual environment, then install new requirements
 RUN pip install -r /tmp/requirements.txt && \
+    set -e && \
     cd /tmp/wheels/; for x in *.whl; do pip install $x --no-cache-dir; done
 
 COPY ./target/versioning_api.py /app/versioning_api.py

foundation/foundation-configuration-store/src/main/java/com/boozallen/aissemble/configuration/store/YamlProperty.java

@@ -44,7 +44,7 @@ public boolean equals(Object o) {
             return false;
         }
 
-        // does not include value to enable check of erroneously indistinguisabe properties,
+        // does not include value to enable check of erroneously indistinguishable properties,
         // where two or more properties have identical groupName and name (but different values)
         YamlProperty yamlProperty = (YamlProperty) o;
         return Objects.equals(name, yamlProperty.getName()) &&

foundation/foundation-mda/src/main/resources/templates/general-docker/airflow.docker.file.vm

@@ -29,13 +29,13 @@ ENV VENV=$AIRFLOW_HOME/pipelines-env
 ENV KRAUSENING_BASE ${AIRFLOW_HOME}/config/
 
 WORKDIR /
-RUN find /installation/requirements -path '/installation/requirements/*/*/*' -name requirements.txt -type f -exec "$VENV/bin/python3" -m pip install --no-cache-dir -r '{}' ';'
+RUN set -e && files=$(find /installation/requirements -path '/installation/requirements/*/*/*' -name requirements.txt -type f); for file in $files; do "$VENV/bin/python3" -m pip install --no-cache-dir -r $file || exit 1; done;
 
 #PIPELINES
 COPY ./target/dockerbuild/. $AIRFLOW_HOME/pipelines/
 
 #Run pip install on *.tar.gz if any are found -could be none for a project without training steps
-RUN find $AIRFLOW_HOME/pipelines -path "$AIRFLOW_HOME/pipelines/*/*/*" -name *.tar.gz -type f -exec "$VENV/bin/python3" -m pip install --no-deps --no-cache-dir '{}' ';'
+RUN set -e && files=$(find $AIRFLOW_HOME/pipelines -path "$AIRFLOW_HOME/pipelines/*/*/*" -name *.tar.gz -type f); for file in $files; do "$VENV/bin/python3" -m pip install --no-deps --no-cache-dir $file || exit 1; done;
 
 ENV PIP_USER=true
 

foundation/foundation-mda/src/main/resources/templates/general-docker/inference.docker.file.vm

@@ -46,7 +46,8 @@ RUN python -m pip install --ignore-installed -r /installation/${inferenceModule}
 
 # Install ${inferenceModule}
 COPY target/${inferenceModule}/dist /modules/${inferenceModule}
-RUN cd /modules/${inferenceModule}; for x in *.whl; do python -m pip install $x --no-cache-dir --no-deps; done
+RUN set -e && \
+    cd /modules/${inferenceModule}; for x in *.whl; do python -m pip install $x --no-cache-dir --no-deps; done
 
 # Start the inference API drivers for FastAPI and gRPC
 CMD python -m ${inferenceModuleSnakeCase}.inference_api_driver "fastAPI" & python -m ${inferenceModuleSnakeCase}.inference_api_driver "grpc"

foundation/foundation-mda/src/main/resources/templates/general-docker/sagemaker-training.docker.file.vm

@@ -29,6 +29,6 @@ RUN python3.11 -m pip install -r /installation/${trainingModule}/requirements.tx
 
 # Install ${trainingModule}
 COPY ./target/dockerbuild/${trainingPipeline}/${trainingModule}/*.tar.gz /installation/${trainingModule}/
-RUN find /installation/${trainingModule}/ -name *.tar.gz -type f -exec python3.11 -m pip install --no-deps --no-cache-dir '{}' ';'
+RUN set -e && files=$(find /installation/${trainingModule}/ -name *.tar.gz -type f); for file in $files; do python3.11 -m pip install --no-deps --no-cache-dir $file || exit 1; done;
 
 ENTRYPOINT ["python3.11", "-m", "${trainingModuleSnakeCase}.${trainingPipelineSnakeCase}"]

foundation/foundation-mda/src/main/resources/templates/general-docker/spark-worker.docker.file.vm

@@ -23,17 +23,17 @@ WORKDIR /
 
 #Install 3rd party Pyspark pipeline dependencies (does nothing if you only have Spark pipelines)
 COPY ./target/dockerbuild/requirements/. /tmp/requirements/
-RUN find /tmp/requirements -path '/tmp/requirements/*/*' -name requirements.txt -type f -exec python3 -m pip install --no-cache-dir -r '{}' ';'
+RUN set -e && files=$(find /tmp/requirements -path '/tmp/requirements/*/*' -name requirements.txt -type f); for file in $files; do python3 -m pip install --no-cache-dir -r $file || exit 1; done;
 
 #Install monorepo Pyspark pipeline dependencies (does nothing if you only have Spark pipelines)
 COPY ./target/dockerbuild/wheels/. /tmp/wheels/
-RUN find /tmp/wheels -path '/tmp/wheels/*' -name '*.whl' -type f -exec python3 -m pip install --no-cache-dir '{}' ';'
+RUN set -e && files=$(find /tmp/wheels -path '/tmp/wheels/*' -name '*.whl' -type f); for file in $files; do python3 -m pip install --no-cache-dir $file || exit 1; done;
 
 #Pipelines
 COPY --chown=spark:spark --chmod=777 ./target/dockerbuild/. /opt/spark/jobs/pipelines/
 
 #Install Pyspark pipelines (does nothing if you only have Spark pipelines)
-RUN find /opt/spark/jobs -path '/opt/spark/jobs/pipelines/*/*' -name '*.tar.gz' -type f -exec python3 -m pip install --no-deps --no-cache-dir '{}' ';'
+RUN set -e && files=$(find /opt/spark/jobs -path '/opt/spark/jobs/pipelines/*/*' -name '*.tar.gz' -type f); for file in $files; do python3 -m pip install --no-deps --no-cache-dir $file || exit 1; done;
 
 COPY --chown=spark ./src/main/resources/krausening/ ${SPARK_HOME}/krausening/
 

foundation/foundation-mda/src/main/resources/templates/general-docker/training.docker.file.vm

@@ -38,6 +38,7 @@ RUN python3.11 -m pip install --ignore-installed -r /installation/${trainingModu
 
 # Install ${trainingModule}
 COPY target/${trainingModule}/dist /modules/${trainingModule}
-RUN cd /modules/${trainingModule}; for x in *.whl; do pip install $x --no-cache-dir --no-deps; done
+RUN set -e && \
+    cd /modules/${trainingModule}; for x in *.whl; do pip install $x --no-cache-dir --no-deps; done
 
 CMD python3.11 -m ${trainingModuleSnakeCase}.${trainingPipelineSnakeCase}_driver

foundation/foundation-upgrade/src/main/java/com/boozallen/aissemble/upgrade/migration/v1_7_0/DockerPipInstallMigration.java

@@ -0,0 +1,139 @@
+package com.boozallen.aissemble.upgrade.migration.v1_7_0;
+
+/*-
+ * #%L
+ * aiSSEMBLE::Foundation::Upgrade
+ * %%
+ * Copyright (C) 2021 Booz Allen
+ * %%
+ * This software package is licensed under the Booz Allen Public License. All Rights Reserved.
+ * #L%
+ */
+
+import com.boozallen.aissemble.upgrade.migration.AbstractAissembleMigration;
+import com.boozallen.aissemble.upgrade.util.FileUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+public class DockerPipInstallMigration extends AbstractAissembleMigration {
+
+    private static final Logger logger = LoggerFactory.getLogger(DockerPipInstallMigration.class);
+    // starts with a negative look-ahead to ensure we are not matching lines that already contain
+    // "RUN set -e &&"
+    private static final String forDoPattern = "^(?!.*RUN set -e &&)RUN .*for .+; do .+; done";
+    // () denote various capture groups to separate relevant portions of the regex match
+    // (?:) enables the enclosed portion of the regex pattern to not be evaluated as a capture group
+    private static final String findExecPattern =
+            "(^RUN )(find )(.*)( -exec)(.*python.* install (?:-\\S*\\s)*)(.+)";
+    private static final String RUN = "RUN ";
+    private static final String RUN_WITH_SET_E = "RUN set -e && ";
+    private static final String FILES_FIND = "files=$(find ";
+    private static final String FOR_FILE_IN_FILES_DO = "); for file in $files; do";
+    private static final String FILE_OR_EXIT_1_DONE = "$file || exit 1; done;";
+    private static final String upgradableInstructionsPattern =
+            String.join("|", findExecPattern, forDoPattern);
+
+    /**
+     * @param dockerFile the file to check
+     * @return boolean denoting if the dockerfile has at least one instruction to be migrated
+     */
+    @Override
+    protected boolean shouldExecuteOnFile(File dockerFile) {
+        boolean shouldExecute = false;
+
+        if (dockerFile != null && dockerFile.exists()) {
+            try {
+                shouldExecute = FileUtils.hasRegExMatch(upgradableInstructionsPattern, dockerFile);
+            } catch (IOException e) {
+                logger.error(String.format("Unable to load '%s' due to exception:", dockerFile.getAbsolutePath()), e);
+            }
+        }
+
+        return shouldExecute;
+    }
+
+    /**
+     * @param dockerFile the file to update instruction(s)
+     * @return boolean denoting if the dockerfile has had at least one instruction migrated
+     */
+    @Override
+    protected boolean performMigration(File dockerFile) {
+        logger.info("Migrating file: {}", dockerFile.getAbsolutePath());
+        boolean migratedSuccessfully = false;
+
+        try {
+            // first, migrate for-do pattern instruction(s)
+            boolean forDoMigratedSuccessfully = FileUtils.modifyRegexMatchInFile(
+                    dockerFile,
+                    forDoPattern,
+                    RUN,
+                    RUN_WITH_SET_E
+            );
+
+            // second, migrate find-exec pattern instruction(s)
+            boolean findExecMigratedSuccessfully =
+                    migrateFindExecInstructions(dockerFile);
+            migratedSuccessfully = forDoMigratedSuccessfully || findExecMigratedSuccessfully;
+        } catch (Exception e) {
+            logger.error(String.format("Unable to migrate '%s' due to exception:", dockerFile.getAbsolutePath()), e);
+            return false;
+        }
+
+        return migratedSuccessfully;
+    }
+
+    /**
+     *
+     * @param dockerFile the file to migrate find-exec docker instruction(s)
+     * @return boolean denoting if the dockerfile has had at least one find-exec instruction migrated
+     */
+    protected boolean migrateFindExecInstructions(File dockerFile) {
+        if (dockerFile == null || !dockerFile.exists()) {
+            return false;
+        }
+
+        boolean modified = false;
+
+        try {
+            Path path = dockerFile.toPath();
+            Charset charset = StandardCharsets.UTF_8;
+            List<String> resultLines = new ArrayList<>();
+
+            Pattern pattern = Pattern.compile(DockerPipInstallMigration.findExecPattern);
+            Matcher lineMatcher;
+            for (String line : Files.readAllLines(path, charset)) {
+                // check each line for a regex match
+                lineMatcher = pattern.matcher(line);
+                String result = line;
+                if (lineMatcher.find()) {
+                    // if match found, rebuild the line with relevant capture groups replaced
+                    result = RUN_WITH_SET_E
+                            + FILES_FIND
+                            + lineMatcher.group(3)
+                            + FOR_FILE_IN_FILES_DO
+                            + lineMatcher.group(5)
+                            + FILE_OR_EXIT_1_DONE;
+                    modified = true;
+                }
+                resultLines.add(result);
+            }
+            if (modified) {
+                Files.write(path, resultLines, charset);
+            }
+        } catch (IOException e) {
+            return false;
+        }
+        return modified;
+    }
+}

foundation/foundation-upgrade/src/main/java/com/boozallen/aissemble/upgrade/util/FileUtils.java

@@ -19,6 +19,7 @@
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.regex.Matcher;
@@ -31,7 +32,7 @@ public final class FileUtils {
      * @param file the File
      * @param regex a regex representing the text to replace, as a String
      * @param replacement the replacement text to substitute the regex
-     * @return An ArrayList of Strings representing each capture group in the regex that was matched
+     * @return a boolean set to true if at least one replacement was performed in the file
      */
     public static boolean replaceInFile(File file, String regex, String replacement) throws IOException {
         boolean replacedInFile = false;
@@ -48,6 +49,44 @@ public static boolean replaceInFile(File file, String regex, String replacement)
         return replacedInFile;
     }
 
+    /**
+     * Evaluates a file against a regex pattern and replaces a substring of each regex match
+     * with a specified replacement
+     * @param file the File
+     * @param regex a regex representing the text to replace a substring of
+     * @param substring the substring of the regex match that will be replaced
+     * @param replacement the replacement of the match substring
+     * @return a boolean set to true if at least one modification was performed in the file
+     */
+    public static boolean modifyRegexMatchInFile(File file, String regex, String substring, String replacement) {
+        if (file == null || !file.exists()) {
+            return false;
+        }
+
+        boolean modified = false;
+
+        try {
+            Path path = file.toPath();
+            Charset charset = StandardCharsets.UTF_8;
+            List<String> resultLines = new ArrayList<>();
+
+            Pattern pattern = Pattern.compile(regex);
+            for (String line : Files.readAllLines(path, charset)) {
+                if (pattern.matcher(line).find()) {
+                    line = line.replace(substring, replacement);
+                    modified = true;
+                }
+                resultLines.add(line);
+            }
+            if (modified) {
+                Files.write(path, resultLines, charset);
+            }
+        } catch (IOException e) {
+            return false;
+        }
+        return modified;
+    }
+
     /**
      * Function to read in the {@link File} object and return a {@link List} of the contents.
      * @param file {@link File} to read
@@ -104,6 +143,24 @@ public static ArrayList<String> getRegExCaptureGroups(String regex, String input
         return captured;
     }
 
+    /**
+     * Evaluates a regex pattern against a file to determine if at least one regex match exists
+     *
+     * @param regex a regex pattern, as a String
+     * @param file the file to search for matching substrings
+     * @return true if there is at least one regex match, otherwise false
+     */
+    public static boolean hasRegExMatch(String regex, File file) throws IOException {
+        String fileContent;
+        if (file != null && file.exists()) {
+            Charset charset = StandardCharsets.UTF_8;
+            fileContent = Files.readString(file.toPath(), charset);
+            return Pattern.compile(regex, Pattern.MULTILINE).matcher(fileContent).find();
+        } else {
+            return false;
+        }
+    }
+
     /**
      * Infers the indentation style from the given line.
      *

foundation/foundation-upgrade/src/main/resources/migrations.json

@@ -45,5 +45,15 @@
         "excludes": ["**/target/**/Chart.yaml"]
       }
     ]
+  },
+  {
+    "name": "upgrade-docker-pip-install-migration",
+    "implementation": "com.boozallen.aissemble.upgrade.migration.v1_7_0.DockerPipInstallMigration",
+    "fileSets": [
+      {
+        "includes": ["*-docker/*/src/main/resources/docker/Dockerfile"],
+        "excludes": ["*-docker/*/target/Dockerfile"]
+      }
+    ]
   }
 ]