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

Add kafka cluster create test #988

Merged
merged 74 commits into from
Jun 28, 2023
Merged

Add kafka cluster create test #988

merged 74 commits into from
Jun 28, 2023

Conversation

Kuvesz
Copy link
Contributor

@Kuvesz Kuvesz commented May 30, 2023 •
edited
Loading

Description

This PR adds a test case for creating a kafka cluster, to be more specific it is configurable which example we want to test with. By default simplekafkacluster is set.

Type of Change

  • Other (test)

Checklist

  • I have read the contributing guidelines
  • I have verified this change is not present in other open pull requests
  • All code style checks pass
  • New code contribution is covered by automated tests
  • All new and existing tests pass

@Kuvesz Kuvesz requested a review from a team as a code owner May 30, 2023 13:02
@CLAassistant
Copy link

CLAassistant commented May 30, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@Kuvesz Kuvesz marked this pull request as draft May 30, 2023 13:02
@Kuvesz Kuvesz force-pushed the test/de2e-koperator-simplekafkacluster branch from 4238237 to d681d09 Compare May 30, 2023 13:10
@Kuvesz Kuvesz mentioned this pull request May 30, 2023
Merged
2 tasks
@bartam1
Copy link
Contributor

bartam1 commented May 31, 2023 •
edited
Loading

I think we should do the zookeeper cluster renaming in different PR.

@Kuvesz
Copy link
Contributor Author

Kuvesz commented May 31, 2023

I think we should do the zookeeper cluster renaming in different PR.

I tend to agree, I was just a bit lazy because this PR would then have to be based on that one, which means another branch added to this testing stuff, I'd rather not bother if possible, though it would be nicer.

bartam1
bartam1 reviewed May 31, 2023
View reviewed changes
tests/e2e/k8s_test.go Outdated Show resolved Hide resolved
@Kuvesz Kuvesz requested a review from bartam1 May 31, 2023 09:30
bartam1
bartam1 reviewed May 31, 2023
View reviewed changes
tests/e2e/k8s_test.go Outdated Show resolved Hide resolved
@Kuvesz Kuvesz force-pushed the test/de2e-koperator-simplekafkacluster branch from 14977fd to 8d8cbb9 Compare June 21, 2023 15:03
panyuenlau
panyuenlau previously approved these changes Jun 21, 2023
View reviewed changes
Copy link
Member

@panyuenlau panyuenlau left a comment •
edited
Loading

Choose a reason for hiding this comment

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

looking sharp

bartam1
bartam1 requested changes Jun 21, 2023
View reviewed changes
tests/e2e/install_cluster.go Outdated Show resolved Hide resolved
tests/e2e/install_cluster.go Show resolved Hide resolved
tests/e2e/koperator.go Outdated Show resolved Hide resolved
tests/e2e/koperator.go Outdated Show resolved Hide resolved
tests/e2e/koperator.go Outdated Show resolved Hide resolved
tests/e2e/koperator.go Outdated Show resolved Hide resolved
tests/e2e/test_install_cluster.go Outdated Show resolved Hide resolved
tests/e2e/test_install_cluster.go Outdated Show resolved Hide resolved
tests/e2e/test_install_cluster.go Outdated Show resolved Hide resolved
@Kuvesz Kuvesz changed the title Add kafka cluster create test with our simplekafkacluster and ssl example and rename zookeeper cluster Add kafka cluster create test with our simplekafkacluster and ssl example Jun 22, 2023
@Kuvesz Kuvesz changed the title Add kafka cluster create test with our simplekafkacluster and ssl example Add kafka cluster create test Jun 22, 2023
pregnor
pregnor reviewed Jun 22, 2023
View reviewed changes
Copy link
Member

@pregnor pregnor left a comment

Choose a reason for hiding this comment

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

LGTM in general, left a couple comments.

pkg/resources/kafka/configmap_test.go Outdated Show resolved Hide resolved
tests/e2e/helm.go Outdated Show resolved Hide resolved
tests/e2e/k8s.go Outdated Show resolved Hide resolved
tests/e2e/koperator_suite_test.go Outdated
//testProduceConsumeInternal()
testUninstallZookeeperCluster()
testInstallZookeeperCluster(1)
testInstallKafkaCluster("local", "simplekafkacluster.yaml")
Copy link
Member

Choose a reason for hiding this comment

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

Required: IMO we MUST add both variants here.

tests/e2e/test_install_cluster.go Outdated Show resolved Hide resolved
tests/e2e/koperator.go Outdated Show resolved Hide resolved
@Kuvesz Kuvesz requested a review from pregnor June 22, 2023 12:37
pregnor
pregnor previously approved these changes Jun 22, 2023
View reviewed changes
Copy link
Member

@pregnor pregnor left a comment

Choose a reason for hiding this comment

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

LGTM, one little thingie, it can be done separately

tests/e2e/k8s.go Outdated Show resolved Hide resolved
@Kuvesz Kuvesz requested a review from pregnor June 22, 2023 12:45
pregnor
pregnor previously approved these changes Jun 23, 2023
View reviewed changes
bartam1
bartam1 approved these changes Jun 28, 2023
View reviewed changes
Copy link
Contributor

@bartam1 bartam1 left a comment

Choose a reason for hiding this comment

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

LGTM!

@Kuvesz Kuvesz merged commit 111348f into master Jun 28, 2023
@Kuvesz Kuvesz deleted the test/de2e-koperator-simplekafkacluster branch June 28, 2023 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bartam1 bartam1 bartam1 approved these changes

@pregnor pregnor pregnor approved these changes

@panyuenlau panyuenlau Awaiting requested review from panyuenlau

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Kuvesz @CLAassistant @bartam1 @pregnor @panyuenlau