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/03/15 15:29:34 UTC
[kudu-CR] [mini-kdc] a couple of fixes
Alexey Serbin has uploaded a new change for review.
http://gerrit.cloudera.org:8080/6404
Change subject: [mini-kdc] a couple of fixes
......................................................................
[mini-kdc] a couple of fixes
Updated MiniKdc code to fix the following minor bugs:
* hiding non-OK return code from kinit in MiniKdc::Kinit()
* not waiting for krb5kdc process to start listening on ports
in case of restart sequence: i.e. calling Start(), then Stop(),
and then Start() again.
Added unit test to verify how MiniKdc::CreateUserPrincipal(),
MiniKdc::Kinit() and MiniKdc::Klist() work when krb5kdc is shut down
and then started again.
Also, added WARN_UNUSED_RESULT attribute for kudu::Subprocess methods
returning Status.
Change-Id: Ibe8cf39a2dcca29627f1e40104ea2e44d7f69d5c
---
M src/kudu/security/test/mini_kdc-test.cc
M src/kudu/security/test/mini_kdc.cc
M src/kudu/util/subprocess.h
3 files changed, 65 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/6404/1
--
To view, visit http://gerrit.cloudera.org:8080/6404
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibe8cf39a2dcca29627f1e40104ea2e44d7f69d5c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [mini-kdc] a couple of fixes
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: [mini-kdc] a couple of fixes
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6404/3/src/kudu/security/test/mini_kdc.cc
File src/kudu/security/test/mini_kdc.cc:
Line 178: const bool need_config_update = (options_.port == 0);
why extract this into a variable?
--
To view, visit http://gerrit.cloudera.org:8080/6404
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe8cf39a2dcca29627f1e40104ea2e44d7f69d5c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [mini-kdc] a couple of fixes
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: [mini-kdc] a couple of fixes
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6404/3/src/kudu/security/test/mini_kdc.cc
File src/kudu/security/test/mini_kdc.cc:
Line 178: const bool need_config_update = (options_.port == 0);
> Because options_.port is modified by WaitForKdcPorts() call below, and due
ah ok, I missed that.
--
To view, visit http://gerrit.cloudera.org:8080/6404
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe8cf39a2dcca29627f1e40104ea2e44d7f69d5c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [mini-kdc] a couple of fixes
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [mini-kdc] a couple of fixes
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6404/3/src/kudu/security/test/mini_kdc.cc
File src/kudu/security/test/mini_kdc.cc:
Line 178: const bool need_config_update = (options_.port == 0);
> Because options_.port is modified by WaitForKdcPorts() call below, and due
I tried alternative approaches (output parameter of WaitForKdcPorts() for result port, etc.), but then I found it would be necessary to port from options_ in that case and pass it along as parameter elsewhere.
--
To view, visit http://gerrit.cloudera.org:8080/6404
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe8cf39a2dcca29627f1e40104ea2e44d7f69d5c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [mini-kdc] a couple of fixes
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/6404
to look at the new patch set (#3).
Change subject: [mini-kdc] a couple of fixes
......................................................................
[mini-kdc] a couple of fixes
Updated MiniKdc code to fix the following minor bugs:
* hiding non-OK return code from kinit in MiniKdc::Kinit()
* not waiting for krb5kdc process to start listening on ports
in case of restart sequence: i.e. calling Start(), then Stop(),
and then Start() again.
Added unit test to verify how MiniKdc::CreateUserPrincipal(),
MiniKdc::Kinit() and MiniKdc::Klist() work when krb5kdc is shut down
and then started again.
Also, added WARN_UNUSED_RESULT attribute for kudu::Subprocess methods
returning Status.
Change-Id: Ibe8cf39a2dcca29627f1e40104ea2e44d7f69d5c
---
M src/kudu/integration-tests/minidump_generation-itest.cc
M src/kudu/security/test/mini_kdc-test.cc
M src/kudu/security/test/mini_kdc.cc
M src/kudu/util/subprocess.h
4 files changed, 69 insertions(+), 20 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/6404/3
--
To view, visit http://gerrit.cloudera.org:8080/6404
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibe8cf39a2dcca29627f1e40104ea2e44d7f69d5c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [mini-kdc] a couple of fixes
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [mini-kdc] a couple of fixes
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6404/3/src/kudu/security/test/mini_kdc.cc
File src/kudu/security/test/mini_kdc.cc:
Line 178: const bool need_config_update = (options_.port == 0);
> why extract this into a variable?
Because options_.port is modified by WaitForKdcPorts() call below, and due to the sequence of calls, it's necessary to check for that pre-condition to understand whether to call CreateKrb5Conf() and CreateKdcConf()
--
To view, visit http://gerrit.cloudera.org:8080/6404
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe8cf39a2dcca29627f1e40104ea2e44d7f69d5c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [mini-kdc] a couple of fixes
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged.
Change subject: [mini-kdc] a couple of fixes
......................................................................
[mini-kdc] a couple of fixes
Updated MiniKdc code to fix the following minor bugs:
* hiding non-OK return code from kinit in MiniKdc::Kinit()
* not waiting for krb5kdc process to start listening on ports
in case of restart sequence: i.e. calling Start(), then Stop(),
and then Start() again.
Added unit test to verify how MiniKdc::CreateUserPrincipal(),
MiniKdc::Kinit() and MiniKdc::Klist() work when krb5kdc is shut down
and then started again.
Also, added WARN_UNUSED_RESULT attribute for kudu::Subprocess methods
returning Status.
Change-Id: Ibe8cf39a2dcca29627f1e40104ea2e44d7f69d5c
Reviewed-on: http://gerrit.cloudera.org:8080/6404
Reviewed-by: Dan Burkert <da...@apache.org>
Tested-by: Kudu Jenkins
---
M src/kudu/integration-tests/minidump_generation-itest.cc
M src/kudu/security/test/mini_kdc-test.cc
M src/kudu/security/test/mini_kdc.cc
M src/kudu/util/subprocess.h
4 files changed, 69 insertions(+), 20 deletions(-)
Approvals:
Dan Burkert: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/6404
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibe8cf39a2dcca29627f1e40104ea2e44d7f69d5c
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [mini-kdc] a couple of fixes
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/6404
to look at the new patch set (#2).
Change subject: [mini-kdc] a couple of fixes
......................................................................
[mini-kdc] a couple of fixes
Updated MiniKdc code to fix the following minor bugs:
* hiding non-OK return code from kinit in MiniKdc::Kinit()
* not waiting for krb5kdc process to start listening on ports
in case of restart sequence: i.e. calling Start(), then Stop(),
and then Start() again.
Added unit test to verify how MiniKdc::CreateUserPrincipal(),
MiniKdc::Kinit() and MiniKdc::Klist() work when krb5kdc is shut down
and then started again.
Also, added WARN_UNUSED_RESULT attribute for kudu::Subprocess methods
returning Status.
Change-Id: Ibe8cf39a2dcca29627f1e40104ea2e44d7f69d5c
---
M src/kudu/integration-tests/minidump_generation-itest.cc
M src/kudu/security/test/mini_kdc-test.cc
M src/kudu/security/test/mini_kdc.cc
M src/kudu/util/subprocess.h
4 files changed, 69 insertions(+), 20 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/6404/2
--
To view, visit http://gerrit.cloudera.org:8080/6404
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibe8cf39a2dcca29627f1e40104ea2e44d7f69d5c
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>