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

Allow dashes when parsing broker rack #68

Merged
merged 1 commit into from
Jun 28, 2023

Conversation

ctrlaltluc
Copy link

Description

This change allows for - (dash) in broker rack values when building the broker to rack map used when enabling parallel broker restarts.

Type of Change

  • Bug Fix

@ctrlaltluc ctrlaltluc self-assigned this Jun 28, 2023
@ctrlaltluc ctrlaltluc force-pushed the luciani/allow-dashes-in-broker-rack branch from 16b8306 to 73118d7 Compare June 28, 2023 09:47
@ctrlaltluc ctrlaltluc force-pushed the luciani/allow-dashes-in-broker-rack branch from 73118d7 to 63e24a0 Compare June 28, 2023 10:06
@amuraru
Copy link

amuraru commented Jun 28, 2023

the tests are failing though with there is no scale mock

Copy link

@dobrerazvan dobrerazvan left a comment

Choose a reason for hiding this comment

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

LGTM

@ctrlaltluc
Copy link
Author

the tests are failing though with there is no scale mock

@amuraru it is the flaky test again from kafkacluster_controller_externalnodeport_test, not related to this PR. It seems that 83868b8 did not fix it.

It is also odd that it is flaky only in our fork, not in upstream. Cannot replicate it on local either.

I will create a task for it, I already spent 1 day earlier this month on it, but could not find the root cause. :(

@ctrlaltluc ctrlaltluc merged commit c8446a6 into master Jun 28, 2023
@ctrlaltluc ctrlaltluc deleted the luciani/allow-dashes-in-broker-rack branch June 28, 2023 11:44
@ctrlaltluc
Copy link
Author

ctrlaltluc commented Jul 27, 2023

Found it in upstream too. That test is a flaky one not only in our repo.
https://github.com/banzaicloud/koperator/actions/runs/5681154322/job/15396691867?pr=1002

LE: See explanation for upstream failure here, seems the same as we've seen in #67, but curiously enough, the nodeports are still not deleted even with that explicit cleanup.

amuraru pushed a commit that referenced this pull request Dec 12, 2023
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.

5 participants