Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ST] enable all system test disable due to UTO and add admin client utils #9876

Merged
merged 11 commits into from
Apr 29, 2024
Merged

[ST] enable all system test disable due to UTO and add admin client utils #9876

merged 11 commits into from
Apr 29, 2024

Conversation

henryZrncik
Copy link
Contributor

@henryZrncik henryZrncik commented Mar 26, 2024 •
edited

Type of change

  • Refactoring

Description

Allow all tests disabled due to UTONotSupported
refactor of all checks and tests in ST suites to run with UTO.

  • removing some steps from recovery tests and whole test altogether (TopicST#testCreateTopicViaKafka) which is now (with UTO only) doing nothing
  • removal of UTONotSupported and UTONotSupportedCondition

Some of Kafka Scripts usages transition to Admin Client.

  • implementing functionality for adminClient command describeTopic which can parse KafkaTopicDescription object with partitionCount, name, replicaCount (which are effectively all we care about the kafka topic in tests so far).
  • adding 8 methods into AdminClientTemplates for deploying admin client on any of combination SSL, PLAINTEXT, SASL_PLAINTEXT , SASL_SSL security with/without extra configuration parameter.
  • removal of some methods from KafkaTopicUtils which works with kafka scripts which can be now ignored due to using adminClient.
  • add new label into admin client which would resolve problem of having unique labels in case there are 2 admin clients at the same namespace.

Minor refactors

  • admin client deployment.
  • util methods in cruise control and their calls in Cruise Control STs.

@henryZrncik henryZrncik self-assigned this Mar 26, 2024
@henryZrncik henryZrncik added this to the 0.41.0 milestone Mar 26, 2024
@henryZrncik henryZrncik marked this pull request as draft March 26, 2024 16:44
@henryZrncik henryZrncik mentioned this pull request Apr 9, 2024
Closed
@henryZrncik henryZrncik changed the title [ST] enable all UTO checks and system tests [ST] enable all system test disable due to UTO + admin client utils Apr 11, 2024
@henryZrncik henryZrncik changed the title [ST] enable all system test disable due to UTO + admin client utils [ST] enable all system test disable due to UTO and add admin client utils Apr 11, 2024
@see-quick
Copy link
Member

/azp run regression

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@henryZrncik henryZrncik marked this pull request as ready for review April 12, 2024 08:07
@henryZrncik
Copy link
Contributor Author

Do not merge before release of new test clients.

see-quick
see-quick reviewed Apr 12, 2024
View reviewed changes
Copy link
Member

@see-quick see-quick left a comment

Choose a reason for hiding this comment

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

Looks good. That's a lot of work done thanks 💯 ! I have a few comments ...

.../main/java/io/strimzi/systemtest/kafkaclients/internalClients/admin/KafkaAdminException.java Show resolved Hide resolved
systemtest/src/main/java/io/strimzi/systemtest/templates/specific/AdminClientTemplates.java Outdated Show resolved Hide resolved
systemtest/src/main/java/io/strimzi/systemtest/templates/specific/AdminClientTemplates.java Outdated Show resolved Hide resolved
systemtest/src/main/java/io/strimzi/systemtest/templates/specific/AdminClientTemplates.java Outdated Show resolved Hide resolved
systemtest/src/main/java/io/strimzi/systemtest/templates/specific/AdminClientTemplates.java Outdated Show resolved Hide resolved
systemtest/src/main/java/io/strimzi/systemtest/utils/kubeUtils/objects/PodUtils.java Outdated Show resolved Hide resolved
systemtest/src/test/java/io/strimzi/systemtest/cruisecontrol/CruiseControlConfigurationST.java Outdated Show resolved Hide resolved
systemtest/src/test/java/io/strimzi/systemtest/mirrormaker/MirrorMaker2ST.java Outdated Show resolved Hide resolved
systemtest/src/test/java/io/strimzi/systemtest/mirrormaker/MirrorMaker2ST.java Outdated Show resolved Hide resolved
systemtest/src/test/java/io/strimzi/systemtest/mirrormaker/MirrorMaker2ST.java Outdated Show resolved Hide resolved
see-quick
see-quick reviewed Apr 17, 2024
View reviewed changes
Copy link
Member

@see-quick see-quick left a comment

Choose a reason for hiding this comment

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

Looks good thanks., a few suggestions to consider

systemtest/src/main/java/io/strimzi/systemtest/templates/specific/AdminClientTemplates.java Show resolved Hide resolved
systemtest/src/main/java/io/strimzi/systemtest/templates/specific/AdminClientTemplates.java Outdated Show resolved Hide resolved
systemtest/src/main/java/io/strimzi/systemtest/templates/specific/AdminClientTemplates.java Outdated Show resolved Hide resolved
systemtest/src/main/java/io/strimzi/systemtest/templates/specific/AdminClientTemplates.java Outdated Show resolved Hide resolved
systemtest/src/test/java/io/strimzi/systemtest/cruisecontrol/CruiseControlST.java Show resolved Hide resolved
systemtest/src/test/java/io/strimzi/systemtest/mirrormaker/MirrorMaker2ST.java Outdated Show resolved Hide resolved
see-quick
see-quick approved these changes Apr 18, 2024
View reviewed changes
Copy link
Member

@see-quick see-quick left a comment

Choose a reason for hiding this comment

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

LGTM, Assuming that the tests will pass :)

@see-quick
Copy link
Member

/azp run regression

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@henryZrncik
[ST] enable all test stopped on UTO not supported
46e9271 
Signed-off-by: hzrncik <hzrncik@redhat.com>
@henryZrncik
refactor parsing JSON output to use Class instead of ReferenceType
ee03e02 
Signed-off-by: hzrncik <hzrncik@redhat.com>
@henryZrncik
move some helper methods from kafka admin utils into kafka admin temp…
b2e85ab 
…lates

Signed-off-by: hzrncik <hzrncik@redhat.com>
@henryZrncik
style fixes
a48c063 
Signed-off-by: hzrncik <hzrncik@redhat.com>
@henryZrncik
address recommendations
ab3e24f 
Signed-off-by: hzrncik <hzrncik@redhat.com>
@henryZrncik
upper case TLS
95c5327 
Signed-off-by: hzrncik <hzrncik@redhat.com>
@henryZrncik
fix admin assignment in throtling quota
0843010 
Signed-off-by: hzrncik <hzrncik@redhat.com>
@henryZrncik
address recommendation
aa42f9f 
Signed-off-by: hzrncik <hzrncik@redhat.com>
im-konge
im-konge reviewed Apr 23, 2024
View reviewed changes
Copy link
Member

@im-konge im-konge left a comment

Choose a reason for hiding this comment

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

I left couple of comments, but otherwise this looks good. Thanks a lot :)

...ain/java/io/strimzi/systemtest/kafkaclients/internalClients/admin/KafkaTopicDescription.java Show resolved Hide resolved
...ain/java/io/strimzi/systemtest/kafkaclients/internalClients/admin/KafkaTopicDescription.java Outdated Show resolved Hide resolved
...test/src/main/java/io/strimzi/systemtest/kafkaclients/internalClients/admin/AdminClient.java Outdated Show resolved Hide resolved
systemtest/src/main/java/io/strimzi/systemtest/templates/specific/AdminClientTemplates.java Outdated Show resolved Hide resolved
systemtest/src/main/java/io/strimzi/systemtest/templates/specific/AdminClientTemplates.java Outdated Show resolved Hide resolved
systemtest/src/main/java/io/strimzi/systemtest/templates/specific/AdminClientTemplates.java Outdated Show resolved Hide resolved
systemtest/src/main/java/io/strimzi/systemtest/templates/specific/AdminClientTemplates.java Outdated Show resolved Hide resolved
systemtest/src/main/java/io/strimzi/systemtest/utils/AdminClientUtils.java Outdated Show resolved Hide resolved
@henryZrncik
added java doc and few renamings in admin client utils
c308c2b 
Signed-off-by: hzrncik <hzrncik@redhat.com>
@henryZrncik
remove static properties settings in admin client for all commands
5e34cad 
Signed-off-by: hzrncik <hzrncik@redhat.com>
@see-quick
Copy link
Member

/azp run regression

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

im-konge
im-konge approved these changes Apr 24, 2024
View reviewed changes
Copy link
Member

@im-konge im-konge 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 :) good job

@henryZrncik henryZrncik added ready for merge Label for PRs which are ready for merge and removed needs review labels Apr 24, 2024
@im-konge im-konge removed the ready for merge Label for PRs which are ready for merge label Apr 24, 2024
@im-konge
Copy link
Member

/azp run kraft-regression

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@henryZrncik
incrase controller replica cout to 3 to cover roll of controllers in …
f4c7e2c 
…config change

Signed-off-by: hzrncik <hzrncik@redhat.com>
@see-quick
Copy link
Member

/azp run regression

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@im-konge
Copy link
Member

/azp run kraft-regression

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@henryZrncik henryZrncik added the ready for merge Label for PRs which are ready for merge label Apr 29, 2024
@im-konge im-konge merged commit cd3705e into strimzi:main Apr 29, 2024
29 checks passed
@im-konge im-konge deleted the esnure-annotation branch April 29, 2024 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@see-quick see-quick see-quick approved these changes

@im-konge im-konge im-konge approved these changes

Assignees

@henryZrncik henryZrncik

Labels
ready for merge Label for PRs which are ready for merge System tests
Projects
None yet
Milestone
0.41.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@henryZrncik @see-quick @im-konge