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 paused connector status when Connect is scaled to zero #9926

Merged
merged 2 commits into from
Apr 9, 2024

Conversation

fvaleri
Copy link
Contributor

@fvaleri fvaleri commented Apr 6, 2024

The common strategy across operators is that a paused resource should overwrite any error in terms of status update. This is currently not happening when a connector is paused after Connect is scaled to zero.

The connector is first updated to NotReady by connectorEventHandler, and then ReconciliationPaused by reconcileConnectors. The testConnectorResourceMetricsScaledToZero fails randmonly because of this double status update. Besides, the test is wrong, as it does not expect the connector to be paused.

This should fix #9843.

The common strategy across operators is that a paused resource should overwrite any error in terms of status update.
This is currently not happening when a connector is paused after Connect is scaled to zero.

The connector is first updated to `NotReady` by `connectorEventHandler`, and then `ReconciliationPaused` by `reconcileConnectors`.
The `testConnectorResourceMetricsScaledToZero` fails randmonly because of this double status update.
Besides, the test is wrong, as it does not expect the connector to be paused.

Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
@fvaleri fvaleri added this to the 0.41.0 milestone Apr 6, 2024
Copy link
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

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

One nit ... it seems to look good otherwise.

@scholzj
Copy link
Member

scholzj commented Apr 7, 2024

/azp run regression

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
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.

Thanks @fvaleri. Good job! 👍

@scholzj
Copy link
Member

scholzj commented Apr 7, 2024

/azp run regression

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

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

LGTM. I found more issues related to scale-to-zero while testing this, but they are not related to the changes from this PR (this Pr fixes what it promises to fix) and should be addressed separately.

Copy link
Contributor

@henryZrncik henryZrncik left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@ppatierno ppatierno 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!

@scholzj scholzj merged commit c8b5d9d into strimzi:main Apr 9, 2024
21 checks passed
@fvaleri fvaleri deleted the fix-paused-kctrs branch April 9, 2024 09:49
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.

Flaky test: ConnectorMockTest.testConnectorResourceMetricsScaledToZero
5 participants