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.

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

[system test] [doc] cruise control package #10615

Merged
merged 19 commits into from
Sep 24, 2024

Conversation

see-quick
Copy link
Member

@see-quick see-quick commented Sep 19, 2024

Type of change

  • Enhancement / new feature
  • Refactoring

Description

This PR adds system test documentation to the cruise control package.

Checklist

  • Write tests
  • Make sure all tests pass

Signed-off-by: see-quick <maros.orsak159@gmail.com>
Signed-off-by: see-quick <maros.orsak159@gmail.com>
Signed-off-by: see-quick <maros.orsak159@gmail.com>
@see-quick see-quick self-assigned this Sep 19, 2024
@see-quick see-quick requested a review from a team September 19, 2024 18:12
@see-quick see-quick added this to the 0.44.0 milestone Sep 19, 2024
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.

Few comments, thanks for the PR

Comment on lines 161 to 173
@TestDoc(
description = @Desc("Test the Cruise Control functionality when API security is disabled."),
steps = {
@Step(value = "Initialize test storage and resource manager objects", expected = "Test storage and resource manager objects are created"),
@Step(value = "Create Kafka Node Pools with 3 brokers each", expected = "Kafka Node Pools are successfully created and ready"),
@Step(value = "Create a Kafka cluster with Cruise Control having API security disabled", expected = "Kafka cluster with Cruise Control and no API security is deployed"),
@Step(value = "Create a Kafka Rebalance resource", expected = "Kafka Rebalance resource is successfully created"),
@Step(value = "Wait for the Kafka Rebalance custom resource state to become ProposalReady", expected = "Kafka Rebalance custom resource state is ProposalReady")
},
labels = {
@Label(value = "cruise-control"),
}
)
Copy link
Member

Choose a reason for hiding this comment

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

I guess it's not for this PR, but what benefit this test brings in comparison with the one in the CruiseControlApiST?

Copy link
Member Author

Choose a reason for hiding this comment

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

I mean, we can possibly remove such test case in CruiseControlST, because it seems it does less then test in CruiseControlApiST IMHO. So I can create different PR to removing such test?

Signed-off-by: see-quick <maros.orsak159@gmail.com>
Copy link
Contributor

@PaulRMellor PaulRMellor 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, Maros. I left a few suggestions for consistency and a couple of questions where I wasn't sure about the description.

see-quick and others added 13 commits September 20, 2024 13:00
…ol.CruiseControlApiST.md

Co-authored-by: PaulRMellor <47596553+PaulRMellor@users.noreply.github.com>
Signed-off-by: Maros Orsak <maros.orsak159@gmail.com>
…ol.CruiseControlConfigurationST.md

Co-authored-by: PaulRMellor <47596553+PaulRMellor@users.noreply.github.com>
Signed-off-by: Maros Orsak <maros.orsak159@gmail.com>
…ol.CruiseControlApiST.md

Co-authored-by: PaulRMellor <47596553+PaulRMellor@users.noreply.github.com>
Signed-off-by: Maros Orsak <maros.orsak159@gmail.com>
…ol.CruiseControlConfigurationST.md

Co-authored-by: PaulRMellor <47596553+PaulRMellor@users.noreply.github.com>
Signed-off-by: Maros Orsak <maros.orsak159@gmail.com>
…ol.CruiseControlConfigurationST.md

Co-authored-by: PaulRMellor <47596553+PaulRMellor@users.noreply.github.com>
Signed-off-by: Maros Orsak <maros.orsak159@gmail.com>
…ol.CruiseControlST.md

Co-authored-by: PaulRMellor <47596553+PaulRMellor@users.noreply.github.com>
Signed-off-by: Maros Orsak <maros.orsak159@gmail.com>
…ol.CruiseControlST.md

Co-authored-by: PaulRMellor <47596553+PaulRMellor@users.noreply.github.com>
Signed-off-by: Maros Orsak <maros.orsak159@gmail.com>
…ol.CruiseControlST.md

Co-authored-by: PaulRMellor <47596553+PaulRMellor@users.noreply.github.com>
Signed-off-by: Maros Orsak <maros.orsak159@gmail.com>
…ol.CruiseControlST.md

Co-authored-by: PaulRMellor <47596553+PaulRMellor@users.noreply.github.com>
Signed-off-by: Maros Orsak <maros.orsak159@gmail.com>
…ol.CruiseControlST.md

Co-authored-by: PaulRMellor <47596553+PaulRMellor@users.noreply.github.com>
Signed-off-by: Maros Orsak <maros.orsak159@gmail.com>
Signed-off-by: see-quick <maros.orsak159@gmail.com>
Signed-off-by: see-quick <maros.orsak159@gmail.com>
…notation

Signed-off-by: see-quick <maros.orsak159@gmail.com>
Copy link
Contributor

@PaulRMellor PaulRMellor left a comment

Choose a reason for hiding this comment

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

LGTM. Couple of places with periods.

@see-quick
Copy link
Member Author

LGTM. Couple of places with periods.

Thanks updated :)

Signed-off-by: see-quick <maros.orsak159@gmail.com>
Signed-off-by: see-quick <maros.orsak159@gmail.com>
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 :)

@Step(value = "Deploy the cluster operator", expected = "Cluster operator is deployed")
},
labels = {
@Label(value = "cruise-control"),
Copy link
Member

Choose a reason for hiding this comment

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

I just got idea -> not for this PR, as you have it already hardcoded everywhere -> but what about creating some TestDocsLabels where all these values will be stored - then you can just use it everywhere else instead of copy-pasting it every time.

Copy link
Member Author

Choose a reason for hiding this comment

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

I just got idea -> not for this PR, as you have it already hardcoded everywhere -> but what about creating some TestDocsLabels where all these values will be stored - then you can just use it everywhere else instead of copy-pasting it every time.

TestTags help with that idea I assume :D; Sure +1 it's great. I will create a PR with such a change.

@see-quick see-quick added ready for merge Label for PRs which are ready for merge and removed needs review labels Sep 24, 2024
@see-quick see-quick merged commit a09c20e into strimzi:main Sep 24, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for merge Label for PRs which are ready for merge System tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants