Format resource diffs

Fix kubernetes#973

.gitmodules

@@ -226,3 +226,6 @@
 [submodule "_vendor/github.com/dgrijalva/jwt-go"]
 	path = _vendor/github.com/dgrijalva/jwt-go
 	url = https://github.com/dgrijalva/jwt-go
+[submodule "_vendor/github.com/sergi/go-diff"]
+	path = _vendor/github.com/sergi/go-diff
+	url = https://github.com/sergi/go-diff.git

pkg/diff/diff.go

@@ -0,0 +1,167 @@
+/*
+Copyright 2016 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 diff
+
+import (
+	"bytes"
+	"strings"
+
+	"github.com/golang/glog"
+	"github.com/sergi/go-diff/diffmatchpatch"
+)
+
+func FormatDiff(lString, rString string) string {
+	results := buildDiffLines(lString, rString)
+
+	return renderText(results, 2)
+}
+
+func renderText(results []lineRecord, context int) string {
+	keep := make([]bool, len(results))
+	for i := range results {
+		if results[i].Type == diffmatchpatch.DiffEqual {
+			continue
+		}
+		for j := i - context; j <= i+context; j++ {
+			if j >= 0 && j < len(keep) {
+				keep[j] = true
+			}
+		}
+	}
+
+	var b bytes.Buffer
+	wroteSkip := false
+	for i := range results {
+		if !keep[i] {
+			if !wroteSkip {
+				b.WriteString("...\n")
+				wroteSkip = true
+			}
+			continue
+		}
+
+		switch results[i].Type {
+		case diffmatchpatch.DiffDelete:
+			b.WriteString("- ")
+		case diffmatchpatch.DiffInsert:
+			b.WriteString("+ ")
+		case diffmatchpatch.DiffEqual:
+			b.WriteString("  ")
+		}
+		b.WriteString(results[i].Line)
+		b.WriteString("\n")
+		wroteSkip = false
+	}
+
+	return b.String()
+}
+
+type lineRecord struct {
+	Type diffmatchpatch.Operation
+	Line string
+}
+
+func buildDiffLines(lString, rString string) []lineRecord {
+	dmp := diffmatchpatch.New()
+	diffs := dmp.DiffMain(lString, rString, false)
+	// No need to cleanup, we're going to do a line based diff
+	//diffs = dmp.DiffCleanupSemantic(diffs)
+
+	l := ""
+	r := ""
+
+	var results []lineRecord
+	for _, diff := range diffs {
+		lines := strings.Split(diff.Text, "\n")
+		switch diff.Type {
+		case diffmatchpatch.DiffInsert:
+			if len(lines) > 0 {
+				r += lines[0]
+				if len(lines) > 1 {
+					results = append(results, lineRecord{Type: diffmatchpatch.DiffInsert, Line: r})
+					r = ""
+				}
+			}
+			for i := 1; i < len(lines)-1; i++ {
+				line := lines[i]
+				results = append(results, lineRecord{Type: diffmatchpatch.DiffInsert, Line: line})
+			}
+			if len(lines) > 1 {
+				r = lines[len(lines)-1]
+			}
+
+		case diffmatchpatch.DiffDelete:
+			if len(lines) > 0 {
+				l += lines[0]
+				if len(lines) > 1 {
+					results = append(results, lineRecord{Type: diffmatchpatch.DiffDelete, Line: l})
+					l = ""
+				}
+			}
+			for i := 1; i < len(lines)-1; i++ {
+				line := lines[i]
+				results = append(results, lineRecord{Type: diffmatchpatch.DiffDelete, Line: line})
+			}
+			if len(lines) > 1 {
+				l = lines[len(lines)-1]
+			}
+
+		case diffmatchpatch.DiffEqual:
+			if len(lines) > 0 {
+				if l != "" || r != "" {
+					l += lines[0]
+					r += lines[0]
+				} else {
+					results = append(results, lineRecord{Type: diffmatchpatch.DiffEqual, Line: lines[0]})
+				}
+				if len(lines) > 1 {
+					if r != "" {
+						results = append(results, lineRecord{Type: diffmatchpatch.DiffInsert, Line: r})
+						r = ""
+					}
+
+					if l != "" {
+						results = append(results, lineRecord{Type: diffmatchpatch.DiffDelete, Line: l})
+						l = ""
+					}
+				}
+			}
+			for i := 1; i < len(lines)-1; i++ {
+				line := lines[i]
+				results = append(results, lineRecord{Type: diffmatchpatch.DiffEqual, Line: line})
+			}
+			if len(lines) > 1 {
+				l = lines[len(lines)-1]
+				r = lines[len(lines)-1]
+			}
+
+		default:
+			glog.Fatalf("unexpected dmp type: %v", diff.Type)
+		}
+	}
+
+	if l != "" && r != "" {
+		if l == r {
+			results = append(results, lineRecord{Type: diffmatchpatch.DiffEqual, Line: l})
+		} else {
+			results = append(results, lineRecord{Type: diffmatchpatch.DiffDelete, Line: l})
+			results = append(results, lineRecord{Type: diffmatchpatch.DiffInsert, Line: r})
+		}
+	}
+
+	return results
+}

pkg/diff/diff_test.go

@@ -0,0 +1,119 @@
+/*
+Copyright 2016 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 diff
+
+import (
+	"github.com/sergi/go-diff/diffmatchpatch"
+	"testing"
+)
+
+func Test_Diff_1(t *testing.T) {
+	l := `A
+B
+C
+D
+E
+F`
+	r := `A
+D
+E
+F`
+	expectedDiff := `  A
+- B
+- C
+  D
+  E
+...
+`
+
+	{
+		dl := buildDiffLines(l, r)
+		actual := ""
+		for i := range dl {
+			switch dl[i].Type {
+			case diffmatchpatch.DiffDelete:
+				actual += "-"
+			case diffmatchpatch.DiffInsert:
+				actual += "+"
+			case diffmatchpatch.DiffEqual:
+				actual += "="
+			default:
+				t.Errorf("Unexpected diff type: %v", dl[i].Type)
+			}
+		}
+		expected := "=--==="
+		if actual != expected {
+			t.Fatalf("unexpected diff.  expected=%v, actual=%v", expected, actual)
+		}
+	}
+
+	actual := FormatDiff(l, r)
+	if actual != expectedDiff {
+		t.Fatalf("unexpected diff.  expected=%v, actual=%v", expectedDiff, actual)
+	}
+}
+
+func Test_Diff_2(t *testing.T) {
+	l := `A
+B
+C
+D
+E
+F`
+	r := `A
+B
+C
+D
+E2
+F`
+	expectedDiff := `...
+  C
+  D
++ E2
+- E
+  F
+`
+	actual := FormatDiff(l, r)
+	if actual != expectedDiff {
+		t.Fatalf("unexpected diff.  expected=%v, actual=%v", expectedDiff, actual)
+	}
+}
+
+func Test_Diff_3(t *testing.T) {
+	l := `A
+B
+C
+D
+E
+F`
+	r := `A
+B
+C
+D
+E
+F2`
+	expectedDiff := `...
+  D
+  E
+- F
++ F2
+`
+	actual := FormatDiff(l, r)
+	if actual != expectedDiff {
+		t.Fatalf("unexpected diff.  expected=%v, actual=%v", expectedDiff, actual)
+	}
+}

upup/pkg/fi/dryrun_target.go

@@ -25,6 +25,7 @@ import (
 	"sync"
 
 	"github.com/golang/glog"
+	"k8s.io/kops/pkg/diff"
 	"k8s.io/kops/upup/pkg/fi/utils"
 )
 
@@ -141,7 +142,11 @@ func (t *DryRunTarget) PrintReport(taskMap map[string]Task, out io.Writer) error
 			fmt.Fprintf(b, "Will modify resources:\n")
 			// We can't use our reflection helpers here - we want corresponding values from a,e,c
 			for _, r := range updates {
-				var changeList []string
+				type change struct {
+					FieldName   string
+					Description string
+				}
+				var changeList []change
 
 				valC := reflect.ValueOf(r.changes)
 				valA := reflect.ValueOf(r.a)
@@ -186,14 +191,22 @@ func (t *DryRunTarget) PrintReport(taskMap map[string]Task, out io.Writer) error
 							switch fieldValE.Interface().(type) {
 							//case SimpleUnit:
 							//	ignored = true
-							default:
+							case Resource, ResourceHolder:
+								resA, okA := tryResourceAsString(fieldValA)
+								resE, okE := tryResourceAsString(fieldValE)
+								if okA && okE {
+									description = diff.FormatDiff(resA, resE)
+								}
+							}
+
+							if !ignored && description == "" {
 								description = fmt.Sprintf(" %v -> %v", ValueAsString(fieldValA), ValueAsString(fieldValE))
 							}
 						}
 						if ignored {
 							continue
 						}
-						changeList = append(changeList, fmt.Sprintf("%-20s\t%s", valC.Type().Field(i).Name, description))
+						changeList = append(changeList, change{FieldName: valC.Type().Field(i).Name, Description: description})
 					}
 				} else {
 					return fmt.Errorf("unhandled change type: %v", valC.Type())
@@ -205,8 +218,16 @@ func (t *DryRunTarget) PrintReport(taskMap map[string]Task, out io.Writer) error
 
 				taskName := getTaskName(r.changes)
 				fmt.Fprintf(b, "  %-20s\t%s\n", taskName, IdForTask(taskMap, r.e))
-				for _, f := range changeList {
-					fmt.Fprintf(b, "  \t%s\n", f)
+				for _, change := range changeList {
+					lines := strings.Split(change.Description, "\n")
+					if len(lines) == 1 {
+						fmt.Fprintf(b, "  \t%-20s\t%s\n", change.FieldName, change.Description)
+					} else {
+						fmt.Fprintf(b, "  \t%-20s\n", change.FieldName)
+						for _, line := range lines {
+							fmt.Fprintf(b, "  \t%-20s\t%s\n", "", line)
+						}
+					}
 				}
 				fmt.Fprintf(b, "\n")
 			}
@@ -224,6 +245,31 @@ func (t *DryRunTarget) PrintReport(taskMap map[string]Task, out io.Writer) error
 	return err
 }
 
+func tryResourceAsString(v reflect.Value) (string, bool) {
+	if !v.CanInterface() {
+		return "", false
+	}
+
+	intf := v.Interface()
+	if res, ok := intf.(Resource); ok {
+		s, err := ResourceAsString(res)
+		if err != nil {
+			glog.Warningf("error converting to resource: %v", err)
+			return "", false
+		}
+		return s, true
+	}
+	if res, ok := intf.(*ResourceHolder); ok {
+		s, err := res.AsString()
+		if err != nil {
+			glog.Warningf("error converting to resource: %v", err)
+			return "", false
+		}
+		return s, true
+	}
+	return "", false
+}
+
 func getTaskName(t Task) string {
 	s := fmt.Sprintf("%T", t)
 	lastDot := strings.LastIndexByte(s, '.')