X Tutup
The Wayback Machine - https://web.archive.org/web/20220323041931/https://github.com/nodejs/node/pull/36884
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

Revert "test: mark test-cluster-bind-privileged-port flaky on arm" #36884

Closed
wants to merge 1 commit into from

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Jan 12, 2021

This reverts commit a45a404.

Solved by marking ports <1024 as privileged on Docker containers.

Ref: #36850
Ref: #36847
Ref: nodejs/build#2521

/cc @nodejs/build

This reverts commit a45a404.

Solved by marking ports <1024 as privileged on Docker containers.

Ref: nodejs#36850
Ref: nodejs#36847
Ref: nodejs/build#2521
@rvagg rvagg requested a review from jasnell Jan 12, 2021
@nodejs-github-bot nodejs-github-bot added the test label Jan 12, 2021
@aduh95 aduh95 added author ready request-ci labels Jan 12, 2021
@github-actions github-actions bot removed the request-ci label Jan 12, 2021
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jan 12, 2021

lpinca
lpinca approved these changes Jan 12, 2021
Trott
Trott approved these changes Jan 13, 2021
Copy link
Member

@Trott Trott left a comment

LGTM. I wonder if as a next step, the test should be modified to allow for either no error or a check that the expected error appears and is propagated. Then the test would at least work no matter where it was run and we wouldn't have to maintain privileged port configuration on Build hosts.

Copy link
Member

@mhdawson mhdawson left a comment

LGTM

aduh95 added a commit that referenced this issue Jan 18, 2021
This reverts commit a45a404.

Solved by marking ports <1024 as privileged on Docker containers.

Ref: #36850
Ref: #36847
Ref: nodejs/build#2521

PR-URL: #36884
Refs: #36850
Refs: #36847
Refs: nodejs/build#2521
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Ash Cripps <acripps@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
@aduh95
Copy link
Contributor

@aduh95 aduh95 commented Jan 18, 2021

Landed in 5716130

@aduh95 aduh95 closed this Jan 18, 2021
ruyadorno added a commit that referenced this issue Jan 22, 2021
This reverts commit a45a404.

Solved by marking ports <1024 as privileged on Docker containers.

Ref: #36850
Ref: #36847
Ref: nodejs/build#2521

PR-URL: #36884
Refs: #36850
Refs: #36847
Refs: nodejs/build#2521
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Ash Cripps <acripps@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
@ruyadorno ruyadorno mentioned this pull request Jan 22, 2021
@targos targos added the dont-land-on-v14.x label May 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready dont-land-on-v14.x test
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

X Tutup