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

CentOS 5 #898

Closed
Trott opened this issue Sep 30, 2017 · 8 comments
Closed

CentOS 5 #898

Trott opened this issue Sep 30, 2017 · 8 comments

Comments

@Trott
Copy link
Member

Trott commented Sep 30, 2017

Hi, @nodejs/build friends.

Given all of that, at what point do we remove it from Ci? Is this sort of thing documented anywhere? Is the Build WG the right collection of people to decide?

(Apologies in advance if this has all ben discussed already, which I think it has.)

@rvagg
Copy link
Member

rvagg commented Sep 30, 2017

We are still building 4 and 6 binaries with CentOS 5 and we can't change that so we should continue to support it there, it's unlikely to cause too much trouble with the small incremental changes going in to 4 and 6, especially when 6 heads into Maintenance. CentOS6 is used for binaries of 8+.
Another problem we have is that we can't even deploy new CentOS5 machines so we're stuck with the ones we have for the life of 6. If we were to need to replace them for whatever reason then we'd have to hack together some workaround like a Docker container or a VM in a VM or some other hackery.
I'm +1 on removing it from the 8+ test mix though since we don't rely on it, leaving it for <8 test runs. Any objections to doing this since it's causing some grief?

@Trott
Copy link
Member Author

Trott commented Sep 30, 2017

@rvagg If I'm understanding you correctly, we can't remove CentOS 5 machines completely until Node.js 6.x is unsupported which won't be until April 2019. Is that right? Yikes! But, yeah, I get it.

@rvagg
Copy link
Member

rvagg commented Sep 30, 2017

yerp, that's correct, kind of tied ourselves to a bit of a donkey with that one but it's a bit hard to change now

@gibfahn
Copy link
Member

gibfahn commented Sep 30, 2017

I'm +1 on removing it from the 8+ test mix though since we don't rely on it, leaving it for <8 test runs. Any objections to doing this since it's causing some grief?

We should definitely do this.

@refack
Copy link
Contributor

refack commented Sep 30, 2017

Seems like if push comes to shove we can at least provision a new CentOS5 machine on AWS (either a comunity AMI or install our own)

@Trott
Copy link
Member Author

Trott commented Oct 1, 2017

Another CentOS 5-only flaky (and thus a probable argument for removing CentOS 5 from CI): nodejs/node#14982

@gibfahn
Copy link
Member

gibfahn commented Oct 7, 2017

Okay, so I've replaced this code in https://ci.nodejs.org/job/node-test-commit-linux :

# clang is only supported in Node versions 7 and lower
MAJOR_VERSION=`cat src/node_version.h |grep "#define NODE_MAJOR_VERSION" | awk '{ print $3}'`
RUN_TESTS="RUN"
echo $SMARTOS_VERSION
if [[ "$nodes" =~ ubuntu1204 && ${MAJOR_VERSION} -gt 7 ]]; then
  RUN_TESTS=DONT_RUN
  SKIP_MESSAGE="ubuntu1204 is not supported for version 8 and above, skipping"
elif [[ "$nodes" = fedora22 && ${MAJOR_VERSION} -gt 7 ]]; then
  RUN_TESTS=DONT_RUN
  SKIP_MESSAGE="fedora22 is not supported for version 8 and above, skipping"
fi
# clang is only supported in Node versions 7 and lower
MAJOR_VERSION=`cat src/node_version.h |grep "#define NODE_MAJOR_VERSION" | awk '{ print $3}'`
RUN_TESTS="RUN"
echo $SMARTOS_VERSION
if [[ ${MAJOR_VERSION} -gt 7 && "$nodes" =~ 'ubuntu1204|fedora22|centos5' ]]; then
  RUN_TESTS=DONT_RUN
  SKIP_MESSAGE="$nodes is not supported for version 8 and above, skipping"
fi

Please let me know if this is wrong!

@refack
Copy link
Contributor

refack commented Oct 7, 2017

"if" sample:
https://ci.nodejs.org/job/node-test-commit-linux/12908/nodes=centos5-32/console

centos5-32 is not supported for version 8 and above, skipping

"else" sample:
https://ci.nodejs.org/job/node-test-commit-linux/12910/nodes=centos5-32/parameters/

python ./configure 
creating ./icu_config.gypi
* Using ICU in deps/icu-small
...

SGTM

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

No branches or pull requests

4 participants