Add support for Centos7/ppc64 #1763

sam-github · 2019-04-15T20:45:47Z

Add the repository for devtoolset-6.
Do not use the "ius" repository, it doesn't support ppc64, and was not
necessary.

sam-github · 2019-04-15T21:06:37Z

Hm, gcc-6 and gcc-4.9 aren't installed. For ubuntu, they appear to have been official packages (gcc-4.8, etc.). For 11.x, I assume it doesn't matter, we'll use devtoolset-6 for 11.x+, and only use the centos machines to build them.

sam-github · 2019-04-15T21:34:18Z

Hm, link failed on -latomic, so I patched v8, and node linked:

[centos@test-osuosl-centos7-ppc64--le-1 node]$ git diff
diff --git a/tools/v8_gypfiles/v8.gyp b/tools/v8_gypfiles/v8.gyp
index 72a8dfe..9835491 100644
--- a/tools/v8_gypfiles/v8.gyp
+++ b/tools/v8_gypfiles/v8.gyp
@@ -391,7 +391,7 @@
         }],
         # Platforms that don't have Compare-And-Swap support need to link atomic
         # library to implement atomic memory access
-        [ 'v8_current_cpu in ["mips", "mipsel", "mips64", "mips64el", "ppc", "ppc64",
+        [ 'v8_current_cpu in ["mips", "mipsel", "mips64", "mips64el", "ppc", "s390", "
             'link_settings': {
               'libraries': [ '-latomic', ],
             },

@nodejs/v8-update any ideas on this?

sam-github · 2019-04-15T21:39:50Z

Hm, went the other way, installed devtoolset-6-libatomic-devel

refack · 2019-04-15T21:48:17Z

Does this need changes in

build/jenkins/scripts/select-compiler.sh

Line 19 in cf02bff

if [ "$SELECT_ARCH" = "PPC64LE" ]; then

, doing something like

build/jenkins/scripts/select-compiler.sh

Line 57 in cf02bff

. /opt/rh/devtoolset-6/enable

?

sam-github · 2019-04-15T21:52:23Z

Yes... and no. Generally we land the ansiblization first, then the select-compiler.sh second in another PR. That said, I have the changes locally to select-compiler, and this machine isn't currently in use, and I'm very unfamiliar with the process of bringin online a set of machines that will (initially) only be used to build master.12.x..

@mhdawson the select-compiler update, want it in this PR, or another?

refack · 2019-04-15T22:05:03Z

I'm very unfamiliar with the process of bringin online a set of machines

Well... We don't do it very often... We'd also need to patch somewhere near

build/jenkins/scripts/VersionSelectorScript.groovy

Lines 51 to 53 in cf02bff

    
           // PPC BE ------------------------------------------------ 
        
           [ /^ppcbe-ubuntu/,                  anyType,     gte(8)  ],

...

IMO in this specific case the order or PRs is not very critical since the change will not go live until we actually add the new machines to the job matrix (VersionSelectorScript.groovy is an exclusion filter on top of that).

ansible/roles/baselayout/vars/main.yml

rvagg · 2019-04-16T05:59:03Z

ansible/roles/baselayout/tasks/partials/repo/centos7.yml

  yum:
    name: "https://centos{{ ansible_distribution_major_version }}.iuscommunity.org/ius-release.rpm"
    state: present

+- name: centos7 | install sclo7 devtoolset-6


I have #1765 which is for x64 centos7 and it's going to conflict with this, one of us probably needs to change, or we need to add arch exclusions. Mine is just:

- name: centos7 | install scl for devtoolset-6 when: "arch != 'arm64'" yum: name: centos-release-scl state: present

and I'm not sure about the arm64 exclusion there, we might be able to get rid of the sclo aarch64 bits further down this file but it'll need some testing (which I don't have time for right now!)

@sam-github might a simple yum install centos-release-scl work for ppc64 too?

I'm not so sure what these incantations do, but I replaced mine with your example above, and am trying it on a fresh host. I'll see if it works.

Unfortunately, no, I don't think that repo works for ppc64le:

TASK [baselayout : install packages] ************************************************** failed: [release-osuosl-centos7-ppc64_le-1] (item=ccache,gcc-c++,devtoolset-6,sudo,git,devtoolset-6-libatomic-devel,python3) => {"changed": false, "msg": "Failure talking to yum: Cannot find a valid baseurl for repo: centos-sclo-sclo/ppc64le", "package": "ccache,gcc-c++,devtoolset-6,sudo,git,devtoolset-6-libatomic-devel,python3"} failed: [release-osuosl-centos7-ppc64_le-1] (item=automake,bash,libtool) => {"changed": false, "msg": "Failure talking to yum: Cannot find a valid baseurl for repo: centos-sclo-sclo/ppc64le", "package": "automake,bash,libtool"}

I'm reverting back to what I did, but with when: "arch == 'ppc64'".

refack · 2019-04-16T14:30:41Z

Closes: #1705

sam-github · 2019-04-16T16:01:31Z

Added groovy exclusion, added more packages to centos7, still looking around to compare to Rod's changes, and for his comments.

jenkins/scripts/VersionSelectorScript.groovy

sam-github · 2019-04-16T16:45:34Z

@refack Sorry, I've had no success finding python 3.

sam-github · 2019-04-16T17:19:18Z

I've ansibled the 3 ppc64_le machines added in inventory.yml, several times (just doing it again now, against all 3 simultaneously, to make sure they are in same state).

What's next, do we land this, or add them as ci machines to make sure the config is sufficient?

sam-github · 2019-04-16T21:43:39Z

ansible/roles/jenkins-worker/vars/main.yml

@@ -91,6 +91,7 @@ needs_monit: [

 # some os'es needs different paths to java. add them here.
 java_path: {
+  'centos7': '/home/iojs/jdk8u172-b11/bin/java',


This java path is actually specific to centos-ppc64le, but I'm not sure if any other centos7 platforms use systemd.

ansible/roles/jenkins-worker/templates/systemd.service.j2

sam-github · 2019-04-16T21:47:49Z

I re-ansibled with the correct jenkins secrets (@mhdawson set the machines up in jenkins), and realized that there was no java on the machine. Please see last commit, it works, but I'm not sure if its the right way, or what other options there are.

jenkins/scripts/VersionSelectorScript.groovy

jenkins/scripts/select-compiler.sh

rvagg · 2019-04-17T00:29:51Z

so we're seeing the fruits of removing java-base as a dependency here again, can someone explain why it was removed, especially since it seems to break everything else--because everything needs java

sam-github · 2019-04-17T15:53:14Z

I'm not sure what java-base is, so I searched git history, and found https://github.com/nodejs/build/pull/1723/files

I'm slightly horrified to see I changed how the java-base role is reached, I don't recall doing that on purpose, or even understand what the changes are, I suspect they made it into my PR via rebasing, but I can't prove it.

Anyhow, I'm not sure if this is the java-base dependency you are talking about. If it is, the java-base role is (and was) executed for centos7-ppc64le, it just so happens that because of the when: clauses, no stanza to install java matched so the role ended up being a no-op.

mhdawson · 2019-04-17T22:27:49Z

jenkins/scripts/VersionSelectorScript.groovy

@@ -18,7 +18,7 @@ def buildExclusions = [
  [ /centos[67]-(arm)?(64|32)-gcc48/, anyType,     gte(10) ],
  [ /centos[67]-(arm)?(64|32)-gcc6/,  anyType,     lt(10)  ],
  [ /centos6-32-gcc6/,                releaseType, gte(10) ], // 32-bit linux for <10 only
-  [ /^centos7-64/,                    releaseType, lt(12)  ],
+  [ /^centos7-64/,                    anyType,     lt(12)  ],


Can you explain the need for this a bit more. Wondering if maybe the addition should be ppc specific

In specific I'm wondering why we would not have already had the exclude for x64 in test. Do we not have centos in the test builds, that seems odd to me.

#1763 (comment)

To start it should instead exclude running centos on versions lower than 12.x. The result should be builds on both ubuntu and centos for 12.x and above.

I am misunderstanding what you meant by that. What did you mean?

maybe that applies only to centos7-ppc64? that version rule is wrong, we want the centos7-64 exclusion to only exist for releaseType because that's how we say that centos6 should be used for making binaries prior to 12. anyType is going to stop it even testing.

My comment was because the tag is centos7-64 and I expected changes to only be affecting ppcle in this PR.

mhdawson · 2019-04-17T22:29:05Z

jenkins/scripts/select-compiler.sh

    *s390x* ) SELECT_ARCH=S390X ;;
    *aix* ) SELECT_ARCH=AIXPPC ;;
  esac
 fi

 # get node version
-if [ -z ${NODEJS_MAJOR_VERSION+x} ]; then
+if [ -z "$NODEJS_MAJOR_VERSION" ]; then


Wondering about this change.

The previous code wasn't working for me, this does. Possibly a shell difference? I am not at all sure why there was such a convoluted way of checking of the env var is set, this seems to work better:

core/build (centos7-ppc-support % u=) % if [ -z ${ZZZ+x} ]; then echo T; fi T core/build (centos7-ppc-support % u=) % if [ -z "$ZZZ" ]; then echo T; fi T core/build (centos7-ppc-support % u=) % env | grep ZZZ

Tomorrow I can revert it and post the error caused by the original line.

This change will match both an empty string and an unset environment variable. I think this change is OK because if it's empty then it's still useless and should be re-generated, but it's a bit weird that it's necessary. In what context is NODEJS_MAJOR_VERSION not set? Are there Jenkins jobs that aren't using the versioning plugin?

This change will match both an empty string and an unset environment variable. I think this change is OK because if it's empty then it's still useless and should be re-generated, but it's a bit weird that it's necessary. In what context is NODEJS_MAJOR_VERSION not set? Are there Jenkins jobs that aren't using the versioning plugin?

Yes. Anything that isn't building Node.js from source but still needs to do compilation (i.e. natve addons), e.g. citgm-smoker-nobuild. node-test-node-addon-api.

mhdawson · 2019-04-17T22:33:44Z

jenkins/scripts/select-compiler.sh

@@ -4,14 +4,14 @@ if [ "$DONTSELECT_COMPILER" != "DONT" ]; then
  NODE_NAME=${NODE_NAME:-$HOSTNAME}
  echo "Selecting compiler based on $NODE_NAME"
  case $NODE_NAME in
-    *ppc64_le* ) SELECT_ARCH=PPC64LE ;;
+    *ppc64*le* ) SELECT_ARCH=PPC64LE ;;


It this because the node names are inconsistent?

because of

build/ansible/roles/baselayout/tasks/main.yml

Line 16 in 6d12c28

safe_hostname: "{{ inventory_hostname | regex_replace('_', '--') }}"

, it means that HOSTNAME := NODE_NAME.replace(_, --), so select compiler's attempt to use the HOSTNAME as a default, which usually works, fails here.

The change should be OK, but again it's an oddity because NODE_NAME should come from Jenkins and the only situation where it's not set is when this is run independently, I guess if this is run as an independent test?

Yes, anyone attempting to trouble shoot build issues or bootstrap a ci machine needs to read select-compiler by hand, after reading it to figure out what they should set NODE_NAME to. I added HOSTNAME as the default. If we don't want that, I can back it out. Given that ansible sets the HOSTNAME to the NODE_NAME (modulo the - and _), I'm not sure why this script depends on ci env vars. If this change is out of scope, I'll back it out. Its more of a public service for future users.

Ok, understood now and makes sense. The result is that you can call select-compiler.sh on the host by hand and get the right behavior without having to export NODE_NAME.

sam-github · 2019-04-18T04:26:52Z

ansible/roles/java-base/tasks/main.yml

  when: os in "ubuntu1404" and arch == "ppc64"
  unarchive:
    src: /tmp/OpenJDK8_ppc64le_Linux_jdk8u172-b11.tar.gz
    remote_src: yes
    dest: /home/iojs
  tags: java

+- name: unarchive java centos7 ppc64


these two stanzas have the effect of putting java in /usr/bin/java, where the systemd jenkins unit file (and anyone else) would expect it to be.

sam-github · 2019-04-18T15:52:43Z

jenkins/scripts/VersionSelectorScript.groovy

@@ -19,6 +19,7 @@ def buildExclusions = [
  [ /centos[67]-(arm)?(64|32)-gcc6/,  anyType,     lt(10)  ],
  [ /centos6-32-gcc6/,                releaseType, gte(10) ], // 32-bit linux for <10 only
  [ /^centos7-64/,                    releaseType, lt(12)  ],
+  [ /^centos7-ppcle/,                 anyType,     lt(12)  ],


OK, the new machines are labelled centos7-ppcle, this excludes them when node lt 12.

mhdawson · 2019-04-18T21:25:06Z

So Java base was not removed it was moved in https://github.com/nodejs/build/pull/1723/files

ansible/roles/jenkins-worker/tasks/main.yml now has

- include_role:
    name: java-base

instead. That was necessary because java base installs java to a location only created earlier on in main.yml (at least for ppc) so it could not work properly on a new machine.

So it was moved (my fault as I added it to Sam's PR to get it working). It worked fine for both PPC and s390 but maybe there is something about it that does not work on other platforms? It may be because of the associated list of packages in build/ansible/roles/java-base/vars/main.yml.

@rvagg we can revert if its causing problems on other platforms. @sam-github can you see if you can figure out another solution which works for ppc, s390 and other platforms? It might be something like moving the package: {} definition but I really don't know.

mhdawson

LGTM, but lets hold off landing until 12.x goes out.

rvagg · 2019-04-18T23:24:44Z

I think the java-base thing should be sorted in the merged smartos18 PR that pulled its inclusion up before the service calls.

rvagg · 2019-04-18T23:30:06Z

ansible/roles/baselayout/vars/main.yml

@@ -43,7 +43,7 @@ packages: {
  centos7_x64: ['git2u','centos-release-scl',], # centos-release-scl is required to enable SCLo
                                               # but we do it manually in partials/repo/centos7.yml for arm64
  centos7: [
-    'ccache,gcc-c++,devtoolset-6,sudo',
+    'ccache,gcc-c++,devtoolset-6,sudo,git,devtoolset-6-libatomic-devel',


I've added git in #1765, if it gets merged first then you'll just have to resolve that conflict

The centos7 machines will be used instead.

- Add the repository for devtoolset-6. - Do not use the "ius" repository, it doesn't support ppc64le, and was not necessary.

mhdawson · 2019-04-24T17:06:21Z

Landing as I think this is ready to go based on the discussion.

refack added ci-change PSA of configuration changes ci-public ci-release platform:ppc labels Apr 15, 2019

refack reviewed Apr 15, 2019

View reviewed changes

ansible/roles/baselayout/vars/main.yml Outdated Show resolved Hide resolved

rvagg mentioned this pull request Apr 16, 2019

ansible: release-centos7 and enable scl for devtoolset-6 #1765

Merged

rvagg reviewed Apr 16, 2019

View reviewed changes

rvagg mentioned this pull request Apr 16, 2019

Node.js 12 infrastructure checklist #1768

Closed

25 tasks

refack reviewed Apr 16, 2019

View reviewed changes

jenkins/scripts/VersionSelectorScript.groovy Outdated Show resolved Hide resolved

sam-github commented Apr 16, 2019

View reviewed changes

ansible/roles/jenkins-worker/templates/systemd.service.j2 Outdated Show resolved Hide resolved

mhdawson reviewed Apr 16, 2019

View reviewed changes

jenkins/scripts/VersionSelectorScript.groovy Outdated Show resolved Hide resolved

mhdawson reviewed Apr 16, 2019

View reviewed changes

jenkins/scripts/select-compiler.sh Outdated Show resolved Hide resolved

sam-github commented Apr 16, 2019

View reviewed changes

jenkins/scripts/select-compiler.sh Show resolved Hide resolved

sam-github mentioned this pull request Apr 17, 2019

Add centos7 PPCle machines #1705

Closed

15 tasks

mhdawson reviewed Apr 17, 2019

View reviewed changes

sam-github commented Apr 18, 2019

View reviewed changes

mhdawson approved these changes Apr 18, 2019

View reviewed changes

rvagg reviewed Apr 18, 2019

View reviewed changes

sam-github added 3 commits April 22, 2019 12:25

Use devtoolset-6 on node > 11 for centos7-ppc64le

e440a2e

Exclude centos7-ppcle for node < 12.x

ab432f5

The centos7 machines will be used instead.

Setup centos7-ppc64le jenkins workers

c6e9433

- Add the repository for devtoolset-6. - Do not use the "ius" repository, it doesn't support ppc64le, and was not necessary.

mhdawson merged commit 0aaa408 into nodejs:master Apr 24, 2019

sam-github deleted the centos7-ppc-support branch April 24, 2019 18:33

richardlau mentioned this pull request Jun 14, 2019

build: link libatomic on mac and linux nodejs/node#28232

Closed

2 tasks

Add support for Centos7/ppc64 #1763

Add support for Centos7/ppc64 #1763

Conversation

sam-github commented Apr 15, 2019 • edited Loading

sam-github commented Apr 15, 2019

sam-github commented Apr 15, 2019

sam-github commented Apr 15, 2019

refack commented Apr 15, 2019

sam-github commented Apr 15, 2019

refack commented Apr 15, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

refack commented Apr 16, 2019

sam-github commented Apr 16, 2019

sam-github commented Apr 16, 2019

sam-github commented Apr 16, 2019

Choose a reason for hiding this comment

sam-github commented Apr 16, 2019

rvagg commented Apr 17, 2019

sam-github commented Apr 17, 2019

Choose a reason for hiding this comment

mhdawson Apr 17, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhdawson commented Apr 18, 2019

mhdawson left a comment

Choose a reason for hiding this comment

rvagg commented Apr 18, 2019

Choose a reason for hiding this comment

mhdawson commented Apr 24, 2019

sam-github commented Apr 15, 2019 •

edited

Loading

mhdawson Apr 17, 2019 •

edited

Loading