Skip to content

Conversation

@pon0marev
Copy link
Contributor

  • add Docker CLI presence check
  • require Compose V2 (Docker CLI plugin); warn if Docker is outdated
  • remove set -e toggling; use guard conditions with returns
  • update docker-*.sh to call $COMPOSE_CMD

Copy link
Contributor

@smatvienko-tb smatvienko-tb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great progress, please check my suggestions

compose-utils.sh Outdated
if [[ $SHELLOPTS =~ errexit ]]; then
set +e
FLAG_SET=true
function composeCmd()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to check compose version and call from this file sourced.

do not call this from another files

compose-utils.sh Outdated
exit 1
# Check Docker Compose V2 availability (plugin)
if ! docker compose version >/dev/null 2>&1; then
echo "Docker Compose V2 plugin is not detected. Your Docker installation may be outdated. Please update Docker to a version that includes Docker Compose V2." >&2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have version 5 now. So please, communicate that V2 is a minimum requirement. Greater version is also fine.

# unknown option
;;
esac
$COMPOSE_CMD $COMPOSE_ARGS
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use directly docker compose $COMPOSE_ARGS

source compose-utils.sh

COMPOSE_VERSION=$(composeVersion) || exit $?
COMPOSE_CMD=$(composeCmd) || exit $?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does not required. docker compose version check should be called inside source compose-utils.sh

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR modernizes Docker Compose support by removing legacy Compose V1 compatibility and requiring Docker Compose v2 or newer (the Docker CLI plugin).

Changes:

  • Replaced the complex composeVersion() function with a simpler checkComposeVersion() that validates Docker CLI presence and requires Compose V2
  • Removed version-switching logic (V1 vs V2 case statements) from all docker-*.sh scripts
  • Updated all docker scripts to use docker compose command directly instead of conditional docker-compose vs docker compose

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
compose-utils.sh Replaced complex version detection function with simpler check that requires Compose V2; added automatic initialization on sourcing
docker-upgrade-tb.sh Removed COMPOSE_VERSION variable and case statement; uses docker compose directly
docker-update-service.sh Removed COMPOSE_VERSION variable and case statement; uses docker compose directly
docker-stop-services.sh Removed COMPOSE_VERSION variable and case statement; uses docker compose directly
docker-start-services.sh Removed COMPOSE_VERSION variable, case statement, and V1-specific --compatibility flag; uses docker compose directly
docker-remove-services.sh Removed COMPOSE_VERSION variable and case statement; uses docker compose directly
docker-install-tb.sh Removed COMPOSE_VERSION variable and multiple case statements; uses docker compose directly

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +341 to +347
# Initialize Compose command on sourcing this file
# If the script is sourced, BASH_SOURCE[0] != $0 and we can safely `return` on failure
if [ "${BASH_SOURCE[0]}" != "$0" ]; then
checkComposeVersion || return $?
else
# If executed directly, run the check and exit on failure
checkComposeVersion || exit $?
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The automatic initialization of checkComposeVersion when compose-utils.sh is sourced will cause scripts that don't use Docker Compose (like docker-check-log-folders.sh and docker-create-log-folders.sh) to fail unnecessarily. These scripts only use utility functions like checkFolders from compose-utils.sh and don't need Docker Compose to be installed. Consider moving the compose version check to only run in scripts that actually execute docker compose commands, or making the check optional based on a flag.

Suggested change
# Initialize Compose command on sourcing this file
# If the script is sourced, BASH_SOURCE[0] != $0 and we can safely `return` on failure
if [ "${BASH_SOURCE[0]}" != "$0" ]; then
checkComposeVersion || return $?
else
# If executed directly, run the check and exit on failure
checkComposeVersion || exit $?
# Initialize Compose command
# If executed directly, run the check and exit on failure.
# If sourced, run the check only when explicitly requested via
# the TB_REQUIRE_DOCKER_COMPOSE_CHECK environment variable.
if [ "${BASH_SOURCE[0]}" = "$0" ]; then
checkComposeVersion || exit $?
elif [ "${TB_REQUIRE_DOCKER_COMPOSE_CHECK:-false}" = "true" ]; then
checkComposeVersion || return $?

Copilot uses AI. Check for mistakes.
Comment on lines +326 to +327
function checkComposeVersion()
{ # Check Docker CLI availability
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent brace placement for function declaration. All other functions in this file have the opening brace on the same line as the function declaration (e.g., 'function deploymentFolder() {' on line 33). This function should follow the same pattern: 'function checkComposeVersion() {' with the comment moved to the next line or above the function.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants