Skip to content
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

Finch and Raven not creating their wps_output under /bird_name #11

Closed
tlvu opened this issue Jan 29, 2020 · 15 comments · Fixed by #203
Closed

Finch and Raven not creating their wps_output under /bird_name #11

tlvu opened this issue Jan 29, 2020 · 15 comments · Fixed by #203
Assignees
Labels
bug Something isn't working

Comments

@tlvu
Copy link
Collaborator

tlvu commented Jan 29, 2020

Migrated from old PAVICS https://github.com/Ouranosinc/PAVICS/issues/151

Does not cause problem now but might cause problem later when we want to restrict access to some wps output from these birds (for privacy reason). Older birds (flyingpigeon, mallefowl, ...) are following this schema.

See discusstion in https://github.com/Ouranosinc/PAVICS/pull/150#discussion_r305084199

@huard
Copy link
Collaborator

huard commented Jul 8, 2020

Has this been fixed with the cookie-cutter template update ?

@tlvu
Copy link
Collaborator Author

tlvu commented Jul 8, 2020

Nope. It's a deployment config. It's probably directly in here, unless pywps do not have that config yet or the config do not support giving a initially non-existing dir.

@fmigneault fmigneault added the bug Something isn't working label Jan 22, 2021
@huard
Copy link
Collaborator

huard commented May 28, 2021

@dbyrns Is this required for cowbird ?

@dbyrns
Copy link
Collaborator

dbyrns commented May 28, 2021

Cowbird will not handle process outputs so the quick answer is no. On the other hand wps_outputs is served by nginx and thredds so the structure could be "visible". We better have something well organized. Permissions management may be harder also if everything is mixed under the same folder.
To fix this, you probably don't have to update the bird directly. Weaver has been "fixed" simply by updating the volume mount in compose by defining the bird name on the server side. See : https://github.com/bird-house/birdhouse-deploy/blob/feature/DAC-300-weaver-compose/birdhouse/components/weaver/default.env#L54

@huard huard pinned this issue Jul 12, 2021
@huard
Copy link
Collaborator

huard commented Oct 27, 2021

Looking at the docker compose, here are the volumes that are mounted:

  • malleefowl: wps_outputs:/opt/birdhouse/var/lib/pywps/outputs
  • hummingbird: wps_outputs:/opt/birdhouse/var/lib/pywps/outputs
  • flyingpigeon: wps_outputs:/data/wpsoutputs
  • finch: wps_outputs:/data/wpsoutputs
  • raven: wps_outputs:/data/wpsoutputs

I'm not seeing where malleefowl and hummingbird actually configure their outputs to their bird name directory.

@dbyrns
Copy link
Collaborator

dbyrns commented Oct 27, 2021

@huard, old birds were using a buildout recipe. The output location is defined here : https://github.com/bird-house/birdhousebuilder.recipe.pywps/blob/master/birdhousebuilder/recipe/pywps/pywps.cfg#L28

@huard
Copy link
Collaborator

huard commented Oct 27, 2021

Ah thanks !

So am I right thinking that we could simply do something like this in the compose:

finch: 
...
volumes:
   - wps_outputs/finch:/data/wpsoutputs

@dbyrns
Copy link
Collaborator

dbyrns commented Oct 27, 2021

wps_outputs is a named volume, I don't think you can append a directory to it.
Since wps_outputs is assumed to be under /data (set by $DATA_PERSIST_ROOT) you could do the same as weaver :
See https://github.com/bird-house/birdhouse-deploy/pull/114/files#diff-bf078310de48415c354a40f99e18c9a5d5145c603579e8d29110585977aa42bfR57 and https://github.com/bird-house/birdhouse-deploy/pull/114/files#diff-a633f61efffbef9b534cc7bbfa6c4bca0acf906deb9bd03908e4645e47a31000R70
and define

finch: 
...
volumes:
   - ${DATA_PERSIST_ROOT}/wps_outputs/finch:/data/wpsoutputs

@tlvu
Copy link
Collaborator Author

tlvu commented Oct 28, 2021

wps_outputs is a named volume, I don't think you can append a directory to it.

Exact !

Since wps_outputs is assumed to be under /data (set by $DATA_PERSIST_ROOT)

The /data/wpsoutputs part is inside the container, not on the host. $DATA_PERSIST_ROOT is on the host side.

Remember this persistance is using data-volume, not direct bind mount from the host.

I was lost with the buildout magic. But I think the cleanest is to somehow reproduce that magic.

I'll try to give it another shot, unless you know more about buildout that you already know that recipe can not be reproduced outside of buildout. Let me know.

@tlvu
Copy link
Collaborator Author

tlvu commented Oct 28, 2021

@huard, old birds were using a buildout recipe. The output location is defined here : https://github.com/bird-house/birdhousebuilder.recipe.pywps/blob/master/birdhousebuilder/recipe/pywps/pywps.cfg#L28

Oh wait, isn't this just a wps.cfg change?! That can easily be reproduced. Thanks for pointing to the right place inside that buildout recipe. It would probably cost me some additional time to trace to this.

@dbyrns
Copy link
Collaborator

dbyrns commented Oct 28, 2021

The /data/wpsoutputs part is inside the container, not on the host. $DATA_PERSIST_ROOT is on the host side.

I know, the finch part doesn't need to exist inside the container, it's only relevant on the host side, where the data must exist along other birds. By doing it like this compose will create the finch directory on the host side and mount it at the same place it used to inside the container and your PR (https://github.com/bird-house/birdhouse-deploy/pull/203/files) will not be required at all.

@tlvu
Copy link
Collaborator Author

tlvu commented Oct 28, 2021

The /data/wpsoutputs part is inside the container, not on the host. $DATA_PERSIST_ROOT is on the host side.

I know, the finch part doesn't need to exist inside the container, it's only relevant on the host side, where the data must exist along other birds. By doing it like this compose will create the finch directory on the host side and mount it at the same place it used to inside the container and your PR (https://github.com/bird-house/birdhouse-deploy/pull/203/files) will not be required at all.

I understood that. But doing what you propose means changing the left side of the volume mount from a data-volume (wps_outputs:/data/wpsoutputs) to a direct bind mount on the host (${DATA_PERSIST_ROOT}/wps_outputs/finch:/data/wpsoutputs).

This means changing all the existing birds that currently use data-volume to direct bind mount. Also I think we previously choose data-volume instead of direct bind mount because there was no backup need for this kind of data.

This also means the current way Weaver tries to share it's wpsoutput is wrong since the Nginx proxy also use the data-volume

- wps_outputs:/pavics-data/wps_outputs
.

@dbyrns
Copy link
Collaborator

dbyrns commented Nov 1, 2021

Thank you @tlvu, after some DM with you thursday, I agreeded that the wps_outputs volume is mounted under ${DATA_PERSIST_ROOT} only in thredds and we cannot mix and match volume and direct bind mount. The best solution is therefore #203

I remembered talking about that with @fmigneault and we were saying that sharing the same volume with all birds is not a very good practice. Sadly some birds expect to create a directory having their bird name. Without that behavior having a named volume per-bird would have solved the issue and be a better practice by isolating each bird.

@fmigneault, after having talk with @tlvu I'm no longer confident that the solution used in Weaver will be correct. The direct bind mount ${DATA_PERSIST_ROOT}/wps_outputs/weaver will probably not be exposed in thredds or nginx since the wps_outputs directory will be replaced by the volume mount. Will need to check that later (another PR 😉).

@fmigneault
Copy link
Collaborator

This also means the current way Weaver tries to share it's wpsoutput is wrong since the Nginx proxy also use the data-volume

The direct bind mount ${DATA_PERSIST_ROOT}/wps_outputs/weaver will probably not be exposed in thredds or nginx since the wps_outputs directory will be replaced by the volume mount.

Not true. Have a try: https://host-140-154.rdext.crim.ca/wpsoutputs/weaver/2dd1d8ca-651c-4644-9ac6-b6bf32718457.log

Without that behavior having a named volume per-bird would have solved the issue

I still believe the fix should be on the bird's side. Some flag to create or not the named directory.
It should not enforce a specific name, and they should definitely remain distinct volumes.

@tlvu
Copy link
Collaborator Author

tlvu commented Feb 14, 2022

This is not yet working, update here #231 (comment)

@tlvu tlvu closed this as completed in #203 Feb 17, 2023
tlvu added a commit that referenced this issue Feb 17, 2023
Before this fix, finch, raven, flyingpigeon were dumping their output
directly under `https://PAVICS_HOST/wpsoutputs/`.

With this fix, it will be under each bird name, ex:
`https://PAVICS_HOST/wpsoutputs/finch/` which is cleaner and follows
what malleefowl and hummingbird already does.

Fixes #11.
Fixes https://crim-ca.atlassian.net/browse/DAC-398

Requires PR Ouranosinc/pavics-sdi#280,
bird-house/finch#273,
Ouranosinc/raven#459

If `optional-components/secure-data-proxy` is enabled, might need some
additional permissions for each bird in

https://github.com/bird-house/birdhouse-deploy/blob/master/birdhouse/optional-components/secure-data-proxy/config/magpie/config.yml.template.
@huard huard unpinned this issue Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants