feat: Add Pod Disruption Budget support to Helm chart #2380

bmlyr · 2025-08-18T22:17:11Z

Add PodDisruptionBudget support to Helm chart (https://kubernetes.io/docs/tasks/run-application/configure-pdb/)
This PR should resolve #2345

adutra

Hi @bmlyr thank you for this contribution!

Do you mind running helm-docs --chart-search-root=helm and committing the result?

If you need to install helm-docs:

https://github.com/norwoodj/helm-docs

Thanks! 🙏

helm/polaris/values.yaml

adutra · 2025-08-19T16:48:59Z

@bmlyr I think you need to rebase your PR, there is some strange compilation failure right now.

In apache#2369 Iceberg table federation was refactored around the new `IcebergRESTExternalCatalogFactory` type based on discussion in the community sync. This has unblocked the ability to federate to more non-Iceberg catalogs, such as in apache#2355. This PR refactors generic table federation to go through the same mechanism. After this, we can go through and implement generic table federation for the existing `IcebergRESTExternalCatalogFactory` implementations.

Classes with a `CallContext` field should call `getRealmConfig` once and store it as a field as well. The idea is that long term we would want to stop relying on the `CallContext` itself but instead inject its individual items. Thus we also add `RealmConfig` to `TestServices`.

…ernal property (apache#2339)

…r-servlet-jakarta to v1.4.1 (apache#2377)

Updates S3Mock testcontainer dependency from 3.11.0 to 4.7.0 and refactors usage into a centralized wrapper class in runtime/test-common. Changes Upgraded S3Mock testcontainer to 4.7.0 Created S3Mock wrapper class for consistent configuration Consolidated S3 config properties generation Updated integration tests to use new wrapper No functional changes to test behavior.

…apache#2387)

…kts (apache#2385)

…he#2386)

This change promotes `CatalogConfig` and `RestCatalogConfig` to top-level, public annotations and introduces a few "hooks" in `PolarisRestCatalogIntegrationBase` that can be overridden by subclasses. This change is a preparatory work for apache#2280 (S3 remote signing).

similar to 7af85be we should prefer the existing helper methods on the entity instead

- fixed dead link to catalog definition in Iceberg docs on Entities page - removed single quotes from credential parameter in the cmdline example for connecting a local spark-sql: env variables need to be resolved in cmdline, they will not be resolved by spark-sql itself.

bmlyr · 2025-08-19T20:00:31Z

@adutra Might be better now

…ruptionbudget_test.yaml no more default for this value

adutra · 2025-08-20T11:16:16Z

helm/polaris/tests/poddisruptionbudget_test.yaml

+    set:
+      podDisruptionBudget.enabled: true
+      podDisruptionBudget.minAvailable: 2
+      podDisruptionBudget.maxUnavailable: null


Explicitly unsetting this is not necessary anymore, but it doesn't hurt to keep this line either.

adutra · 2025-08-20T11:17:02Z

helm/polaris/tests/poddisruptionbudget_test.yaml

+            app.kubernetes.io/instance: polaris-release
+
+  # validation tests
+  - it: should handle both minAvailable and maxUnavailable set (minAvailable takes precedence)


This would be rather a pathological case imho, but OK to test it.

Apparently, this would fail to apply:

one or more objects failed to apply, reason: PodDisruptionBudget.policy [...] is invalid: [...] minAvailable and maxUnavailable cannot be both set.

Should we prevent the user from doing this? Maybe by selecting only minAvailable?

Maybe? We can also fail hard e.g.

{{- if and .Values.podDisruptionBudget.minAvailable .Values.podDisruptionBudget.maxUnavailable }} {{- fail "podDisruptionBudget.minAvailable and podDisruptionBudget.maxUnavailable cannot be both set." -}} {{- end }}

I made a few changes to avoid that. Let me know if you prefer this version

You're right, failing is a better option

Might be better now

Very nice, thank you 👍

…f minAvailable and maxUnavailable

…od disruption budget

adutra · 2025-08-20T12:14:34Z

Thank you for your contribution @bmlyr !

* feat: Add Pod Disruption Budget support to Helm chart (apache#2380) * chore(deps): update quay.io/keycloak/keycloak docker tag to v26.3.3 (apache#2407) * Mention Helm chart support for PodDisruptionBudget in CHANGELOG.md (apache#2408) * chore: Suppress javac deprecation warnings in SparkCatalog (apache#2394) SparkCatalog intentionally overrides and uses deprecated methods from Spark's TableCatalog. This PR adds suppression annotations to allow for clean compilation given that the deprecated method calls and overrides are clearly expected in this case. * Python client auto generate (apache#2192) * Python client auto generate * Python client auto generate * Python client auto generate * Python client auto generate * Python client auto generate * Python client auto generate * Remove auto generated doc * undo * Fix doc * Fix docker ref from CONTAINER_TOOL to DOCKER * Add client help manual to GH action * Add missing region to MinIO getting-started example (apache#2411) The example was missing an AWS region, thus causing Spark to fail with: ``` spark-sql ()> create table ns.t1 as select 'abc'; 25/08/20 16:25:06 ERROR Executor: Exception in task 0.0 in stage 0.0 (TID 0) software.amazon.awssdk.core.exception.SdkClientException: Unable to load region from any of the providers in the chain software.amazon.awssdk.regions.providers.DefaultAwsRegionProviderChain@47578c86: [software.amazon.awssdk.regions.providers.SystemSettingsRegionProvider@1656f847: Unable to load region from system settings. Region must be specified either via environment variable (AWS_REGION) or system property (aws.region)., software.amazon.awssdk.regions.providers.AwsProfileRegionProvider@2bbaabe3: No region provided in profile: default, software.amazon.awssdk.regions.providers.InstanceProfileRegionProvider@54b1cfd8: Unable to contact EC2 metadata service.] ... at org.apache.iceberg.aws.AwsClientFactories$DefaultAwsClientFactory.s3(AwsClientFactories.java:119) at org.apache.iceberg.aws.s3.S3FileIO.client(S3FileIO.java:391) at org.apache.iceberg.aws.s3.S3FileIO.newOutputFile(S3FileIO.java:193) ``` * Add feature config to allow dropping views without purging (apache#2369) * Add feature config to allow dropping views without purging With tables, the client can decide whether to purge the table on drop or not. However, Polaris Servers used to unconditionally perform the purge on dropping a view. After apache#1619 that behaviour effectively prevents dropping views if the admin user does not set `DROP_WITH_PURGE_ENABLED`. The latter, though, is not currently advisable per apache#1617. This change introduces a new feature configuration (`PURGE_VIEWS_ON_DROP`) that allows the admin user to instruct Polaris servers to drop views without purging to achieve operational parity with tables. Fixes apache#2367 * review: rename to PURGE_VIEW_METADATA_ON_DROP * review: re-fix description * Last merged commit c97b150 --------- Co-authored-by: Bryan Maloyer <bryan.mlyr@gmail.com> Co-authored-by: Mend Renovate <bot@renovateapp.com> Co-authored-by: Alexandre Dutra <adutra@apache.org> Co-authored-by: Dmitri Bourlatchkov <dmitri.bourlatchkov@gmail.com> Co-authored-by: Yong Zheng <yongzheng0809@gmail.com>

bmlyr added 2 commits August 19, 2025 00:09

feat: Add Pod Disruption Budget configuration to Helm chart

2d8eebc

fix: Add missing newline at end of poddisruptionbudget.yaml

e7252f2

github-project-automation bot added this to Basic Kanban Board Aug 18, 2025

github-project-automation bot moved this to PRs In Progress in Basic Kanban Board Aug 18, 2025

feat: Add unit tests for Pod Disruption Budget configuration

a6d8916

adutra reviewed Aug 19, 2025

View reviewed changes

helm/polaris/values.yaml Outdated Show resolved Hide resolved

eric-maynard and others added 15 commits August 19, 2025 21:43

Update community meeting dates (apache#2382)

93788ec

Python client: make S3 role-ARN optional and add missing endpoint-int…

0de2204

…ernal property (apache#2339)

fix(deps): update dependency io.prometheus:prometheus-metrics-exporte…

d25393c

…r-servlet-jakarta to v1.4.1 (apache#2377)

Nit: extract getResolvedCatalogEntity method in IcebergCatalogHandler (…

0f7c8b9

…apache#2387)

Nit: remove transitive dependencies from runtime/server/build.gradle.…

b083b49

…kts (apache#2385)

Nit: add methods isExternal and isStaticFacade to CatalogEntity (apac…

b0fa74b

…he#2386)

Remove BaseMetaStoreManager.serializeProperties (apache#2374)

923ef45

similar to 7af85be we should prefer the existing helper methods on the entity instead

Merge branch 'apache:main' into helm-pdb

37e678e

fix: Update maxUnavailable value to allow for undefined configuration

89c2c3f

feat: Add pod disruption budget configuration to README with helm-docs

9943594

bmlyr added 2 commits August 19, 2025 22:02

fix: Add missing newline at end of poddisruptionbudget_test.yaml

e4da1db

fix: Remove redundant test cases for default maxUnavailable in poddis…

fe4aa04

…ruptionbudget_test.yaml no more default for this value

adutra previously approved these changes Aug 20, 2025

View reviewed changes

github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Aug 20, 2025

fix: Update pod disruption budget logic to prevent simultaneous use o…

6b1cb4b

…f minAvailable and maxUnavailable

bmlyr dismissed adutra’s stale review via 6b1cb4b August 20, 2025 11:38

fix: Prevent simultaneous use of minAvailable and maxUnavailable in p…

c348bf0

…od disruption budget

adutra approved these changes Aug 20, 2025

View reviewed changes

adutra merged commit c5fd368 into apache:main Aug 20, 2025
12 checks passed

github-project-automation bot moved this from Ready to merge to Done in Basic Kanban Board Aug 20, 2025

adutra mentioned this pull request Aug 20, 2025

Mention Helm chart support for PodDisruptionBudget in CHANGELOG.md #2408

Merged

feat: Add Pod Disruption Budget support to Helm chart #2380

feat: Add Pod Disruption Budget support to Helm chart #2380

Uh oh!

Conversation

bmlyr commented Aug 18, 2025

Uh oh!

adutra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

adutra commented Aug 19, 2025

Uh oh!

bmlyr commented Aug 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

adutra commented Aug 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants