Skip to content

feat: pause reconciliation#1111

Merged
hemahg merged 14 commits intomainfrom
pause-reconciliation
Oct 28, 2024
Merged

feat: pause reconciliation#1111
hemahg merged 14 commits intomainfrom
pause-reconciliation

Conversation

@hemahg
Copy link
Contributor

@hemahg hemahg commented Oct 10, 2024

feat: pause reconciliation

Design link

@MikeEdgar MikeEdgar added this to the 0.4.0 milestone Oct 11, 2024
@hemahg hemahg force-pushed the pause-reconciliation branch 2 times, most recently from bc39f2e to 4e79f09 Compare October 16, 2024 10:21
@hemahg hemahg changed the title [WIP] feat: pause reconciliation feat: pause reconciliation Oct 16, 2024
@hemahg hemahg force-pushed the pause-reconciliation branch 2 times, most recently from 5d542b9 to 6d69e2b Compare October 16, 2024 10:39
@MikeEdgar
Copy link
Member

@hemahg did everything work as expected with #1103 ? If it is fine, we can merge that then rebase this one. That should make it easier to verify everything working together. Wdyt?

@MikeEdgar MikeEdgar force-pushed the pause-reconciliation branch 2 times, most recently from 92b817c to 18d0ca0 Compare October 17, 2024 10:32
@hemahg hemahg force-pushed the pause-reconciliation branch from 18d0ca0 to e3125a2 Compare October 23, 2024 07:37
Copy link
Member

@MikeEdgar MikeEdgar left a comment

Choose a reason for hiding this comment

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

At root page of the application I am seeing repeated HTTP 400 errors. It looks like the UI is calling getKafkaCluster to fetch a single cluster with a blank/empty kafkaId.

Comment on lines +250 to +251
{isReconciliationPaused && (
<DataListItem aria-labelledby={`reconciliation-warning`}>
Copy link
Member

Choose a reason for hiding this comment

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

The messages array already contains a condition for ReconciliationPaused. Can we use that instead of adding an additional row? It results in the generated row plus the one that Strimzi gives us for ReconciliationPaused.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

hemahg and others added 13 commits October 25, 2024 16:36
Signed-off-by: hemahg <hhg@redhat.com>
Signed-off-by: hemahg <hhg@redhat.com>
Signed-off-by: hemahg <hhg@redhat.com>
Signed-off-by: hemahg <hhg@redhat.com>
Signed-off-by: hemahg <hhg@redhat.com>
Signed-off-by: hemahg <hhg@redhat.com>
Signed-off-by: hemahg <hhg@redhat.com>
Signed-off-by: Michael Edgar <medgar@redhat.com>
Signed-off-by: hemahg <hhg@redhat.com>
Signed-off-by: hemahg <hhg@redhat.com>
Signed-off-by: hemahg <hhg@redhat.com>
Signed-off-by: hemahg <hhg@redhat.com>
Signed-off-by: hemahg <hhg@redhat.com>
@hemahg hemahg force-pushed the pause-reconciliation branch from 2b71e2f to a839cf5 Compare October 25, 2024 11:06
Copy link
Member

@MikeEdgar MikeEdgar left a comment

Choose a reason for hiding this comment

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

One more fix, then good with me. Thanks @hemahg

There is another improvement that can be made in a follow-up PR to hide the pause button when a Kafka cluster's .meta.managed field is false. These Kafka clusters do not have a (known) Kafka CR so we can't pause/resume them.

Comment on lines +40 to +47
{isValidKafkaId ? (
<ReconciliationProvider kafkaId={kafkaId}>
<ReconciliationPausedBanner kafkaId={kafkaId} />
<ClusterDrawer>{children}</ClusterDrawer>
</ReconciliationProvider>
) : (
<></>
)}
Copy link
Member

Choose a reason for hiding this comment

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

This change allows the list of multiple Kafkas to still appear, but blocks the banner if there is no kafkaId.

Suggested change
{isValidKafkaId ? (
<ReconciliationProvider kafkaId={kafkaId}>
<ReconciliationPausedBanner kafkaId={kafkaId} />
<ClusterDrawer>{children}</ClusterDrawer>
</ReconciliationProvider>
) : (
<></>
)}
<ReconciliationProvider kafkaId={kafkaId ?? ""}>
{isValidKafkaId && (
<ReconciliationPausedBanner kafkaId={kafkaId} />
)}
<ClusterDrawer>{children}</ClusterDrawer>
</ReconciliationProvider>

@MikeEdgar MikeEdgar added the ui label Oct 25, 2024
Signed-off-by: hemahg <hhg@redhat.com>
@sonarqubecloud
Copy link

@hemahg hemahg merged commit 90af4d1 into main Oct 28, 2024
@hemahg hemahg deleted the pause-reconciliation branch October 28, 2024 06:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants