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

events: Ensure pipelines are cleaned up on closing subscription #23042

Merged
merged 2 commits into from
Sep 13, 2023

Conversation

tomhjp
Copy link
Contributor

@tomhjp tomhjp commented Sep 13, 2023

I noticed this while reviewing #23024. It seems the broker field was never populated, so we never cleaned up the pipeline on close. However, we only need one method from the broker, so I narrowed the field down to one function as well.

@tomhjp tomhjp added bug Used to indicate a potential bug eventbus hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed backport/1.15.x labels Sep 13, 2023
@tomhjp tomhjp added this to the 1.15.0 milestone Sep 13, 2023
@github-actions
Copy link

Build Results:
All builds succeeded! ✅

@github-actions
Copy link

github-actions bot commented Sep 13, 2023

CI Results:
All Go tests succeeded! ✅

Copy link
Contributor

@peteski22 peteski22 left a comment

Choose a reason for hiding this comment

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

LGTM 😄

@tomhjp
Copy link
Contributor Author

tomhjp commented Sep 13, 2023

The test failures were genuine: https://github.com/hashicorp/vault/actions/runs/6171703869/job/16750516454

The formatter node was never getting deregistered from the broker before, but now it could be if every last subscription is cancelled, so I moved registering it inside the subscribe function. It will be re-registered if there is already an existing subscription, but the broker allows us to do that unless we configure it not to.

Copy link
Contributor

@swenson swenson left a comment

Choose a reason for hiding this comment

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

LGTM

@tomhjp
Copy link
Contributor Author

tomhjp commented Sep 13, 2023

Thanks!

@tomhjp tomhjp merged commit 8e7c6e8 into main Sep 13, 2023
108 checks passed
@tomhjp tomhjp deleted the events-cleanup-pipeline-on-close branch September 13, 2023 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to indicate a potential bug eventbus hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants