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

Configure new version of the quota plugin #719

Merged
merged 3 commits into from
May 12, 2022
Merged

Configure new version of the quota plugin #719

merged 3 commits into from
May 12, 2022

Conversation

k-wall
Copy link
Contributor

@k-wall k-wall commented Apr 27, 2022

The new quota plugin requires additional configuration options. This PR passes those options. The new configuration options will be ignored by the older version of the plugin, so will be safe for older bundle versions.

This PR also let's you override the kafka/zookeeper images via either the configmap or fleetshard environment variables. Just set a env var on the deployment like this and it will override the image provided by the rhosak bundle.

 oc set env deployment -n  redhat-kas-fleetshard-operator kas-fleetshard-operator IMAGE_KAFKA=quay.io/k_wall/kafka-30@sha256:098303da416359ae1ff6509af047ae9af004015e91f57f59d0b37ddbb8da8f80 

@k-wall k-wall marked this pull request as draft April 27, 2022 16:03
@github-actions github-actions bot added the operator changes related to operator label Apr 27, 2022
@k-wall k-wall marked this pull request as ready for review May 9, 2022 08:49
@k-wall k-wall changed the title WIP: config new quota plugin Configure new version of the quota plugin May 9, 2022
@k-wall k-wall added this to the 0.23.0 milestone May 9, 2022
@k-wall k-wall requested a review from grdryn May 11, 2022 07:53
@k-wall k-wall requested a review from MikeEdgar May 11, 2022 10:47
Comment on lines +622 to +626
KafkaInstanceConfiguration.Kafka kafka = instanceConfig.getKafka();
config.put("client.quota.callback.usageMetrics.topic", String.valueOf(kafka.getQuotaCallbackUsageMetricsTopic()));
config.put("client.quota.callback.quotaPolicy.check-interval", String.valueOf(kafka.getQuotaCallbackQuotaPolicyCheckInterval()));
config.put("client.quota.callback.kafka.clientIdPrefix", String.valueOf(kafka.getQuotaCallbackQuotaKafkaClientIdPrefix()));
config.put("client.quota.callback.static.storage.check-interval", String.valueOf(instanceConfig.getStorage().getCheckInterval()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to mention it, the OperandOverride class supports additionalProperties that can be used to configure properties specific to a Strimzi bundle deployment version.

That can be leveraged to only apply new/changed properties alongside the image updates they align with. I think ultimately we'll want to use that (or something like it) to extend support for blast-radius control to include configuration as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks - that's useful. In this case, the new config is just adds so it is harmless to pass new config to the old plugin.

@k-wall k-wall requested a review from SamBarker May 11, 2022 20:55
@k-wall
Copy link
Contributor Author

k-wall commented May 11, 2022

@SamBarker can you re-review, once you are happy I believe this is good to merge.

Copy link
Contributor

@SamBarker SamBarker left a comment

Choose a reason for hiding this comment

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

LGTM

@MikeEdgar MikeEdgar merged commit afae486 into bf2fc6cc711aee1a0c2a:main May 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
operator changes related to operator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants