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

Use installed node version #7

Closed
wants to merge 1 commit into from
Closed

Use installed node version #7

wants to merge 1 commit into from

Conversation

DanHulton
Copy link
Contributor

Currently, when installing a new version of node on a system that has had an existing version installed already, nvm use is not issued, which leads to the old version continuing to be used, even though the desired version is installed.

@morgangraphics
Copy link
Owner

Dan,

Wanted to call your attention to this part of the documentation which I had updated a few weeks ago.

NVM is stateless in that if you have multiple versions of Node installed on a machine, AND you have not set a default e.g. nvm alias default v10.15.1 you may have to run nvm use <VERSION> as part of your script to run the Node version you want/expect.

This is what the nvm_commands: [] is specifically designed to handle.

@DanHulton
Copy link
Contributor Author

Sure, but I would argue that without the use there, you end up with unwelcome surprises. Consider the following use case:

  • Run 1: Install Node 8.x by setting nodejs_version: 8.3.2
  • Run 2: Decide to upgrade to 10.x by setting nodejs_version: 10.15.3

"nodejs_version" seems like it's intended to be the canonical version of node you want the server to be using, not just a version you want to be also installed. In fact, that's what happens the first time you run the role, it works as expected.

But if "nodejs_version" doesn't also set the default to that version, it's effectively useless for the most-common use case I can think of (upgrading to a new version using NVM). It's also useless for the second most-common use case I can think of (maintaining multiple versions). What, then is definition and use of this parameter?

Maybe it makes sense to have "nodejs_version" be the version that is installed and set as default, and offer a second optional parameter for "installed_nodejs_versions" for a list of versions you want installed but not necessarily set to default?

Let me know if any of that makes sense/is appealing and I can modify the PR, or if I'm completely off-base about how that param is intended to be used.

@morgangraphics
Copy link
Owner

morgangraphics commented Apr 9, 2019

The goal of this role was designed for installation of NVM and configuration while also leveraging the NVM interface to set up/run a project.

The end result of that goal was that I never thought of installing multiple versions of Node in one go. I view this role as a scalpel rather than a hammer. So iterating over a list of Node versions to install, while possible, is not a use case I would use. Regardless, the role allows for you to do this by including the role multiple times as shown in the More Complex example.

Also, I had never really thought about setting the version you install as default. If you look at the More Complex example for instance:

nvm_commands:
       - "nvm install {{ config.prod.client-1.nodejs }}"
       - "nvm alias default {{ config.prod.client-1.nodejs }}"
       - "nvm run default app.js"

this is where you could technically use nvm use {{ nodejs_version }} to configure your NVM environment e.g.

nvm_commands:
       - "nvm install {{ config.prod.client-1.nodejs }}"
       - "nvm use {{ config.prod.client-1.nodejs }}"
       - "nvm alias default {{ config.prod.client-1.nodejs }}"
       - "nvm run default app.js"

However, I believe (please correct me if I'm wrong), the default functionality for nvm install is analogous to nvm install X.XX && nvm use X.XX.

To sum it up, perhaps what you are suggesting is that there should be a new role variable that is something like: default: True so whichever version you explicitly state, is aliased as the default NodeJS version to run. All this would do is append/concatenate the nvm alias default {{ nodejs_version }} command to the {{ nvm_commands }} list making the version explicitly declared as the default, mitigating the need for a separate one off "nvm use {{ nodejs_version }} task. (It'd just make it more generic using already existing stuff) e.g.

- hosts: host1
  roles:
    - role: ansible-role-nvm
      nodejs_version: "10.15.1"
      default: True

- hosts: host1
  roles:
    - role: ansible-role-nvm
      nodejs_version: "8.15.0"

The added benefit is that if there are other versions on the machine that you want to leverage, you could then take advantage of the nvm_commands: [] role variable itself. This brings me to the other point of having to update the documentation to explain the nvm_commands: [] variable in further detail.

@DanHulton
Copy link
Contributor Author

Hm, so I was running under the impression that nvm install didn't also use the installed version, but it seems I'm incorrect. Though it's odd that on all our servers when we ran this role, it did install node 10, but didn't switch to it. Potentially I have a local setting somewhere on NVM that doesn't auto-switch to the new version?

At any rate, I do like your suggestion about adding a default: True role variable, since you'd probably want to have the newly-installed version set to default without needing to necessarily drop to nvm_commands and do the whole thing yourself. NVM doesn't change default when you install a new version, so this would make it much easier.

@morgangraphics
Copy link
Owner

morgangraphics commented Apr 9, 2019

While I don't know anything about your infrastructure, if the OS includes NodeJS as part of their base install, or Node was added via their Package Management feature, that will always be the default version of Node. Because at this level, Node is part of the OS infrastructure.

NVM can short circuit this functionality because of the addition to the .bashrc file which loads and overwrites/supplements Node paths to the environment when the user logs in (meaning it's user specific).

export NVM_DIR="$HOME/.nvm"
          [ -s "$NVM_DIR/nvm.sh" ] && . "$NVM_DIR/nvm.sh"

Because NVM is inherently stateless, coming back as the user who installed NVM in a later playbook or even a later play, means you sort of start over right? It's the reason I pointed out NVM is stateless in that if you have multiple versions of Node installed on a machine, AND you have not set a default e.g. nvm alias default v10.15.1 you may have to run nvm use <VERSION> as part of your script to run the Node version you want/expect.

In any case, if you don't explicitly set nvm use X.XX you will get the base Node install as you've seen/experienced and not the one you just installed. Which may be why it runs but not as expected.

Having just reread the NMV documentation it does state: The first version installed becomes the default. New shells will start with the default version of node (e.g., nvm alias default). Which seems to explain the issues you are seeing.

the nvm_commands: [] is really meant to leverage Node stuff without actually managing node stuff directly. Let's say you have a App and a Service running on the same machine

nvm_commands:
       - "nvm install 10.15.1"
       - "nvm alias default 10.15.1" # <= Changes the default NVM version
       - "nvm run default ./app/app.js" # (optional) Makes sure it's invoked when using it
       - "nvm install 8.15.0
       - "nvm alias service-default 8.15.0"
       - "nvm run service-default ./api/api.js" # <= (Required) Makes sure it's invoked when using it

@morgangraphics
Copy link
Owner

I've added the issues addressing what has been discussed here #8 and #11. I'll have something up by the end of the week for you.

@DanHulton
Copy link
Contributor Author

That absolutely explains what I'm seeing, and I definitely should have understood it sooner, I feel. It makes total sense.

There's no rush to update that package, as I have a workaround in my ansible script in place, but I do appreciate the conversation and the update.

@DanHulton DanHulton closed this Apr 10, 2019
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.

None yet

2 participants