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

Dockerize #1

Merged
merged 17 commits into from
Oct 3, 2017
Merged

Dockerize #1

merged 17 commits into from
Oct 3, 2017

Conversation

scjody
Copy link
Contributor

@scjody scjody commented Aug 15, 2017

@etpinard
Copy link
Contributor

Right. Generating an image will hang using plotly.js latest as the renderer assumes plotly/plotly.js#1939.

Originally, I was thinking of putting all the docker stuff in streambed. I believe this repo here will one day become open source and be just an npm module.

By design, the start server script in ths repo won't do all the things we want on prod. For example, it simply prints logs to the terminal as opposed to logging them to a file.

In brief, I'm thinking streambed/image_server/ will consume this module with its own start sever script with all the logging logic and environment variables unique to streambed. I can write this script in like ~1 hour tomorrow, if you want.

If that conflicts with anything on your side @scjody, I'm happy to discuss it.

@scjody
Copy link
Contributor Author

scjody commented Aug 16, 2017

There isn't really much Docker stuff. The Dockerfile is actually really trivial other than the stuff I did to base the image on Ubuntu 14.04 rather than the node-xvfb image.

We could put it in streambed but then it would be harder to keep it in sync with changes in this repo, it doesn't belong with anything else in that repo, and it would make deployment more complicated. For other projects (database connector and Dash On Premise) we have the Dockerfile in the project repo, and database connector is open source.

For logging, printing to STDOUT and STDERR is fine for GKE - it collects logs itself. Ideally:

  • Print one line per request to STDOUT summarizing the request and its status (similar to what nginx or apache does).
  • Print errors, debugging info, etc. to STDERR.

What else do we need to do that's specific to Plotly Cloud?

@etpinard
Copy link
Contributor

For logging, printing to STDOUT and STDERR is fine for GKE - it collects logs itself

Wow. That's a game changer.

What else do we need to do that's specific to Plotly Cloud?

Now that I believe image-exporter is set to be fully open source -- including PDF and EPS plotly graph exports (cc @jackparmer @bpostlethwaite ) -- this means there would be no need for private export components. So yeah, there's not much Plotly-Cloud-specific things to worry about. Adding the docker stuff to this repo is making more and more sense.

That said, we'll need to take special care of the mapbox access token. Looking at the current image_server code, nothing else comes to mind.

@bpostlethwaite bpostlethwaite mentioned this pull request Aug 16, 2017
@scjody
Copy link
Contributor Author

scjody commented Aug 16, 2017

That said, we'll need to take special care of the mapbox access token.

I think GKE can pass that in as part of the container's environment. I'll look into that.

@scjody scjody force-pushed the dockerize branch 4 times, most recently from 125ea28 to 3aedcb5 Compare August 22, 2017 23:28
@etpinard etpinard mentioned this pull request Aug 30, 2017
@scjody scjody force-pushed the dockerize branch 19 times, most recently from a703dcd to 81870aa Compare September 21, 2017 18:17
@scjody scjody force-pushed the dockerize branch 6 times, most recently from d10f902 to 64ea9d8 Compare September 29, 2017 17:25
@scjody
Copy link
Contributor Author

scjody commented Oct 3, 2017

Mapbox access token support is done and documented in 6472479

@scjody
Copy link
Contributor Author

scjody commented Oct 3, 2017

@bpostlethwaite Please review


fc-cache -v /usr/share/fonts/user

xvfb-run --server-args '-screen 0 640x480x24' ./bin/plotly-export-server.js $@
Copy link
Contributor

Choose a reason for hiding this comment

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

These are different screen dimensions than in streambed.

Is this on purpose @scjody ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually we currently run at 1280x1024x24 in production: https://github.com/plotly/streambed/blob/master/deployment/roles/streambed_repo/templates/xvfb_wrapper#L30

I don't think it makes a difference but I'm willing to change it to whatever you think is best.

Copy link
Contributor

@etpinard etpinard Oct 3, 2017

Choose a reason for hiding this comment

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

I was just curious. I have no idea if this makes a difference or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it does. @bpostlethwaite any ideas?

Copy link
Member

Choose a reason for hiding this comment

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

It might cap the maximum screen size but I think Plotly.js has no troubles rendering "off screen" anyway. We used the "off screen" trick for in-browser rendering since forever.

That said, not super sure either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just tested image export on stage and it will happily create (for example) a 3000x3000 image despite the current screen size. So I really don't think it matters.

gcloud beta container clusters create imageserver-stage --enable-autoscaling --min-nodes=1 --max-nodes=3 --num-nodes=1 --zone=us-central1-a --additional-zones=us-central1-b,us-central1-c --enable-autoupgrade --cluster-version=1.7.6-gke.1
# Note: "min", "num", and "max" nodes sets the number PER ZONE.
kubectl apply -f deployment/kube
kubectl get service imageserver # Will show the load balancer IP when it's ready
Copy link
Member

Choose a reason for hiding this comment

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

thanks!

@bpostlethwaite
Copy link
Member

Looks good - it's a lot to digest but the architecture seems sound. 💃

Side note: It's interesting that ansible yaml files are becoming dumb wrappers around local actions. Basically Kube and GCloud with Jinja2 templating and python data structures. I realize this is necessary so that these deployments fit into our current deployment framework but it portends a review at some point once the majority of services are deployed this way.

@scjody scjody merged commit 9775aed into master Oct 3, 2017
@scjody scjody deleted the dockerize branch October 3, 2017 21:31
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.

3 participants