-
Notifications
You must be signed in to change notification settings - Fork 7
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
Update DEFAULT_CONF_DIRS
to the minimal components required to deploy the stack
#399
Conversation
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2216/Result : failure BIRDHOUSE_DEPLOY_BRANCH : minimal-components DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1399/NOTEBOOK TEST RESULTS |
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.
Since the EXTRA_CONF_DIRS
was preemptively set with both config
and component
locations for all items explicitly, and that the CI test suite runs everything without major problem, I am confident the refactor must be good.
However, I would like to hold off this PR for some time, as I am aware that @ChaamC, @Nazim-crim and @ahandan are all working on finalizing some relatively complex implementations that interact with multiple components simultaneously. I would prefer to avoid merge conflict burden on top of their changes that should be interacted in a near future.
@@ -0,0 +1 @@ | |||
./config |
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.
Since we are purposely moving components, should we consider leaving this un-ignored?
Could help detect an invalid definition that refers to the old path.
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.
I think we have to ignore it so that the autoupdate code works. Right @tlvu ?
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.
Since it would be a major update that could remove components if misconfigured, I guess it that it would feel safer to purposely break the autoupdate to have node admins make sure they set up their EXTRA_CONF_DIRS
properly and clean up leftover configs if applicable before updating. However, if others don't feel this could be an issue, I'm fine with having the gitignore also. I'm not personally using autoupdate.
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.
I'm not either so let's let @tlvu decide this one
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.
I'd rather prefer not breaking the autodeploy. We had already the plan to do this: add the new paths in advance to env.local
so it is already forward compatible when this PR merge so no manual interventions required.
However I would also ping PCIC in this PR so they are aware of this major change.
birdhouse/default.env
Outdated
./components/proxy | ||
./components/magpie | ||
./components/twitcher | ||
./components/stac | ||
./components/cowbird | ||
' |
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.
Given this change, I propose we use 2.0.0
for the new version.
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.
I agree
@@ -8,7 +8,7 @@ export PROXY_READ_TIMEOUT_VALUE="240s" | |||
|
|||
# Content of "location /" in file config/proxy/conf.d/all-services.include.template | |||
# Useful to have a custom homepage. | |||
export PROXY_ROOT_LOCATION="return 302 https://\$host/jupyter/hub/login;" | |||
export PROXY_ROOT_LOCATION="return 302 https://\$host/magpie;" |
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.
I saw that the same variable is set by default when enabling jupyterhub
.
Should we have a better strategy to define the desired root location redirect?
It seems that which one will be applied depends on a combination of dependency resolution order + file-system listing order. I'm not sure if this is replicable, unless one explicitly overrides the variable in their env.local
.
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.
It seems that which one will be applied depends on a combination of dependency resolution order + file-system listing order
If jupyterhub is enabled it will always override the magpie settings in default.env
since magpie is enabled in DEFAULT_CONF_DIRS
which will always be read before EXTRA_CONF_DIRS
(where jupyterhub is enabled). Because jupyterhub's default.env
will be read second, it will override the variable set in magpie's default.env
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.
Ok. Makes sense. I would add a comment that explicit this assumption, and indicate that this is only as last resort default if nothing else was enabled. Realistically, Magpie should probably not be the entrypoint, but we need "something" to fill the variable.
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.
Add this comment about magpie as landing page is not the most user-friendly location to the var PROXY_ROOT_LOCATION
in env.local.example
as well.
birdhouse/default.env
Outdated
./components/stac | ||
./components/cowbird |
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.
No difference technically, but can you put cowbird directly after magpie/twitcher given how closely related they are feature wise?
#COMPONENT_DEPENDENCIES=" | ||
# $COMPONENT_DEPENDENCIES | ||
# ./config/mongodb | ||
# ./components/mongodb | ||
#" |
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.
Technically, component/magpie
should be in there, otherwise Cowbird doesn't really have anything to interact with.
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.
Should we even keep this since "this dependency is only required if the mongo instance is the one provided in config/mongodb (include this for Cowbird<2.0.0)"
Is cowbird still <2.0.0? Else this comment block not needed anymore?
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.
I wouldn't worry about cowbird<2.0.0
. No reason to go back. Only add the relevant current dependencies.
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.
That's what I thought, no need to go back to older cowbird so no need to keep this comment block about adding mongodb as dependencies.
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.
Ok, I'll remove it
@@ -35,7 +35,7 @@ cd "${BIRDHOUSE_DEPLOY_COMPONENTS_ROOT}" || true # ignore error for now, empty | |||
|
|||
# note: no quotes in 'ls' on purpose to expand glob patterns | |||
BIRDHOUSE_DEPLOY_COMPONENTS_LIST_KNOWN="$( \ | |||
ls -d1 ./*components/*/ ./config/*/ 2>/dev/null \ | |||
ls -d1 ./*components/*/ ./components/*/ 2>/dev/null \ |
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.
Seems redundant to have component twice. Should be globed by the first one.
tests/pytest.ini
Outdated
@@ -0,0 +1,3 @@ | |||
[pytest] | |||
markers = | |||
online: marks test as requiring access to the internet (to run offline: '-m "not online"' |
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.
Missing closing )
.
The end message is somewhat misleading. It will not "run offline", but skip online tests entirely.
|
||
- add any components no longer in the `DEFAULT_CONF_DIRS` list to the `EXTRA_CONF_DIRS` list. | ||
|
||
For example, to keep the jupyterhub component enabled, add `./components/jupyterhub` to the `EXTRA_CONF_DIRS` list. |
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.
Given that ./components/jupyterhub
is not enabled by default anymore, but ./components/stac
is, it might be worth mentioning that adding PROXY_ROOT_LOCATION
explicitly could be required depending on which components are enabled, and which one the node manager desires to put as default redirect (if any).
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2220/Result : failure BIRDHOUSE_DEPLOY_BRANCH : minimal-components DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-133.rdext.crim.ca Infrastructure deployment failed. Instance has not been destroyed. @matprov |
…ve been there in the first place)
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2347/Result : failure BIRDHOUSE_DEPLOY_BRANCH : minimal-components DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-46.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1468/NOTEBOOK TEST RESULTS |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2349/Result : success BIRDHOUSE_DEPLOY_BRANCH : minimal-components DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-90.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1470/NOTEBOOK TEST RESULTS |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2348/Result : success BIRDHOUSE_DEPLOY_BRANCH : minimal-components DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-20.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1469/NOTEBOOK TEST RESULTS |
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.
LGTM. I assume an autodeploy test has been performed with the old list of components enabled in env.local
?
I will pre-prepare our various env.local
files in advance so once this PR is merged, our staging system will pick this up automatically.
@tlvu export EXTRA_CONF_DIRS="
./config/proxy
./config/magpie
./config/twitcher
./config/canarie-api
./config/geoserver
./config/finch
./config/raven
./config/hummingbird
./config/thredds
./config/portainer
./config/jupyterhub
./components/proxy
./components/magpie
./components/twitcher
./components/canarie-api
./components/geoserver
./components/finch
./components/raven
./components/hummingbird
./components/thredds
./components/portainer
./components/jupyterhub
./components/monitoring
./components/weaver
./components/cowbird
./components/stac
./optional-components/canarie-api-full-monitoring
./optional-components/all-public-access
./optional-components/testthredds
./optional-components/secure-thredds
./optional-components/secure-data-proxy
./optional-components/stac-data-proxy
./optional-components/stac-populator
./optional-components/wps-healthchecks
./optional-components/database-external-ports
./optional-components/test-weaver
./optional-components/test-cowbird-jupyter-access
./optional-components/test-geoserver-secured-access
./optional-components/x-robots-tag-header
" |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2352/Result : success BIRDHOUSE_DEPLOY_BRANCH : minimal-components DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-20.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1471/NOTEBOOK TEST RESULTS |
@fmigneault I've added some very basic documentation for the components (to be added to later on). I'll plan on merging this in early next week. |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2354/Result : success BIRDHOUSE_DEPLOY_BRANCH : minimal-components DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-90.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1472/NOTEBOOK TEST RESULTS |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2353/Result : success BIRDHOUSE_DEPLOY_BRANCH : minimal-components DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-20.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1473/NOTEBOOK TEST RESULTS |
Great addition. More than "basic". Thanks ! Future updates should probably reorganize them slightly so they are sorted similarly to the components directory. |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2358/Result : failure BIRDHOUSE_DEPLOY_BRANCH : minimal-components DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-20.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1476/NOTEBOOK TEST RESULTS |
#419) The `.gitignore` syntax was wrong. Regression from v2.0.0 (#399). **Non-breaking changes** - Fix `.gitignore` syntax <!-- The test suite can be run using a different DACCS config with ``birdhouse_daccs_configs_branch: branch_name`` in the PR description. To globally skip the test suite regardless of the commit message use ``birdhouse_skip_ci: true`` in the PR description. --> birdhouse_daccs_configs_branch: master birdhouse_skip_ci: false
Overview
Changes
DEFAULT_CONF_DIRS
to refer exclusively to the proxy, magpie, twitcher, stac, and cowbird components. Also moves all components that were previously under thebirdhouse/config
directory to thebirdhouse/components
directory. This removes the arbitrary distinction between these groups of components that didn't have any functional or logical reason.Because this change updates the default components, this is not backwards compatible unless the following changes are made to the local environment file (
birdhouse/env.local
by default):DEFAULT_CONF_DIRS
list to theEXTRA_CONF_DIRS
list.For example, to keep the jupyterhub component enabled, add
./components/jupyterhub
to theEXTRA_CONF_DIRS
list.Changes
Non-breaking changes
PROXY_ROOT_LOCATION
to refer to the magpie login page by default but to the jupyterhub login page when jupyterhub is also enabled.Breaking changes
./config
to./components
DEFAULT_CONF_DIRS
to refer exclusively to the proxy, magpie, twitcher, stac, and cowbirdRelated Issue / Discussion
Additional Information
Links to other issues or sources.
birdhouse_daccs_configs_branch: master
birdhouse_skip_ci: false