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

Allow reconcilers to listen to leader promotion events #2688

Merged

Conversation

pierDipi
Copy link
Member

The current approach doesn't allow controllers to hook
additional logic when a reconciler is promoted for buckets,
only when it is demoted with DemoteFunc passed through
the controller.OptionsFn function.

In eventing, we have components running in the background
(aka detached to resource created/update/delete),
which should only take action when a given reconciler
is the leader (the autoscaler).

Fixes #2675

Release Note

Allow reconcilers to listen to leader promotion events 

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
@knative-prow knative-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 21, 2023
@knative-prow knative-prow bot requested review from KauzClay and skonto February 21, 2023 15:12
@knative-prow knative-prow bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 21, 2023
@pierDipi pierDipi changed the title [WIP] Allow reconcilers to listen to leader promotion events Allow reconcilers to listen to leader promotion events Feb 21, 2023
@knative-prow knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 21, 2023
@pierDipi
Copy link
Member Author

/cc @dprotaso

I couldn't find any unit test for this area, any ideas?

@knative-prow knative-prow bot requested a review from dprotaso February 22, 2023 13:30
@dprotaso
Copy link
Member

dprotaso commented Feb 22, 2023

I couldn't find any unit test for this area, any ideas?

Yeah - we at least have coverage that the code generation can compile (via prow/action checks) and we have downstream tests to catch breaking changes.

@dprotaso
Copy link
Member

/lgtm
/approve

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Feb 22, 2023
@knative-prow
Copy link

knative-prow bot commented Feb 22, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprotaso, pierDipi

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:

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

@knative-prow knative-prow bot merged commit 2fdd6bf into knative:main Feb 22, 2023
KauzClay pushed a commit to KauzClay/pkg that referenced this pull request Feb 24, 2023
* Generator: allow reconcilers to listen to leader promotion events

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

* Run hack/update-codegen.sh

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

---------

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
knative-prow bot pushed a commit that referenced this pull request Feb 24, 2023
* Allow reconcilers to listen to leader promotion events  (#2688)

* Generator: allow reconcilers to listen to leader promotion events

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

* Run hack/update-codegen.sh

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

---------

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

* run update-deps

---------

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
Co-authored-by: Pierangelo Di Pilato <pierdipi@redhat.com>
@pierDipi pierDipi deleted the KNATIVE-2675_promote_func_promote_event branch March 13, 2023 13:12
knative-prow bot pushed a commit to knative/eventing that referenced this pull request Mar 14, 2023
Fixes #6732 

The autoscaler runs in every controller replica [1], it tries
to scale down on every replica after the given refresh period,
and sometimes the 2 replicas don't agree on which value to use
for the new replicas since the state is lister/cache based,
leading to a too fast scale up or down behavior or sometime
not converging.
(also because of #6733)

Implementations should be using
knative/pkg#2675
for enabling leader-aware autoscaler. (PR
knative/pkg#2688)

[1]

https://github.com/knative/eventing/blob/1092472f440586099d6a5cbf1d3234bb36431af4/pkg/scheduler/statefulset/autoscaler.go#L85-L103

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
vishal-chdhry pushed a commit to vishal-chdhry/eventing that referenced this pull request Mar 14, 2023
Fixes knative#6732 

The autoscaler runs in every controller replica [1], it tries
to scale down on every replica after the given refresh period,
and sometimes the 2 replicas don't agree on which value to use
for the new replicas since the state is lister/cache based,
leading to a too fast scale up or down behavior or sometime
not converging.
(also because of knative#6733)

Implementations should be using
knative/pkg#2675
for enabling leader-aware autoscaler. (PR
knative/pkg#2688)

[1]

https://github.com/knative/eventing/blob/1092472f440586099d6a5cbf1d3234bb36431af4/pkg/scheduler/statefulset/autoscaler.go#L85-L103

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
vishal-chdhry pushed a commit to vishal-chdhry/eventing that referenced this pull request Apr 25, 2023
Fixes knative#6732 

The autoscaler runs in every controller replica [1], it tries
to scale down on every replica after the given refresh period,
and sometimes the 2 replicas don't agree on which value to use
for the new replicas since the state is lister/cache based,
leading to a too fast scale up or down behavior or sometime
not converging.
(also because of knative#6733)

Implementations should be using
knative/pkg#2675
for enabling leader-aware autoscaler. (PR
knative/pkg#2688)

[1]

https://github.com/knative/eventing/blob/1092472f440586099d6a5cbf1d3234bb36431af4/pkg/scheduler/statefulset/autoscaler.go#L85-L103

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow reconcilers to listen to leader promotion events
2 participants