[GitHub] [ozone] adoroszlai commented on a change in pull request #2056: HDDS-4181. Add acceptance tests for upgrade, finalization and downgrade.

[GitBox <gi...@apache.org>]

adoroszlai commented on a change in pull request #2056:
URL: https://github.com/apache/ozone/pull/2056#discussion_r607907657



##########
File path: hadoop-ozone/dist/src/main/compose/upgrade/compose/ha/docker-compose.yaml
##########
@@ -0,0 +1,160 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+version: "3.4"
+
+# reusable fragments (see https://docs.docker.com/compose/compose-file/#extension-fields)
+x-common-config:
+  &common-config
+  env_file:
+    - docker-config
+  image: ${OZONE_IMAGE}
+
+x-replication:
+  &replication
+  OZONE-SITE.XML_ozone.replication: ${OZONE_REPLICATION_FACTOR:-3}
+
+x-datanode:
+  &datanode
+  command: ["ozone","datanode"]
+  <<: *common-config
+  environment:
+    <<: *replication
+  ports:
+    - 9864
+    - 9882
+
+x-om:
+  &om
+  command: ["ozone","om","${OM_HA_ARGS}"]
+  <<: *common-config
+  environment:
+    ENSURE_OM_INITIALIZED: /data/metadata/om/current/VERSION
+    <<: *replication
+  ports:
+    - 9862
+    - 9872
+
+x-volumes:
+    - &ozone-dir ../../../..:${OZONE_DIR}

Review comment:
       Thanks, this was new for me.

##########
File path: hadoop-ozone/dist/src/main/compose/testlib.sh
##########
@@ -377,23 +377,14 @@ prepare_for_binary_image() {
 ##   (no binaries included)
 ## @param `ozone-runner` image version (optional)
 prepare_for_runner_image() {
-  local default_version=${docker.ozone-runner.version} # set at build-time from Maven property
+  local default_version=20210226-1 # set at build-time from Maven property

Review comment:
       Seems to be unintended change.

##########
File path: hadoop-ozone/dist/src/main/compose/upgrade/upgrades/manual-upgrade/test.sh
##########
@@ -20,32 +20,37 @@
 
 set -e -o pipefail
 
-COMPOSE_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
-export COMPOSE_DIR
-
-: "${OZONE_REPLICATION_FACTOR:=3}"
-: "${OZONE_UPGRADE_FROM:="0.5.0"}"
-: "${OZONE_UPGRADE_TO:="1.0.0"}"
-: "${OZONE_VOLUME:="${COMPOSE_DIR}/data"}"
-
-export OZONE_REPLICATION_FACTOR OZONE_UPGRADE_FROM OZONE_UPGRADE_TO OZONE_VOLUME
-
-source "${COMPOSE_DIR}/testlib.sh"
-
-create_data_dir
-
-prepare_for_binary_image "${OZONE_UPGRADE_FROM}"
-load_version_specifics "${OZONE_UPGRADE_FROM}"
-first_run
-unload_version_specifics
-
-from=$(get_logical_version "${OZONE_UPGRADE_FROM}")
-to=$(get_logical_version "${OZONE_UPGRADE_TO}")
-execute_upgrade_steps "$from" "$to"
-
-prepare_for_binary_image "${OZONE_UPGRADE_TO}"
-load_version_specifics "${OZONE_UPGRADE_TO}"
-second_run
-unload_version_specifics
-
-generate_report
+# Fail if required vars are not set.

Review comment:
       I have a strange feeling about making some vars required for `test.sh`.  Should these script be renamed?

##########
File path: hadoop-ozone/dist/src/main/compose/upgrade/README.md
##########
@@ -12,18 +12,91 @@
   limitations under the License. See accompanying LICENSE file.
 -->
 
-# Compose file for upgrade
+# Ozone Upgrade Acceptance Tests
 
-This directory contains a sample cluster definition and script for
-testing upgrade from previous version to the current one.
+This directory contains cluster definitions and scripts for testing upgrades from any previous version to another
+previous version, or to the local build of the code. It is designed to catch backwards incompatible changes made between
+an older release of Ozone and a later release (which may be the local build).
 
-Data for each container is persisted in mounted volume (by default it's
-`data` under the `compose/upgrade` directory, but can be overridden via
-`OZONE_VOLUME` environment variable).
+## IMPORTANT NOTES
 
-Prior version is run using an official `apache/ozone` image, while the
-current version is run with the `ozone-runner` image using locally built
-source code.
+1. Backwards Incompatibility
+    - These tests will not catch backwards incompatible changes against commits in between releases.
+        - Example:
+            1. After 1.0.0, a change *c1* is made that is backwards compatible with *1.0.0*.
+            2. After *c1*, a new change *c2* is made that is also backwards compatible with 1.0.0 but backwards *incompatible* with *c1*.
 
-Currently the test script only supports a single version upgrade (eg.
-from 0.5.0 to 1.0.0).
+            - This test suite will not raise an error for *c2*, because it only tests against the last release
+            (1.0.0), and not the last commit (*c1*).
+
+2. Downgrade Support
+    - Downgrades will not be supported until upgrading from 1.1.0 to 1.2.0
+
+    - Until 1.1.0 is released, downgrades cannot be tested, so they are commented out of the current non-rolling upgrade tests.
+
+## Directory Layout
+
+### upgrades
+
+Each type of upgrade has a subdirectory under the *upgrades* directory. Each upgrade's steps are controlled by a *test.sh* script in its *upgrades/\<upgrade-type>* directory. Callbacks to execute throughout the upgrade are called by this script and should be placed in a file called *callback.sh* in the *upgrades/\<upgrade-type>/\<upgrade-from>-\<upgrade-to>* directory. After the test is run, results and docker volume data for the upgrade for these versions will also be placed in this directory. The results of all upgrades run as part of the tests will be placed in a *results* folder in the top level upgrade directory.
+
+#### manual-upgrade
+
+- Any necessary conversion of on disk structures from the old version to the new version must be done explicitly.
+
+- This is primarily for testing upgrades from versions before the non-rolling upgrade framework was introduced.
+
+- Supported Callbacks:
+    1. `setup_with_old_version`: Run before ozone is started in the old version.
+    3. `with_old_version`: Run while ozone is running in the old version.
+    3. `setup_with_new_version`: Run after ozone is stopped in the old version, but before it is restarted in the new version.
+    4. `with_new_version`: Run while ozone is running in the new version.
+
+#### non-rolling-upgrade
+
+- Any necessary conversion of on disk structures from the old version to the new version are handled by Ozone's non-rolling upgrade framework.
+
+- Supported Callbacks:
+    1. `setup`: Run before ozone is started in the old version.
+    3. `with_old_version`: Run while ozone is running in the old version.
+    3. `with_new_version_pre_finalized`: Run after ozone is stopped in the old version, and brought back up and running in the new version pre-finalized.

Review comment:
       I would like to suggest another callback: after old version is stopped but before starting new version.  This would be useful for testing upgrade to versions that first support eg. OM HA or SCM HA.  We need to change the compose cluster definition between the runs.  Or is this scenario covered in any other way?

##########
File path: hadoop-ozone/dist/src/main/compose/testlib.sh
##########
@@ -377,23 +377,14 @@ prepare_for_binary_image() {
 ##   (no binaries included)
 ## @param `ozone-runner` image version (optional)
 prepare_for_runner_image() {
-  local default_version=${docker.ozone-runner.version} # set at build-time from Maven property
+  local default_version=20210226-1 # set at build-time from Maven property
   local runner_version=${OZONE_RUNNER_VERSION:-${default_version}} # may be specified by user running the test
   local v=${1:-${runner_version}} # prefer explicit argument
 
   export OZONE_DIR=/opt/hadoop
   export OZONE_IMAGE="apache/ozone-runner:${v}"
 }
 
-## @description Print the logical version for a specific release
-## @param the release for which logical version should be printed
-get_logical_version() {
-  local v="$1"
-
-  # shellcheck source=/dev/null
-  echo $(source "${_testlib_dir}/versions/${v}.sh" && ozone_logical_version)

Review comment:
       Definitions of `ozone_logical_version` function could be removed from `hadoop-ozone/dist/src/main/compose/versions/*.sh`.

##########
File path: hadoop-ozone/dist/src/main/compose/upgrade/test.sh
##########
@@ -15,18 +15,27 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-SCRIPT_DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null && pwd )
-ALL_RESULT_DIR="$SCRIPT_DIR/result"
-mkdir -p "$ALL_RESULT_DIR"
-rm "$ALL_RESULT_DIR"/* || true
-source "$SCRIPT_DIR/../testlib.sh"
+# Version that will be run using the local build.
+: "${OZONE_CURRENT_VERSION:=1.1.0}"
+export OZONE_CURRENT_VERSION
 
-tests=$(find_tests)
-cd "$SCRIPT_DIR"
+TEST_DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null && pwd )
+source "$TEST_DIR/testlib.sh"
 
-run_test_scripts ${tests}
-RESULT=$?
+# Export variables needed by tests and ../testlib.sh.
+export TEST_DIR
+export COMPOSE_DIR="$TEST_DIR"
 
-generate_report "upgrade" "${ALL_RESULT_DIR}"
+RESULT_DIR="$ALL_RESULT_DIR" create_results_dir
 
-exit ${RESULT}
+# Upgrade tests to be run.
+# Run all upgrades even if one fails.
+# Any failure will save a failing return code to $RESULT.
+set +e
+run_test manual-upgrade 0.5.0 1.1.0
+run_test non-rolling-upgrade 1.0.0 1.1.0

Review comment:
       What happens after 1.1.0 is released?  Do we need to update these versions to 1.2.0 or add new entries?

##########
File path: hadoop-ozone/dist/src/main/compose/upgrade/testlib.sh
##########
@@ -17,64 +17,133 @@
 
 set -e -o pipefail
 
+# Fail if required variables are not set.
+set -u
+: "${OZONE_CURRENT_VERSION}"
+set +u
+
 _upgrade_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
 
+# Cumulative result of all tests run with run_test function.
+# 0 if all passed, 1 if any failed.
+: "${RESULT:=0}"
 : "${OZONE_REPLICATION_FACTOR:=3}"
-: "${OZONE_VOLUME:="${COMPOSE_DIR}/data"}"
 : "${OZONE_VOLUME_OWNER:=}"
+: "${OZONE_CURRENT_VERSION:=}"
+: "${ALL_RESULT_DIR:="$_upgrade_dir"/result}"
+
+# export for docker-compose
+export OZONE_REPLICATION_FACTOR
 
 source "${_upgrade_dir}/../testlib.sh"
 
 ## @description Create the directory tree required for persisting data between
 ##   compose cluster restarts
-create_data_dir() {
+create_data_dirs() {
+  local dirs_to_create="$@"
+
   if [[ -z "${OZONE_VOLUME}" ]]; then
     return 1
   fi
 
   rm -fr "${OZONE_VOLUME}" 2> /dev/null || sudo rm -fr "${OZONE_VOLUME}"
-  mkdir -p "${OZONE_VOLUME}"/{dn1,dn2,dn3,om,recon,s3g,scm}
+  mkdir -p $dirs_to_create
   fix_data_dir_permissions
 }
 
-## @description Run upgrade steps required for going from one logical version to another.
-## @param Starting logical version
-## @param Target logical version
-execute_upgrade_steps() {
-  local -i from=$1
-  local -i to=$2
+## @description Prepares to run an image with `start_docker_env`.
+## @param the version of Ozone to be run.
+##   If this is equal to `OZONE_CURRENT_VERSION`, then the ozone runner image wil be used.
+##   Else, a binary image will be used.
+prepare_for_image() {
+  local image_version="$1"
 
-  if [[ ${from} -ge ${to} ]]; then
-    return
+  if [[ "$image_version" = "$OZONE_CURRENT_VERSION" ]]; then
+      prepare_for_runner_image
+  else
+      prepare_for_binary_image "$image_version"
   fi
+}
 
-  pushd ${_testlib_dir}/../libexec/upgrade
+## @description Runs a callback function only if it exists.
+## @param The name of the function to run.
+callback() {
+  local func="$1"
+  if type -t "$func" > /dev/null; then

Review comment:
       Should it check for output `function` from `type -t`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org