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

Do not use strategicmergepatch for CRDs #937

Merged
merged 1 commit into from
Oct 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 22 additions & 23 deletions pkg/controller/instance/plan_execution_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
kudoengine "github.com/kudobuilder/kudo/pkg/engine"
"github.com/kudobuilder/kudo/pkg/util/health"
errwrap "github.com/pkg/errors"
apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -202,35 +203,33 @@ func prettyPrint(i interface{}) string {
func patchExistingObject(newResource runtime.Object, existingResource runtime.Object, c client.Client) error {
newResourceJSON, _ := apijson.Marshal(newResource)
key, _ := client.ObjectKeyFromObject(newResource)
err := c.Patch(context.TODO(), existingResource, client.ConstantPatch(types.StrategicMergePatchType, newResourceJSON))
if err != nil {
// Right now applying a Strategic Merge Patch to custom resources does not work. There is
// certain metadata needed, which when missing, leads to an invalid Content-Type Header and
// causes the request to fail.
// ( see https://github.com/kubernetes-sigs/kustomize/issues/742#issuecomment-458650435 )
//
// We temporarily solve this by checking for the specific error when a SMP is applied to
// custom resources and handle it by defaulting to a Merge Patch.
//
// The error message for which we check is:
// the body of the request was in an unknown format - accepted media types include:
// application/json-patch+json, application/merge-patch+json
//
// Reason: "UnsupportedMediaType" Code: 415
if apierrors.IsUnsupportedMediaType(err) {
err = c.Patch(context.TODO(), newResource, client.ConstantPatch(types.MergePatchType, newResourceJSON))
if err != nil {
log.Printf("PlanExecution: Error when applying merge patch to object %v: %v", key, err)
return err
}
} else {
log.Printf("PlanExecution: Error when applying StrategicMergePatch to object %v: %v", key, err)
_, isUnstructured := newResource.(runtime.Unstructured)
_, isCRD := newResource.(*apiextv1beta1.CustomResourceDefinition)

if isUnstructured || isCRD || isKudoType(newResource) {
// strategic merge patch is not supported for these types, falling back to mergepatch
err := c.Patch(context.TODO(), newResource, client.ConstantPatch(types.MergePatchType, newResourceJSON))
if err != nil {
log.Printf("PlanExecution: Error when applying merge patch to object %v: %v", key, err)
return err
}
} else {
err := c.Patch(context.TODO(), existingResource, client.ConstantPatch(types.StrategicMergePatchType, newResourceJSON))
if err != nil {
log.Printf("PlanExecution: Error when applying StrategicMergePatch to object %v: %w", key, err)
return err
}
}
return nil
}

func isKudoType(object runtime.Object) bool {
_, isOperator := object.(*v1alpha1.OperatorVersion)
_, isOperatorVersion := object.(*v1alpha1.Operator)
_, isInstance := object.(*v1alpha1.Instance)
return isOperator || isOperatorVersion || isInstance
}

// prepareKubeResources takes all resources in all tasks for a plan and renders them with the right parameters
// it also takes care of applying KUDO specific conventions to the resources like commond labels
func prepareKubeResources(plan *activePlan, meta *executionMetadata, renderer kubernetesObjectEnhancer) (*planResources, error) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
apiVersion: kudo.dev/v1alpha1
kind: Instance
metadata:
name: crd-instance
status:
aggregatedStatus:
status: COMPLETE
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
apiVersion: kudo.dev/v1alpha1
kind: TestStep
kubectl:
- kudo install --instance crd-instance ./crd-operator
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
name: "crd-operator"
version: "0.1.0"
kubernetesVersion: 1.13
maintainers:
- name: Your name
email: <your@email.com>
url: https://kudo.dev
tasks:
app:
resources:
- crd.yaml
plans:
deploy:
strategy: serial
phases:
- name: main
strategy: parallel
steps:
- name: everything
tasks:
- app
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
version:
description: Version of image
default: 1.7.9
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
creationTimestamp: null
labels:
app: kudo-manager
controller-tools.k8s.io: "1.0"
name: abcs.kudo.dev
spec:
group: kudo.dev
names:
kind: Abc
plural: abcs
singular: abc
scope: Namespaced
version: v1alpha1