Re: [PR] HDDS-14821. Version specific data validation in rolling-upgrade suite [ozone]
dombizita commented on PR #10033: URL: https://github.com/apache/ozone/pull/10033#issuecomment-4440154948 Thanks for the review @errose28! -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HDDS-14821. Version specific data validation in rolling-upgrade suite [ozone]
dombizita merged PR #10033: URL: https://github.com/apache/ozone/pull/10033 -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HDDS-14821. Version specific data validation in rolling-upgrade suite [ozone]
dombizita commented on code in PR #10033:
URL: https://github.com/apache/ozone/pull/10033#discussion_r3229379979
##
hadoop-ozone/dist/src/main/compose/upgrade/upgrades/rolling-upgrade/driver.sh:
##
@@ -35,72 +34,85 @@ echo "--- RUNNING ROLLING UPGRADE TEST FROM
$OZONE_UPGRADE_FROM TO $OZONE_UPGRAD
source "$TEST_DIR"/testlib.sh
-# Restart one service with the target image.
+## @description Restart one service with the target image.
+## @param name of the service
+## @param stage prefix, used for the data prefix
rolling_restart_service() {
- SERVICE="$1"
+ local service="$1"
+ local stage_prefix="$2"
- echo "--- RESTARTING ${SERVICE} WITH IMAGE ${TARGET_IMAGE} ---"
+ local callback_data_prefix="generate-${service}-${stage_prefix}"
# Stop service
- stop_containers "${SERVICE}"
+ stop_containers "${service}"
- callback before_service_restart
+ callback with_service_stopped "$callback_data_prefix"
- # Restart service with the requested image.
- prepare_for_image "${TARGET_IMAGE}"
- create_containers "${SERVICE}"
+ # Start service
+ create_containers "${service}"
- callback after_service_restart
+ callback with_service_restarted "$callback_data_prefix"
# Service-specific readiness checks.
- case "${SERVICE}" in
+ case "${service}" in
om*)
- wait_for_port "${SERVICE}" 9862 120
+ wait_for_port "${service}" 9862 120
;;
scm*)
# SCM hostnames in this compose are scmX.org
- wait_for_port "${SERVICE}.org" 9876 120
+ wait_for_port "${service}.org" 9876 120
;;
dn*)
- wait_for_port "${SERVICE}" 9882 120
+ wait_for_port "${service}" 9882 120
;;
s3g*)
- wait_for_port "${SERVICE}" 9878 120
+ wait_for_port "${service}" 9878 120
;;
esac
}
+## @description Restart all services with the target image.
+## @param stage prefix, used for the generated data as prefix, also for the
OUTPUT_NAME
+## @param target image
rolling_restart_all_services() {
- STAGE_PREFIX="$1"
- TARGET_IMAGE="$2"
+ local stage_prefix="$1"
+ local target_image="$2"
local s
+ # Prepare the requested image
+ prepare_for_image "${target_image}"
+ echo "--- PREPARED ${target_image} IMAGE ---"
+
# SCMs first
for s in scm2 scm1 scm3; do
-
OUTPUT_NAME="${OZONE_UPGRADE_FROM}-${OZONE_UPGRADE_TO}-${STAGE_PREFIX}-${s}"
-rolling_restart_service "$s"
+
OUTPUT_NAME="${OZONE_UPGRADE_FROM}-${OZONE_UPGRADE_TO}-${stage_prefix}-${s}"
Review Comment:
Thanks, I missed that, added in latest commit.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-14821. Version specific data validation in rolling-upgrade suite [ozone]
errose28 commented on code in PR #10033:
URL: https://github.com/apache/ozone/pull/10033#discussion_r3228361879
##
hadoop-ozone/dist/src/main/compose/upgrade/upgrades/rolling-upgrade/driver.sh:
##
@@ -131,7 +143,7 @@ rolling_restart_all_services "4-upgrade" "$OZONE_UPGRADE_TO"
# Upgrade client after all server components are upgraded but before
finalization,
# so new client APIs can be exercised against pre-finalized servers.
echo "--- UPGRADING CLIENT TO $OZONE_UPGRADE_TO ---"
-OUTPUT_NAME="${OZONE_UPGRADE_FROM}-${OZONE_UPGRADE_TO}-2-client"
+OUTPUT_NAME="${OZONE_UPGRADE_FROM}-${OZONE_UPGRADE_TO}-4-upgrade-client"
Review Comment:
Discussed offline. Step 4 is considered the entire upgrade step. Upgrading
the server and client are treated as subtasks of that step so the numbering is
intentional.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-14821. Version specific data validation in rolling-upgrade suite [ozone]
errose28 commented on code in PR #10033:
URL: https://github.com/apache/ozone/pull/10033#discussion_r3221804298
##
hadoop-ozone/dist/src/main/compose/upgrade/upgrades/rolling-upgrade/driver.sh:
##
@@ -35,72 +34,85 @@ echo "--- RUNNING ROLLING UPGRADE TEST FROM
$OZONE_UPGRADE_FROM TO $OZONE_UPGRAD
source "$TEST_DIR"/testlib.sh
-# Restart one service with the target image.
+## @description Restart one service with the target image.
+## @param name of the service
+## @param stage prefix, used for the data prefix
rolling_restart_service() {
- SERVICE="$1"
+ local service="$1"
+ local stage_prefix="$2"
- echo "--- RESTARTING ${SERVICE} WITH IMAGE ${TARGET_IMAGE} ---"
+ local callback_data_prefix="generate-${service}-${stage_prefix}"
# Stop service
- stop_containers "${SERVICE}"
+ stop_containers "${service}"
- callback before_service_restart
+ callback with_service_stopped "$callback_data_prefix"
- # Restart service with the requested image.
- prepare_for_image "${TARGET_IMAGE}"
- create_containers "${SERVICE}"
+ # Start service
+ create_containers "${service}"
- callback after_service_restart
+ callback with_service_restarted "$callback_data_prefix"
# Service-specific readiness checks.
- case "${SERVICE}" in
+ case "${service}" in
om*)
- wait_for_port "${SERVICE}" 9862 120
+ wait_for_port "${service}" 9862 120
;;
scm*)
# SCM hostnames in this compose are scmX.org
- wait_for_port "${SERVICE}.org" 9876 120
+ wait_for_port "${service}.org" 9876 120
;;
dn*)
- wait_for_port "${SERVICE}" 9882 120
+ wait_for_port "${service}" 9882 120
;;
s3g*)
- wait_for_port "${SERVICE}" 9878 120
+ wait_for_port "${service}" 9878 120
;;
esac
}
+## @description Restart all services with the target image.
+## @param stage prefix, used for the generated data as prefix, also for the
OUTPUT_NAME
+## @param target image
rolling_restart_all_services() {
- STAGE_PREFIX="$1"
- TARGET_IMAGE="$2"
+ local stage_prefix="$1"
+ local target_image="$2"
local s
+ # Prepare the requested image
+ prepare_for_image "${target_image}"
+ echo "--- PREPARED ${target_image} IMAGE ---"
+
# SCMs first
for s in scm2 scm1 scm3; do
-
OUTPUT_NAME="${OZONE_UPGRADE_FROM}-${OZONE_UPGRADE_TO}-${STAGE_PREFIX}-${s}"
-rolling_restart_service "$s"
+
OUTPUT_NAME="${OZONE_UPGRADE_FROM}-${OZONE_UPGRADE_TO}-${stage_prefix}-${s}"
Review Comment:
Is this supposed to have an `echo` line for SCM restart?
##
hadoop-ozone/dist/src/main/compose/upgrade/upgrades/rolling-upgrade/driver.sh:
##
@@ -131,7 +143,7 @@ rolling_restart_all_services "4-upgrade" "$OZONE_UPGRADE_TO"
# Upgrade client after all server components are upgraded but before
finalization,
# so new client APIs can be exercised against pre-finalized servers.
echo "--- UPGRADING CLIENT TO $OZONE_UPGRADE_TO ---"
-OUTPUT_NAME="${OZONE_UPGRADE_FROM}-${OZONE_UPGRADE_TO}-2-client"
+OUTPUT_NAME="${OZONE_UPGRADE_FROM}-${OZONE_UPGRADE_TO}-4-upgrade-client"
Review Comment:
Should we number this `5` instead of `4`? Looks like this is just cosmetic
but it seems like the number should increment at each step.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-14821. Version specific data validation in rolling-upgrade suite [ozone]
dombizita commented on code in PR #10033:
URL: https://github.com/apache/ozone/pull/10033#discussion_r3217504602
##
hadoop-ozone/dist/src/main/compose/upgrade/upgrades/rolling-upgrade/callbacks/common/callback.sh:
##
@@ -20,9 +20,43 @@ source "$TEST_DIR"/testlib.sh
### CALLBACKS ###
before_service_restart() {
- generate "generate-${SERVICE}" "$CLIENT"
+ generate "generate-${SERVICE}-${STAGE_PREFIX}" "$CLIENT"
}
after_service_restart() {
- validate "generate-${SERVICE}" "$CLIENT"
+ validate "generate-${SERVICE}-${STAGE_PREFIX}" "$CLIENT"
+}
+
+with_old_version() {
+ execute_robot_test "$SCM" -N "${OUTPUT_NAME}-check-finalization" --include
finalized upgrade/check-finalization.robot
Review Comment:
Yes, you are right, I changed it.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-14821. Version specific data validation in rolling-upgrade suite [ozone]
dombizita commented on code in PR #10033:
URL: https://github.com/apache/ozone/pull/10033#discussion_r3217501010
##
hadoop-ozone/dist/src/main/compose/upgrade/upgrades/rolling-upgrade/callbacks/common/callback.sh:
##
@@ -20,9 +20,43 @@ source "$TEST_DIR"/testlib.sh
### CALLBACKS ###
before_service_restart() {
Review Comment:
Good idea, done.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-14821. Version specific data validation in rolling-upgrade suite [ozone]
dombizita commented on code in PR #10033:
URL: https://github.com/apache/ozone/pull/10033#discussion_r3217499254
##
hadoop-ozone/dist/src/main/compose/upgrade/upgrades/rolling-upgrade/callbacks/common/callback.sh:
##
@@ -20,9 +20,43 @@ source "$TEST_DIR"/testlib.sh
### CALLBACKS ###
before_service_restart() {
- generate "generate-${SERVICE}" "$CLIENT"
+ generate "generate-${SERVICE}-${STAGE_PREFIX}" "$CLIENT"
Review Comment:
I did some refactoring around this, let me know what you think, overall I
reduced the number of global environment variables.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-14821. Version specific data validation in rolling-upgrade suite [ozone]
dombizita commented on code in PR #10033:
URL: https://github.com/apache/ozone/pull/10033#discussion_r3217495029
##
hadoop-ozone/dist/src/main/compose/upgrade/upgrades/rolling-upgrade/callbacks/common/callback.sh:
##
@@ -20,9 +20,43 @@ source "$TEST_DIR"/testlib.sh
### CALLBACKS ###
before_service_restart() {
- generate "generate-${SERVICE}" "$CLIENT"
+ generate "generate-${SERVICE}-${STAGE_PREFIX}" "$CLIENT"
}
after_service_restart() {
- validate "generate-${SERVICE}" "$CLIENT"
+ validate "generate-${SERVICE}-${STAGE_PREFIX}" "$CLIENT"
+}
+
+with_old_version() {
+ execute_robot_test "$SCM" -N "${OUTPUT_NAME}-check-finalization" --include
finalized upgrade/check-finalization.robot
+ generate old1
+ validate old1
+}
+
+with_this_version_pre_finalized() {
Review Comment:
Nice catch, it's good to keep the same tests here as before for non-rolling
upgrade. I added them.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-14821. Version specific data validation in rolling-upgrade suite [ozone]
errose28 commented on code in PR #10033:
URL: https://github.com/apache/ozone/pull/10033#discussion_r3157293868
##
hadoop-ozone/dist/src/main/compose/upgrade/upgrades/rolling-upgrade/callbacks/common/callback.sh:
##
@@ -20,9 +20,43 @@ source "$TEST_DIR"/testlib.sh
### CALLBACKS ###
before_service_restart() {
- generate "generate-${SERVICE}" "$CLIENT"
+ generate "generate-${SERVICE}-${STAGE_PREFIX}" "$CLIENT"
Review Comment:
To reduce the amount of global env variables, `STAGE_PREFIX` and `SERVICE`
and be combined into a single `prefix` string that is assembled by the driver
and then passed into this function as a parameter.
We can also pass `STAGE_PREFIX` as a parameter from
`rolling_restart_all_services` into `rolling_restart_service` where the final
prefix is assembled.
##
hadoop-ozone/dist/src/main/compose/upgrade/upgrades/rolling-upgrade/callbacks/common/callback.sh:
##
@@ -20,9 +20,43 @@ source "$TEST_DIR"/testlib.sh
### CALLBACKS ###
before_service_restart() {
- generate "generate-${SERVICE}" "$CLIENT"
+ generate "generate-${SERVICE}-${STAGE_PREFIX}" "$CLIENT"
}
after_service_restart() {
- validate "generate-${SERVICE}" "$CLIENT"
+ validate "generate-${SERVICE}-${STAGE_PREFIX}" "$CLIENT"
+}
+
+with_old_version() {
+ execute_robot_test "$SCM" -N "${OUTPUT_NAME}-check-finalization" --include
finalized upgrade/check-finalization.robot
+ generate old1
+ validate old1
+}
+
+with_this_version_pre_finalized() {
Review Comment:
The non-rolling upgrade has a test for key overwrite as well. I think we
should add that here. We can do it as a separate generate/validate step which
will be clearer than what the non-rolling suite does now. For example:
```
# In the old version:
generate overwrite
# In all following callbacks:
validate overwrite
generate overwrite
```
##
hadoop-ozone/dist/src/main/compose/upgrade/upgrades/rolling-upgrade/callbacks/common/callback.sh:
##
@@ -20,9 +20,43 @@ source "$TEST_DIR"/testlib.sh
### CALLBACKS ###
before_service_restart() {
- generate "generate-${SERVICE}" "$CLIENT"
+ generate "generate-${SERVICE}-${STAGE_PREFIX}" "$CLIENT"
}
after_service_restart() {
- validate "generate-${SERVICE}" "$CLIENT"
+ validate "generate-${SERVICE}-${STAGE_PREFIX}" "$CLIENT"
+}
+
+with_old_version() {
+ execute_robot_test "$SCM" -N "${OUTPUT_NAME}-check-finalization" --include
finalized upgrade/check-finalization.robot
Review Comment:
This should run on `CLIENT` not `SCM` right? If so, the requirement that
`SCM` is set can be removed from the top of `driver.sh`.
##
hadoop-ozone/dist/src/main/compose/upgrade/upgrades/rolling-upgrade/callbacks/common/callback.sh:
##
@@ -20,9 +20,43 @@ source "$TEST_DIR"/testlib.sh
### CALLBACKS ###
before_service_restart() {
Review Comment:
I think these names are somewhat confusing because `before_service_restart`
is actually run when the service is stopped but before it is started. Maybe we
change these names to something like `with_service_stopped` and
`with_service_restarted`.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-14821. Version specific data validation in rolling-upgrade suite [ozone]
github-actions[bot] commented on PR #10033: URL: https://github.com/apache/ozone/pull/10033#issuecomment-4309492243 This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
