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

Fix topicId status update #10491

Merged
merged 2 commits into from
Sep 2, 2024
Merged

Fix topicId status update #10491

merged 2 commits into from
Sep 2, 2024

Conversation

fvaleri
Copy link
Contributor

@fvaleri fvaleri commented Aug 26, 2024

Type of change

Bugfix

Description

The KafkaTopic.status.topicId keeps the old value when the topic is recreated in Kafka while the reconciliation is paused/disabled, or the operator is not running.

This change adds the topicId when the KafkaTopic is managed and topicId is not set, and removes the topicId when KafkaTopic is paused or unmanaged.

Should close #10467.

Checklist

  • Write tests
  • Make sure all tests pass
  • Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
  • Reference relevant issue(s) and close them after merging

@fvaleri fvaleri added this to the 0.44.0 milestone Aug 26, 2024
@fvaleri fvaleri requested a review from tombentley August 26, 2024 06:50
@fvaleri
Copy link
Contributor Author

fvaleri commented Aug 26, 2024

@strimzi/maintainers can we run STs on this? Thanks.

@Frawless
Copy link
Member

/azp run regression

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@tombentley tombentley left a comment

Choose a reason for hiding this comment

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

Thanks @fvaleri

@fvaleri
Copy link
Contributor Author

fvaleri commented Aug 27, 2024

Failed test is unrelated:

[ERROR] io.strimzi.operator.cluster.operator.assembly.KafkaConnectorIT.testConnectorIsAutoRestarted(VertxTestContext)
[ERROR]   Run 1: KafkaConnectorIT.testConnectorIsAutoRestarted(VertxTestContext) java.lang.AssertionError: 
Expected: is "RESTARTING"
     but: was "UNASSIGNED"
[ERROR]   Run 2: KafkaConnectorIT.testConnectorIsAutoRestarted(VertxTestContext) java.lang.AssertionError: 
Expected: is "RESTARTING"
     but: was "UNASSIGNED"
[ERROR]   Run 3: KafkaConnectorIT.testConnectorIsAutoRestarted(VertxTestContext) java.lang.AssertionError: 
Expected: is "RESTARTING"
     but: was "UNASSIGNED"

@fvaleri
Copy link
Contributor Author

fvaleri commented Aug 27, 2024

/packit test --labels regression

@fvaleri
Copy link
Contributor Author

fvaleri commented Aug 27, 2024

@tombentley system tests looks mostly good with the exception of OpenTelemetryST, which is a known issue unrelated to this PR.

Let me know if you are good with latest changes.
If you are happy, I would say we can rebase/merge it after #10451.

Thanks.

@fvaleri
Copy link
Contributor Author

fvaleri commented Aug 28, 2024

/packit test --labels regression

The KafkaTopic.status.topicId keeps the old value when the topic is recreated in Kafka while the reconciliation is paused/disabled, or the operator is not running.

This change adds the topicId when the KafkaTopic is managed and topicId is not set, and removes the topicId when KafkaTopic is paused or unmanaged.

Signed-off-by: Federico Valeri <[email protected]>
Signed-off-by: Federico Valeri <[email protected]>
@fvaleri
Copy link
Contributor Author

fvaleri commented Aug 31, 2024

/packit test --labels regression

Copy link
Member

@tombentley tombentley 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 @fvaleri

@fvaleri
Copy link
Contributor Author

fvaleri commented Sep 2, 2024

Thanks, I think we are good to go.

@scholzj
Copy link
Member

scholzj commented Sep 2, 2024

/azp run regression

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@fvaleri
Copy link
Contributor Author

fvaleri commented Sep 2, 2024

Failed tests are unrelated and succeeded on packit.

@scholzj scholzj merged commit e7e8b11 into strimzi:main Sep 2, 2024
27 checks passed
@fvaleri fvaleri deleted the to-tid-recon branch September 3, 2024 07:09
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.

[Bug]: The KafkaTopic.status.topicId is never updated
4 participants