Add ComputeDomain v1beta2 as a hub, CRD conversion webhook, and Helm-templated CRD#1005
Draft
shivamerla wants to merge 1 commit intokubernetes-sigs:mainfrom
Draft
Add ComputeDomain v1beta2 as a hub, CRD conversion webhook, and Helm-templated CRD#1005shivamerla wants to merge 1 commit intokubernetes-sigs:mainfrom
shivamerla wants to merge 1 commit intokubernetes-sigs:mainfrom
Conversation
3ec6cec to
4ca7248
Compare
4ca7248 to
85b0a0c
Compare
Contributor
Author
Contributor
Author
|
The linter failures look like false positives. It flags |
Contributor
Author
|
Create CD using v1beta1. kubectl create -f imex-injection.yaml: Get is served as Get v1beta1 version explicitly: Get v1beta2 version explicitly: |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: shivamerla The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
49c8fd5 to
012b531
Compare
…templated CRD This change introduces: - A new 'resource.nvidia.com/v1beta2' version as the storage/conversion hub for ComputeDomain and keep 'v1beta1' as a deprecated served version, and wire the CRD to a conversion webhook so the apiserver can round-trip between API specs. - Webook configuration for conversion webhooks have to be embedded into the CRD unlike validating/mutating webhooks. So ship the computedomains CRD as a Helm chart template so webhook clientConfig (service name/namespace/port, optional caBundle, cert-manager CA injection) resolves at install time instead of baking static cluster settings into the manifest. - Helm validation now rejects resources.computeDomains.enabled=true when webhook.enabled=false, matching the CRD's conversion.webhook requirement. - Deprecated 'numNodes' field is stored as an annotation to facilitate the conversion between stored v1beta2 to served v1beta1 version. Some caveats with the conversion webhooks: - The conversion webhook must be enabled when ComputeDomains are enabled: spec.conversion.strategy=Webhook on the CRD requires a reachable webhook, with webhook.enabled=false the cluster cannot honor v1beta1/v1beta2 conversion. - Users must install with webhook.enabled=true (and valid TLS) or disable ComputeDomains (resources.computeDomains.enabled=false). - For the above reason, for production environments 'cert-manager' will be a required pre-requisite. - The ComputeComains CRD is intentionally templatized (Helm templates/, not only chart crds/): With that CRDs cannot be directly applied by kubectl, instead install or upgrade via Helm (or reproduce the same templating) so clientConfig and optional cert-manager annotations align with the deployed webhook. - Because of the above reason, helm chart deletion will remove the CRD as well and any CR instances associated with it. Ideal thing to do is to create a separate helm chart for CRDs, so added a TODO for that. Signed-off-by: Shiva Krishna, Merla <smerla@nvidia.com>
bdba9e3 to
a0b6153
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change introduces:
resource.nvidia.com/v1beta2version as the storage/conversion hub for ComputeDomain and keepv1beta1as a deprecated served version, and wire the CRD to a conversion webhook so the apiserver can round-trip between API specs.ComputeDomainsCRD as a Helm chart template so webhook clientConfig (service name/namespace/port, optional caBundle, cert-manager CA injection) resolves at install time instead of baking static cluster settings into the manifest.numNodesfield is stored as an annotation to facilitate the conversion between stored v1beta2 to served v1beta1 version.Some caveats with the conversion webhooks:
spec.conversion.strategy=Webhookon the CRD requires a reachable webhook, withwebhook.enabled=falsethe cluster cannot honor v1beta1/v1beta2 conversion.webhook.enabled=true(and valid TLS) or disable ComputeDomains (resources.computeDomains.enabled=false).cert-manageror equivalent will be a required pre-requisite.