OPS-1850: use nodeenv for edxapp by maxrothman · Pull Request #3444 · openedx-unsupported/configuration

maxrothman · 2016-11-01T21:40:24Z

@edx/devops @bjacobel

bjacobel · 2016-11-02T13:28:05Z

@maxrothman How should I go about testing this?

maxrothman · 2016-11-02T13:50:28Z

I think building a sandbox off of this branch would work.

bjacobel · 2016-11-02T14:07:15Z

ok, nodeenv.sandbox.edx.org is launching

maxrothman · 2016-11-02T14:24:06Z

@bjacobel it's probably going to fail because I don't think nodeenv is in edxapp's requirements

EDIT: verified. Apps that use nodeenv have it in their requirements.txt, edx-platform does not. I'll make a quick PR.

maxrothman · 2016-11-03T15:04:37Z

@bjacobel have you had a chance to test yet?

bjacobel · 2016-11-03T16:09:00Z

@maxrothman I haven't, I didn't check the first sandbox I built after you said it would probably fail. Now that edx/edx-platform#13890 is merged I will build a box with configuration_version as origin/max/edxapp-nodeenv and see how it goes.

bjacobel · 2016-11-03T17:18:28Z

@maxrothman Got an error building the sandbox: http://jenkins.edx.org:8080/view/Ansible/job/ansible-provision/9885/

Could be I just configured something wrong, I've never launched one with a non-master configuration version before. You probably know much more about this than me, you want to just ping me when there's a box I can SSH into and poke around?

maxrothman · 2016-11-03T18:23:27Z

It's an issue with my code. Hang on, I'll fix it.

EDIT: @bjacobel fixed

maxrothman · 2016-11-04T14:01:25Z

@bjacobel I built this sandbox from scratch. Though the build failed, it got past the edxapp section so you should be able to use it for testing.

bjacobel · 2016-11-04T14:25:28Z

I SSHed in and did some poking around, everything seemed ok up until the point I started trying to run things with node modules that (I think?) should be installed on sandboxes.

 wheatley > ~ > code > edx-pattern-library > master > $ > ssh nodeenv.sandbox.edx.org
Welcome to Ubuntu 12.04.5 LTS (GNU/Linux 3.2.0-101-virtual x86_64)


*******************************************************************
*     ___                         _ __  __                        *
*    / _ \ _ __ ___ _ _    ___ __| |\ \/ / (R)                    *
*   | |_| | '_ \ -_) ' \  / -_) _` | >  <                         *
*    \___/| .__/___|_|_|  \___\__,_|/_/\_\                        *
*         |_|                                                     *
*                                                                 *
* This system is for the use of authorized users only.  Usage of  *
* this system may be monitored and recorded by system personnel.  *
*                                                                 *
* Anyone using this system expressly consents to such monitoring  *
* and is advised that if such monitoring reveals possible         *
* evidence of criminal activity, system personnel may provide the *
* evidence from such monitoring to law enforcement officials.     *
*                                                                 *
* Need help? https://open.edx.org/getting-help                    *
*                                                                 *
*******************************************************************
This system will be terminated on Nov 10, 2016
Last login: Fri Nov  4 14:15:39 2016 from dhcp-18-189-127-242.dyn.mit.edu
[nodeenv] bjacobel@nodeenv i-12022c8b:~$ which node
/usr/bin/node
[nodeenv] bjacobel@nodeenv i-12022c8b:~$ node --version
v0.10.37
[nodeenv] bjacobel@nodeenv i-12022c8b:~$ which npm
/usr/bin/npm
[nodeenv] bjacobel@nodeenv i-12022c8b:~$ npm version
{ http_parser: '1.0',
  node: '0.10.37',
  v8: '3.14.5.9',
  ares: '1.9.0-DEV',
  uv: '0.10.36',
  zlib: '1.2.8',
  modules: '11',
  openssl: '1.0.1l',
  npm: '1.4.28' }
[nodeenv] bjacobel@nodeenv i-12022c8b:~$ npm bin
/home/bjacobel/node_modules/.bin
[nodeenv] bjacobel@nodeenv i-12022c8b:~$ sudo su edxapp -s /bin/bash
edxapp@nodeenv:/home/bjacobel$ source ~/edxapp_env
edxapp@nodeenv:/home/bjacobel$ cd /edx/app/edxapp/edx-platform
edxapp@nodeenv:~/edx-platform$ which node
/edx/app/edxapp/nodeenvs/edxapp/bin/node
edxapp@nodeenv:~/edx-platform$ node --version
v0.10.37
edxapp@nodeenv:~/edx-platform$ which npm
/edx/app/edxapp/nodeenvs/edxapp/bin/npm
edxapp@nodeenv:~/edx-platform$ npm --version
1.4.28

(everything looked ok up to this point)

edxapp@nodeenv:~/edx-platform$ npm bin
/edx/app/edxapp/edx-platform/node_modules/.bin
edxapp@nodeenv:~/edx-platform$ ls -al $(npm bin)
ls: cannot access /edx/app/edxapp/edx-platform/node_modules/.bin: No such file or directory
edxapp@nodeenv:~/edx-platform$ $(npm bin)/eslint
bash: /edx/app/edxapp/edx-platform/node_modules/.bin/eslint: No such file or directory
edxapp@nodeenv:~/edx-platform$ ls -al /edx/app/edxapp/edx-platform/node_modules
ls: cannot access /edx/app/edxapp/edx-platform/node_modules: No such file or directory
edxapp@nodeenv:~/edx-platform$

Can't find any node modules in this nodeenv. Or maybe I'm not actually in the nodeenv? It's confusing because I named the sandbox nodeenv. But none of the node modules I would expect to be installed are in /edx/app/edxapp/nodeenvs/edxapp/lib either.

maxrothman · 2016-11-04T14:31:31Z

@bjacobel let's connect later this morning and figure it out.

bjacobel · 2016-11-04T18:40:14Z

👍 maxrothman.sandbox.edx.org is looking good with these changes

maxrothman · 2016-11-04T21:06:22Z

Tests are running in https://github.com/edx/configuration/pull/3449 because this requires a change on master in edx-platform but the tests run on release.

maxrothman · 2016-11-04T21:56:01Z

Tests have passed in #3449

e0d · 2016-11-07T19:06:41Z

I'm 👍 for this change.

@benpatterson is there anything you'd like to see before merging this?

benpatterson · 2016-11-07T19:11:03Z

I'd look for some solution in the jenkins_worker role; that's currently using the chrislea PPA and that's how we'll be executing tests....

Where did you see that in the overall plan? Do we move one without the other?

e0d · 2016-11-07T19:37:17Z

@benpatterson Hmm, I'm a little surprised the worker is using different code -- I hadn't realized that when we discussed last week. I think they should be the same, but that the key thing is that we are using the same node and npm versions. The chrislea PPA is not available for xenial, so I think moving the nodeenv is the right thing to do. Who can we talk to about your usecases for node/npm?

benpatterson · 2016-11-07T19:46:11Z

@e0d Oh, I am totally on board with moving to nodeenv. I was moreso pointing out this. I think the reason that's there today might be legacy...when it was originally added, we did not have the edxapp_common role. So arguably it's a matter of cleaning up that side of things.

What I was also inferring was that using nodeenv has not been built into edxapp CI yet. Currently it's using the node/npm versions it finds on the machine it's running on (supplied by the jenkins_worker role). The questions at this juncture would revolve around how we'd want to roll out changes here. If you merge this and deploy using this, then there is the risk of different node environments. If, OTOH, we build this onto edx-platform first, then there is the risk of waiting for that to land (might be a couple of weeks? if we wait for the Tools team).

Happy to take this offline...it's a somewhat strategic/logistical set of questions. But I think we want the same thing here ultimately: both the test and prod infrastructures to be as similar as possible.

e0d · 2016-11-07T20:05:49Z

I'd really like to take the steps to ensure that we're testing the same we we deploy. Doesn't feel like this would be a huge effort, but I'd like to talk through the repercussions.

@nedbat FYI

benpatterson · 2016-11-07T23:27:46Z


+- name: create nodeenv
+  shell: >
+    {{ edxapp_venv_dir }}/bin/nodeenv {{ edxapp_nodeenv_dir }} --node={{ edxapp_node_version }} --prebuilt


If you use --force then this can be idempotent

The creates arg below means that it won't attempt to create it when the directory is present. However, this won't replace a nodeenv with the wrong node version. Are you saying we should ditch creates and only use --force? That's not how we're doing it elsewhere.

I think the right approach might be to make that change in all the places that create nodeenvs in a separate PR.

Ah I missed the creates arg. No, you are right...that's how idempotency is achieved. I was more concerned with running the same script twice than I was changing the node version. Perhaps you need to register the node version and ensure that's what was created?

The more I think about it, the more I think creates actually does the wrong thing. I think --force is safer, but we should do it in a separate PR in all the places that make nodeenvs.

@bjacobel

This change: * gives us the flexibility of managing node from pull-request to pull-request. It'll be a huge lift to folks that want to test out node upgrades like @bjacobel or @andy-armstrong * Sets us up for a port to Xenial, which will be accomplished in part by using nodeenv for managing the node environment. See a companion PR on the configuration repo for edxapp: edx/configuration#3444 This pull request does not: * Attempt to solve any stability issues in builds related to node. * Make node installs any faster * Create any caching infrastructure to speed up node installs. They will be approximately as fast as they were before. Downloading the node bits build-to-build adds about 3 seconds, which is worth it for the added flexibility.

@bjacobel

This change: * gives us the flexibility of managing node from pull-request to pull-request. It'll be a huge lift to folks that want to test out node upgrades like @bjacobel or @andy-armstrong * Sets us up for a port to Xenial, which will be accomplished in part by using nodeenv for managing the node environment. See a companion PR on the configuration repo for edxapp: edx/configuration#3444 This pull request does not: * Attempt to solve any stability issues in builds related to node. * Make node installs any faster * Create any caching infrastructure to speed up node installs. They will be approximately as fast as they were before. Downloading the node bits build-to-build adds about 3 seconds, which is worth it for the added flexibility.

benpatterson · 2016-11-11T13:50:13Z

Closing the loop on this, I am 👍

benpatterson · 2016-11-17T15:02:51Z

🎉

maxrothman mentioned this pull request Nov 2, 2016

Switch to using nodeenv openedx/openedx-platform#13890

Merged

maxrothman force-pushed the max/edxapp-nodeenv branch from 3ad270e to 7b6418b Compare November 3, 2016 16:05

OPS-1850: use nodeenv for edxapp

7ca232d

maxrothman force-pushed the max/edxapp-nodeenv branch from 7b6418b to 7ca232d Compare November 3, 2016 18:26

maxrothman force-pushed the max/edxapp-nodeenv branch from da744a3 to 7ca232d Compare November 4, 2016 20:43

benpatterson reviewed Nov 7, 2016

View reviewed changes

benpatterson mentioned this pull request Nov 8, 2016

Use nodeenv to manage node version in Jenkins builds openedx/openedx-platform#13911

Merged

feanil mentioned this pull request Nov 10, 2016

Feanil/retire xenial hosts #3461

Merged

1 task

maxrothman merged commit 1a75f79 into master Nov 17, 2016

maxrothman deleted the max/edxapp-nodeenv branch November 17, 2016 15:02

Uh oh!

Conversation

maxrothman commented Nov 1, 2016

Uh oh!

bjacobel commented Nov 2, 2016

Uh oh!

maxrothman commented Nov 2, 2016

Uh oh!

bjacobel commented Nov 2, 2016

Uh oh!

maxrothman commented Nov 2, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maxrothman commented Nov 3, 2016

Uh oh!

bjacobel commented Nov 3, 2016

Uh oh!

bjacobel commented Nov 3, 2016

Uh oh!

maxrothman commented Nov 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maxrothman commented Nov 4, 2016

Uh oh!

bjacobel commented Nov 4, 2016

Uh oh!

maxrothman commented Nov 4, 2016

Uh oh!

bjacobel commented Nov 4, 2016

Uh oh!

maxrothman commented Nov 4, 2016

Uh oh!

maxrothman commented Nov 4, 2016

Uh oh!

e0d commented Nov 7, 2016

Uh oh!

benpatterson commented Nov 7, 2016

Uh oh!

e0d commented Nov 7, 2016

Uh oh!

benpatterson commented Nov 7, 2016

Uh oh!

e0d commented Nov 7, 2016

Uh oh!

benpatterson Nov 7, 2016

Choose a reason for hiding this comment

Uh oh!

maxrothman Nov 8, 2016

Choose a reason for hiding this comment

Uh oh!

benpatterson Nov 8, 2016

Choose a reason for hiding this comment

Uh oh!

maxrothman Nov 8, 2016

Choose a reason for hiding this comment

Uh oh!

benpatterson Nov 11, 2016

Choose a reason for hiding this comment

Uh oh!

benpatterson commented Nov 11, 2016

Uh oh!

benpatterson commented Nov 17, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

maxrothman commented Nov 2, 2016 •

edited

Loading

maxrothman commented Nov 3, 2016 •

edited

Loading