[camel-k] 01/03: feat: Use server-side apply to create and patch owned resources

[as...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

astefanutti pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/camel-k.git

commit cf48cd2f7553a04c624c10520b5a80461f915ddf
Author: Antonin Stefanutti <an...@stefanutti.fr>
AuthorDate: Mon Feb 15 18:16:58 2021 +0100

    feat: Use server-side apply to create and patch owned resources
    
    Server-side apply has been introduced to improve co-operation
    of controllers in defining the desire state for shared resources.
    
    We implemented a custom strategy relying on a combination of merge
    patch and pruning of nil values, but that fails to leverage the
    semantic provided by the strategic merge patch field markers,
    as strategic merge patch is not supported by CRDs, like Knative
    Service.
    
    With server-side apply, markers can be applied to data
    structures (Lists, Maps), to make valid and finer-grained merge
    strategies.
    
    This particularly helps cooperation of the Camel K operator with
    its Knative counterpart, into updating the Knative service pod
    template fields concurrently, such as the `Env []EnvVar` field,
    that declares the `+patchStrategy=merge` marker, which translates
    into the server-side apply `listType` and `+listMapKey` markers.
---
 pkg/trait/deployer.go          | 42 +++++------------------------
 pkg/util/kubernetes/replace.go | 22 ++++++---------
 pkg/util/patch/patch.go        | 61 +++++++++++++++---------------------------
 3 files changed, 36 insertions(+), 89 deletions(-)

diff --git a/pkg/trait/deployer.go b/pkg/trait/deployer.go
index 062b91d..d9fa584 100644
--- a/pkg/trait/deployer.go
+++ b/pkg/trait/deployer.go
@@ -19,12 +19,10 @@ package trait
 
 import (
 	"github.com/pkg/errors"
-	"k8s.io/apimachinery/pkg/types"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
-	"github.com/apache/camel-k/pkg/util/kubernetes"
 	"github.com/apache/camel-k/pkg/util/patch"
 )
 
@@ -62,15 +60,6 @@ func (t *deployerTrait) Configure(e *Environment) (bool, error) {
 func (t *deployerTrait) Apply(e *Environment) error {
 	switch e.Integration.Status.Phase {
 
-	case v1.IntegrationPhaseNone, v1.IntegrationPhaseWaitingForPlatform, v1.IntegrationPhaseInitialization, v1.IntegrationPhaseDeploying:
-		// Register a post action that updates the resources generated by the traits
-		e.PostActions = append(e.PostActions, func(env *Environment) error {
-			if err := kubernetes.ReplaceResources(env.C, env.Client, env.Resources.Items()); err != nil {
-				return errors.Wrap(err, "error during replace resource")
-			}
-			return nil
-		})
-
 	case v1.IntegrationPhaseBuildingKit, v1.IntegrationPhaseResolvingKit:
 		if e.IntegrationKitInPhase(v1.IntegrationKitPhaseReady) {
 			e.PostProcessors = append(e.PostProcessors, func(environment *Environment) error {
@@ -80,38 +69,19 @@ func (t *deployerTrait) Apply(e *Environment) error {
 			})
 		}
 
-	case v1.IntegrationPhaseRunning, v1.IntegrationPhaseWaitingForBindings:
+	case v1.IntegrationPhaseNone, v1.IntegrationPhaseInitialization,
+		v1.IntegrationPhaseWaitingForPlatform, v1.IntegrationPhaseWaitingForBindings,
+		v1.IntegrationPhaseDeploying, v1.IntegrationPhaseRunning:
 		// Register a post action that patches the resources generated by the traits
 		e.PostActions = append(e.PostActions, func(env *Environment) error {
 			for _, resource := range env.Resources.Items() {
-				key, err := client.ObjectKeyFromObject(resource)
+				target, err := patch.PositiveApplyPatch(resource)
 				if err != nil {
 					return err
 				}
-
-				object := resource.DeepCopyObject()
-				err = env.Client.Get(env.C, key, object)
+				err = env.Client.Patch(env.C, target, client.Apply, client.ForceOwnership, client.FieldOwner("camel-k-operator"))
 				if err != nil {
-					return err
-				}
-
-				// If both objects have "ObjectMeta" and "Spec" fields and they contain all the expected fields
-				// (plus optional others), then avoid patching.
-				if !patch.ObjectMetaEqualDeepDerivative(object, resource) ||
-					!patch.SpecEqualDeepDerivative(object, resource) {
-
-					p, err := patch.PositiveMergePatch(object, resource)
-					if err != nil {
-						return err
-					} else if len(p) == 0 {
-						// Avoid triggering a patch request for nothing
-						continue
-					}
-
-					err = env.Client.Patch(env.C, resource, client.RawPatch(types.MergePatchType, p))
-					if err != nil {
-						return errors.Wrap(err, "error during patch resource")
-					}
+					return errors.Wrapf(err, "error during apply resource: %v", resource)
 				}
 			}
 			return nil
diff --git a/pkg/util/kubernetes/replace.go b/pkg/util/kubernetes/replace.go
index 55c8dcf..a9f045d 100644
--- a/pkg/util/kubernetes/replace.go
+++ b/pkg/util/kubernetes/replace.go
@@ -20,27 +20,21 @@ package kubernetes
 import (
 	"context"
 
-	"github.com/apache/camel-k/pkg/client"
-	routev1 "github.com/openshift/api/route/v1"
 	"github.com/pkg/errors"
+
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	serving "knative.dev/serving/pkg/apis/serving/v1"
+
 	k8sclient "sigs.k8s.io/controller-runtime/pkg/client"
-)
 
-// ReplaceResources allows to completely replace a list of resources on Kubernetes, taking care of immutable fields and resource versions
-func ReplaceResources(ctx context.Context, c client.Client, objects []runtime.Object) error {
-	for _, object := range objects {
-		err := ReplaceResource(ctx, c, object)
-		if err != nil {
-			return err
-		}
-	}
-	return nil
-}
+	serving "knative.dev/serving/pkg/apis/serving/v1"
+
+	routev1 "github.com/openshift/api/route/v1"
+
+	"github.com/apache/camel-k/pkg/client"
+)
 
 // ReplaceResource allows to completely replace a resource on Kubernetes, taking care of immutable fields and resource versions
 func ReplaceResource(ctx context.Context, c client.Client, res runtime.Object) error {
diff --git a/pkg/util/patch/patch.go b/pkg/util/patch/patch.go
index e2c54b1..09c4265 100644
--- a/pkg/util/patch/patch.go
+++ b/pkg/util/patch/patch.go
@@ -21,7 +21,8 @@ import (
 	"reflect"
 
 	jsonpatch "github.com/evanphx/json-patch"
-	"k8s.io/apimachinery/pkg/api/equality"
+
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/json"
@@ -63,6 +64,26 @@ func PositiveMergePatch(source runtime.Object, target runtime.Object) ([]byte, e
 	return json.Marshal(positivePatch)
 }
 
+func PositiveApplyPatch(source runtime.Object) (runtime.Object, error) {
+	sourceJSON, err := json.Marshal(source)
+	if err != nil {
+		return nil, err
+	}
+
+	var positivePatch map[string]interface{}
+	err = json.Unmarshal(sourceJSON, &positivePatch)
+	if err != nil {
+		return nil, err
+	}
+
+	// The following is a work-around to remove null fields from the apply patch,
+	// so that ownership is not taken for non-managed fields.
+	// See https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/2155-clientgo-apply
+	removeNilValues(reflect.ValueOf(positivePatch), reflect.Value{})
+
+	return &unstructured.Unstructured{Object: positivePatch}, nil
+}
+
 func removeNilValues(v reflect.Value, parent reflect.Value) {
 	for v.Kind() == reflect.Ptr || v.Kind() == reflect.Interface {
 		v = v.Elem()
@@ -90,41 +111,3 @@ func removeNilValues(v reflect.Value, parent reflect.Value) {
 		}
 	}
 }
-
-func ObjectMetaEqualDeepDerivative(object runtime.Object, expected runtime.Object) (res bool) {
-	defer func() {
-		if r := recover(); r != nil {
-			res = false
-		}
-	}()
-
-	if expected == nil {
-		return true
-	} else if object == nil {
-		return false
-	}
-
-	objectMeta := reflect.ValueOf(object).Elem().FieldByName("ObjectMeta").Interface()
-	expectedMeta := reflect.ValueOf(expected).Elem().FieldByName("ObjectMeta").Interface()
-
-	return equality.Semantic.DeepDerivative(expectedMeta, objectMeta)
-}
-
-func SpecEqualDeepDerivative(object runtime.Object, expected runtime.Object) (res bool) {
-	defer func() {
-		if r := recover(); r != nil {
-			res = false
-		}
-	}()
-
-	if expected == nil {
-		return true
-	} else if object == nil {
-		return false
-	}
-
-	objectSpec := reflect.ValueOf(object).Elem().FieldByName("Spec").Interface()
-	expectedSpec := reflect.ValueOf(expected).Elem().FieldByName("Spec").Interface()
-
-	return equality.Semantic.DeepDerivative(expectedSpec, objectSpec)
-}