Changes based on Code Review

- Removes all sudo, pulls dependencies into plan file - Removes _aws, _curl, and _jq - Redirects pgrep and pkill stderr to /dev/null - Changes use of | tee to &> on minio hab execs - Adds messaging about possible long runtime of cp command - Adds --endpoint-url back to aws command calls - Cuts down envvar usage in favor of plan vars Signed-off-by: Jason Heath <[email protected]>
habitat-sh · Dec 17, 2024 · 67f0098 · 67f0098
1 parent 943d9e5
commit 67f0098
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 94 deletions.
diff --git a/components/builder-minio/habitat/config_install/minio-migration.sh b/components/builder-minio/habitat/config_install/minio-migration.sh
@@ -1,90 +1,24 @@
 #!/bin/bash
 
-# JAH: Consider this function more closely and maybe eliminated it
-function sudo() {
-  [[ $EUID = 0 ]] || set -- command sudo -E "$@"
-  "$@"
-}
-
-function _hab_exec() {
-  pkg=$1
-  shift
-  hab pkg exec "$pkg" -- "$@"
-}
-
-function _aws() {
-  _hab_exec 'core/aws-cli' aws --endpoint-url "{{cfg.env.MINIO_ENDPOINT}}" "$@"
-}
-
-function _curl() {
-  _hab_exec 'core/curl' curl "$@"
-}
-
-function _jq() {
-  _hab_exec 'core/jq-static' jq "$@"
-}
-
-function _hab_pkg_install() {
-  pkg=$1
-  shift
-  echo "$pkg not installed, installing"
-  if ! sudo hab pkg install "$pkg" -- "$@"; then
-    echo "ERROR: install of $pkg FAILED"
-    exit 4
-  fi
-  return 0
-}
-
-function _are_dependencies_installed() {
-  echo "CHECKING dependencies"
-  declare -a deps
-  deps=('core/aws-cli' 'core/curl' 'core/jq-static')
-  for d in "${deps[@]}"; do
-    if hab pkg env "$d" &>/dev/null; then
-      if ! _hab_pkg_install "$d"; then
-        return 5
-      fi
-    fi
-  done
-}
-
-function _minio_check {
-  echo "CHECKING MinIO"
-  local output
-  output=$(_aws s3 ls)
-  if [[ ! $output =~ $MINIO_BUCKET ]]; then
-    echo "ERROR: Invalid MinIO credentials"
-    return 6
-  fi
-  return 0
-}
-
-function preflight_checks() {
-  if ! _are_dependencies_installed; then
-    echo "ERROR: one or more preflight checks FAILED"
-    exit 7
-  fi
-}
-
 function download_bucket_objects() {
   echo "DOWNLOADING objects from the MinIO that we are migrating from"
-  if ! _aws s3 sync "$s3_url" "$WAYPOINT"; then
+  if ! aws --endpoint-url "$MINIO_ENDPOINT" s3 sync "$s3_url" "$WAYPOINT"; then
     echo "ERROR: download of objects FAILED"
     exit 8
   fi
   return 0
 }
 
 function _ensure_bucket_exists() {
-  if ! _aws s3 ls "$MINIO_BUCKET" &>/dev/null; then
-    _aws s3 mb "$s3_url"
+  if ! aws --endpoint-url "$MINIO_ENDPOINT" s3 ls "{{cfg.bucket_name}}" &>/dev/null; then
+    aws --endpoint-url "$MINIO_ENDPOINT" s3 mb "$s3_url"
   fi
 }
 
 function upload_bucket_objects() {
   _ensure_bucket_exists
   echo "UPLOADING objects to the MinIO that we are migrating to"
-  if ! _aws s3 sync "$WAYPOINT" "$s3_url"; then
+  if ! aws --endpoint-url "$MINIO_ENDPOINT" s3 sync "$WAYPOINT" "$s3_url"; then
     echo "ERROR: upload of objects FAILED"
     exit 9
   fi
@@ -94,7 +28,7 @@ function upload_bucket_objects() {
 function is_migration_from_removed_fs_backend_needed() {
   echo "CHECKING if migration from removed fs backend is needed"
   if [[ -f /hab/svc/builder-minio/data/.minio.sys/format.json ]]; then
-    format_value=$(_jq -r '.format' /hab/svc/builder-minio/data/.minio.sys/format.json)
+    format_value=$(jq -r '.format' /hab/svc/builder-minio/data/.minio.sys/format.json)
     if [[ "${format_value}" == 'fs' ]]; then
       return 0
     fi
@@ -105,7 +39,7 @@ function is_migration_from_removed_fs_backend_needed() {
 function minio_health_live_check() {
   for ((n = 0; n < 20; n++)); do
     curl_http_code_args=(-fs -o /dev/null -w "%{http_code}" --retry 4 --retry-delay 1)
-    code=$(_curl "${curl_http_code_args[@]}" "$MINIO_ENDPOINT/minio/health/live")
+    code=$(curl "${curl_http_code_args[@]}" "$MINIO_ENDPOINT/minio/health/live")
     if [[ $code == 200 ]]; then
       return 0
     else
@@ -117,8 +51,8 @@ function minio_health_live_check() {
 
 function minio_stop() {
   for ((n = 0; n < 20; n++)); do
-    if pgrep minio >/dev/null; then
-      sudo pkill minio >/dev/null
+    if pgrep minio &>/dev/null; then
+      pkill minio &>/dev/null
       sleep 1
     else
       return 0
@@ -130,17 +64,21 @@ function config_environment_for_migration() {
   if [[ ! $1 =~ ^[0-9]+$ ]]; then
     echo "ERROR: Invalid timestamp"
   fi
-  export MINIO_ENDPOINT="{{cfg.env.MINIO_ENDPOINT}}"
+  if [ -f "{{pkg.svc_files_path}}/private.key" ]; then
+    MINIO_ENDPOINT="https://localhost:{{cfg.bind_port}}"
+  else
+    MINIO_ENDPOINT="http://localhost:{{cfg.bind_port}}"
+  fi
+  export MINIO_ENDPOINT
   export AWS_ACCESS_KEY_ID="{{cfg.env.MINIO_ACCESS_KEY}}"
   export AWS_SECRET_ACCESS_KEY="{{cfg.env.MINIO_SECRET_KEY}}"
-  export s3_url="s3://{{cfg.env.MINIO_BUCKET}}"
-  export MINIO_BUCKET="{{cfg.env.MINIO_BUCKET}}"
+  export s3_url="s3://{{cfg.bucket_name}}"
   WAYPOINT=$(mktemp -d -t minio-waypoint-"$1"-XXXXXXXXXX)
   export WAYPOINT
 }
 
 function _enumerate_bucket_objects() {
-  _aws s3 ls "$s3_url" --recursive --summarize >"$1"
+  aws --endpoint-url "$MINIO_ENDPOINT" s3 ls "$s3_url" --recursive --summarize >"$1"
 }
 
 function summarize_old_minio_bucket_objects() {

diff --git a/components/builder-minio/habitat/default.toml b/components/builder-minio/habitat/default.toml
@@ -35,8 +35,6 @@ bind_port = 9000
 # Setup additional environment variables to pass to minio
 # Doc: https://docs.min.io/docs/minio-server-configuration-guide.html
 [env]
-MINIO_ENDPOINT="http://localhost:9000"
-MINIO_BUCKET="habitat-builder-artifact-store.local"
 # Set minio admin user and password
 # MINIO_ACCESS_KEY and MINIO_SECRET_KEY are deprecated.
 # Use MINIO_ROOT_USER and MINIO_ROOT_PASSWORD

diff --git a/components/builder-minio/habitat/hooks/install b/components/builder-minio/habitat/hooks/install
@@ -8,7 +8,6 @@ source "{{pkg.svc_config_install_path}}/minio-migration.sh"
 
 timestamp=$EPOCHSECONDS # provides a shared element in naming items below
 config_environment_for_migration "$timestamp"
-preflight_checks
 
 echo "Checking if MinIO migration is needed"
 if is_migration_from_removed_fs_backend_needed; then
@@ -30,12 +29,14 @@ if is_migration_from_removed_fs_backend_needed; then
     # which was on-prem-stable for 6+ years so this should be a safe choice
 
     if ! hab pkg list $minio_pkg_old >/dev/null; then
-      hab pkg install $minio_pkg_old
+      echo "ERROR: $minio_pkg_old is not installed and we require it for the migration."
+      exit 11
     fi
 
     old_minio_stdout=$(mktemp -t "minio-old-stdout-$timestamp-XXXXXXXXXX")
-    # shellcheck disable=SC2024
-    sudo --background hab pkg exec $minio_pkg_old minio -- server /hab/svc/builder-minio/data >"$old_minio_stdout"
+    hab pkg exec $minio_pkg_old minio -- server \
+      --config-dir "{{pkg.svc_config_path}}" \
+      /hab/svc/builder-minio/data &>"$old_minio_stdout" &
 
     if ! minio_health_live_check; then
       echo "MinIO is not running so MinIO migration cannot begin"
@@ -50,16 +51,22 @@ if is_migration_from_removed_fs_backend_needed; then
 
   minio_stop
 
-  export MIGRATION_BACKUP_DIRECTORY="/hab/svc/builder-minio/data-backup-$EPOCHSECONDS"
-  sudo mv /hab/svc/builder-minio/data/ "$MIGRATION_BACKUP_DIRECTORY"
-  sudo mkdir /hab/svc/builder-minio/data/
+  MIGRATION_BACKUP_DIRECTORY=$(mktemp -d -t minio-data-backup-"$timestamp"-XXXXXXXXXX)
+  echo "NOTE: Copying the old minio data to $MIGRATION_BACKUP_DIRECTORY"
+  echo "NOTE: Depending on the size of your on-prem bldr, this could take quite some time"
+  cp -r /hab/svc/builder-minio/data "$MIGRATION_BACKUP_DIRECTORY"
+  GLOBIGNORE=".:.."
+  for x in /hab/svc/builder-minio/data/*; do
+    rm -rf "$x"
+  done
 
-  sudo hab pkg install core/minio --channel stable
   export MINIO_ROOT_USER="{{cfg.env.MINIO_ACCESS_KEY}}"
   export MINIO_ROOT_PASSWORD="{{cfg.env.MINIO_SECRET_KEY}}"
   new_minio_stdout=$(mktemp -t "minio-new-stdout-$timestamp-XXXXXXXXXX")
   # shellcheck disable=SC2024
-  sudo --background hab pkg exec core/minio minio -- server /hab/svc/builder-minio/data >"$new_minio_stdout"
+  hab pkg exec core/minio minio -- server \
+    /hab/svc/builder-minio/data \
+    --config-dir "{{pkg.svc_config_path}}" &>"$new_minio_stdout" &
 
   if ! minio_health_live_check; then
     echo "MinIO did not come back up so we cannot upload the habitat artifacts into the new MinIO"
@@ -89,7 +96,5 @@ if is_migration_from_removed_fs_backend_needed; then
 
   minio_stop
 
-  sudo chown --recursive hab:hab /hab/svc/builder-minio/data/
-
   echo "END MinIO migration"
 fi
diff --git a/components/builder-minio/habitat/plan.sh b/components/builder-minio/habitat/plan.sh
@@ -1,10 +1,10 @@
-# shellcheck shell=bash
+# shellcheck shell=bash disable=SC2034
 
 pkg_name=builder-minio
 pkg_origin=habitat
 pkg_maintainer="The Habitat Maintainers <[email protected]>"
 pkg_license=('Apache-2.0')
-pkg_deps=(core/minio core/cacerts core/openssl core/aws-cli core/bash)
+pkg_deps=(core/aws-cli core/bash core/cacerts core/curl core/jq-static core/minio core/openssl)
 pkg_build_deps=(core/git)
 
 pkg_exports=(
@@ -17,7 +17,8 @@ pkg_exports=(
 pkg_version() {
   # TED: After migrating the builder repo we needed to add to
   # the rev-count to keep version sorting working
-  echo "$(($(git rev-list HEAD --count) + 5000))"
+#   echo "$(($(git rev-list HEAD --count) + 5000))"
+  echo "$(($(git rev-list HEAD --count) + 5001))"
 }
 
 do_before() {