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

[Question] Status of $patch (Strategic Merge Patch) support for Custom Resources #3694

Closed
yanniszark opened this issue Mar 8, 2021 · 8 comments
Labels
area/api issues for api module area/kyaml issues for kyaml kind/support Categorizes issue or PR as a support question.

Comments

@yanniszark
Copy link
Contributor

yanniszark commented Mar 8, 2021

Question

Do CustomResources support Strategic Merge Patch in kustomize? Specifically for $patch: delete.
Can I rely on this feature? It seems to work on 4.0.4 but not on 3.2.0. I couldn't find an issue / pr solving it (except #2056 which doesn't include a closing PR), so I'm not sure if this was solved by accident or is a supported feature.

Scenario

Given the kustomization:

kustomization.yaml

resources:
- example-cr.yaml

patches:
- patch: |-
    $patch: delete
    apiVersion: "*"
    kind: "*"
    metadata:
      name: "*"
  target:
    kind: ExampleCR

example-cr.yaml

apiVersion: example/v1
kind: ExampleCR
metadata:
  name: example
spec:
  example: example

Outcomes

Version 3.2.0 outputs:

$patch: delete
apiVersion: example/v1
kind: ExampleCR
metadata:
  name: example
spec:
  example: example

Version 4.0.4 outputs nothing (as expected).

@yanniszark
Copy link
Contributor Author

cc'ing @monopole because of comments in #2056

@Shell32-Natsu Shell32-Natsu added area/api issues for api module area/kyaml issues for kyaml kind/support Categorizes issue or PR as a support question. labels Mar 8, 2021
@Shell32-Natsu
Copy link
Contributor

I am not sure about the custom resources. I am afraid is not a good idea to rely on this behavior.

@yanniszark
Copy link
Contributor Author

@Shell32-Natsu @monopole shouldn't kustomize support it though, as part of supporting SMP?
It seems the current versions already work. How about establishing the intention to support this feature, by including a unit test?
This is an immensely useful feature that is otherwise missing from kustomize.

@Shell32-Natsu
Copy link
Contributor

Not familiar with this, but IMO a correct strategic merge patch with directive needs schema for the CRD. kustomize hasn't fully supported it yet.

@KnVerey
Copy link
Contributor

KnVerey commented Mar 15, 2021

I agree with @Shell32-Natsu that the schema is required to correctly implement SMPs in general. I'm guessing that $delete started working in v4 because of the switch to kyaml merge libraries under the hood. There is work in progress to allow users to provide OpenAPI schemas for their resources and get full, correct SMP support. Please see the KEP here: https://github.com/kubernetes/enhancements/tree/master/keps/sig-cli/2206-openapi-features-in-kustomize

@natasha41575
Copy link
Contributor

Closing this issue because using custom OpenAPI schemas is now implemented , see this example here. (More documentation is coming.) SMPs for CRDs will be supported as long as users are able to provide their own schema files.

@yhrn
Copy link
Contributor

yhrn commented May 27, 2021

@natasha41575 even though Kustomize now supports OpenAPI schemas I'm not sure that fully solves the CRD problem. From what I can see CustomResourceDefinition does not support x-kubernetes-patch-merge-key or x-kubernetes-patch-strategy. I got it to work by using kustomize openapi fetch the get the schemas from the cluster and then patching the schema to add the aforementioned properties but that is not a scalable solution for us.

@KnVerey
Copy link
Contributor

KnVerey commented Jun 9, 2021

Re: support for those fields in CRD, please follow kubernetes/kubernetes#82942.

apo-ger added a commit to apo-ger/manifests that referenced this issue Nov 22, 2022
Istio 1.6.0 generated manifests include some policy/v1
PodDisruptionBudget resources that we need to remove. See:
- istio/istio#12602
- istio/istio#24000

The current manifests utilize two "delete" patches:
- common/istio-1-16/istio-install/base/patches/remove-pdb.yaml
- common/istio-1-16/cluster-local-gateway/base/patches/remove-pdb.yaml

However these patches do not work with kustomize v3.2.0. The root
cause is that v3.2.0 doesn't have the appropriate openapi schema for
the policy/v1 API version resources. This is fixed in kustomize v4+.
See:
- kubernetes-sigs/kustomize#3694 (comment)
- kubernetes-sigs/kustomize#4495

Changes:
- We disable the delete patches until we upgrade to kustomize v4+.
  - tracked in: kubeflow#1797

- As a temporary workaraound we remove PodDisruptionBudget resources
  manually with yq before deploying Istio manifests.

- Update README file with instructions.

Refs: kubeflow#2325

Signed-off-by: Apostolos Gerakaris <[email protected]>
apo-ger added a commit to apo-ger/manifests that referenced this issue Nov 22, 2022
Istio 1.6.0 generated manifests include some policy/v1
PodDisruptionBudget resources that we need to remove. See:
- istio/istio#12602
- istio/istio#24000

The current manifests utilize two "delete" patches:
- common/istio-1-16/istio-install/base/patches/remove-pdb.yaml
- common/istio-1-16/cluster-local-gateway/base/patches/remove-pdb.yaml

However these patches do not work with kustomize v3.2.0. The root
cause is that v3.2.0 doesn't have the appropriate openapi schema for
the policy/v1 API version resources. This is fixed in kustomize v4+.
See:
- kubernetes-sigs/kustomize#3694 (comment)
- kubernetes-sigs/kustomize#4495

Changes:
- We disable the delete patches until we upgrade to kustomize v4+.
  - tracked in: kubeflow#1797

- As a temporary workaraound we remove PodDisruptionBudget resources
  manually with yq before deploying Istio manifests.

- Update README file with instructions.

Refs: kubeflow#2325

Signed-off-by: Apostolos Gerakaris <[email protected]>
apo-ger added a commit to apo-ger/manifests that referenced this issue Nov 22, 2022
Istio 1.6.0 generated manifests include some policy/v1
PodDisruptionBudget resources that we need to remove. See:
- istio/istio#12602
- istio/istio#24000

The current manifests utilize two "delete" patches:
- common/istio-1-16/istio-install/base/patches/remove-pdb.yaml
- common/istio-1-16/cluster-local-gateway/base/patches/remove-pdb.yaml

However these patches do not work with kustomize v3.2.0. The root
cause is that v3.2.0 doesn't have the appropriate openapi schema for
the policy/v1 API version resources. This is fixed in kustomize v4+.
See:
- kubernetes-sigs/kustomize#3694 (comment)
- kubernetes-sigs/kustomize#4495

Changes:
- We disable the delete patches until we upgrade to kustomize v4+.
  - tracked in: kubeflow#1797

- As a temporary workaraound we remove PodDisruptionBudget resources
  manually with yq before deploying Istio manifests.

- Update README file with instructions.

Refs: kubeflow#2325

Signed-off-by: Apostolos Gerakaris <[email protected]>
apo-ger added a commit to apo-ger/manifests that referenced this issue Nov 24, 2022
Istio 1.6.0 generated manifests include some policy/v1
PodDisruptionBudget resources that we need to remove. See:
- istio/istio#12602
- istio/istio#24000

The current manifests utilize two "delete" patches:
- common/istio-1-16/istio-install/base/patches/remove-pdb.yaml
- common/istio-1-16/cluster-local-gateway/base/patches/remove-pdb.yaml

However these patches do not work with kustomize v3.2.0. The root
cause is that v3.2.0 doesn't have the appropriate openapi schema for
the policy/v1 API version resources. This is fixed in kustomize v4+.
See:
- kubernetes-sigs/kustomize#3694 (comment)
- kubernetes-sigs/kustomize#4495

Changes:
- We disable the delete patches until we upgrade to kustomize v4+.
  - tracked in: kubeflow#1797

- As a temporary workaraound we remove PodDisruptionBudget resources
  manually with yq before deploying Istio manifests.

- Update README file with instructions.

Refs: kubeflow#2325

Signed-off-by: Apostolos Gerakaris <[email protected]>
google-oss-prow bot pushed a commit to kubeflow/manifests that referenced this issue Nov 24, 2022
* common: Add Istio v1.16.0 manifests

Signed-off-by: Apostolos Gerakaris <[email protected]>

* Update kustomization file in example to deploy istio-1-16

Signed-off-by: Apostolos Gerakaris <[email protected]>

* tests: Update install Istio GH action script

Use Istio 1.16 for testing

Signed-off-by: Apostolos Gerakaris <[email protected]>

* common: Remove PodDisruptionBudget resources for Istio

Istio 1.6.0 generated manifests include some policy/v1
PodDisruptionBudget resources that we need to remove. See:
- istio/istio#12602
- istio/istio#24000

The current manifests utilize two "delete" patches:
- common/istio-1-16/istio-install/base/patches/remove-pdb.yaml
- common/istio-1-16/cluster-local-gateway/base/patches/remove-pdb.yaml

However these patches do not work with kustomize v3.2.0. The root
cause is that v3.2.0 doesn't have the appropriate openapi schema for
the policy/v1 API version resources. This is fixed in kustomize v4+.
See:
- kubernetes-sigs/kustomize#3694 (comment)
- kubernetes-sigs/kustomize#4495

Changes:
- We disable the delete patches until we upgrade to kustomize v4+.
  - tracked in: #1797

- As a temporary workaraound we remove PodDisruptionBudget resources
  manually with yq before deploying Istio manifests.

- Update README file with instructions.

Refs: #2325

Signed-off-by: Apostolos Gerakaris <[email protected]>

* Update README

Use the --cluster-specific flag when generating Istio 1.16 manifests
for K8s-1.25. See:
* istio/istio#41220

Signed-off-by: Apostolos Gerakaris <[email protected]>

* tests: Update GH Action workflows

Trigger the workflows when the Istio version changes

Signed-off-by: Apostolos Gerakaris <[email protected]>

Signed-off-by: Apostolos Gerakaris <[email protected]>
kevin85421 pushed a commit to juliusvonkohout/manifests that referenced this issue Feb 28, 2023
* common: Add Istio v1.16.0 manifests

Signed-off-by: Apostolos Gerakaris <[email protected]>

* Update kustomization file in example to deploy istio-1-16

Signed-off-by: Apostolos Gerakaris <[email protected]>

* tests: Update install Istio GH action script

Use Istio 1.16 for testing

Signed-off-by: Apostolos Gerakaris <[email protected]>

* common: Remove PodDisruptionBudget resources for Istio

Istio 1.6.0 generated manifests include some policy/v1
PodDisruptionBudget resources that we need to remove. See:
- istio/istio#12602
- istio/istio#24000

The current manifests utilize two "delete" patches:
- common/istio-1-16/istio-install/base/patches/remove-pdb.yaml
- common/istio-1-16/cluster-local-gateway/base/patches/remove-pdb.yaml

However these patches do not work with kustomize v3.2.0. The root
cause is that v3.2.0 doesn't have the appropriate openapi schema for
the policy/v1 API version resources. This is fixed in kustomize v4+.
See:
- kubernetes-sigs/kustomize#3694 (comment)
- kubernetes-sigs/kustomize#4495

Changes:
- We disable the delete patches until we upgrade to kustomize v4+.
  - tracked in: kubeflow#1797

- As a temporary workaraound we remove PodDisruptionBudget resources
  manually with yq before deploying Istio manifests.

- Update README file with instructions.

Refs: kubeflow#2325

Signed-off-by: Apostolos Gerakaris <[email protected]>

* Update README

Use the --cluster-specific flag when generating Istio 1.16 manifests
for K8s-1.25. See:
* istio/istio#41220

Signed-off-by: Apostolos Gerakaris <[email protected]>

* tests: Update GH Action workflows

Trigger the workflows when the Istio version changes

Signed-off-by: Apostolos Gerakaris <[email protected]>

Signed-off-by: Apostolos Gerakaris <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api issues for api module area/kyaml issues for kyaml kind/support Categorizes issue or PR as a support question.
Projects
None yet
Development

No branches or pull requests

5 participants