remove orphaned subscribers from nats jetstream channels - #679
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #679 +/- ##
==========================================
- Coverage 26.69% 26.24% -0.46%
==========================================
Files 44 44
Lines 2772 2839 +67
==========================================
+ Hits 740 745 +5
- Misses 1974 2035 +61
- Partials 58 59 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/cherrypick release-1.18 |
|
@astelmashenko: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/assign @Cali0707 |
| subsUids.Insert(string(s.UID)) | ||
| } | ||
|
|
||
| orphanedSubs := subsUids.Difference(allSubsInNsUids) |
There was a problem hiding this comment.
I think we should have some sort of check for the type of Channel the subscription is referencing, as there are scenarios where users have multiple types of Channels (e.g. In Memory + NATS)
My concern is that we would be deleting too many subscriptions in this scenario
There was a problem hiding this comment.
@Cali0707 , it is difference, it works the next way:
Difference returns a set of objects that are not in s2. For example: s1 = {a1, a2, a3} s2 = {a1, a2, a4, a5} s1.Difference(s2) = {a3} s2.Difference(s1) = {a4, a5}
s1 in our case is Channel.Spec.Subscriptions, s2 is all sub in a namespace, so there is no issue if we take all subs in a namespace, making difference of UIDS we just remove all subs from s1 which are actually exist and leftovers are those do not exist. And we do not delete any CRDs, we only delete orphaned subscriptions from a Channel.Spec.Subscriptions and Channel.Status.Subscriptions.
|
@creydr , could you please skip codecov for this one? |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: astelmashenko, Cali0707 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@dprotaso , could you please skip codecov for this PR? |
|
/override "codecov/patch" you can adjust the settings here - https://github.com/knative-extensions/eventing-natss/blob/main/.codecov.yaml |
|
@dprotaso: Overrode contexts on behalf of dprotaso: codecov/patch DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
64abebf
into
knative-extensions:main
|
@astelmashenko: new pull request created: #685 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/cherrypick release-1.18 |
|
/cherrypick release-1.19 |
|
@astelmashenko: new pull request could not be created: failed to create pull request against knative-extensions/eventing-natss#release-1.18 from head knative-prow-robot:cherry-pick-679-to-release-1.18: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"A pull request already exists for knative-prow-robot:cherry-pick-679-to-release-1.18."}],"documentation_url":"https://docs.github.com/rest/pulls/pulls#create-a-pull-request","status":"422"} DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@astelmashenko: new pull request created: #686 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
There was an issues that sometime we observeved incosistency in actual subscribers CRDs and channels' list of subs. There were non-existent subs in channel.
Proposed Changes
This PR fixes that by checking list of subscription CRD against NastJetStreamChannel list of subscriptions.
Release Note