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

test: fix IPv6 checks on IBM i #46546

Merged
merged 2 commits into from
Feb 23, 2023

Conversation

abmusse
Copy link
Contributor

@abmusse abmusse commented Feb 7, 2023

This fix should help resolve some test failures:

That fail with:

AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
+ actual - expected

+ 'Unknown system error -42'
- 'EADDRNOTAVAIL'
    at Socket.<anonymous> (/home/IOJS/build/workspace/node-test-commit-ibmi/nodes/ibmi73-ppc64/test/parallel/test-net-autoselectfamilydefault.js:131:18)
    at Socket.<anonymous> (/home/IOJS/build/workspace/node-test-commit-ibmi/nodes/ibmi73-ppc64/test/common/index.js:448:15)
    at Socket.emit (node:events:518:28)
    at emitErrorNT (node:internal/streams/destroy:151:8)
    at emitErrorCloseNT (node:internal/streams/destroy:116:3)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: 'Unknown system error -42',
  expected: 'EADDRNOTAVAIL',
  operator: 'strictEqual'
}

Node.js v20.0.0-pre

The issues I discovered were 2 fold:

  1. The hasIPv6 function cannot determine IPv6 on IBM i. The loop back interface on IBM i is named *LOOPBACK.

  2. When IPv6 is disabled IBM i returns EUNATCH (errno 42). For now, I've added an errno check in test case for. I believe that libuv needs to get updated to handle EUNATCH so that the error code can be addressed as EUNATCH. Similar to include: add EOVERFLOW status code mapping libuv/libuv#3145

CC @nodejs/platform-ibmi
CC @richardlau

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Feb 7, 2023
test/parallel/test-net-autoselectfamily.js Outdated Show resolved Hide resolved
@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 8, 2023
@github-actions github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Feb 8, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Feb 8, 2023

Failed to start CI
- Validating Jenkins credentials
✖  Jenkins credentials invalid
https://github.com/nodejs/node/actions/runs/4127678728

@richardlau richardlau added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Feb 8, 2023
@richardlau richardlau removed the request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. label Feb 9, 2023
@richardlau richardlau added ibm i Issues and PRs related to the IBM i platform. request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Feb 20, 2023
@richardlau
Copy link
Member

@abmusse Can you rebase this to fix the conflict?

@abmusse
Copy link
Contributor Author

abmusse commented Feb 21, 2023

@abmusse Can you rebase this to fix the conflict?

@richardlau
Fixed up the conflict by rebasing and squashed all intermediate commits into one commit.

@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@richardlau
Copy link
Member

This fix should help resolve some test failures:

FWIW I ran an IBM i CI build for this:
https://ci.nodejs.org/job/node-test-commit-ibmi/nodes=ibmi73-ppc64/1072
when compared to today's daily IBM i CI:
https://ci.nodejs.org/job/node-test-commit-ibmi/nodes=ibmi73-ppc64/1070
parallel/test-net-autoselectfamily no longer fails 🎉. However parallel/test-net-autoselectfamilydefault is still failing -- presumably this PR should be modifying that test as well based on the PR description.

@abmusse
Copy link
Contributor Author

abmusse commented Feb 22, 2023

This fix should help resolve some test failures:

FWIW I ran an IBM i CI build for this: https://ci.nodejs.org/job/node-test-commit-ibmi/nodes=ibmi73-ppc64/1072 when compared to today's daily IBM i CI: https://ci.nodejs.org/job/node-test-commit-ibmi/nodes=ibmi73-ppc64/1070 parallel/test-net-autoselectfamily no longer fails tada. However parallel/test-net-autoselectfamilydefault is still failing -- presumably this PR should be modifying that test as well based on the PR description.

Good catch @richardlau I need to apply the same errno 42 check in that file as well!

        else if (common.isIBMi) {
          // IBMi returns EUNATCH (ERRNO 42) when IPv6 is disabled
          // keep this errno assertion until EUNATCH is recognized by libuv
          assert.strictEqual(error.errno, -42);
        }

Update added fix in

e8a7d20

@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 22, 2023

@richardlau richardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 23, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 23, 2023
@nodejs-github-bot nodejs-github-bot merged commit c2c61e0 into nodejs:main Feb 23, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in c2c61e0

@abmusse abmusse deleted the ibmi-ipv6-test-fix branch February 23, 2023 15:13
@juanarbol juanarbol added the backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. label Mar 1, 2023
@juanarbol
Copy link
Member

This is not landing cleanly on v18.x

@richardlau
Copy link
Member

@juanarbol This hasn't gone out in a current release yet so it's too early to be considered for v18.x.

targos pushed a commit that referenced this pull request Mar 13, 2023
PR-URL: #46546
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@abmusse abmusse mentioned this pull request May 17, 2023
mhdawson pushed a commit that referenced this pull request Aug 23, 2023
When IPv6 is disabled IBM i returns EUNATCH (errno 42)
instead of EADDRNOTAVAIL.

libuv 1.46.0 adds EUNATCH errno

We can now use error.code to refer to EUNATCH
in node versions that use libuv 1.46.0.

PR-URL: #48050
Refs: #48049
Refs: #46546
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
UlisesGascon pushed a commit that referenced this pull request Sep 10, 2023
When IPv6 is disabled IBM i returns EUNATCH (errno 42)
instead of EADDRNOTAVAIL.

libuv 1.46.0 adds EUNATCH errno

We can now use error.code to refer to EUNATCH
in node versions that use libuv 1.46.0.

PR-URL: #48050
Refs: #48049
Refs: #46546
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
When IPv6 is disabled IBM i returns EUNATCH (errno 42)
instead of EADDRNOTAVAIL.

libuv 1.46.0 adds EUNATCH errno

We can now use error.code to refer to EUNATCH
in node versions that use libuv 1.46.0.

PR-URL: nodejs#48050
Refs: nodejs#48049
Refs: nodejs#46546
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
targos pushed a commit that referenced this pull request Nov 10, 2023
PR-URL: #46546
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
PR-URL: nodejs/node#46546
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
PR-URL: nodejs/node#46546
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. ibm i Issues and PRs related to the IBM i platform. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants