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

Use capabilities introspection to know which API version should be targeted for the resources #114

Merged
merged 4 commits into from
Dec 6, 2021

Conversation

yorinasub17
Copy link
Contributor

Description

This allows the k8s-service chart to be more flexible in targeting different Kubernetes versions, while still avoiding deprecated interfaces. For example, PodDisruptionBudget was located in policy/v1beta1, but has been moved to policy/v1, and the version in policy/v1beta1 is now marked for deprecation since k8s 1.21. If the helm chart is being installed to a k8s cluster <1.21, then it is preferred to use policy/v1beta1, while targeting k8s cluster >=1.21 requires using policy/v1. To support this, we can introspect the Capabilities built in object to get metadata about the target k8s cluster, and select the right version to use based on that.

This PR addresses this kind of API discrepancy for Ingress and PodDisruptionBudget resources.

TODOs

  • Ensure the branch is named correctly with the issue number. e.g: feature/new-vpc-endpoints-955 or bug/missing-count-param-434.
  • Update the docs.
  • Keep the changes backwards compatible where possible.
  • Run the pre-commit checks successfully.
  • Run the relevant tests successfully.

Copy link
Contributor

@autero1 autero1 left a comment

Choose a reason for hiding this comment

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

This LGTM!

Would it make sense to make a note about this in the README, so users would know which API versions are introspected without having to go through chart source code?

@yorinasub17
Copy link
Contributor Author

Thanks for the review! Going to go ahead and merge this in as is. If you disagree with my assessment below, happy to open a follow up PR with the README update.

Would it make sense to make a note about this in the README, so users would know which API versions are introspected without having to go through chart source code?

I think it's ok. I think it's considered standard helm practice to use the capabilities feature to ensure the right API version is always in use.

@yorinasub17 yorinasub17 merged commit 87a5cb7 into master Dec 6, 2021
@yorinasub17 yorinasub17 deleted the feature/capability-aware-api-selection branch December 6, 2021 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants