-
Notifications
You must be signed in to change notification settings - Fork 7
Enforce the load order of components defined in env.local #304
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.
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
Changes from 9 commits
096509a
25daaea
a91b7dc
d91dba0
d4e7ea4
fb92700
8d80642
8272c93
11d0279
81a2e41
d580911
94ffced
343e26d
21825f9
d747892
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| name: Unit tests | ||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize, reopened, ready_for_review] | ||
| jobs: | ||
| test: | ||
| if: github.event.pull_request.draft == false | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - name: Set up python for testing | ||
| uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: "3.11" | ||
| cache: 'pip' | ||
| - name: Install python test dependencies | ||
| run: | | ||
| pip install -r ./tests/requirements.txt | ||
| - name: Test with pytest | ||
| run: | | ||
| pytest ./tests/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,13 +56,50 @@ | |
|
|
||
| the load order is: | ||
|
|
||
| - ./config/twitcher | ||
| - ./config/magpie (loaded as a dependency of twitcher, not loaded a second time after project-api) | ||
| - ./config/project-api | ||
| - ./optional-components/generic_bird | ||
| - ./config/twitcher | ||
| - ./config/project-api | ||
| - ./config/wps_outputs-volume (loaded as a dependency of generic_bird) | ||
| - ./optional-components/generic_bird | ||
| - ./components/cowbird | ||
|
|
||
| This load order also applies to the order that docker-compose-extra.yml files are specified. If a component also | ||
| includes an override file for another component (eg: ./config/finch/config/proxy/docker-compose-extra.yml overrides | ||
| ./config/proxy/docker-compose-extra.yml), the following additional load order rules apply: | ||
|
|
||
| - if the component that is being overridden has already been loaded, the override file is loaded immediately | ||
| - otherwise, the override files will be loaded immediately after the component that is being overridden has been loaded | ||
|
|
||
| For example, with the following files in place: | ||
|
|
||
| ```shell | ||
| # env.local | ||
| DEFAULT_CONF_DIRS=" | ||
| ./config/finch | ||
| ./config/proxy | ||
| " | ||
| ``` | ||
| ```yaml | ||
| # config/proxy/docker-compose-extra.yml | ||
| ... | ||
| # config/finch/docker-compose-extra.yml | ||
| ... | ||
| # config/finch/config/proxy/docker-compose-extra.yml | ||
| ... | ||
| ``` | ||
|
|
||
| the docker compose files will be loaded in the following order: | ||
|
|
||
| - config/finch/docker-compose-extra.yml | ||
| - config/proxy/docker-compose-extra.yml | ||
| - config/finch/config/proxy/docker-compose-extra.yml | ||
|
Comment on lines
+98
to
+100
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to contradict what the test evaluates, or there is something I missed?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Finch doesn't have proxy as an explicit dependency (it is not included in the COMPONENT_DEPENDENCIES variable defined in config/finch/default.env) so proxy is not loaded before finch. Because of this, the order that the components appear in the DEFAULT_CONF_DIRS list is respected (finch is before proxy). However, the file that overrides proxy settings (config/finch/config/proxy/docker-compose-extra.yml) can't be loaded until after both finch and proxy are loaded, since it could potentially override settings in either one, so it is loaded as soon as both of finch an proxy are loaded. From the CHANGES.md file, here is the relevant explanation:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. So in the tests, |
||
|
|
||
| - Add tests to ensure override capabilities are preserved which allows all default | ||
| behaviors of the platform can be customized. | ||
|
|
||
| See [birdhouse/README.rst](birdhouse/README.rst) for instruction to run the | ||
| tests. | ||
|
|
||
| [1.25.1](https://github.com/bird-house/birdhouse-deploy/tree/1.25.1) (2023-04-11) | ||
| ------------------------------------------------------------------------------------------------------------------ | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -66,17 +66,19 @@ read_default_env() { | |
| read_env_local() { | ||
| # we don't use usual .env filename, because docker-compose uses it | ||
|
|
||
| if [ -e "$COMPOSE_DIR/env.local" ]; then | ||
| echo "Using local environment file at: ${BIRDHOUSE_LOCAL_ENV:="$COMPOSE_DIR/env.local"}" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a new feature that
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. I intended it as a hidden feature and I think I'd like to keep it hidden for now. |
||
|
|
||
| if [ -e "$BIRDHOUSE_LOCAL_ENV" ]; then | ||
| saved_shell_options="$(set +o)" | ||
| set +xv # hide passwd in env.local in logs | ||
|
|
||
| . "$COMPOSE_DIR/env.local" | ||
| . "$BIRDHOUSE_LOCAL_ENV" | ||
|
|
||
| # restore saved shell options | ||
| eval "$saved_shell_options" | ||
|
|
||
| else | ||
| echo "WARNING: '$COMPOSE_DIR/env.local' not found" 1>&2 | ||
| echo "WARNING: '$BIRDHOUSE_LOCAL_ENV' not found" 1>&2 | ||
| fi | ||
|
|
||
| } | ||
|
|
@@ -93,23 +95,37 @@ source_conf_files() { | |
| # ignore directories that are already in ALL_CONF_DIRS | ||
| continue | ||
| fi | ||
| if [ -e "$adir" ]; then | ||
| ALL_CONF_DIRS="$ALL_CONF_DIRS | ||
| $adir | ||
| " | ||
| else | ||
| # push current adir onto the stack (this helps keep track of current adir when recursing) | ||
| _adir_stack="$_adir_stack\n$adir" | ||
| if [ ! -e "$adir" ]; then | ||
| # Do not exit to not break unattended autodeploy since no human around to | ||
| # fix immediately. | ||
| # The new adir with typo will not be active but at least all the existing | ||
| # will still work. | ||
| echo "WARNING: '$adir' in $conf_locations does not exist" 1>&2 | ||
| fi | ||
| COMPONENT_DEFAULT_ENV="$adir/default.env" | ||
| if [ -f "$COMPONENT_DEFAULT_ENV" ]; then | ||
| echo "reading '$COMPONENT_DEFAULT_ENV'" | ||
| . "$COMPONENT_DEFAULT_ENV" | ||
| source_conf_files "$COMPONENT_DEPENDENCIES" "a dependency of $adir" | ||
| if [ -f "$adir/default.env" ]; then | ||
| # Source config settings of dependencies first if they haven't been sourced previously. | ||
| # Note that this will also define the order that docker-compose-extra.yml files will be loaded. | ||
| unset COMPONENT_DEPENDENCIES | ||
| dependencies="$(. "$adir/default.env" && echo "$COMPONENT_DEPENDENCIES")" | ||
| if [ -n "$dependencies" ]; then | ||
| source_conf_files "$dependencies" "a dependency of $adir" | ||
| # reset the adir variable in case it was changed in a recursive call | ||
| adir="$(printf '%b' "$_adir_stack" | tail -1)" | ||
| fi | ||
| echo "reading '$adir/default.env'" | ||
| . "$adir/default.env" | ||
| fi | ||
| if echo "$ALL_CONF_DIRS" | grep -qE "^\s*$adir\s*$"; then | ||
| # check again in case a dependency has already added this to the ALL_CONF_DIRS variable | ||
| continue | ||
| fi | ||
| ALL_CONF_DIRS="$ALL_CONF_DIRS | ||
| $adir | ||
| " | ||
| # pop current adir from the stack once we're done with it | ||
| _adir_stack="$(printf '%b' "$_adir_stack" | sed '$d')" | ||
| done | ||
| } | ||
|
|
||
|
|
@@ -151,6 +167,60 @@ process_delayed_eval() { | |
| } | ||
|
|
||
|
|
||
| # Sets the COMPOSE_CONF_LIST variable to a string that contains all the docker-compose*.yml files that are | ||
| # required to pass to the docker compose command. This is determined by the contents of the ALL_CONF_DIRS | ||
| # variable (set using the read_components_default_env function). | ||
| # The file order is determined by the order of the config directories in ALL_CONF_DIRS. | ||
| # If a config directory also includes an override file for another component | ||
| # (eg: ./config/finch/config/proxy/docker-compose-extra.yml overrides ./config/proxy/docker-compose-extra.yml), | ||
| # the following additional order rules apply: | ||
| # | ||
| # - if the component that is being overridden has already been added, the override file is added immediately | ||
| # - otherwise, the override files will be added immediately after the component that is being overridden | ||
| # has been added | ||
| create_compose_conf_list() { | ||
| # ALL_CONF_DIRS relative paths are relative to COMPOSE_DIR. | ||
| discover_compose_dir | ||
| if [ -d "$COMPOSE_DIR" ]; then | ||
| cd "$COMPOSE_DIR" || return | ||
| fi | ||
|
|
||
| COMPOSE_CONF_LIST="-f docker-compose.yml" | ||
| COMPONENT_OVERRIDES='' | ||
| LOADED_COMPONENTS='' | ||
| for adir in $ALL_CONF_DIRS; do | ||
| service_name=$(basename "$adir") | ||
| LOADED_COMPONENTS="${LOADED_COMPONENTS}\n${service_name}" | ||
|
|
||
| if [ -f "$adir/docker-compose-extra.yml" ]; then | ||
| COMPOSE_CONF_LIST="${COMPOSE_CONF_LIST} -f $adir/docker-compose-extra.yml" | ||
| fi | ||
|
|
||
| # If previously loaded components specified overrides for the component that was just loaded, load those overrides now | ||
| previous_overrides=$(printf '%b' "${COMPONENT_OVERRIDES}" | grep "^$service_name " | sed "s/^${service_name}//g" | tr '\n' ' ') | ||
| COMPOSE_CONF_LIST="${COMPOSE_CONF_LIST} ${previous_overrides}" | ||
| # Load overrides for other components unless the component to be overridden hasn't been loaded yet. If the component | ||
| # hasn't been loaded yet, store a reference to it so that it can be added in as soon as the component is loaded. | ||
| for conf_dir in "$adir"/config/*; do | ||
| override_service_name=$(basename "$conf_dir") | ||
| extra_compose="$conf_dir/docker-compose-extra.yml" | ||
| if [ -f "$extra_compose" ]; then | ||
| if printf '%b' "${LOADED_COMPONENTS}" | grep -q "^$override_service_name$"; then | ||
| COMPOSE_CONF_LIST="${COMPOSE_CONF_LIST} -f $extra_compose" | ||
| else | ||
| COMPONENT_OVERRIDES="${COMPONENT_OVERRIDES}\n${override_service_name} -f ${extra_compose}" | ||
| fi | ||
| fi | ||
| done | ||
| done | ||
|
|
||
| # Return to previous pwd. | ||
| if [ -d "$COMPOSE_DIR" ]; then | ||
| cd - || return | ||
| fi | ||
| } | ||
|
|
||
|
|
||
| # Main function to read all config files in appropriate order and call | ||
| # process_delayed_eval() at the appropriate moment. | ||
| read_configs() { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| pytest==7.2.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice delayed load ! Components do not have to know the load ordering amount themselves.