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.

Sign up for GitHub

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

Jump to bottom

[v0.21] [Consolidated Channel] Backport of #487 #497

Merged
merged 3 commits into from Apr 9, 2021
Merged

[v0.21] [Consolidated Channel] Backport of #487 #497

merged 3 commits into from Apr 9, 2021

Conversation

devguyio
Copy link
Contributor

@devguyio devguyio commented Apr 1, 2021

Proposed Changes

Release Note

🐛 A bug fix was back ported to the consolidated channel so that subscriptions becomes eventually ready even when dispatchers scale up or down.

@devguyio
[Consolidated Channel] Update target pod IPs when dispatcher pods sca…
c5472ad 
…le before the subscription is marked ready (knative-extensions#487)

* Restart probing dispatchers if the pods change before being ready

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* fix when to return subscribers reconciled

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Proper locking in status prober

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Add a debug log when dispatcher pods change

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Refresh pods probing on dispatcher pod changes

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* change some bad var names

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Add prober test for succeeding only after refreshing pod probing

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Update struct creation after unused field removed

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Reuse same handler for pod informer

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Fix a couple of typos

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>
@google-cla google-cla bot added the cla: yes Indicates the PR's author has signed the CLA. label Apr 1, 2021
@knative-prow-robot knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 1, 2021
@codecov
Copy link

codecov bot commented Apr 1, 2021 •
edited

Codecov Report

❗ No coverage uploaded for pull request base (release-0.21@3d0fa6d). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##             release-0.21     #497   +/-   ##
===============================================
  Coverage                ?   73.00%           
===============================================
  Files                   ?      131           
  Lines                   ?     5157           
  Branches                ?        0           
===============================================
  Hits                    ?     3765           
  Misses                  ?     1158           
  Partials                ?      234

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d0fa6d...624f62b. Read the comment docs.

@matzew
Copy link
Contributor

matzew commented Apr 7, 2021

Ah... upgrade test ... I guess, we can ignore that here, on this branch, @cardil ?

@devguyio
Copy link
Contributor Author

devguyio commented Apr 7, 2021

pull-knative-sandbox-eventing-kafka-upgrade-tests fails cause they're not part of this branch.

@devguyio
Copy link
Contributor Author

devguyio commented Apr 7, 2021

I guess, we can ignore that here, on this branch

yep.

matzew
matzew previously approved these changes Apr 7, 2021
View reviewed changes
Copy link
Contributor

@matzew matzew left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@knative-prow-robot knative-prow-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Apr 7, 2021
@knative-prow-robot knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Apr 9, 2021
slinkydeveloper
slinkydeveloper approved these changes Apr 9, 2021
View reviewed changes
Copy link
Contributor

@slinkydeveloper slinkydeveloper left a comment

Choose a reason for hiding this comment

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

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 9, 2021
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: matzew, slinkydeveloper

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [matzew,slinkydeveloper]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow-robot knative-prow-robot merged commit fece5fa into knative-extensions:release-0.21 Apr 9, 2021
matzew pushed a commit to matzew/eventing-kafka that referenced this pull request May 4, 2021
@devguyio @matzew
[v0.21] [Consolidated Channel] Backport of knative-extensions#487 (kn…
3641f1f 
…ative-extensions#497)

* [Consolidated Channel] Update target pod IPs when dispatcher pods scale before the subscription is marked ready  (knative-extensions#487)

* Restart probing dispatchers if the pods change before being ready

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* fix when to return subscribers reconciled

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Proper locking in status prober

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Add a debug log when dispatcher pods change

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Refresh pods probing on dispatcher pod changes

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* change some bad var names

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Add prober test for succeeding only after refreshing pod probing

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Update struct creation after unused field removed

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Reuse same handler for pod informer

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Fix a couple of typos

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Update codegen

* Adding dummy e2e-upgrade-test
openshift-merge-robot referenced this pull request in openshift-knative/eventing-kafka May 4, 2021
@matzew @devguyio
[v0.21] [Consolidated Channel] Backport of #487 (#497) (#190)
b8b7ebb 
* [Consolidated Channel] Update target pod IPs when dispatcher pods scale before the subscription is marked ready  (#487)

* Restart probing dispatchers if the pods change before being ready

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* fix when to return subscribers reconciled

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Proper locking in status prober

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Add a debug log when dispatcher pods change

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Refresh pods probing on dispatcher pod changes

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* change some bad var names

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Add prober test for succeeding only after refreshing pod probing

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Update struct creation after unused field removed

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Reuse same handler for pod informer

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Fix a couple of typos

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>

* Update codegen

* Adding dummy e2e-upgrade-test

Co-authored-by: Ahmed Abdalla Abdelrehim <aabdelre@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slinkydeveloper slinkydeveloper slinkydeveloper approved these changes

@matzew matzew matzew left review comments

@lberk lberk Awaiting requested review from lberk

@lionelvillard lionelvillard Awaiting requested review from lionelvillard

Assignees

@matzew matzew

@slinkydeveloper slinkydeveloper

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@devguyio @matzew @knative-prow-robot @slinkydeveloper