Skip to content

Add datasets and models to the upgrade script #288

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 13 commits into
base: release-1.4.0
Choose a base branch
from
Draft

Add datasets and models to the upgrade script #288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
61d0be8
remove default UI password from the helm chart
jdanieck Aug 1, 2025
6503095
add k8s deployment to deploy.sh
jdanieck Aug 1, 2025
d513b00
prettier style issue fixes
jdanieck Aug 1, 2025
096e377
Merge branch 'release-1.4.0-rc1' into k8s-default-ui-pass
jdanieck Aug 1, 2025
b620f91
Merge branch 'release-1.4.0-rc1' into k8s-default-ui-pass
jdanieck Aug 4, 2025
8fad87b
Merge branch 'release-1.4.0-rc1' into k8s-default-ui-pass
jdanieck Aug 6, 2025
aa2d875
Merge branch 'release-1.4.0-rc1' into k8s-default-ui-pass
jdanieck Aug 7, 2025
98e957d
revise README
jdanieck Aug 7, 2025
e7cb986
Merge branch 'release-1.4.0-rc1' into k8s-default-ui-pass
jdanieck Aug 7, 2025
d49e6e4
Merge branch 'release-1.4.0-rc1' into k8s-default-ui-pass
jdanieck Aug 7, 2025
b510818
fix indentation issue (orccured after merge conflict)
jdanieck Aug 7, 2025
15d8d7f
Move datasets and models to volumes
saratpoluri Aug 8, 2025
0e532c6
Merge branch 'release-1.4.0-rc1' into fix/extra-upgrade-steps
saratpoluri Aug 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 17 additions & 20 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ help:
@echo " list-dependencies List all apt/pip dependencies for all microservices"
@echo " build-sources-image Build the image with 3rd party sources"
@echo " install-models Install custom OpenVINO Zoo models to models volume"
@echo " check-db-upgrade Check if the database needs to be upgraded"
@echo " upgrade-database Backup and upgrade database to a newer PostgreSQL version"
@echo " check-upgrade Check if upgrade is needed"
@echo " upgrade-scenescape Upgrade an existing Intel® SceneScape installation"
@echo " (automatically transfers data to Docker volumes)"
@echo ""
@echo " rebuild Clean and build all images"
Expand Down Expand Up @@ -485,36 +485,33 @@ certificates:
auth-secrets:
$(MAKE) -C ./tools/authsecrets SECRETSDIR=$(SECRETSDIR)

# Database upgrade target
.PHONY: check-db-upgrade upgrade-database
# Existing install upgrade target
.PHONY: check-upgrade upgrade-scenescape

check-db-upgrade:
@if manager/tools/upgrade-database --check >/dev/null 2>&1; then \
echo "Database upgrade is required."; \
check-upgrade:
@if manager/tools/upgrade-scenescape --check >/dev/null 2>&1; then \
echo "Upgrade is required."; \
exit 0; \
else \
echo "No database upgrade needed."; \
echo "No upgrade needed."; \
exit 1; \
fi

upgrade-database:
@echo "Starting database upgrade process..."
@if ! manager/tools/upgrade-database --check >/dev/null 2>&1; then \
echo "No database upgrade needed."; \
upgrade-scenescape:
@echo "Starting upgrade process..."
@if ! manager/tools/upgrade-scenescape --check >/dev/null 2>&1; then \
echo "No upgrade needed."; \
exit 0; \
fi
@UPGRADE_LOG=/tmp/upgrade.$(shell date +%s).log; \
echo "Upgrading database (log at $$UPGRADE_LOG)..."; \
manager/tools/upgrade-database 2>&1 | tee $$UPGRADE_LOG; \
NEW_DB=$$(grep -E 'Upgraded database .* has been created in Docker volumes' $$UPGRADE_LOG | sed -e 's/.*created in Docker volumes.*//'); \
echo "Upgrading installation (log at $$UPGRADE_LOG)..."; \
manager/tools/upgrade-scenescape 2>&1 | tee $$UPGRADE_LOG; \
if [ $$? -ne 0 ]; then \
echo ""; \
echo "ABORTING"; \
echo "Automatic upgrade of database failed"; \
echo "Automatic upgrade of installation failed"; \
exit 1; \
fi; \
echo ""; \
echo "Database upgrade completed successfully."; \
echo "Database is now stored in Docker volumes:"; \
echo " - Database: scenescape_vol-db"; \
echo " - Migrations: scenescape_vol-migrations"
. tools/yaml_parse.sh; \
echo "Upgrade completed successfully.";
4 changes: 2 additions & 2 deletions deploy.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ echo '########################################'
make -C docs clean
make CERTPASS="${CERTPASS}" DBPASS="${DBPASS}"

if manager/tools/upgrade-database --check ; then
if manager/tools/upgrade-scenescape --check ; then
UPGRADEDB=0
echo "No upgrade needed"

Expand Down Expand Up @@ -176,7 +176,7 @@ else
fi

UPGRADE_LOG=/tmp/upgrade.$$.log
manager/tools/upgrade-database 2>&1 | tee ${UPGRADE_LOG}
manager/tools/upgrade-scenescape 2>&1 | tee ${UPGRADE_LOG}
NEW_DB=$(egrep 'Upgraded database .* has been created' ${UPGRADE_LOG} | awk '{print $NF}')
if [ ! -d "${NEW_DB}/db" -o ! -d "${NEW_DB}/migrations" ] ; then
echo
Expand Down
33 changes: 20 additions & 13 deletions docs/user-guide/How-to-upgrade.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ Before You Begin, ensure the following:
make build-all
```

3. **Run the upgrade-database script**:
3. **Run the upgrade script**:

```bash
bash manager/tools/upgrade-database
bash manager/tools/upgrade-scenescape
```

4. **Bring up services to verify upgrade**:
Expand All @@ -40,7 +40,24 @@ Before You Begin, ensure the following:
make demo
```

5. **Log in to the Web UI** and verify that data and configurations are intact.
5. **Verify the volumes are created**:
```bash
docker volume ls
```

The results will look like:
```console
local scenescape_vol-datasets
local scenescape_vol-db
local scenescape_vol-dlstreamer-pipeline-server-pipeline-root
local scenescape_vol-media
local scenescape_vol-migrations
local scenescape_vol-models
local scenescape_vol-netvlad_models
local scenescape_vol-sample-data
```

6. **Log in to the Web UI** and verify that data and configurations are intact.

## Model Management During Upgrade

Expand All @@ -50,16 +67,6 @@ Starting from 1.4.0 version, Intel® SceneScape stores models in Docker volumes
- **No Manual Copy Required**: You no longer need to manually copy `model_installer/models/` during upgrades.
- **Reduced Disk Usage**: Models are not duplicated between host filesystem and containers.

### Managing Models

- **To reinstall models**: `make install-models`
- **To clean models**: `make clean-models` (this will remove the Docker volume)
- **To check existing models in volume**: `docker volume ls | grep vol-models`

### Legacy Installations

If upgrading from a version that used host filesystem model storage (`model_installer/models/`), the models will be automatically reinstalled to the new Docker volume during the first deployment.

## Troubleshooting

1. **pg_backup Container Already Running Error**:
Expand Down
46 changes: 46 additions & 0 deletions manager/tools/upgrade-database → manager/tools/upgrade-scenescape
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,54 @@ run_migration() {
return 0
}

copy_models_to_volume() {
local temp_container="init_models"
local models_src="model_installer/models"

log_message "Initializing models volume..."
docker volume rm scenescape_vol-models 2>/dev/null || true
Copy link

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

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

The script forcibly removes existing volumes without checking if they contain important data. Consider adding a backup or confirmation step before removing volumes to prevent accidental data loss.

Suggested change
docker volume rm scenescape_vol-models 2>/dev/null || true
if docker volume inspect scenescape_vol-models >/dev/null 2>&1; then
echo "WARNING: This will remove the existing 'scenescape_vol-models' volume and all its data."
read -p "Are you sure you want to continue? [y/N]: " confirm
if [[ "$confirm" != "y" && "$confirm" != "Y" ]]; then
echo "Aborting models volume initialization."
return 1
fi
docker volume rm scenescape_vol-models
fi

Copilot uses AI. Check for mistakes.
docker volume create scenescape_vol-models

docker run --name ${temp_container} \
Copy link

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

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

The temporary container creation doesn't handle the case where a container with the same name already exists. Consider adding docker rm -f ${temp_container} 2>/dev/null || true before creating the container to prevent conflicts.

Copilot uses AI. Check for mistakes.
-d \
-v ${PWD}/${models_src}:/source/models \
-v scenescape_vol-models:/dest/models \
alpine:latest sleep 10

log_message "Copying models to volume..."
docker exec ${temp_container} /bin/sh -c "cp -r /source/models/* /dest/models/"
Copy link

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

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

The copy command will fail if the source directory /source/models/ is empty or doesn't contain any files, as the shell expansion /* will not match anything. Consider using cp -r /source/models/. /dest/models/ or adding a check for directory contents first.

Suggested change
docker exec ${temp_container} /bin/sh -c "cp -r /source/models/* /dest/models/"
docker exec ${temp_container} /bin/sh -c "cp -r /source/models/. /dest/models/"

Copilot uses AI. Check for mistakes.
docker stop ${temp_container} && docker rm ${temp_container}
log_message "Models copied to scenescape_vol-models volume"
}

copy_datasets_to_volume() {
local temp_container="init_datasets"
local datasets_src="datasets"

log_message "Initializing datasets volume..."
Comment on lines +359 to +360
Copy link

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

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

Similar to the models volume, the datasets volume is removed without verification. This could lead to data loss if the volume contains user data that should be preserved during upgrade.

Suggested change
log_message "Initializing datasets volume..."
local backup_dir="backup_datasets_$(date +%Y%m%d_%H%M%S)"
log_message "Checking for existing data in scenescape_vol-datasets volume..."
# Use a temporary container to check if the volume is non-empty
docker run --rm \
-v scenescape_vol-datasets:/data \
alpine:latest /bin/sh -c "ls -A /data" > /tmp/datasets_volume_contents.txt 2>/dev/null || true
if [ -s /tmp/datasets_volume_contents.txt ]; then
echo "WARNING: Existing data found in scenescape_vol-datasets volume:"
cat /tmp/datasets_volume_contents.txt
echo "Do you want to back up the existing datasets before removal? [y/N]"
read -r backup_choice
if [ "$backup_choice" = "y" ] || [ "$backup_choice" = "Y" ]; then
mkdir -p "$backup_dir"
docker run --rm \
-v scenescape_vol-datasets:/data \
-v ${PWD}/$backup_dir:/backup \
alpine:latest /bin/sh -c "cp -r /data/* /backup/"
echo "Backup completed: $backup_dir"
fi
echo "Do you want to delete the existing datasets volume? This will permanently remove all data. [y/N]"
read -r delete_choice
if [ ! "$delete_choice" = "y" ] && [ ! "$delete_choice" = "Y" ]; then
echo "Aborting datasets volume removal."
return 1
fi
fi
rm -f /tmp/datasets_volume_contents.txt
log_message "Removing and re-creating datasets volume..."

Copilot uses AI. Check for mistakes.
docker volume rm scenescape_vol-datasets 2>/dev/null || true
docker volume create scenescape_vol-datasets

Copy link

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

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

Same issue as with the models container - potential naming conflicts if a container with the same name already exists from a previous failed run.

Suggested change
# Remove any existing container with the same name to avoid naming conflicts
docker rm -f ${temp_container} 2>/dev/null || true

Copilot uses AI. Check for mistakes.
docker run --name ${temp_container} \
-d \
-v ${PWD}/${datasets_src}:/source/datasets \
-v scenescape_vol-datasets:/dest/datasets \
alpine:latest sleep 10

log_message "Copying datasets to volume..."
docker exec ${temp_container} /bin/sh -c "cp -r /source/datasets/* /dest/datasets/"
Copy link

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

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

Same issue as with the models copy - the command will fail if /source/datasets/ is empty. The shell glob /* expansion requires at least one file to match.

Suggested change
docker exec ${temp_container} /bin/sh -c "cp -r /source/datasets/* /dest/datasets/"
docker exec ${temp_container} /bin/sh -c "if [ \"\$(ls -A /source/datasets)\" ]; then cp -r /source/datasets/* /dest/datasets/; else echo 'No datasets to copy.'; fi"

Copilot uses AI. Check for mistakes.
docker stop ${temp_container} && docker rm ${temp_container}
log_message "Datasets copied to scenescape_vol-datasets volume"
}

# Main script execution
main() {
log_message "Copy models to volume mount..."
copy_models_to_volume

log_message "Copy datasets to volume mount..."
copy_datasets_to_volume

if [[ -f "$migration_file" ]] && grep -q 'sscape' "$migration_file"; then
echo "'$migration_file' exists and contains 'sscape'. Renaming tables and sscape references in migration file..."
run_table_rename
Expand Down
Loading
Loading