Make apply conflict errors more readable

staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go

@@ -27,7 +27,6 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/validation/field"
-	"sigs.k8s.io/structured-merge-diff/merge"
 )
 
 const (
@@ -186,16 +185,7 @@ func NewConflict(qualifiedResource schema.GroupResource, name string, err error)
 }
 
 // NewApplyConflict returns an error including details on the requests apply conflicts
-func NewApplyConflict(conflicts merge.Conflicts) *StatusError {
-	causes := make([]metav1.StatusCause, 0, len(conflicts))
-	for _, conflict := range conflicts {
-		causes = append(causes, metav1.StatusCause{
-			Type:    metav1.CauseType("conflict"),
-			Message: conflict.Error(),
-			Field:   conflict.Path.String(),
-		})
-	}
-
+func NewApplyConflict(causes []metav1.StatusCause, message string) *StatusError {
 	return &StatusError{ErrStatus: metav1.Status{
 		Status: metav1.StatusFailure,
 		Code:   http.StatusConflict,
@@ -204,7 +194,7 @@ func NewApplyConflict(conflicts merge.Conflicts) *StatusError {
 			// TODO: Get obj details here?
 			Causes: causes,
 		},
-		Message: fmt.Sprintf("Apply failed with %d conflicts: %s", len(conflicts), conflicts.Error()),
+		Message: message,
 	}}
 }
 

staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go

@@ -825,6 +825,9 @@ const (
 	// without the expected return type. The presence of this cause indicates the error may be
 	// due to an intervening proxy or the server software malfunctioning.
 	CauseTypeUnexpectedServerResponse CauseType = "UnexpectedServerResponse"
+	// FieldManagerConflict is used to report when another client claims to manage this field,
+	// It should only be returned for a request using server-side apply.
+	CauseTypeFieldManagerConflict CauseType = "FieldManagerConflict"
 )
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go

@@ -20,7 +20,6 @@ import (
 	"fmt"
 	"time"
 
-	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -178,7 +177,7 @@ func (f *FieldManager) Apply(liveObj runtime.Object, patch []byte, force bool) (
 	newObjTyped, managed, err := f.updater.Apply(liveObjTyped, patchObjTyped, apiVersion, managed, manager, force)
 	if err != nil {
 		if conflicts, ok := err.(merge.Conflicts); ok {
-			return nil, errors.NewApplyConflict(conflicts)
+			return nil, internal.NewConflictError(conflicts)
 		}
 		return nil, err
 	}

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/conflict.go

@@ -0,0 +1,82 @@
+/*
+Copyright 2019 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package internal
+
+import (
+	"encoding/json"
+	"fmt"
+	"sort"
+	"strings"
+	"time"
+
+	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/structured-merge-diff/fieldpath"
+	"sigs.k8s.io/structured-merge-diff/merge"
+)
+
+// NewConflictError returns an error including details on the requests apply conflicts
+func NewConflictError(conflicts merge.Conflicts) *errors.StatusError {
+	causes := []metav1.StatusCause{}
+	for _, conflict := range conflicts {
+		causes = append(causes, metav1.StatusCause{
+			Type:    metav1.CauseTypeFieldManagerConflict,
+			Message: fmt.Sprintf("conflict with %v", printManager(conflict.Manager)),
+			Field:   conflict.Path.String(),
+		})
+	}
+	return errors.NewApplyConflict(causes, getConflictMessage(conflicts))
+}
+
+func getConflictMessage(conflicts merge.Conflicts) string {
+	if len(conflicts) == 1 {
+		return fmt.Sprintf("Apply failed with 1 conflict: conflict with %v: %v", printManager(conflicts[0].Manager), conflicts[0].Path)
+	}
+
+	m := map[string][]fieldpath.Path{}
+	for _, conflict := range conflicts {
+		m[conflict.Manager] = append(m[conflict.Manager], conflict.Path)
+	}
+
+	uniqueManagers := []string{}
+	for manager := range m {
+		uniqueManagers = append(uniqueManagers, manager)
+	}
+
+	// Print conflicts by sorted managers.
+	sort.Strings(uniqueManagers)
+
+	messages := []string{}
+	for _, manager := range uniqueManagers {
+		messages = append(messages, fmt.Sprintf("conflicts with %v:", printManager(manager)))
+		for _, path := range m[manager] {
+			messages = append(messages, fmt.Sprintf("- %v", path))
+		}
+	}
+	return fmt.Sprintf("Apply failed with %d conflicts: %s", len(conflicts), strings.Join(messages, "\n"))
+}
+
+func printManager(manager string) string {
+	encodedManager := &metav1.ManagedFieldsEntry{}
+	if err := json.Unmarshal([]byte(manager), encodedManager); err != nil {
+		return fmt.Sprintf("%q", manager)
+	}
+	if encodedManager.Operation == metav1.ManagedFieldsOperationUpdate {
+		return fmt.Sprintf("%q using %v at %v", encodedManager.Manager, encodedManager.APIVersion, encodedManager.Time.UTC().Format(time.RFC3339))
+	}
+	return fmt.Sprintf("%q", encodedManager.Manager)
+}

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/conflict_test.go

@@ -0,0 +1,106 @@
+/*
+Copyright 2019 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package internal_test
+
+import (
+	"net/http"
+	"reflect"
+	"testing"
+
+	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal"
+	"sigs.k8s.io/structured-merge-diff/fieldpath"
+	"sigs.k8s.io/structured-merge-diff/merge"
+)
+
+// TestNewConflictError tests that NewConflictError creates the correct StatusError for a given smd Conflicts
+func TestNewConflictError(t *testing.T) {
+	testCases := []struct {
+		conflict merge.Conflicts
+		expected *errors.StatusError
+	}{
+		{
+			conflict: merge.Conflicts{
+				merge.Conflict{
+					Manager: `{"manager":"foo","operation":"Update","apiVersion":"v1","time":"2001-02-03T04:05:06Z"}`,
+					Path:    fieldpath.MakePathOrDie("spec", "replicas"),
+				},
+			},
+			expected: &errors.StatusError{
+				ErrStatus: metav1.Status{
+					Status: metav1.StatusFailure,
+					Code:   http.StatusConflict,
+					Reason: metav1.StatusReasonConflict,
+					Details: &metav1.StatusDetails{
+						Causes: []metav1.StatusCause{
+							{
+								Type:    metav1.CauseTypeFieldManagerConflict,
+								Message: `conflict with "foo" using v1 at 2001-02-03T04:05:06Z`,
+								Field:   ".spec.replicas",
+							},
+						},
+					},
+					Message: `Apply failed with 1 conflict: conflict with "foo" using v1 at 2001-02-03T04:05:06Z: .spec.replicas`,
+				},
+			},
+		},
+		{
+			conflict: merge.Conflicts{
+				merge.Conflict{
+					Manager: `{"manager":"foo","operation":"Update","apiVersion":"v1","time":"2001-02-03T04:05:06Z"}`,
+					Path:    fieldpath.MakePathOrDie("spec", "replicas"),
+				},
+				merge.Conflict{
+					Manager: `{"manager":"bar","operation":"Apply"}`,
+					Path:    fieldpath.MakePathOrDie("metadata", "labels", "app"),
+				},
+			},
+			expected: &errors.StatusError{
+				ErrStatus: metav1.Status{
+					Status: metav1.StatusFailure,
+					Code:   http.StatusConflict,
+					Reason: metav1.StatusReasonConflict,
+					Details: &metav1.StatusDetails{
+						Causes: []metav1.StatusCause{
+							{
+								Type:    metav1.CauseTypeFieldManagerConflict,
+								Message: `conflict with "foo" using v1 at 2001-02-03T04:05:06Z`,
+								Field:   ".spec.replicas",
+							},
+							{
+								Type:    metav1.CauseTypeFieldManagerConflict,
+								Message: `conflict with "bar"`,
+								Field:   ".metadata.labels.app",
+							},
+						},
+					},
+					Message: `Apply failed with 2 conflicts: conflicts with "bar":
+- .metadata.labels.app
+conflicts with "foo" using v1 at 2001-02-03T04:05:06Z:
+- .spec.replicas`,
+				},
+			},
+		},
+	}
+	for _, tc := range testCases {
+		actual := internal.NewConflictError(tc.conflict)
+		if !reflect.DeepEqual(tc.expected, actual) {
+			t.Errorf("Expected to get\n%+v\nbut got\n%+v", tc.expected.ErrStatus, actual.ErrStatus)
+		}
+	}
+}