Skip to content
This repository has been archived by the owner on Oct 25, 2019. It is now read-only.

start server via docker #373

Merged
merged 9 commits into from
Jun 20, 2019
Merged

start server via docker #373

merged 9 commits into from
Jun 20, 2019

Conversation

de-code
Copy link
Collaborator

@de-code de-code commented May 31, 2019

No description provided.

@de-code
Copy link
Collaborator Author

de-code commented May 31, 2019

@giorgiosironi
In the current deployment we run npm run bundle for the client as part of the formula. That will inject HTML into the index.html for NewRelic and Google Tag Manager. The PeerScout Python server, serving the API, also serves the client as static files (it only needs the JavaScript bundle and the index.html).

The problem is, that the peerscout image doesn't include node, and therefore we wouldn't be able to run npm run bundle. We could run the client container separately (and run npm run bundle as part of the deployment). Then we would use something like nginx to serve client and API under the same port.

However, it seems we should probably already include the bundled JavaScript in some container. We just need to inject the HTML into the index.html (into head, end of body and end of file). We could write a Python script to do that. I am just wondering whether that is already a solved problem?
How do we do that for other Single Page App projects? (the client could still be served by nginx - we could remove it from the peerscout image)

@de-code de-code requested a review from giorgiosironi May 31, 2019 10:00
@de-code de-code self-assigned this May 31, 2019
@giorgiosironi
Copy link
Contributor

How do we do that for other Single Page App projects? (the client could still be served by nginx - we could remove it from the peerscout image)

Maybe the xpub people know of a solution as they integrated New Relic as well. There's ids derived from the account in the generated HTML/JavaScript so I'd avoid building it into the image as it should be configuration. We may have to mount an HTML file or a fragment of it as a volume?

@de-code
Copy link
Collaborator Author

de-code commented May 31, 2019

How do we do that for other Single Page App projects? (the client could still be served by nginx - we could remove it from the peerscout image)

Maybe the xpub people know of a solution as they integrated New Relic as well. There's ids derived from the account in the generated HTML/JavaScript so I'd avoid building it into the image as it should be configuration. We may have to mount an HTML file or a fragment of it as a volume?

Yes, mounting the HTML fragements (that we already provide). It's just the small "script" (or similar) that bakes it into the correct place of the index.html (which currently is part of the webpack bundle process).

@diversemix how do you manage that in xpub?

@diversemix
Copy link

diversemix commented Jun 3, 2019

How do we do that for other Single Page App projects? (the client could still be served by nginx - we could remove it from the peerscout image)

Maybe the xpub people know of a solution as they integrated New Relic as well. There's ids derived from the account in the generated HTML/JavaScript so I'd avoid building it into the image as it should be configuration. We may have to mount an HTML file or a fragment of it as a volume?

Yes, mounting the HTML fragements (that we already provide). It's just the small "script" (or similar) that bakes it into the correct place of the index.html (which currently is part of the webpack bundle process).

@diversemix how do you manage that in xpub?

Because we are a SPA ... then its quite simple for us... https://github.com/elifesciences/elife-xpub/blob/cbb9bb9067d688090ea482cb634846dad4a61602/app.js#L1

The newrelic.js then pulls in the relevant stuff from the config:

  newrelic: {
    licenseKey: 'xxx',
    applicationID: '162979288',
  },

You could make the configuration then pull from the environment. We don't do this with the new relic config but do do it for other variables in config.

@de-code
Copy link
Collaborator Author

de-code commented Jun 3, 2019

Because we are a SPA ... then its quite simple for us... https://github.com/elifesciences/elife-xpub/blob/cbb9bb9067d688090ea482cb634846dad4a61602/app.js#L1

Okay, thank you. I am guessing that is hooking into the express server or similar that pubsweet-server is using, to modify the HTML sent by the server. I could do something similar to the Python server (or in express if we changed it to serve the client separately). But might prefer re-using the existing HTML fragments.

@de-code
Copy link
Collaborator Author

de-code commented Jun 12, 2019

Will implement the use of html fragments separately #387

@de-code de-code changed the title [wip] start server via docker start server via docker Jun 12, 2019
@@ -26,7 +26,7 @@ ARG install_dev
COPY requirements.dev.txt ./
RUN if [ "${install_dev}" = "y" ]; then pip install -r requirements.dev.txt; fi

COPY --from=client --chown=elife:elife /home/node/client/ ${PROJECT_FOLDER}/client/
COPY --from=client --chown=elife:elife /home/node/client/dist/ ${PROJECT_FOLDER}/client/dist/
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only need dist when running the server (no need to copy node_modules & co)

@@ -35,7 +35,7 @@ COPY --chown=elife:elife app-defaults.cfg ${PROJECT_FOLDER}/

USER root
RUN mkdir .data && chown www-data:www-data .data
RUN mkdir logs && chown www-data:www-data logs
RUN mkdir logs && chown www-data:www-data logs && chmod -R a+w logs
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be www-data and elife writing to the logs

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be just www-data if we run everything in the container? But fine to keep this backward compatibility if needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The image is (or would be) used to run the server as well as the "ETL" scripts (migrate schema, load data etc.). The latter is currently meant to be run via the elife user. But we could use www-data for both if you think that would be better.

@de-code
Copy link
Collaborator Author

de-code commented Jun 12, 2019

@giorgiosironi any objections to merging this PR?

dockerfile: Dockerfile
image: elifesciences/peerscout_init:${IMAGE_TAG}
volumes:
- config-aws:/home/elife/volume-config-aws
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some weird copying from one folder to the other to avoid ownership clashes?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem with that, but why needing a volume if the copying is executed every time the container starts anyway?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find that when the container is not run as root or the developer's user, then just mounting the credentials won't work because the permissions are meant to set up that only the user can read them. This makes the developers credentials available via the volume. (The init container is run as root, and therefore has permissions)

One alternative would maybe be to run the container using the developer's user (which seem to also be more complicated than it should). I experimented with a few approaches across the projects. Not sure which one is best. Any suggestions?

Copy link
Contributor

@giorgiosironi giorgiosironi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No particular objections

@de-code
Copy link
Collaborator Author

de-code commented Jun 20, 2019

Also updated the base image because the previous base Python image had LANG set to en_US.UTF-8 which didn't actually exist in the config. Only C.UTF-8 was present. Which is also what LANG is set to in the latest version of our Python base image as well as other Python base images.

@de-code de-code merged commit 94dcc14 into develop Jun 20, 2019
@de-code de-code deleted the docker-server-start branch June 20, 2019 15:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants