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

Expose concurrent reconcile configuration flags in helm chart values #517

Merged

Conversation

a-hilaly
Copy link
Member

@a-hilaly a-hilaly commented Mar 5, 2024

This patch adds configuration options for reconciler concurrency tuning
(reconcileDefaultMaxConcurrency and reconcileResourceMaxConcurrency)
in the Helm chart values, allowing users to customize reconciliation concurrency
for ACK service controllers deployed via Helm charts.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link

ack-prow bot commented Mar 5, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@ack-prow ack-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 Mar 5, 2024
@ack-prow ack-prow bot requested review from jlbutler and jljaco March 5, 2024 03:44
@ack-prow ack-prow bot added the approved label Mar 5, 2024
@a-hilaly a-hilaly marked this pull request as ready for review March 5, 2024 03:44
@a-hilaly a-hilaly force-pushed the max-concurrent-reconciles branch 3 times, most recently from 02af30b to 78048d9 Compare March 5, 2024 08:44
@a-hilaly
Copy link
Member Author

a-hilaly commented Mar 5, 2024

/retest

@a-hilaly a-hilaly force-pushed the max-concurrent-reconciles branch 3 times, most recently from e990560 to 65c1db1 Compare March 5, 2024 11:03
@a-hilaly a-hilaly changed the title [WIP] Expose controlle-runtime MaxConcurrentReconcile parameter Expose concurrent reconcile configuration flags in helm chart values Mar 5, 2024
@ack-prow ack-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 Mar 5, 2024
@a-hilaly a-hilaly force-pushed the max-concurrent-reconciles branch from 65c1db1 to 381cbfe Compare March 5, 2024 11:05
@a-hilaly
Copy link
Member Author

a-hilaly commented Mar 5, 2024

/retest

1 similar comment
@a-hilaly
Copy link
Member Author

a-hilaly commented Mar 5, 2024

/retest

@a-hilaly a-hilaly force-pushed the max-concurrent-reconciles branch from 381cbfe to 27826c1 Compare March 6, 2024 11:57
@a-hilaly
Copy link
Member Author

a-hilaly commented Mar 6, 2024

/retest

1 similar comment
@a-hilaly
Copy link
Member Author

a-hilaly commented Mar 6, 2024

/retest

@@ -128,6 +128,12 @@ reconcile:
# An object representing the reconcile resync configuration for each specific resource.
resourceResyncPeriods: {}

# The default number of concurrent syncs that a reconciler can perform.
defaultMaxConcurrentSyncs: 5
Copy link

Choose a reason for hiding this comment

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

leftover from testing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yesir! thank you!

fixed :)

@a-hilaly a-hilaly force-pushed the max-concurrent-reconciles branch from 27826c1 to 9513780 Compare March 6, 2024 16:21
@jlbutler
Copy link

jlbutler commented Mar 6, 2024

/lgtm

@ack-prow ack-prow bot added the lgtm Indicates that a PR is ready to be merged. label Mar 6, 2024
Copy link

ack-prow bot commented Mar 6, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: a-hilaly, jlbutler

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

@a-hilaly
Copy link
Member Author

a-hilaly commented Mar 6, 2024

/retest

1 similar comment
@a-hilaly
Copy link
Member Author

a-hilaly commented Mar 6, 2024

/retest

@ack-prow ack-prow bot merged commit d5ae02a into aws-controllers-k8s:main Mar 6, 2024
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants