Skip to content
This repository has been archived by the owner on Aug 1, 2024. It is now read-only.

Commit

Permalink
fix: Error out of provisioning when any part errors (fix silent CI fa…
Browse files Browse the repository at this point in the history
…il) (#767)

CI tests were silently failing during provisioning since the provisioning
scripts did not set the `-e` option in bash. This commit changes the
scripts to include the recommended `set -eu -o pipefail` stanza as well as
include a more conservative `-e` on any docker-exec bash calls.

In places where existing errors were uncovered, I suppressed errors just
for the offending code with a `+e`/`-e` pair and left a comment starting
with `# FIXME[bash-e]`. Owning teams can prioritize and fix these at their
own pace.

This may cause some false positive errors (there are commands which use a
non-zero exit code to signal things other than an error) but they should
be individually reverted or fixed as they are discovered.

Also:

- Extract ANSI color code variables to a shared file and use them.
  Most of the provisioning scripts tried to use the color codes, but
  they weren't defined. I discovered this when I tried to add
  `set -eu` to the scripts.
- Remove the `set -e` from the CI Github Action, since it is almost
  certainly a no-op.
- Add `set -x` to all provisioning scripts for easier debugging (some
  already had it)
- Ensure provisioning scripts have bash shebang
- Tweak function in `programs/provision.sh` to accept variable list of
  arguments (and stop defaulting one of the args)
  • Loading branch information
timmc-edx authored Jun 10, 2021
1 parent 774e406 commit 4f21c4d
Show file tree
Hide file tree
Showing 23 changed files with 155 additions and 92 deletions.
3 changes: 0 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,6 @@ jobs:
- name: pull
run: make dev.pull.${{matrix.services}}

- name: set exit option
run: set -e

- name: provision
run: make dev.provision.${{matrix.services}}

Expand Down
4 changes: 1 addition & 3 deletions check.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@
# Note that passing in a non-existent service will not fail if there are
# other successful checks.

set -e
set -o pipefail
set -u
set -eu -o pipefail

# Grab all arguments into one string, replacing plus signs with spaces.
# Pad on either side with spaces so that the regex in `should_check` works correctly.
Expand Down
3 changes: 1 addition & 2 deletions destroy.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#!/usr/bin/env bash

set -e
set -eu -o pipefail

read -p "This will delete all data in your devstack. Would you like to proceed? [y/n] " -r
if [[ $REPLY =~ ^[Yy]$ ]]
Expand Down
8 changes: 6 additions & 2 deletions enterprise/provision.sh
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
docker-compose exec -T lms bash -c 'source /edx/app/edxapp/edxapp_env && python /edx/app/edxapp/edx-platform/manage.py lms --settings=devstack_docker manage_user enterprise_worker [email protected] --staff'
cat enterprise/worker_permissions.py | docker-compose exec -T lms bash -c 'source /edx/app/edxapp/edxapp_env && python /edx/app/edxapp/edx-platform/manage.py lms shell --settings=devstack_docker'
#!/usr/bin/env bash
set -eu -o pipefail
set -x

docker-compose exec -T lms bash -e -c 'source /edx/app/edxapp/edxapp_env && python /edx/app/edxapp/edx-platform/manage.py lms --settings=devstack_docker manage_user enterprise_worker [email protected] --staff'
cat enterprise/worker_permissions.py | docker-compose exec -T lms bash -e -c 'source /edx/app/edxapp/edxapp_env && python /edx/app/edxapp/edx-platform/manage.py lms shell --settings=devstack_docker'
3 changes: 1 addition & 2 deletions load-db.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@
#
# This will load the edxapp database from a file named exapp.sql.

set -e
set -o pipefail
set -eu -o pipefail

if [ -z "$1" ]
then
Expand Down
19 changes: 12 additions & 7 deletions programs/provision.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/bin/sh
#!/usr/bin/env bash
set -eu -o pipefail
set -x

set -e
#
# To add programs support, we need to tweak/add certain rows in the database.
# We want to go through Django for this (rather than direct db modification), since we have a lot of Python
Expand Down Expand Up @@ -44,19 +45,20 @@ docker_exec() {
/edx/app/$app/$repo/manage.py $cmd
"

docker-compose exec -T "$service" bash -c "$CMDS"
docker-compose exec -T "$service" bash -e -c "$CMDS"
}

provision_ida() {
service=${1}
cmd=${2:-"shell"}
service=$1
cmd=$2
shift 2

# Escape double quotes and backticks from the Python
PROGRAM_SCRIPT="$(sed -E 's/("|`)/\\\1/g' < "$BASEDIR/$service.py")"

cmd="$cmd -c \"$PROGRAM_SCRIPT\""

docker_exec "$service" "$cmd" "$3" "$4"
docker_exec "$service" "$cmd" "$@"
}

trap reset_color 1 2 3 6 15
Expand All @@ -68,7 +70,10 @@ fi

if [ "$1" = "discovery" -o -z "$1" ]; then
notice Adding demo program to Discovery...
provision_ida discovery
set +e
# FIXME[bash-e]: Bash scripts should use -e -- but this command fails
provision_ida discovery "shell"
set -e
fi

if [ "$1" = "cache" -o -z "$1" ]; then
Expand Down
22 changes: 17 additions & 5 deletions provision-credentials.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
#!/usr/bin/env bash
set -eu -o pipefail

. scripts/colors.sh
set -x

# NOTE (CCB): We do NOT call provision-ida because it expects a virtualenv.
# The new images for Credentials do not use virtualenv.
Expand All @@ -9,20 +13,28 @@ port=18150
docker-compose up -d $name

echo -e "${GREEN}Installing requirements for ${name}...${NC}"
docker-compose exec -T ${name} bash -c 'source /edx/app/credentials/credentials_env && cd /edx/app/credentials/credentials && make requirements' -- "$name"
docker-compose exec -T ${name} bash -e -c 'source /edx/app/credentials/credentials_env && cd /edx/app/credentials/credentials && make requirements' -- "$name"

echo -e "${GREEN}Running migrations for ${name}...${NC}"
docker-compose exec -T ${name} bash -c 'source /edx/app/credentials/credentials_env && cd /edx/app/credentials/credentials && make migrate' -- "$name"
docker-compose exec -T ${name} bash -e -c 'source /edx/app/credentials/credentials_env && cd /edx/app/credentials/credentials && make migrate' -- "$name"

set +e
# FIXME[bash-e]: Bash scripts should use -e -- but this script fails
# with missing manage.py (need another "credentials" in that cd path?)
echo -e "${GREEN}Creating super-user for ${name}...${NC}"
docker-compose exec -T ${name} bash -c 'source /edx/app/credentials/credentials_env && cd /edx/app/credentials/credentials && echo "from django.contrib.auth import get_user_model; User = get_user_model(); User.objects.create_superuser(\"edx\", \"[email protected]\", \"edx\") if not User.objects.filter(username=\"edx\").exists() else None" | python /edx/app/$1/$1/manage.py shell' -- "$name"
docker-compose exec -T ${name} bash -e -c 'source /edx/app/credentials/credentials_env && cd /edx/app/credentials/credentials && echo "from django.contrib.auth import get_user_model; User = get_user_model(); User.objects.create_superuser(\"edx\", \"[email protected]\", \"edx\") if not User.objects.filter(username=\"edx\").exists() else None" | python /edx/app/$1/$1/manage.py shell' -- "$name"
set -e

set +e
# FIXME[bash-e]: Bash scripts should use -e -- but this script fails
# with missing manage.py (need another "credentials" in that cd path?)
echo -e "${GREEN}Configuring site for ${name}...${NC}"
docker-compose exec -T ${name} bash -c 'source /edx/app/credentials/credentials_env && cd /edx/app/credentials/ && ./manage.py create_or_update_site --site-id=1 --site-domain=localhost:18150 --site-name="Open edX" --platform-name="Open edX" --company-name="Open edX" --lms-url-root=http://localhost:18000 --catalog-api-url=http://edx.devstack.discovery:18381/api/v1/ --tos-url=http://localhost:18000/tos --privacy-policy-url=http://localhost:18000/privacy --homepage-url=http://localhost:18000 --certificate-help-url=http://localhost:18000/faq --records-help-url=http://localhost:18000/faq --theme-name=openedx'
docker-compose exec -T ${name} bash -e -c 'source /edx/app/credentials/credentials_env && cd /edx/app/credentials/ && ./manage.py create_or_update_site --site-id=1 --site-domain=localhost:18150 --site-name="Open edX" --platform-name="Open edX" --company-name="Open edX" --lms-url-root=http://localhost:18000 --catalog-api-url=http://edx.devstack.discovery:18381/api/v1/ --tos-url=http://localhost:18000/tos --privacy-policy-url=http://localhost:18000/privacy --homepage-url=http://localhost:18000 --certificate-help-url=http://localhost:18000/faq --records-help-url=http://localhost:18000/faq --theme-name=openedx'
set -e

./provision-ida-user.sh ${name} ${name} ${port}

# Compile static assets last since they are absolutely necessary for all services. This will allow developers to get
# started if they do not care about static assets
echo -e "${GREEN}Compiling static assets for ${name}...${NC}"
docker-compose exec -T ${name} bash -c ' if ! source /edx/app/credentials/credentials_env && cd /edx/app/credentials/credentials && make static 2>creds_make_static.err; then echo "------- Last 100 lines of stderr"; tail creds_make_static.err -n 100; echo "-------"; fi;' -- "$name"
docker-compose exec -T ${name} bash -e -c ' if ! source /edx/app/credentials/credentials_env && cd /edx/app/credentials/credentials && make static 2>creds_make_static.err; then echo "------- Last 100 lines of stderr"; tail creds_make_static.err -n 100; echo "-------"; fi;' -- "$name"
18 changes: 14 additions & 4 deletions provision-discovery.sh
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
#!/usr/bin/env bash
# Provisioning script for the discovery service
set -eu -o pipefail
set -x

./provision-ida.sh discovery discovery 18381

docker-compose exec -T discovery bash -c 'rm -rf /edx/var/discovery/*'
docker-compose exec -T discovery bash -c 'source /edx/app/discovery/discovery_env && python /edx/app/discovery/discovery/manage.py create_or_update_partner --site-id 1 --site-domain localhost:18381 --code edx --name edX --courses-api-url "http://edx.devstack.lms:18000/api/courses/v1/" --lms-coursemode-api-url "http://edx.devstack.lms:18000/api/course_modes/v1/" --ecommerce-api-url "http://edx.devstack.ecommerce:18130/api/v2/" --organizations-api-url "http://edx.devstack.lms:18000/api/organizations/v0/" --lms-url "http://edx.devstack.lms:18000/" --studio-url "http://edx.devstack.studio:18010/" --publisher-url "http://edx.devstack.frontend-app-publisher:18400/"'
docker-compose exec -T discovery bash -c 'source /edx/app/discovery/discovery_env && python /edx/app/discovery/discovery/manage.py refresh_course_metadata'
docker-compose exec -T discovery bash -c 'source /edx/app/discovery/discovery_env && python /edx/app/discovery/discovery/manage.py update_index --disable-change-limit'
docker-compose exec -T discovery bash -e -c 'rm -rf /edx/var/discovery/*'
docker-compose exec -T discovery bash -e -c 'source /edx/app/discovery/discovery_env && python /edx/app/discovery/discovery/manage.py create_or_update_partner --site-id 1 --site-domain localhost:18381 --code edx --name edX --courses-api-url "http://edx.devstack.lms:18000/api/courses/v1/" --lms-coursemode-api-url "http://edx.devstack.lms:18000/api/course_modes/v1/" --ecommerce-api-url "http://edx.devstack.ecommerce:18130/api/v2/" --organizations-api-url "http://edx.devstack.lms:18000/api/organizations/v0/" --lms-url "http://edx.devstack.lms:18000/" --studio-url "http://edx.devstack.studio:18010/" --publisher-url "http://edx.devstack.frontend-app-publisher:18400/"'

set +e
# FIXME[bash-e]: Bash scripts should use -e -- but this script fails
# (after many retries) when trying to talk to ecommerce
docker-compose exec -T discovery bash -e -c 'source /edx/app/discovery/discovery_env && python /edx/app/discovery/discovery/manage.py refresh_course_metadata'
set -e

docker-compose exec -T discovery bash -e -c 'source /edx/app/discovery/discovery_env && python /edx/app/discovery/discovery/manage.py update_index --disable-change-limit'

# Add demo program
./programs/provision.sh discovery
13 changes: 7 additions & 6 deletions provision-e2e.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
set -e
set -o pipefail
#!/usr/bin/env bash

set -eu -o pipefail
set -x

if [ -z "$DEVSTACK_WORKSPACE" ]; then
Expand All @@ -13,11 +14,11 @@ fi
docker cp ${DEVSTACK_WORKSPACE}/edx-e2e-tests/upload_files/course.tar.gz "$(make --silent --no-print-directory dev.print-container.studio)":/tmp/

# Extract the test course tarball
docker-compose exec -T studio bash -c 'cd /tmp && tar xzf course.tar.gz'
docker-compose exec -T studio bash -e -c 'cd /tmp && tar xzf course.tar.gz'

# Import the course content
docker-compose exec -T studio bash -c 'source /edx/app/edxapp/edxapp_env && python /edx/app/edxapp/edx-platform/manage.py cms --settings=devstack_docker import /tmp course'
docker-compose exec -T studio bash -e -c 'source /edx/app/edxapp/edxapp_env && python /edx/app/edxapp/edx-platform/manage.py cms --settings=devstack_docker import /tmp course'

# Clean up the temp files
docker-compose exec -T studio bash -c 'rm /tmp/course.tar.gz'
docker-compose exec -T studio bash -c 'rm -r /tmp/course'
docker-compose exec -T studio bash -e -c 'rm /tmp/course.tar.gz'
docker-compose exec -T studio bash -e -c 'rm -r /tmp/course'
10 changes: 7 additions & 3 deletions provision-ecommerce.sh
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
#!/usr/bin/env bash
set -eu -o pipefail
set -x

# Load database dumps for the largest databases to save time
./load-db.sh ecommerce

./provision-ida.sh ecommerce ecommerce 18130

# Configure ecommerce
docker-compose exec -T ecommerce bash -c 'source /edx/app/ecommerce/ecommerce_env && python /edx/app/ecommerce/ecommerce/manage.py create_or_update_site --site-id=1 --site-domain=localhost:18130 --partner-code=edX --partner-name="Open edX" --lms-url-root=http://edx.devstack.lms:18000 --lms-public-url-root=http://localhost:18000 --client-side-payment-processor=cybersource --payment-processors=cybersource,paypal --sso-client-id=ecommerce-sso-key --sso-client-secret=ecommerce-sso-secret --backend-service-client-id=ecommerce-backend-service-key --backend-service-client-secret=ecommerce-backend-service-secret --from-email [email protected] --discovery_api_url=http://edx.devstack.discovery:18381/api/v1/ --enable-microfrontend-for-basket-page=1 --payment-microfrontend-url=http://localhost:1998'
docker-compose exec -T ecommerce bash -c 'source /edx/app/ecommerce/ecommerce_env && python /edx/app/ecommerce/ecommerce/manage.py oscar_populate_countries --initial-only'
docker-compose exec -T ecommerce bash -c 'source /edx/app/ecommerce/ecommerce_env && python /edx/app/ecommerce/ecommerce/manage.py create_demo_data --partner=edX'
docker-compose exec -T ecommerce bash -e -c 'source /edx/app/ecommerce/ecommerce_env && python /edx/app/ecommerce/ecommerce/manage.py create_or_update_site --site-id=1 --site-domain=localhost:18130 --partner-code=edX --partner-name="Open edX" --lms-url-root=http://edx.devstack.lms:18000 --lms-public-url-root=http://localhost:18000 --client-side-payment-processor=cybersource --payment-processors=cybersource,paypal --sso-client-id=ecommerce-sso-key --sso-client-secret=ecommerce-sso-secret --backend-service-client-id=ecommerce-backend-service-key --backend-service-client-secret=ecommerce-backend-service-secret --from-email [email protected] --discovery_api_url=http://edx.devstack.discovery:18381/api/v1/ --enable-microfrontend-for-basket-page=1 --payment-microfrontend-url=http://localhost:1998'
docker-compose exec -T ecommerce bash -e -c 'source /edx/app/ecommerce/ecommerce_env && python /edx/app/ecommerce/ecommerce/manage.py oscar_populate_countries --initial-only'
docker-compose exec -T ecommerce bash -e -c 'source /edx/app/ecommerce/ecommerce_env && python /edx/app/ecommerce/ecommerce/manage.py create_demo_data --partner=edX'
6 changes: 3 additions & 3 deletions provision-forum.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
set -e
set -o pipefail
#!/usr/bin/env bash
set -eu -o pipefail
set -x

docker-compose up -d forum
docker-compose exec -T forum bash -c 'source /edx/app/forum/ruby_env && cd /edx/app/forum/cs_comments_service && bundle install --deployment --path /edx/app/forum/.gem/'
docker-compose exec -T forum bash -e -c 'source /edx/app/forum/ruby_env && cd /edx/app/forum/cs_comments_service && bundle install --deployment --path /edx/app/forum/.gem/'
25 changes: 22 additions & 3 deletions provision-ida-user.sh
Original file line number Diff line number Diff line change
@@ -1,14 +1,33 @@
#!/usr/bin/env bash
set -eu -o pipefail
set -x

# FIXME[bash-e]: Bash scripts should use -e -- but this script fails
# with the following errors for ecommerce & edx_notes_api:
# - RuntimeError: Model class openedx.core.djangoapps.content_libraries.models.ContentLibrary doesn't declare an explicit app_label and isn't in an application in INSTALLED_APPS.
# - django.db.utils.ProgrammingError: (1146, "Table 'edxapp.auth_user' doesn't exist")
set +e

#This script depends on the LMS being up!

. scripts/colors.sh

app_name=$1
client_name=$2
client_port=$3

echo -e "${GREEN}Creating service user and OAuth2 applications for ${app_name}...${NC}"

# Create the service user.
docker-compose exec -T lms bash -c 'source /edx/app/edxapp/edxapp_env && python /edx/app/edxapp/edx-platform/manage.py lms --settings=devstack_docker manage_user $1_worker [email protected] --staff --superuser' -- "$app_name"
docker-compose exec -T lms bash -e -c 'source /edx/app/edxapp/edxapp_env && python /edx/app/edxapp/edx-platform/manage.py lms --settings=devstack_docker manage_user $1_worker [email protected] --staff --superuser' -- "$app_name"

# Create the DOT applications - one for single sign-on and one for backend service IDA-to-IDA authentication.
docker-compose exec -T lms bash -c 'source /edx/app/edxapp/edxapp_env && python /edx/app/edxapp/edx-platform/manage.py lms --settings=devstack_docker create_dot_application --grant-type authorization-code --skip-authorization --redirect-uris "http://localhost:$3/complete/edx-oauth2/" --client-id "$1-sso-key" --client-secret "$1-sso-secret" --scopes "user_id" $1-sso $1_worker' -- "$app_name" "$client_name" "$client_port"
docker-compose exec -T lms bash -c 'source /edx/app/edxapp/edxapp_env && python /edx/app/edxapp/edx-platform/manage.py lms --settings=devstack_docker create_dot_application --grant-type client-credentials --client-id "$1-backend-service-key" --client-secret "$1-backend-service-secret" $1-backend-service $1_worker' -- "$app_name" "$client_name" "$client_port"
docker-compose exec -T lms bash -e -c 'source /edx/app/edxapp/edxapp_env && python /edx/app/edxapp/edx-platform/manage.py lms --settings=devstack_docker create_dot_application --grant-type authorization-code --skip-authorization --redirect-uris "http://localhost:$3/complete/edx-oauth2/" --client-id "$1-sso-key" --client-secret "$1-sso-secret" --scopes "user_id" $1-sso $1_worker' -- "$app_name" "$client_name" "$client_port"
docker-compose exec -T lms bash -e -c 'source /edx/app/edxapp/edxapp_env && python /edx/app/edxapp/edx-platform/manage.py lms --settings=devstack_docker create_dot_application --grant-type client-credentials --client-id "$1-backend-service-key" --client-secret "$1-backend-service-secret" $1-backend-service $1_worker' -- "$app_name" "$client_name" "$client_port"


# FIXME[bash-e]: Suppress last error so that calling script can set -e
# for itself. Remove this once *this* script is run entirely with `-e`
# in effect (or at least the last command is no longer erroring for
# any services).
true
15 changes: 10 additions & 5 deletions provision-ida.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
#!/bin/sh -x
#!/usr/bin/env bash
set -eu -o pipefail

. scripts/colors.sh
set -x

app_name=$1 # The name of the IDA application, i.e. /edx/app/<app_name>
client_name=$2 # The name of the Oauth client stored in the edxapp DB.
client_port=$3 # The port corresponding to this IDA service in devstack.
Expand All @@ -7,17 +12,17 @@ container_name=${4:-$1} # (Optional) The name of the container. If missing, wil
docker-compose up -d $app_name

echo -e "${GREEN}Installing requirements for ${app_name}...${NC}"
docker-compose exec -T ${container_name} bash -c 'source /edx/app/$1/$1_env && cd /edx/app/$1/$1/ && make requirements' -- "$app_name"
docker-compose exec -T ${container_name} bash -e -c 'source /edx/app/$1/$1_env && cd /edx/app/$1/$1/ && make requirements' -- "$app_name"

echo -e "${GREEN}Running migrations for ${app_name}...${NC}"
docker-compose exec -T ${container_name} bash -c 'source /edx/app/$1/$1_env && cd /edx/app/$1/$1/ && make migrate' -- "$app_name"
docker-compose exec -T ${container_name} bash -e -c 'source /edx/app/$1/$1_env && cd /edx/app/$1/$1/ && make migrate' -- "$app_name"

echo -e "${GREEN}Creating super-user for ${app_name}...${NC}"
docker-compose exec -T ${container_name} bash -c 'source /edx/app/$1/$1_env && echo "from django.contrib.auth import get_user_model; User = get_user_model(); User.objects.create_superuser(\"edx\", \"[email protected]\", \"edx\") if not User.objects.filter(username=\"edx\").exists() else None" | python /edx/app/$1/$1/manage.py shell' -- "$app_name"
docker-compose exec -T ${container_name} bash -e -c 'source /edx/app/$1/$1_env && echo "from django.contrib.auth import get_user_model; User = get_user_model(); User.objects.create_superuser(\"edx\", \"[email protected]\", \"edx\") if not User.objects.filter(username=\"edx\").exists() else None" | python /edx/app/$1/$1/manage.py shell' -- "$app_name"

./provision-ida-user.sh $app_name $client_name $client_port

# Compile static assets last since they are absolutely necessary for all services. This will allow developers to get
# started if they do not care about static assets
echo -e "${GREEN}Compiling static assets for ${app_name}...${NC}"
docker-compose exec -T ${container_name} bash -c 'source /edx/app/$1/$1_env && cd /edx/app/$1/$1/ && make static' -- "$app_name"
docker-compose exec -T ${container_name} bash -e -c 'source /edx/app/$1/$1_env && cd /edx/app/$1/$1/ && make static' -- "$app_name"
Loading

0 comments on commit 4f21c4d

Please sign in to comment.