[watcher] Use MergePatch type instead of JSON Patch.

If a resource is missing an annotation field, the JSONPatch add will
fail because the parent is missing. Instead, use MergePatch to
automatically create, update, or add the annotation.

go.mod

@@ -24,7 +24,6 @@ require (
 	golang.org/x/sys v0.0.0-20201201145000-ef89a241ccb3 // indirect
 	golang.org/x/text v0.3.4 // indirect
 	golang.org/x/tools v0.0.0-20201202100533-7534955ac86b // indirect
-	gomodules.xyz/jsonpatch/v2 v2.1.0
 	google.golang.org/api v0.31.0
 	google.golang.org/appengine v1.6.7 // indirect
 	google.golang.org/genproto v0.0.0-20201019141844-1ed22bb0c154

pkg/watcher/reconciler/annotation/annotation.go

@@ -16,40 +16,23 @@ package annotation
 
 import (
 	"encoding/json"
-	"fmt"
-	"strings"
-
-	"gomodules.xyz/jsonpatch/v2"
 )
 
 const (
 	Result = "results.tekton.dev/result"
 	Record = "results.tekton.dev/record"
 )
 
-var (
-	resultPath = path(Result)
-	recordPath = path(Record)
-)
-
-func path(s string) string {
-	return fmt.Sprintf("/metadata/annotations/%s", strings.ReplaceAll(s, "/", "~1"))
-}
-
 // Add creates a jsonpatch path used for adding result / record identifiers
 // an object's annotations field.
 func Add(result, record string) ([]byte, error) {
-	patches := []jsonpatch.JsonPatchOperation{
-		{
-			Operation: "add",
-			Path:      resultPath,
-			Value:     result,
-		},
-		{
-			Operation: "add",
-			Path:      recordPath,
-			Value:     record,
+	data := map[string]interface{}{
+		"metadata": map[string]interface{}{
+			"annotations": map[string]string{
+				Result: result,
+				Record: record,
+			},
 		},
 	}
-	return json.Marshal(patches)
+	return json.Marshal(data)
 }

pkg/watcher/reconciler/pipelinerun/pipelinerun.go

@@ -80,7 +80,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, key string) error {
 		log.Errorf("error adding Result annotations: %v", err)
 		return err
 	}
-	if _, err := r.pipelineclientset.TektonV1beta1().PipelineRuns(pr.GetNamespace()).Patch(pr.Name, types.JSONPatchType, patch); err != nil {
+	if _, err := r.pipelineclientset.TektonV1beta1().PipelineRuns(pr.GetNamespace()).Patch(pr.Name, types.MergePatchType, patch); err != nil {
 		log.Errorf("PipelineRun.Patch: %v", err)
 		return err
 	}

pkg/watcher/reconciler/taskrun/taskrun.go

@@ -80,7 +80,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, key string) error {
 		log.Errorf("error adding Result annotations: %v", err)
 		return err
 	}
-	if _, err := r.pipelineclientset.TektonV1beta1().TaskRuns(tr.GetNamespace()).Patch(tr.Name, types.JSONPatchType, patch); err != nil {
+	if _, err := r.pipelineclientset.TektonV1beta1().TaskRuns(tr.GetNamespace()).Patch(tr.Name, types.MergePatchType, patch); err != nil {
 		log.Errorf("TaskRun.Patch: %v", err)
 		return err
 	}

test/e2e/02-test.sh

@@ -16,27 +16,30 @@
 set -e
 
 ROOT="$(git rev-parse --show-toplevel)"
-CONFIG="${ROOT}/test/e2e/pipelinerun.yaml"
 
-kubectl delete -f "${CONFIG}" || true
-kubectl apply -f "${CONFIG}"
-echo "Waiting for runs to complete..."
-kubectl wait -f "${CONFIG}" --for=condition=Succeeded
+for f in "taskrun.yaml" "pipelinerun.yaml"; do
+    CONFIG="${ROOT}/test/e2e/${f}"
+    echo "==========${CONFIG}=========="
+    kubectl delete -f "${CONFIG}" || true
+    kubectl apply -f "${CONFIG}" --record=false
+    echo "Waiting for runs to complete..."
+    kubectl wait -f "${CONFIG}" --for=condition=Succeeded
+
+    # Try a few times to get the result, since we might query before the reconciler
+    # picks it up.
+    for n in $(seq 10); do
+        result_id=$(kubectl get -f "${CONFIG}" -o json | jq -r '.metadata.annotations."results.tekton.dev/result"')
+        if [[ "${result_id}" == "null" ]]; then
+            echo "Attempt #${n}: Could not find 'results.tekton.dev/result' for ${CONFIG}"
+            sleep 1
+        fi
+    done
 
-# Try a few times to get the result, since we might query before the reconciler
-# picks it up.
-for n in $(seq 10); do
-    result_id=$(kubectl get -f "${CONFIG}" -o json | jq -r '.metadata.annotations."results.tekton.dev/result"')
     if [[ "${result_id}" == "null" ]]; then
-        echo "Attempt #${n}: Could not find 'results.tekton.dev/result' for ${CONFIG}"
-        sleep 1
+        echo "Giving up."
+        exit 1
     fi
-done
-
-if [[ "${result_id}" == "null" ]]; then
-    echo "Giving up."
-    exit 1
-fi
 
-echo "Found result ${result_id}"
-echo "Success!"
+    echo "Found result ${result_id}"
+    echo "Success!"
+done