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

docker: add alt assets inclusion during build #56

Merged
merged 3 commits into from
Mar 24, 2020

Conversation

ppanero
Copy link
Member

@ppanero ppanero commented Mar 19, 2020

Closes #54

Building the assets when running containerize is not desired since it is time consuming and it will happen twice. However, when deploying in the "cloud" they are needed. Therefore, this PR allows it conditionally. This way docker-compose will not pass the arg and containerize stays as is.

This is part of a bigger picture solution mentioned in the above issue.

Method 1: flag the docker build and launch a job

Since we do not want to incur into the "double building" of assets
(Once in the docker build, a second if updating). One option could
be to flag the building of assets in the Dockerfile (e.g. build only
if include_assets env var is True or is set).

Since as per docker documentation, when a volume is mounted the files of the folder (i.e. /opt/... of the web node) will be pass to the volume, giving access to them to the nginx.

Fixes on deployment and future todos can be found here: inveniosoftware/helm-invenio#8

@ppanero ppanero changed the title WIP docker: add alt assets inclusion during build docker: add alt assets inclusion during build Mar 20, 2020
@ppanero
Copy link
Member Author

ppanero commented Mar 20, 2020

Build is breaking on dependencies. If we agree with the change I will fix it before merging ofc.

@fenekku
Copy link
Contributor

fenekku commented Mar 23, 2020

Completely naive thought looking at the different tasks around getting the statics to work:

Is there a way to have an openshift task / docker tools to just create a volume?

Seems to me that what it boils down to is: can we ship a refreshed static volume at deployment? If we can, then there wouldn't be any issue with moving things around and creating other initContainers or other additional code (but maybe we have to). That is, it would be nice to have a containerized volume holding statics we can just ship.

@fenekku
Copy link
Contributor

fenekku commented Mar 23, 2020

Other note: agreed on removing the --pre from the test code. We don't need/want that anymore.

@ppanero
Copy link
Member Author

ppanero commented Mar 23, 2020

Other note: agreed on removing the --pre from the test code. We don't need/want that anymore.

Fixed. Running tests now.

I will investigate the containerize volume in the meantime :D

Thanks for the review

@ppanero
Copy link
Member Author

ppanero commented Mar 23, 2020

From what I have found there are options, tho they do not convince me:

1- Create a new image with the FS and push it, then share that container image with the others.... Is the same that what we are doing but more cumbersome by maintaining two images.

2- Use "data containers" I am still trying to understand it, I am not 100% sure of the difference from above (1).

In any case, users might want to not run invenio-cli containerize, just work in local and ship. In which case they will need the changes of this PR.

I would open an issue (#58) to further investigate the data containers and merge this.

@ppanero ppanero requested a review from fenekku March 23, 2020 15:30
@ppanero ppanero merged commit 195d30b into inveniosoftware:master Mar 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docker: unable to run UI with bare built image
3 participants