Skip to content

KAFKA-4590: SASL/SCRAM system tests#2355

Closed
rajinisivaram wants to merge 2 commits intoapache:trunkfrom
rajinisivaram:KAFKA-4590
Closed

KAFKA-4590: SASL/SCRAM system tests#2355
rajinisivaram wants to merge 2 commits intoapache:trunkfrom
rajinisivaram:KAFKA-4590

Conversation

@rajinisivaram
Copy link
Copy Markdown
Contributor

Runs sanity test and one replication test using SASL/SCRAM.

@rajinisivaram
Copy link
Copy Markdown
Contributor Author

Started a system test run here: https://jenkins.confluent.io/job/system-test-kafka-branch-builder/658/

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 12, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/793/
Test FAILed (JDK 7 and Scala 2.10).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 12, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/793/
Test PASSed (JDK 8 and Scala 2.12).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 12, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/795/
Test FAILed (JDK 8 and Scala 2.11).

@rajinisivaram
Copy link
Copy Markdown
Contributor Author

System test run https://jenkins.confluent.io/job/system-test-kafka-branch-builder/658/ has three unrelated failures - one replication test (KAFKA-4558) and two stream tests. The SASL/SCRAM tests from this PR have passed.

Copy link
Copy Markdown
Contributor

@ewencp ewencp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM, but probably @ijuma or @junrao should also take a look. One question I have is if replication test is the best test to use for validating interbroker protocol -- I think that one is relatively expensive and we probably have cheaper tests that at lest validate that data is replicated. Since we're already parameterizing for interbroker protocol there it's probably not a big deal, but would be good to think about this for efficiency reasons.

@rajinisivaram
Copy link
Copy Markdown
Contributor Author

@ewencp Thank you for the review. Since we are only adding one test (apart from the minimal sanity test) for each new mechanism, replication test felt like a good one since it does a few authentications during restarts. To reduce the time, I am using SCRAM-SHA-256 for client and SCRAM-SHA-512 for inter-broker. So that should test the two protocols as well as the co-existence of both protocols with just one test.

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jan 16, 2017

@rajinisivaram, how about changing the following to only test PLAINTEXT and SASL_SSL?

@matrix(failure_mode=["clean_shutdown", "hard_shutdown", "clean_bounce", "hard_bounce"],
             broker_type=["leader"],
             security_protocol=["PLAINTEXT", "SSL", "SASL_PLAINTEXT", "SASL_SSL"])```

We already do that for `broker_type=["controller"]` and it would "make space" for the new variations added in this PR.

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 16, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/917/
Test PASSed (JDK 8 and Scala 2.11).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 16, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/915/
Test PASSed (JDK 7 and Scala 2.10).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 16, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/915/
Test FAILed (JDK 8 and Scala 2.12).

Copy link
Copy Markdown
Member

@ijuma ijuma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks. Can we run the replication tests in the branch builder (since that is the only thing that changed since the last run) to make sure it's all OK still?

@rajinisivaram
Copy link
Copy Markdown
Contributor Author

@ijuma Thank you for the review. Started test run of replication tests: https://jenkins.confluent.io/job/system-test-kafka-branch-builder-2/174/. Will keep an eye on it and add an update later.

@asfgit asfgit closed this in 55abe65 Jan 17, 2017
asfgit pushed a commit that referenced this pull request Jan 17, 2017
Runs sanity test and one replication test using SASL/SCRAM.

Author: Rajini Sivaram <rajinisivaram@googlemail.com>

Reviewers: Ewen Cheslack-Postava <ewen@confluent.io>, Ismael Juma <ismael@juma.me.uk>

Closes #2355 from rajinisivaram/KAFKA-4590
@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jan 17, 2017

System test passed, merged to trunk and 0.10.2.

soenkeliebau pushed a commit to soenkeliebau/kafka that referenced this pull request Feb 7, 2017
Runs sanity test and one replication test using SASL/SCRAM.

Author: Rajini Sivaram <rajinisivaram@googlemail.com>

Reviewers: Ewen Cheslack-Postava <ewen@confluent.io>, Ismael Juma <ismael@juma.me.uk>

Closes apache#2355 from rajinisivaram/KAFKA-4590
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants