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

Add env var to disable creation of PodDisruptionBudget #10614

Merged
merged 4 commits into from
Sep 25, 2024
Merged

Add env var to disable creation of PodDisruptionBudget #10614

merged 4 commits into from
Sep 25, 2024

Conversation

katheris
Copy link
Contributor

@katheris katheris commented Sep 19, 2024 •
edited
Loading

Type of change

Select the type of your PR

  • Enhancement / new feature

Description

Add env var to disable creation of PodDisruptionBudget

Checklist

Please go through this checklist and make sure all applicable tasks have been done

  • Write tests
  • Make sure all tests pass
  • Update documentation
  • Check RBAC rights for Kubernetes / OpenShift roles
  • Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
  • Reference relevant issue(s) and close them after merging
  • Update CHANGELOG.md
  • Supply screenshots for visual changes, such as Grafana dashboards

@scholzj scholzj added this to the 0.44.0 milestone Sep 19, 2024
@katheris katheris marked this pull request as ready for review September 23, 2024 09:39
scholzj
scholzj reviewed Sep 23, 2024
View reviewed changes
Copy link
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

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

Some nits, but LGTM otherwise.

...perator/src/main/java/io/strimzi/operator/cluster/operator/assembly/ZooKeeperReconciler.java Outdated Show resolved Hide resolved
...er-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/KafkaReconciler.java Outdated Show resolved Hide resolved
...or/src/main/java/io/strimzi/operator/cluster/operator/assembly/AbstractAssemblyOperator.java Outdated Show resolved Hide resolved
scholzj
scholzj reviewed Sep 23, 2024
View reviewed changes
Copy link
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

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

Oh, one more thing ... can you add it to the docs as well ... probably here where the network policy env var is already documented? https://strimzi.io/docs/operators/latest/full/deploying.html#ref-operator-cluster-str

@katheris
Copy link
Contributor Author

Oh, one more thing ... can you add it to the docs as well ... probably here where the network policy env var is already documented? https://strimzi.io/docs/operators/latest/full/deploying.html#ref-operator-cluster-str

I chatted to Paul and added it in two places, there and in a section in the configuring part of the docs.

scholzj
scholzj approved these changes Sep 23, 2024
View reviewed changes
Copy link
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

@scholzj
Copy link
Member

scholzj commented Sep 23, 2024

/azp run regression

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

PaulRMellor
PaulRMellor approved these changes Sep 23, 2024
View reviewed changes
Copy link
Contributor

@PaulRMellor PaulRMellor left a comment

Choose a reason for hiding this comment

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

Great. Just one comment.

documentation/modules/configuring/proc-disable-pod-disruption-budget-generation.adoc

Strimzi generates pod disruption budget resources for Kafka, Kafka Connect worker, MirrorMaker2 worker, and Kafka Bridge worker nodes.

If you want to use custom pod disruption budget resources, you can set the `STRIMZI_POD_DISRUPTION_BUDGET_GENERATION` environment variable to `false` in the Cluster Operator configuration.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
If you want to use custom pod disruption budget resources, you can set the `STRIMZI_POD_DISRUPTION_BUDGET_GENERATION` environment variable to `false` in the Cluster Operator configuration.
If you don't want to use custom pod disruption budget resources, you can set the `STRIMZI_POD_DISRUPTION_BUDGET_GENERATION` environment variable to `false` in the Cluster Operator configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By "custom pod disruption budget resources" I meant if the user wants to provide their own, so for that they need to disable the generation, so I think the existing text is correct

ppatierno
ppatierno approved these changes Sep 25, 2024
View reviewed changes
Copy link
Member

@ppatierno ppatierno left a comment

Choose a reason for hiding this comment

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

LGTM

@scholzj
Copy link
Member

scholzj commented Sep 25, 2024

@katheris Could you please rebase it?

@katheris
Add env var to disable creation of PodDisruptionBudget
bce79c5 
Closes strimzi#6996

Signed-off-by: Katherine Stanley <[email protected]>
@katheris
Update Helm charts and CHANGELOG.md
3283929 
Signed-off-by: Katherine Stanley <[email protected]>
@katheris
Add docs
ad294fe 
Signed-off-by: Katherine Stanley <[email protected]>
@katheris
Review comments scholzj
23b6129 
Signed-off-by: Katherine Stanley <[email protected]>
@katheris
Copy link
Contributor Author

@katheris Could you please rebase it?

Done :)

@scholzj
Copy link
Member

scholzj commented Sep 25, 2024

/azp run regression

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scholzj
Copy link
Member

scholzj commented Sep 25, 2024

/azp run helm-acceptance

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scholzj scholzj merged commit 54a7661 into strimzi:main Sep 25, 2024
23 checks passed
@scholzj
Copy link
Member

scholzj commented Sep 25, 2024

Thanks for the PR @katheris

@scholzj scholzj mentioned this pull request Sep 25, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scholzj scholzj scholzj approved these changes

@ppatierno ppatierno ppatierno approved these changes

@PaulRMellor PaulRMellor PaulRMellor approved these changes

Assignees
No one assigned
Labels
None yet
Projects
Roadmap
Status: 0.44.0 (Work in progress)
Milestone
0.44.0
Development

Successfully merging this pull request may close these issues.
4 participants
@katheris @scholzj @ppatierno @PaulRMellor