You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2017/04/10 19:31:56 UTC

[kudu-CR] [mini-kdc] /dev/stderr instead of STDERR for kdc logs

Alexey Serbin has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/6602

Change subject: [mini-kdc] /dev/stderr instead of STDERR for kdc logs
......................................................................

[mini-kdc] /dev/stderr instead of STDERR for kdc logs

This is a workaround for kdc5krb trying to open STDERR in read-write
mode "a+" (krb5/src/lib/kadm5/logger.c).  Since mini-kdc is run via
the SubProcess utility, its STDERR is piped via write end of the pipe
and cannot be opened in read mode.  That leads to error messages like

  krb5kdc: cannot parse <STDERR>
  krb5kdc: warning - logging entry syntax error

Specifiying 'FILE:/dev/stderr' allows for using the same destination
for the error output, but not opening the standard error in read mode.

Change-Id: I212e2cceb80acbe88040b16d9055cf01ac8a761b
---
M src/kudu/security/test/mini_kdc.cc
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/6602/1
-- 
To view, visit http://gerrit.cloudera.org:8080/6602
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I212e2cceb80acbe88040b16d9055cf01ac8a761b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>

[kudu-CR] [mini-kdc] /dev/stderr instead of STDERR for kdc logs

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: [mini-kdc] /dev/stderr instead of STDERR for kdc logs
......................................................................


Patch Set 3: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/6602
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I212e2cceb80acbe88040b16d9055cf01ac8a761b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [mini-kdc] /dev/stderr instead of STDERR for kdc logs

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: [mini-kdc] /dev/stderr instead of STDERR for kdc logs
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6602/1//COMMIT_MSG
Commit Message:

Line 18: for the error output, but not opening the standard error in read mode.
> Thanks for pointing to that.  Yes, that's also the case: if running the krb
I meant '... if running the krb5adm in Java security tests ...'


-- 
To view, visit http://gerrit.cloudera.org:8080/6602
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I212e2cceb80acbe88040b16d9055cf01ac8a761b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [mini-kdc] /dev/stderr instead of STDERR for kdc logs

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: [mini-kdc] /dev/stderr instead of STDERR for kdc logs
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6602/2/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKdc.java
File java/kudu-client/src/test/java/org/apache/kudu/client/MiniKdc.java:

Line 232:         "   kdc = /dev/stderr",
Should this use FILE: like the c++ side?


-- 
To view, visit http://gerrit.cloudera.org:8080/6602
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I212e2cceb80acbe88040b16d9055cf01ac8a761b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [mini-kdc] /dev/stderr instead of STDERR for kdc logs

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/6602

to look at the new patch set (#2).

Change subject: [mini-kdc] /dev/stderr instead of STDERR for kdc logs
......................................................................

[mini-kdc] /dev/stderr instead of STDERR for kdc logs

This is a workaround for kdc5krb trying to open STDERR in read-write
mode "a+" (krb5/src/lib/kadm5/logger.c) in versions prior to 1.13.

Since mini-kdc is run via the SubProcess utility, its STDERR is piped
via write end of the pipe and cannot be opened in read mode.  That leads
to error messages like:

  krb5kdc: cannot parse <STDERR>
  krb5kdc: warning - logging entry syntax error

Specifiying 'FILE:/dev/stderr' allows for using the same destination
for the error output, but not opening the standard error in read mode.

NOTE: the issue in logger.c has been addressed by changelist a8592307
      in Kerberos repository and the fix is included in krb5 starting
      version 1.13.

Change-Id: I212e2cceb80acbe88040b16d9055cf01ac8a761b
---
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKdc.java
M src/kudu/security/test/mini_kdc.cc
2 files changed, 2 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/6602/2
-- 
To view, visit http://gerrit.cloudera.org:8080/6602
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I212e2cceb80acbe88040b16d9055cf01ac8a761b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [mini-kdc] /dev/stderr instead of STDERR for kdc logs

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: [mini-kdc] /dev/stderr instead of STDERR for kdc logs
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6602/2/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKdc.java
File java/kudu-client/src/test/java/org/apache/kudu/client/MiniKdc.java:

Line 232:         "   kdc = /dev/stderr",
> Should this use FILE: like the c++ side?
oops, my bad.


-- 
To view, visit http://gerrit.cloudera.org:8080/6602
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I212e2cceb80acbe88040b16d9055cf01ac8a761b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [mini-kdc] /dev/stderr instead of STDERR for kdc logs

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: [mini-kdc] /dev/stderr instead of STDERR for kdc logs
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6602/1//COMMIT_MSG
Commit Message:

Line 18: for the error output, but not opening the standard error in read mode.
Really interesting.  We also do something similar here: https://github.com/apache/kudu/blob/master/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKdc.java#L232 on the Java side.  Does that need to get updated as well?  I'm not sure if Java is using the same pipe mechanisms.


-- 
To view, visit http://gerrit.cloudera.org:8080/6602
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I212e2cceb80acbe88040b16d9055cf01ac8a761b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [mini-kdc] /dev/stderr instead of STDERR for kdc logs

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged.

Change subject: [mini-kdc] /dev/stderr instead of STDERR for kdc logs
......................................................................


[mini-kdc] /dev/stderr instead of STDERR for kdc logs

This is a workaround for kdc5krb trying to open STDERR in read-write
mode "a+" (krb5/src/lib/kadm5/logger.c) in versions prior to 1.13.

Since mini-kdc is run via the SubProcess utility, its STDERR is piped
via write end of the pipe and cannot be opened in read mode.  That leads
to error messages like:

  krb5kdc: cannot parse <STDERR>
  krb5kdc: warning - logging entry syntax error

Specifiying 'FILE:/dev/stderr' allows for using the same destination
for the error output, but not opening the standard error in read mode.

NOTE: the issue in logger.c has been addressed by changelist a8592307
      in Kerberos repository and the fix is included in krb5 starting
      version 1.13.

Change-Id: I212e2cceb80acbe88040b16d9055cf01ac8a761b
Reviewed-on: http://gerrit.cloudera.org:8080/6602
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <da...@apache.org>
---
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKdc.java
M src/kudu/security/test/mini_kdc.cc
2 files changed, 2 insertions(+), 2 deletions(-)

Approvals:
  Dan Burkert: Looks good to me, approved
  Kudu Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/6602
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I212e2cceb80acbe88040b16d9055cf01ac8a761b
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [mini-kdc] /dev/stderr instead of STDERR for kdc logs

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/6602

to look at the new patch set (#3).

Change subject: [mini-kdc] /dev/stderr instead of STDERR for kdc logs
......................................................................

[mini-kdc] /dev/stderr instead of STDERR for kdc logs

This is a workaround for kdc5krb trying to open STDERR in read-write
mode "a+" (krb5/src/lib/kadm5/logger.c) in versions prior to 1.13.

Since mini-kdc is run via the SubProcess utility, its STDERR is piped
via write end of the pipe and cannot be opened in read mode.  That leads
to error messages like:

  krb5kdc: cannot parse <STDERR>
  krb5kdc: warning - logging entry syntax error

Specifiying 'FILE:/dev/stderr' allows for using the same destination
for the error output, but not opening the standard error in read mode.

NOTE: the issue in logger.c has been addressed by changelist a8592307
      in Kerberos repository and the fix is included in krb5 starting
      version 1.13.

Change-Id: I212e2cceb80acbe88040b16d9055cf01ac8a761b
---
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKdc.java
M src/kudu/security/test/mini_kdc.cc
2 files changed, 2 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/6602/3
-- 
To view, visit http://gerrit.cloudera.org:8080/6602
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I212e2cceb80acbe88040b16d9055cf01ac8a761b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [mini-kdc] /dev/stderr instead of STDERR for kdc logs

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: [mini-kdc] /dev/stderr instead of STDERR for kdc logs
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6602/1//COMMIT_MSG
Commit Message:

Line 18: for the error output, but not opening the standard error in read mode.
> Really interesting.  We also do something similar here: https://github.com/
Thanks for pointing to that.  Yes, that's also the case: if running the krb5adm in Kudu security tests, the same issue appears.

I found this issue is specific for versions of krb5adm prior to 1.13.  It was fixed in changelist a8592307 (dated back to August 2014).  So, it has been fixed in newer version of krb5adm binaries, but we have many machines where we run RedHat/CentOS 6.x.

I'll update the commit the Java counterpart to use /dev/stderr and the commit message to include the information on the fix.


-- 
To view, visit http://gerrit.cloudera.org:8080/6602
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I212e2cceb80acbe88040b16d9055cf01ac8a761b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes