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

MGDSTRM-9050: expose generic mechanism for injecting additional broker configuration from the operand override #767

Merged
merged 3 commits into from
Jul 5, 2022

Conversation

k-wall
Copy link
Contributor

@k-wall k-wall commented Jul 1, 2022

Using this mechanism, we can override configuration passed to the broker by populating the brokerConfig object on the operator override. This will have utility for temporary feature flags as well as experimentation in dev. Note you can remove a broker config key by passing a null value.

oc edit cm -n redhat-managed-kafka-operator  strimzi-cluster-operator.v0.26.0-15
  fleetshard_operands.yaml: |
    canary:
      init:
        image: null
    admin-server:
      image: null
    kafka:
      brokerConfig:
        kas.policy.topic-config.topic-config-policy-enforced: "true" # Inject a new broker config item
        auto.create.topics.enable: ~ # Null causes an existing one to be removed.

and showing it works end to end:

% oc logs -f kafka-instance-kafka-0 | grep kas.policy.topic-config.topic-config-policy-enforced
kas.policy.topic-config.topic-config-policy-enforced=true

@k-wall k-wall requested review from MikeEdgar and showuon July 1, 2022 15:34
@github-actions github-actions bot added the operator changes related to operator label Jul 1, 2022
@k-wall k-wall added this to the 0.25.0 milestone Jul 1, 2022
Copy link
Contributor

@showuon showuon left a comment

Choose a reason for hiding this comment

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

Thanks for the feature. Overall LGTM. Left a comment. Also, I think we should put the PR description into somewhere, maybe README? wiki? Or...?

@k-wall
Copy link
Contributor Author

k-wall commented Jul 4, 2022

Thanks for the feature. Overall LGTM. Left a comment. Also, I think we should put the PR description into somewhere, maybe README? wiki? Or...?

I added a section to ADVANCED.md

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.

Happy for this to merge as is, my comments are optional.

Copy link
Contributor

@showuon showuon left a comment

Choose a reason for hiding this comment

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

Thanks for adding doc in ADVANCED.md. LGTM! Left a minor comment.

ADVANCED.md Outdated Show resolved Hide resolved
@k-wall
Copy link
Contributor Author

k-wall commented Jul 5, 2022

@MikeEdgar any comments from you?

Copy link
Contributor

@MikeEdgar MikeEdgar left a comment

Choose a reason for hiding this comment

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

LGTM

@k-wall k-wall merged commit 1ddde4c into bf2fc6cc711aee1a0c2a:main Jul 5, 2022
@k-wall k-wall deleted the MGDSTRM-9050 branch July 5, 2022 11:22
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