From 7389385a205968768c340675fe66c73cfc6ca2b7 Mon Sep 17 00:00:00 2001
From: Claudiu Belu <cbelu@cloudbasesolutions.com>
Date: Fri, 13 Mar 2020 03:55:54 -0700
Subject: [PATCH] test images: Adds various fixes and cleanups to the image
 building process

Adds splitOsArch function to image-util.sh, which makes the script DRY-er.

When building a Windows test image, if REMOTE_DOCKER_URL is not set, skip the rest of the
building process for that image, which will save some time (no need to build binaries).

If a REMOTE_DOCKER_URL was not set for a particular OS version, exclude that image from the
manifest list. This fixes an issue where, if REMOTE_DOCKER_URL was not set for Windows Server 1909,
the Windows were completely excluded from the manifest list, including for Windows Server 1809
and 1903 which could have been built and pushed.

Sets "test-webserver" as the default CMD for kitten and nautilus. Since they are now based on
agnhost, they should be set to run test-webserver to maintain previous behaviour.
---
 test/images/image-util.sh       | 98 +++++++++++++--------------------
 test/images/kitten/Dockerfile   |  1 +
 test/images/nautilus/Dockerfile |  1 +
 3 files changed, 39 insertions(+), 61 deletions(-)

diff --git a/test/images/image-util.sh b/test/images/image-util.sh
index cd4b929692d..9ded1068baa 100755
--- a/test/images/image-util.sh
+++ b/test/images/image-util.sh
@@ -34,6 +34,33 @@ listOsArchs() {
   cut -d "=" -f 1 "${image}"/BASEIMAGE
 }
 
+splitOsArch() {
+    image=$1
+    os_arch=$2
+
+    if [[ $os_arch =~ .*/.*/.* ]]; then
+      # for Windows, we have to support both LTS and SAC channels, so we're building multiple Windows images.
+      # the format for this case is: OS/ARCH/OS_VERSION.
+      os_name=$(echo "$os_arch" | cut -d "/" -f 1)
+      arch=$(echo "$os_arch" | cut -d "/" -f 2)
+      os_version=$(echo "$os_arch" | cut -d "/" -f 3)
+      suffix="$os_name-$arch-$os_version"
+
+      # currently, GCE does not have Hyper-V support, which means that the same node cannot be used to build
+      # multiple versions of Windows images. Which is why we have $REMOTE_DOCKER_URL_$os_version URLs configured.
+      # TODO(claudiub): once Hyper-V support has been added to GCE, revert this to just $REMOTE_DOCKER_URL.
+      remote_docker_url_name="REMOTE_DOCKER_URL_$os_version"
+      REMOTE_DOCKER_URL=$(eval echo "\${${remote_docker_url_name}:-}")
+    elif [[ $os_arch =~ .*/.* ]]; then
+      os_name=$(echo "$os_arch" | cut -d "/" -f 1)
+      arch=$(echo "$os_arch" | cut -d "/" -f 2)
+      suffix="$os_name-$arch"
+    else
+      echo "The BASEIMAGE file for the ${image} image is not properly formatted. Expected entries to start with 'os/arch', found '${os_arch}' instead."
+      exit 1
+    fi
+}
+
 # Returns baseimage need to used in Dockerfile for any given architecture
 getBaseImage() {
   os_arch=$1
@@ -56,24 +83,12 @@ build() {
   kube::util::ensure-gnu-sed
 
   for os_arch in ${os_archs}; do
-    if [[ $os_arch =~ .*/.*/.* ]]; then
-      # for Windows, we have to support both LTS and SAC channels, so we're building multiple Windows images.
-      # the format for this case is: OS/ARCH/OS_VERSION.
-      os_name=$(echo "$os_arch" | cut -d "/" -f 1)
-      arch=$(echo "$os_arch" | cut -d "/" -f 2)
-      os_version=$(echo "$os_arch" | cut -d "/" -f 3)
-
-      # currently, GCE does not have Hyper-V support, which means that the same node cannot be used to build
-      # multiple versions of Windows images. Which is why we have $REMOTE_DOCKER_URL_$os_version URLs configured.
-      # TODO(claudiub): once Hyper-V support has been added to GCE, revert this to just $REMOTE_DOCKER_URL.
-      remote_docker_url_name="REMOTE_DOCKER_URL_$os_version"
-      REMOTE_DOCKER_URL=$(eval echo "\${${remote_docker_url_name}:-}")
-    elif [[ $os_arch =~ .*/.* ]]; then
-      os_name=$(echo "$os_arch" | cut -d "/" -f 1)
-      arch=$(echo "$os_arch" | cut -d "/" -f 2)
-    else
-      echo "The BASEIMAGE file for the ${image} image is not properly formatted. Expected entries to start with 'os/arch', found '${os_arch}' instead."
-      exit 1
+    splitOsArch "${image}" "${os_arch}"
+    if [[ "${os_name}" == "windows" && -z "${REMOTE_DOCKER_URL}" ]]; then
+      # If we have a Windows os_arch entry but no Remote Docker Daemon for it,
+      # we should skip it, so we don't have to build any binaries for it.
+      echo "Cannot build the image '${image}' for ${os_arch}. REMOTE_DOCKER_URL_$os_version should be set, containing the URL to a Windows docker daemon."
+      continue
     fi
 
     echo "Building image for ${image} OS/ARCH: ${os_arch}..."
@@ -139,8 +154,6 @@ build() {
         --tlscert "${HOME}/.docker-${os_version}/cert.pem" --tlskey "${HOME}/.docker-${os_version}/key.pem" \
         -H "${REMOTE_DOCKER_URL}" build --pull -t "${REGISTRY}/${image}:${TAG}-${os_name}-${arch}-${os_version}" \
         --build-arg BASEIMAGE="${BASEIMAGE}" -f $dockerfile_name .
-    else
-      echo "Cannot build the image '${image}' for ${os_arch}. REMOTE_DOCKER_URL_$os_version should be set, containing the URL to a Windows docker daemon."
     fi
     popd
   done
@@ -168,25 +181,7 @@ push() {
     os_archs=$(printf 'linux/%s\n' "${!QEMUARCHS[*]}")
   fi
   for os_arch in ${os_archs}; do
-    if [[ $os_arch =~ .*/.*/.* ]]; then
-      # for Windows, we have to support both LTS and SAC channels, so we're building multiple Windows images.
-      # the format for this case is: OS/ARCH/OS_VERSION.
-      os_name=$(echo "$os_arch" | cut -d "/" -f 1)
-      arch=$(echo "$os_arch" | cut -d "/" -f 2)
-      os_version=$(echo "$os_arch" | cut -d "/" -f 3)
-
-      # currently, GCE does not have Hyper-V support, which means that the same node cannot be used to build
-      # multiple versions of Windows images. Which is why we have $REMOTE_DOCKER_URL_$os_version URLs configured.
-      # TODO(claudiub): once Hyper-V support has been added to GCE, revert this to just $REMOTE_DOCKER_URL.
-      remote_docker_url_name="REMOTE_DOCKER_URL_$os_version"
-      REMOTE_DOCKER_URL=$(eval echo "\${${remote_docker_url_name}:-}")
-    elif [[ $os_arch =~ .*/.* ]]; then
-      os_name=$(echo "$os_arch" | cut -d "/" -f 1)
-      arch=$(echo "$os_arch" | cut -d "/" -f 2)
-    else
-      echo "The BASEIMAGE file for the ${image} image is not properly formatted. Expected entries to start with 'os/arch', found '${os_arch}' instead."
-      exit 1
-    fi
+    splitOsArch "${image}" "${os_arch}"
 
     if [[ "$os_name" = "linux" ]]; then
       docker push "${REGISTRY}/${image}:${TAG}-${os_name}-${arch}"
@@ -197,16 +192,11 @@ push() {
         -H "${REMOTE_DOCKER_URL}" push "${REGISTRY}/${image}:${TAG}-${os_name}-${arch}-${os_version}"
     else
       echo "Cannot push the image '${image}' for ${os_arch}. REMOTE_DOCKER_URL_${os_version} should be set, containing the URL to a Windows docker daemon."
+      # we should exclude this image from the manifest list as well, we couldn't build / push it.
+      os_archs=$(printf "%s\n" "$os_archs" | grep -v "$os_arch" || true)
     fi
   done
 
-  # NOTE(claudiub): if the REMOTE_DOCKER_URL var is not set, or it is an empty string, we mustn't include
-  # Windows images into the manifest list.
-  if test -z "${REMOTE_DOCKER_URL:-}" && printf "%s\n" "$os_archs" | grep -q '^windows'; then
-    echo "Skipping pushing the image '${image}' for Windows. REMOTE_DOCKER_URL_\${os_version} should be set, containing the URL to a Windows docker daemon."
-    os_archs=$(printf "%s\n" "$os_archs" | grep -v "^windows" || true)
-  fi
-
   if test -z "${os_archs}"; then
     # this can happen for Windows-only images if they have been skipped entirely.
     echo "No image for the manifest list. Skipping ${image}."
@@ -223,21 +213,7 @@ push() {
   while IFS='' read -r line; do manifest+=("$line"); done < <(echo "$os_archs" | ${SED} "s~\/~-~g" | ${SED} -e "s~[^ ]*~$REGISTRY\/$image:$TAG\-&~g")
   docker manifest create --amend "${REGISTRY}/${image}:${TAG}" "${manifest[@]}"
   for os_arch in ${os_archs}; do
-    if [[ $os_arch =~ .*/.*/.* ]]; then
-      # for Windows, we have to support both LTS and SAC channels, so we're building multiple Windows images.
-      # the format for this case is: OS/ARCH/OS_VERSION.
-      os_name=$(echo "$os_arch" | cut -d "/" -f 1)
-      arch=$(echo "$os_arch" | cut -d "/" -f 2)
-      os_version=$(echo "$os_arch" | cut -d "/" -f 3)
-      suffix="$os_name-$arch-$os_version"
-    elif [[ $os_arch =~ .*/.* ]]; then
-      os_name=$(echo "$os_arch" | cut -d "/" -f 1)
-      arch=$(echo "$os_arch" | cut -d "/" -f 2)
-      suffix="$os_name-$arch"
-    else
-      echo "The BASEIMAGE file for the ${image} image is not properly formatted. Expected entries to start with 'os/arch', found '${os_arch}' instead."
-      exit 1
-    fi
+    splitOsArch "${image}" "${os_arch}"
     docker manifest annotate --os "${os_name}" --arch "${arch}" "${REGISTRY}/${image}:${TAG}" "${REGISTRY}/${image}:${TAG}-${suffix}"
   done
   docker manifest push --purge "${REGISTRY}/${image}:${TAG}"
diff --git a/test/images/kitten/Dockerfile b/test/images/kitten/Dockerfile
index 48d6f860d88..f92dd7a89d4 100644
--- a/test/images/kitten/Dockerfile
+++ b/test/images/kitten/Dockerfile
@@ -16,3 +16,4 @@ ARG BASEIMAGE
 FROM $BASEIMAGE
 COPY html/kitten.jpg kitten.jpg
 COPY html/data.json data.json
+CMD ["test-webserver"]
diff --git a/test/images/nautilus/Dockerfile b/test/images/nautilus/Dockerfile
index c344116bf8c..2cb4784a8fa 100644
--- a/test/images/nautilus/Dockerfile
+++ b/test/images/nautilus/Dockerfile
@@ -16,3 +16,4 @@ ARG BASEIMAGE
 FROM $BASEIMAGE
 COPY html/nautilus.jpg nautilus.jpg
 COPY html/data.json data.json
+CMD ["test-webserver"]