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

tmp dir needed on ubuntu 1604 and fedora23 #873

Closed
MylesBorins opened this issue Sep 11, 2017 · 25 comments
Closed

tmp dir needed on ubuntu 1604 and fedora23 #873

MylesBorins opened this issue Sep 11, 2017 · 25 comments
Assignees
Labels

Comments

@MylesBorins
Copy link
Contributor

Still seeing errors like

       undefinedadded 958 packages in 35.901s
 > [email protected] test /home/iojs/build/workspace/citgm-smoker/nodes/fedora23/citgm_tmp/a63c932d-40c7-4f51-861d-e335a9ca3092/readable-stream
 > tap test/parallel/*.js test/ours/*.js && node test/verify-dependencies.js
 not ok test/parallel/test-stream2-base64-single-char-read-end.js  0/1
     Command: "/home/iojs/build/workspace/citgm-smoker/nodes/fedora23/smoker/bin/node test-stream2-base64-single-char-read-end.js"
     TAP version 13
     not ok 1 test/parallel/test-stream2-base64-single-char-read-end.js
       ---
         exit:    1
         stderr:  |
           fs.js:1658
                 binding.lstat(baseLong);
                         ^
           
           Error: ENOENT: no such file or directory, lstat '/home/iojs/tmp'
               at Object.realpathSync (fs.js:1658:15)
               at Object.<anonymous> (/home/iojs/build/workspace/citgm-smoker/nodes/fedora23/citgm_tmp/a63c932d-40c7-4f51-861d-e335a9ca3092/readable-stream/test/common.js:69:47)
               at Module._compile (module.js:624:30)
               at Object.Module._extensions..js (module.js:635:10)
               at Module.load (module.js:545:32)
               at tryModuleLoad (module.js:508:12)
               at Function.Module._load (module.js:500:3)
               at Module.require (module.js:568:17)
               at require (internal/module.js:11:18)
               at Object.<anonymous> (/home/iojs/build/workspace/citgm-smoker/nodes/fedora23/citgm_tmp/a63c932d-40c7-4f51-861d-e335a9ca3092/readable-stream/test/parallel/test-stream2-base64-single-char-read-end.js:25:1)
         command: "/home/iojs/build/workspace/citgm-smoker/nodes/fedora23/smoker/bin/node test-stream2-base64-single-char-read-end.js"

Please halp

@refack refack self-assigned this Sep 11, 2017
@gibfahn
Copy link
Member

gibfahn commented Sep 11, 2017

Isn't this just because #785 still hasn't landed?

@gibfahn
Copy link
Member

gibfahn commented Sep 11, 2017

i.e. dup of #833 ?

@gibfahn
Copy link
Member

gibfahn commented Sep 11, 2017

So that's landed now, I think the ansible scripts would need to be rerun on the machines.

@MylesBorins
Copy link
Contributor Author

ping on this for ubuntu. Have the ansible scripts not been run yet? I'm still seeing failures in CITGM

@MylesBorins
Copy link
Contributor Author

:(

@MylesBorins
Copy link
Contributor Author

Hey all.. this is still not fixed, and has not been fixed for months

I'm getting really frustrated as this is breaking our ability to have green CI runs with CITGM. Can someone please prioritize this

/cc @nodejs/tsc

@rvagg
Copy link
Member

rvagg commented Oct 10, 2017

What's the purpose of the tsc-agenda label here?

@rvagg
Copy link
Member

rvagg commented Oct 10, 2017

You have nodejs_build_test access don't you @MylesBorins, your ability to execute on this is pretty much the same as ours. Someone's either going to have to manually log in to each of these machines to make this directory or run parallel-ssh or Ansible across them all. The blocker here is having a person with the time to work through all of the hosts but everyone with nodejs_build_test is equally able to do this.

@refack
Copy link
Contributor

refack commented Oct 10, 2017

I'll pick this up.

There might be a "take home" here, in that maybe whomever lands an ansible change should run the scripts. Alternatively we could have point man for parts of the inventory.
Since this seems like a "bystander effect" issue otherwise

@gibfahn
Copy link
Member

gibfahn commented Oct 10, 2017

Alternatively the CitGM job could just do a mkdir -p "$NODE_TEST_DIR" in the config.

@rvagg
Copy link
Member

rvagg commented Oct 10, 2017

Alternatively the CitGM job could just do a mkdir -p "$NODE_TEST_DIR" in the config

yes! let's just do that eh?

@refack
Copy link
Contributor

refack commented Oct 10, 2017

Alternatively the CitGM job could just do a mkdir -p "$NODE_TEST_DIR" in the config

yes! let's just do that eh?

👍
running an ansible playbook just for syncing each of the needed machine still needs some yak shaving...

@MylesBorins
Copy link
Contributor Author

The reason to TSC-Agenda item was added was that this is something that has been brought up for months. It was last brought up in #833 which was claimed that a fix was to be landed in #785. An early request #658 was also closed, an issue opened in March.

To be honest I have never been on boarded to using our ansible scripts and I don't feel comfortable doing a mass update of our infrastructure because of that. This did not seem like a laborious request for help, and is blocking our ability to test Readable-Stream, which is a foundation managed module.

I think that we should be having a conversation both at the WG + TSC level about why a request like this has remained unresolved for over 6 months.

@rvagg
Copy link
Member

rvagg commented Oct 11, 2017

Anyone know if $NODE_TEST_DIR is actually what's used here? I'm not seeing any reference to it in the citgm-smoker job so it might not be what we need. Perhaps it'd be useful to figure out how ~/node-tmp is referenced here by the tools that use it—and what tools actually use it! Then we can work backward from there.

@refack
Copy link
Contributor

refack commented Oct 11, 2017

@trevnorris
Copy link

Can that line be replaced with os.tmpdir()?

@rvagg
Copy link
Member

rvagg commented Oct 11, 2017

Interesting, but I still don't see where NODE_TEST_DIR is set! I don't believe this is something we initialise in Jenkins.

@mcollina
Copy link
Member

It comes from readable-stream test code - https://github.com/nodejs/readable-stream/blob/56d9356637b5d20aba8e8e26faa602222f8500ab/test/common.js#L69

Those things are pulled from node core.

At some point we discussed changing that line or fixing CI, and the general sentiment was that fixing CI was the best path forward. I can dig up the issue if you'd like. However I prefer changing as little as possible in our tests.

@rvagg
Copy link
Member

rvagg commented Oct 11, 2017

OK, so there's a bunch of this kind of things in our node-test-commit-XXX configs:

NODE_TEST_DIR=${HOME}/node-tmp FLAKY_TESTS=$FLAKY_TESTS_MODE make run-ci -j $(getconf _NPROCESSORS_ONLN)

I don't know the history of those (wasn't me so I have no clue why they were set). But there's nothing in the citgm jobs for it and there's no mkdir for it in any of the jobs either. So I guess at some point we (a) had a requirement for NODE_TEST_DIR imposed on the infra and (b) decided that it's best done at infra setup time rather than at test time like we do with ~/node-icu/ and a smattering of others. The thing I still don't understand is why citgm jobs would be looking for ~/node-tmp/ if we're not setting NODE_TEST_DIR for them.

Personally I'd like to keep our specific hardware config to a minimum as it keep on being a pain, and if that means pushing more into Jenkins jobs then so be it (for the record, that sucks too cause the config isn't public and only a few of us can even see it! But it's one thing that GitHub-hosted pipeline jobs could solve for us).

@rvagg
Copy link
Member

rvagg commented Oct 11, 2017

Just had a brain-wave before I drop off to bed. We have this check-java-version job that we've been using to get ourselves up to Java 8 on all our Jenkins machines and it has all our non-Windows machines wired up to it, so I just stuck mkdir -p ${HOME}/node-tmp in the script for it and ran it! So @MylesBorins & @mcollina I think you should be able to try out your jobs now and 🤞.

@refack
Copy link
Contributor

refack commented Oct 11, 2017

Might be one caveat:

export NODE_TEST_DIR=${HOME}/tmp
sets the deafult NODE_TEST_DIR
So any code in the tested modules that looks up NODE_TEST_DIR will take the default. So we should either set it in the job, or remove it from openrc.initd

@gibfahn
Copy link
Member

gibfahn commented Oct 11, 2017

We have this check-java-version job that we've been using to get ourselves up to Java 8 on all our Jenkins machines and it has all our non-Windows machines wired up to it,

It runs on our Windows machines too.

@gibfahn
Copy link
Member

gibfahn commented Oct 11, 2017

To be honest I have never been on boarded to using our ansible scripts and I don't feel comfortable doing a mass update of our infrastructure because of that. This did not seem like a laborious request for help, and is blocking our ability to test Readable-Stream, which is a foundation managed module.

@MylesBorins this seems like the issue then. You're one of the senior members of the Build WG, and you're not comfortable updating our machines. That's not a judgement, I'm not comfortable doing it either, and I'm not sure anyone else is.

@rvagg
Copy link
Member

rvagg commented Nov 1, 2017

From my understanding from the discussion over the last couple of meetings, this is resolved in the specific but the general concern about our confidence in our Ansible scripts remains so I've opened that up as a separate issue @ #959

@rvagg rvagg closed this as completed Nov 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants