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

Rely on the NATS cluster connection status when reporting the Eventing CR status #331

Conversation

marcobebway
Copy link
Contributor

@marcobebway marcobebway commented Dec 14, 2023

Description

Rely on the NATS cluster connection status when reporting the Eventing CR status.
As part of this PR, remove unsupported conditions from the Eventing Status. Because currently we preserve conditions even though it is removed in old releases.

Related issue(s)

@marcobebway marcobebway requested a review from a team as a code owner December 14, 2023 11:15
@kyma-bot kyma-bot added cla: yes Indicates the PR's author has signed the CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 14, 2023
@marcobebway marcobebway force-pushed the 267-eventing-readiness-based-on-nats-availability branch from e5a8405 to 1a355aa Compare December 14, 2023 11:16
@marcobebway marcobebway linked an issue Dec 14, 2023 that may be closed by this pull request
@marcobebway marcobebway self-assigned this Dec 14, 2023
@marcobebway marcobebway added area/eventing Issues or PRs related to eventing kind/bug Categorizes issue or PR as related to a bug. labels Dec 14, 2023
@marcobebway marcobebway force-pushed the 267-eventing-readiness-based-on-nats-availability branch 2 times, most recently from 601af6a to 4edf614 Compare December 14, 2023 16:23
@marcobebway
Copy link
Contributor Author

/hold

@kyma-bot kyma-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 14, 2023
@marcobebway
Copy link
Contributor Author

marcobebway commented Dec 15, 2023

Remaining tasks:

  • Fix failing tests.
  • Add more tests if needed.

@marcobebway marcobebway force-pushed the 267-eventing-readiness-based-on-nats-availability branch 4 times, most recently from 31c731c to 5bfa88a Compare January 15, 2024 12:55
@marcobebway marcobebway force-pushed the 267-eventing-readiness-based-on-nats-availability branch from 5bfa88a to 874c749 Compare January 18, 2024 10:33
@kyma-bot kyma-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 18, 2024
@marcobebway marcobebway force-pushed the 267-eventing-readiness-based-on-nats-availability branch from 4eecb1e to df439d7 Compare January 18, 2024 16:17
internal/connection/nats/interface.go Outdated Show resolved Hide resolved
RegisterReconnectHandlerIfNotRegistered(natsio.ConnHandler)

// RegisterDisconnectErrHandlerIfNotRegistered registers a DisconnectErrHandler only if it was not registered before.
RegisterDisconnectErrHandlerIfNotRegistered(natsio.ConnErrHandler)
Copy link
Contributor

Choose a reason for hiding this comment

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

(nitpick): Can we shorten the name to RegisterDisconnectErrHandler? The if not registered can be documented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

) error {
// check if the backend was changed.
if !eventing.IsSpecBackendTypeChanged() {
if eventingCR.IsPreviousBackendEmpty() || !eventingCR.IsSpecBackendTypeChanged() {
Copy link
Contributor

@muralov muralov Jan 26, 2024

Choose a reason for hiding this comment

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

👍🏼

internal/controller/operator/eventing/controller.go Outdated Show resolved Hide resolved
@marcobebway marcobebway force-pushed the 267-eventing-readiness-based-on-nats-availability branch from 3195dcb to 7c9d981 Compare January 26, 2024 15:39
@kyma-bot kyma-bot added the lgtm Looks good to me! label Jan 29, 2024
@marcobebway
Copy link
Contributor Author

/hold cancel

@kyma-bot kyma-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 29, 2024
@kyma-bot kyma-bot merged commit f4a813e into kyma-project:main Jan 29, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/eventing Issues or PRs related to eventing cla: yes Indicates the PR's author has signed the CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm Looks good to me! size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NATS-Cr in in deletion state results in eventing-cr in error state
3 participants