Isolate zk client config for suppressing authentication #9438

ggezer · 2019-07-09T20:25:45Z

For when more than one zk servers are accessed within a single jvm, global authentication settings are shared via system properties. This PR bumps zk client version for getting a support for client config isolation and makes use of it. Curator has also been updated for compatibility.

AmplabJenkins · 2019-07-09T20:55:51Z

Merged build finished. Test PASSed.

AmplabJenkins · 2019-07-09T20:55:52Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/4532/
Test PASSed.

ggezer · 2019-07-09T20:56:40Z

@bf8086 @madanadit PTAL. Thanks!

madanadit · 2019-07-09T21:55:35Z

core/common/pom.xml

@@ -57,7 +57,8 @@
    </dependency>
    <dependency>
      <groupId>io.netty</groupId>
-      <artifactId>netty-all</artifactId>
+      <artifactId>netty</artifactId>
+      <version>3.7.0.Final</version>


why change the version only for this specific module?

This module uses ConcurrentIdentityMap implementation from netty which is dropped after this version. And that's the only use of netty from this module.

Actually we don't need an explicit netty dependency anymore. I can have another PR to remove netty dependencies throughout the code. (Locally I tested that removing netty dependencies, except this one, has no effect on build.)

did you create a tech dept issue for this?

If we can remove the direct dependency on Netty that would be helpful. (In a separate effort of course)

Created #9449

core/common/src/main/java/alluxio/master/ZkMasterInquireClient.java

AmplabJenkins · 2019-07-09T22:25:48Z

Merged build finished. Test FAILed.

AmplabJenkins · 2019-07-09T22:25:49Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/4537/
Test FAILed.

core/common/src/main/java/alluxio/master/MasterInquireClient.java

AmplabJenkins · 2019-07-10T03:13:04Z

Merged build finished. Test FAILed.

AmplabJenkins · 2019-07-10T03:13:05Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/4540/
Test FAILed.

AmplabJenkins · 2019-07-10T03:20:17Z

Merged build finished. Test FAILed.

AmplabJenkins · 2019-07-10T03:20:18Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/4541/
Test FAILed.

AmplabJenkins · 2019-07-10T04:36:24Z

Merged build finished. Test FAILed.

AmplabJenkins · 2019-07-10T04:36:25Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/4542/
Test FAILed.

madanadit

can you also add documentation for the new param?

core/common/src/main/java/alluxio/master/ZkMasterInquireClient.java

AmplabJenkins · 2019-07-10T06:58:13Z

Merged build finished. Test PASSed.

AmplabJenkins · 2019-07-10T06:58:14Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/4544/
Test PASSed.

core/common/src/main/java/alluxio/conf/PropertyKey.java

bf8086

LGTM. Thanks!

ggezer · 2019-07-11T18:29:10Z

@madanadit PTAL.

AmplabJenkins · 2019-07-11T18:37:02Z

Merged build finished. Test PASSed.

AmplabJenkins · 2019-07-11T18:37:03Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/4548/
Test PASSed.

madanadit · 2019-07-11T21:06:26Z

core/common/pom.xml

@@ -57,7 +57,8 @@
    </dependency>
    <dependency>
      <groupId>io.netty</groupId>
-      <artifactId>netty-all</artifactId>
+      <artifactId>netty</artifactId>
+      <version>3.7.0.Final</version>


did you create a tech dept issue for this?

ggezer · 2019-07-11T22:25:19Z

core/common/src/main/java/alluxio/conf/PropertyKey.java

@@ -525,7 +525,13 @@ public String toString() {
          .setDescription("Session timeout to use when connecting to Zookeeper")
          .setConsistencyCheckLevel(ConsistencyCheckLevel.WARN)
          .build();
-
+  public static final PropertyKey ZOOKEEPER_AUTH_ENABLED =
+      new Builder(Name.ZOOKEEPER_AUTH_ENABLED)


@ns1123 FYI.

madanadit · 2019-07-11T23:30:23Z

alluxio-bot, merge this please.

madanadit · 2019-07-11T23:32:28Z

alluxio-bot, cherry-pick this to branch-2.0 please.

alluxio-bot · 2019-07-11T23:32:44Z

Auto cherry-pick successful to branch: branch-2.0

Auto cherry-pick of existing commit. orig-pr: #9438 orig-commit: 22e0027 orig-commit-author: Göktürk Gezer <gokturk@alluxio.com> pr-link: #9438 change-id: cid-d9607acb78abb6550721a741de095ad2c2e2acd8

madanadit · 2019-07-11T23:32:53Z

@ggezer please cherry-pick to branch-1.8

For when more than one zk servers are accessed within a single jvm, global authentication settings are shared via system properties. This PR bumps zk client version for getting a support for client config isolation and makes use of it. Curator has also been updated for compatibility. pr-link: Alluxio#9438 change-id: cid-d9607acb78abb6550721a741de095ad2c2e2acd8

Cherry-pick of existing commit. orig-pr: #9438 orig-commit: 22e0027 orig-commit-author: Göktürk Gezer <gokturk@alluxio.com> pr-link: #9451 change-id: cid-d9607acb78abb6550721a741de095ad2c2e2acd8

For when more than one zk servers are accessed within a single jvm, global authentication settings are shared via system properties. This PR bumps zk client version for getting a support for client config isolation and makes use of it. Curator has also been updated for compatibility. pr-link: Alluxio#9438 change-id: cid-d9607acb78abb6550721a741de095ad2c2e2acd8

Gokturk Gezer added 3 commits July 9, 2019 12:51

Isolate zk client config for suppressing authentication

496875b

Remove restartable testing server hack

955ed6f

Bump curator to 4.2.0

bb99e9e

alluxio-bot added the POM Change label Jul 9, 2019

alluxio-bot assigned ZacBlanco Jul 9, 2019

Unify zookeeper creation

53e9123

madanadit reviewed Jul 9, 2019

View reviewed changes

core/common/src/main/java/alluxio/master/ZkMasterInquireClient.java Outdated Show resolved Hide resolved

madanadit reviewed Jul 9, 2019

View reviewed changes

core/common/src/main/java/alluxio/master/MasterInquireClient.java Outdated Show resolved Hide resolved

ggezer force-pushed the isolate-zk branch from 76714f6 to 7fae9bb Compare July 10, 2019 03:11

ggezer force-pushed the isolate-zk branch from 7fae9bb to e83c560 Compare July 10, 2019 03:15

Use configuration for isolating config

542e81e

ggezer force-pushed the isolate-zk branch from e83c560 to 542e81e Compare July 10, 2019 04:06

madanadit reviewed Jul 10, 2019

View reviewed changes

core/common/src/main/java/alluxio/master/ZkMasterInquireClient.java Outdated Show resolved Hide resolved

doc update

e6c81fa

bf8086 reviewed Jul 10, 2019

View reviewed changes

core/common/src/main/java/alluxio/conf/PropertyKey.java Show resolved Hide resolved

Update prop definition with scope and consistency level

31fa30a

bf8086 approved these changes Jul 11, 2019

View reviewed changes

ZacBlanco removed their assignment Jul 11, 2019

alluxio-bot assigned ZacBlanco Jul 11, 2019

madanadit approved these changes Jul 11, 2019

View reviewed changes

ggezer commented Jul 11, 2019

View reviewed changes

alluxio-bot merged commit 22e0027 into Alluxio:master Jul 11, 2019

ggezer mentioned this pull request Jul 12, 2019

Isolate zk client config for suppressing authentication #9451

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Isolate zk client config for suppressing authentication #9438

Isolate zk client config for suppressing authentication #9438

ggezer commented Jul 9, 2019

AmplabJenkins commented Jul 9, 2019

AmplabJenkins commented Jul 9, 2019

ggezer commented Jul 9, 2019

madanadit Jul 9, 2019

ggezer Jul 9, 2019

madanadit Jul 11, 2019

calvinjia Jul 11, 2019 •

edited

ggezer Jul 11, 2019 •

edited

AmplabJenkins commented Jul 9, 2019

AmplabJenkins commented Jul 9, 2019

AmplabJenkins commented Jul 10, 2019

AmplabJenkins commented Jul 10, 2019

AmplabJenkins commented Jul 10, 2019

AmplabJenkins commented Jul 10, 2019

AmplabJenkins commented Jul 10, 2019

AmplabJenkins commented Jul 10, 2019

madanadit left a comment

AmplabJenkins commented Jul 10, 2019

AmplabJenkins commented Jul 10, 2019

bf8086 left a comment

ggezer commented Jul 11, 2019

AmplabJenkins commented Jul 11, 2019

AmplabJenkins commented Jul 11, 2019

madanadit Jul 11, 2019

ggezer Jul 11, 2019

madanadit commented Jul 11, 2019

madanadit commented Jul 11, 2019

alluxio-bot commented Jul 11, 2019

madanadit commented Jul 11, 2019

Isolate zk client config for suppressing authentication #9438

Isolate zk client config for suppressing authentication #9438

Conversation

ggezer commented Jul 9, 2019

AmplabJenkins commented Jul 9, 2019

AmplabJenkins commented Jul 9, 2019

ggezer commented Jul 9, 2019

madanadit Jul 9, 2019

Choose a reason for hiding this comment

ggezer Jul 9, 2019

Choose a reason for hiding this comment

madanadit Jul 11, 2019

Choose a reason for hiding this comment

calvinjia Jul 11, 2019 • edited

Choose a reason for hiding this comment

ggezer Jul 11, 2019 • edited

Choose a reason for hiding this comment

AmplabJenkins commented Jul 9, 2019

AmplabJenkins commented Jul 9, 2019

AmplabJenkins commented Jul 10, 2019

AmplabJenkins commented Jul 10, 2019

AmplabJenkins commented Jul 10, 2019

AmplabJenkins commented Jul 10, 2019

AmplabJenkins commented Jul 10, 2019

AmplabJenkins commented Jul 10, 2019

madanadit left a comment

Choose a reason for hiding this comment

AmplabJenkins commented Jul 10, 2019

AmplabJenkins commented Jul 10, 2019

bf8086 left a comment

Choose a reason for hiding this comment

ggezer commented Jul 11, 2019

AmplabJenkins commented Jul 11, 2019

AmplabJenkins commented Jul 11, 2019

madanadit Jul 11, 2019

Choose a reason for hiding this comment

ggezer Jul 11, 2019

Choose a reason for hiding this comment

madanadit commented Jul 11, 2019

madanadit commented Jul 11, 2019

alluxio-bot commented Jul 11, 2019

madanadit commented Jul 11, 2019

calvinjia Jul 11, 2019 •

edited

ggezer Jul 11, 2019 •

edited