[STORM-3616] add optional flag to fail upload-credentials cmd if no creds were uploaded #3243

RuiLi8080 · 2020-04-02T03:48:46Z

-bash-4.1$ storm upload-credentials ec2 -c "password=password"
Running: /home/y/share/yjava_jdk/java/bin/java -client -Ddaemon.name= -Dstorm.options=password%3Dpassword -Dstorm.home=/home/y/lib64/storm/2.2.0.y -Dstorm.log.dir=/home/y/lib64/storm/2.2.0.y/logs -Djava.library.path=/home/y/lib64:/usr/local/lib64:/usr/lib64:/lib64: -Dstorm.conf.file= -cp /home/y/lib64/storm/2.2.0.y/*:/home/y/lib64/storm/2.2.0.y/lib/*:/home/y/lib64/storm/2.2.0.y/extlib/*:/home/y/lib64/storm/2.2.0.y/extlib-daemon/*:/home/y/lib64/storm/current/conf:/home/y/lib64/storm/2.2.0.y/bin org.apache.storm.command.UploadCredentials ec2
03:31:54.653 [main] INFO o.a.s.m.n.Login - Successfully logged in to context StormClient using /etc/grid-keytabs/jaas.conf
03:31:54.662 [Refresh-TGT] INFO o.a.s.m.n.Login - TGT refresh thread started.
03:31:54.691 [Refresh-TGT] INFO o.a.s.m.n.Login - TGT valid starting at: Wed Apr 01 05:14:11 UTC 2020
03:31:54.691 [Refresh-TGT] INFO o.a.s.m.n.Login - TGT expires: Thu Apr 02 05:14:11 UTC 2020
03:31:54.691 [Refresh-TGT] WARN o.a.s.m.n.Login - TGT refresh thread time adjusted from : Thu Apr 02 01:30:31 UTC 2020 to : Thu Apr 02 03:32:54 UTC 2020 since the former is sooner than the minimum refresh interval (60 seconds) from now.
03:31:54.692 [Refresh-TGT] INFO o.a.s.m.n.Login - TGT refresh sleeping until: Thu Apr 02 03:32:54 UTC 2020
03:31:54.835 [main] INFO o.a.s.u.NimbusClient - Found leader nimbus : openstorm13blue-n1.blue.ygrid.yahoo.com:50560
03:31:54.897 [main] INFO o.a.s.c.UploadCredentials - Using topology conf from ec2-1-1585797718 as basis for getting new creds
03:31:54.924 [main] INFO o.a.s.s.a.ClientAuthUtils - Got AutoCreds []
03:31:54.924 [main] INFO o.a.s.StormSubmitter - Full list of creds available: {}
03:31:54.924 [main] WARN o.a.s.StormSubmitter - No credentials were found to push to ec2
03:31:54.924 [main] INFO o.a.s.c.UploadCredentials - Uploaded new creds to topology: ec2
-bash-4.1$ echo $?
0

 

-bash-4.1$ storm upload-credentials ec2 -c "password=password" -e

Running: /home/y/share/yjava_jdk/java/bin/java -client -Ddaemon.name= -Dstorm.options=password%3Dpassword -Dstorm.home=/home/y/lib64/storm/2.2.0.y -Dstorm.log.dir=/home/y/lib64/storm/2.2.0.y/logs -Djava.library.path=/home/y/lib64:/usr/local/lib64:/usr/lib64:/lib64: -Dstorm.conf.file= -cp /home/y/lib64/storm/2.2.0.y/*:/home/y/lib64/storm/2.2.0.y/lib/*:/home/y/lib64/storm/2.2.0.y/extlib/*:/home/y/lib64/storm/2.2.0.y/extlib-daemon/*:/home/y/lib64/storm/current/conf:/home/y/lib64/storm/2.2.0.y/bin org.apache.storm.command.UploadCredentials ec2 -e
03:32:30.449 [main] INFO o.a.s.m.n.Login - Successfully logged in to context StormClient using /etc/grid-keytabs/jaas.conf
03:32:30.471 [Refresh-TGT] INFO o.a.s.m.n.Login - TGT refresh thread started.
03:32:30.492 [Refresh-TGT] INFO o.a.s.m.n.Login - TGT valid starting at: Wed Apr 01 05:14:11 UTC 2020
03:32:30.493 [Refresh-TGT] INFO o.a.s.m.n.Login - TGT expires: Thu Apr 02 05:14:11 UTC 2020
03:32:30.493 [Refresh-TGT] WARN o.a.s.m.n.Login - TGT refresh thread time adjusted from : Thu Apr 02 01:25:36 UTC 2020 to : Thu Apr 02 03:33:30 UTC 2020 since the former is sooner than the minimum refresh interval (60 seconds) from now.
03:32:30.493 [Refresh-TGT] INFO o.a.s.m.n.Login - TGT refresh sleeping until: Thu Apr 02 03:33:30 UTC 2020
03:32:30.641 [main] INFO o.a.s.u.NimbusClient - Found leader nimbus : openstorm13blue-n1.blue.ygrid.yahoo.com:50560
03:32:30.702 [main] INFO o.a.s.c.UploadCredentials - Using topology conf from ec2-1-1585797718 as basis for getting new creds
03:32:30.729 [main] INFO o.a.s.s.a.ClientAuthUtils - Got AutoCreds []
03:32:30.729 [main] INFO o.a.s.StormSubmitter - Full list of creds available: {}
03:32:30.729 [main] WARN o.a.s.StormSubmitter - No credentials were found to push to ec2
03:32:30.730 [main] ERROR o.a.s.c.UploadCredentials - No credentials were uploaded for ec2

Exception in thread "main" java.lang.RuntimeException: No credentials were uploaded for ec2
     at org.apache.storm.command.UploadCredentials.main(UploadCredentials.java:120)

-bash-4.1$ echo $?
1

…reds were uploaded

Ethanlm · 2020-04-02T13:41:28Z

docs/Command-line-client.md

@@ -319,7 +319,9 @@ eg: `storm shell resources/ python topology.py arg1 arg2`

 Syntax: `storm upload_credentials topology-name [credkey credvalue]*`

-Uploads a new set of credentials to a running topology
+Uploads a new set of credentials to a running topology  
+   * `-e --exception`: optioanl flag. If set, command will fail and throw exception if no credentials were uploaded.


type "optioanl"

Ethanlm · 2020-04-02T13:45:48Z

bin/storm.py

+    # If set, this flag will become true meaning that user expects non-empty creds to be uploaded.
+    # Command exits with non-zero code if uploaded creds collection is empty.
+    sub_parser.add_argument(
+        "-e", "--exception", action='store_true',


--exception is too general. Maybe --fail-on-empty or something

I changed --exception-when-empty here since we already use short name -f for file.

This is also good

Where is args.exception_when_empty passed to java command (in storm.py:upload_credentials()?

@bipinprasad

storm/storm-core/src/jvm/org/apache/storm/command/UploadCredentials.java

Line 45 in 088e722

.boolOpt("e", "exception-when-empty")

Seems there is a slight problem. In python code, when exec_java_class() is called, the arguments to the java class is constructed partly from main_class_args. main_class_args is constructed by simply eliminating some arguments passed to the python code (and keeping the rest intact). So if python code got passed "-e" ii will get passed thru to java class as "-e". However, if "--exception was passed to python, then --exception will get passed to java. This wont match what java class is expecting. So in order to do a passthru, the option names should be the same, otherwise main_class_args should be suitably modified based on the args. Looks like passthru might be preferred solution.

Yea, the argument in python is the same: "--exception-when-empty".

Ethanlm · 2020-04-02T13:49:26Z

storm-client/src/jvm/org/apache/storm/StormSubmitter.java

@@ -126,7 +127,7 @@ public static void pushCredentials(String name, Map<String, Object> topoConf, Ma
     * @throws NotAliveException        if the topology is not alive
     * @throws InvalidTopologyException if any other error happens
     */
-    public static void pushCredentials(String name, Map<String, Object> topoConf, Map<String, String> credentials, String expectedUser)
+    public static boolean pushCredentials(String name, Map<String, Object> topoConf, Map<String, String> credentials, String expectedUser)


missing javadoc @return

There is another pushCredentials method in this class needs to be taken care of

Ethanlm · 2020-04-02T13:52:03Z

storm-core/src/jvm/org/apache/storm/command/UploadCredentials.java

@@ -111,7 +112,13 @@ public static void main(String[] args) throws Exception {
        // use the local setting for the login config rather than the topology's
        topologyConf.remove("java.security.auth.login.config");

-        StormSubmitter.pushCredentials(topologyName, topologyConf, credentialsMap, (String) cl.get("u"));
+        boolean throwExceptionForEmptyCreds = (boolean) cl.getOrDefault("e", false);


No need for default value here. above boolOpt is already default to false

Ethanlm · 2020-04-02T19:33:17Z

docs/Command-line-client.md

@@ -319,7 +319,9 @@ eg: `storm shell resources/ python topology.py arg1 arg2`

 Syntax: `storm upload_credentials topology-name [credkey credvalue]*`

-Uploads a new set of credentials to a running topology
+Uploads a new set of credentials to a running topology  
+   * `-e --exception`: optional flag. If set, command will fail and throw exception if no credentials were uploaded.


This needs to be updated too

Ethanlm · 2020-04-02T19:40:15Z

bin/storm.py

+    # Command exits with non-zero code if uploaded creds collection is empty.
+    sub_parser.add_argument(
+        "-e", "--exception-when-empty", action='store_true',
+        help="""whether to throw exception if there are no credentials uploaded, default to be false"""


It means true when this config is specified, otherwise it's false.

So more precisely, "If specified, throws an exception if no credentials are uploaded"

Ethanlm · 2020-04-02T19:42:30Z

storm-client/src/jvm/org/apache/storm/StormSubmitter.java

@@ -106,27 +106,30 @@ public static boolean validateZKDigestPayload(String payload) {
     * @param name        the name of the topology to push credentials to.
     * @param topoConf    the topology-specific configuration, if desired. See {@link Config}.
     * @param credentials the credentials to push.
+     * @return whether the pushed credential collection fullCreds is non-empty. Return false if empty.


fullCreds doesn't really mean anything and can be deleted; image anyone changed the variable name

Sure. Fixed

only one Travis build failed. Should be irrelevant.

Ethanlm

+1

[STORM-3616] add optional flag to fail upload-credentials cmd if no c…

a4da4a7

…reds were uploaded

Ethanlm reviewed Apr 2, 2020

View reviewed changes

address comments

568bb57

Ethanlm reviewed Apr 2, 2020

View reviewed changes

address comments

088e722

Ethanlm approved these changes Apr 7, 2020

View reviewed changes

Ethanlm merged commit b38e159 into apache:master Apr 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[STORM-3616] add optional flag to fail upload-credentials cmd if no creds were uploaded #3243

[STORM-3616] add optional flag to fail upload-credentials cmd if no creds were uploaded #3243

RuiLi8080 commented Apr 2, 2020 •

edited

Ethanlm Apr 2, 2020

RuiLi8080 Apr 2, 2020

Ethanlm Apr 2, 2020

RuiLi8080 Apr 2, 2020

Ethanlm Apr 2, 2020

bipinprasad Apr 3, 2020

RuiLi8080 Apr 3, 2020

bipinprasad Apr 3, 2020

Ethanlm Apr 7, 2020

Ethanlm Apr 2, 2020

RuiLi8080 Apr 2, 2020

Ethanlm Apr 2, 2020

RuiLi8080 Apr 2, 2020

Ethanlm Apr 2, 2020

RuiLi8080 Apr 3, 2020

Ethanlm Apr 2, 2020

RuiLi8080 Apr 3, 2020

Ethanlm Apr 2, 2020 •

edited

RuiLi8080 Apr 3, 2020

RuiLi8080 Apr 3, 2020

Ethanlm left a comment

[STORM-3616] add optional flag to fail upload-credentials cmd if no creds were uploaded #3243

[STORM-3616] add optional flag to fail upload-credentials cmd if no creds were uploaded #3243

Conversation

RuiLi8080 commented Apr 2, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ethanlm Apr 2, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ethanlm left a comment

Choose a reason for hiding this comment

RuiLi8080 commented Apr 2, 2020 •

edited

Ethanlm Apr 2, 2020 •

edited