Isolate patch conflation problem to one test.

See #3394

api/krusty/multiplepatch_test.go

@@ -4,13 +4,60 @@
 package krusty_test
 
 import (
+	"fmt"
 	"strings"
 	"testing"
 
 	kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest"
 )
 
-func TestVolumePatch1(t *testing.T) {
+func TestRemoveEmptyDirWithNullFieldInSmp(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+	th.WriteK(".", `
+resources:
+- deployment.yaml
+patchesStrategicMerge:
+- patch.yaml
+`)
+	th.WriteF("deployment.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx
+spec:
+  template:
+    spec:
+      volumes:
+      - name: fancyDisk
+        emptyDir: {}
+`)
+	th.WriteF("patch.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx
+spec:
+  template:
+    spec:
+      volumes:
+      - name: fancyDisk
+        emptyDir: null
+`)
+	m := th.Run(".", th.MakeDefaultOptions())
+	th.AssertActualEqualsExpected(m, `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx
+spec:
+  template:
+    spec:
+      volumes:
+      - name: fancyDisk
+`)
+}
+
+func TestRemoveEmptyDirAddPersistentDisk(t *testing.T) {
 	th := kusttest_test.MakeHarness(t)
 	th.WriteK(".", `
 resources:
@@ -60,7 +107,7 @@ spec:
 `)
 }
 
-func TestVolumePatch2(t *testing.T) {
+func TestVolumeRemoveEmptyDirInOverlay(t *testing.T) {
 	th := kusttest_test.MakeHarness(t)
 	th.WriteK("base", `
 resources:
@@ -190,17 +237,11 @@ metadata:
 `)
 }
 
-func TestVolumePatch3(t *testing.T) {
+func TestRemoveEmptyDirWithPatchesAtSameLevel(t *testing.T) {
 	th := kusttest_test.MakeHarness(t)
 	th.WriteK("base", `
-commonLabels:
-  team: foo
 resources:
 - deployment.yaml
-configMapGenerator:
-- name: configmap-in-base
-  literals:
-  - foo=bar
 `)
 	th.WriteF("base/deployment.yaml", `
 apiVersion: apps/v1
@@ -218,13 +259,8 @@ spec:
       volumes:
       - name: nginx-persistent-storage
         emptyDir: {}
-      - configMap:
-          name: configmap-in-base
-        name: configmap-in-base
 `)
 	th.WriteK("overlay", `
-commonLabels:
-  env: staging
 patchesStrategicMerge:
 - deployment-patch1.yaml
 - deployment-patch2.yaml
@@ -262,25 +298,14 @@ spec:
       volumes:
       - name: nginx-persistent-storage
 `)
-	m := th.Run("overlay", th.MakeDefaultOptions())
-	th.AssertActualEqualsExpected(m, `
-apiVersion: apps/v1
+	opts := th.MakeDefaultOptions()
+	m := th.Run("overlay", opts)
+	expFmt := `apiVersion: apps/v1
 kind: Deployment
 metadata:
-  labels:
-    env: staging
-    team: foo
   name: nginx
 spec:
-  selector:
-    matchLabels:
-      env: staging
-      team: foo
   template:
-    metadata:
-      labels:
-        env: staging
-        team: foo
     spec:
       containers:
       - env:
@@ -290,159 +315,20 @@ spec:
         name: nginx
       - image: sidecar:latest
         name: sidecar
-      volumes:
-      - gcePersistentDisk:
-          pdName: nginx-persistent-storage
+      volumes:%s
         name: nginx-persistent-storage
-      - configMap:
-          name: configmap-in-base-798k5k7g9f
-        name: configmap-in-base
----
-apiVersion: v1
-data:
-  foo: bar
-kind: ConfigMap
-metadata:
-  labels:
-    env: staging
-    team: foo
-  name: configmap-in-base-798k5k7g9f
-`)
-}
-
-func TestEmptyDirOverrideMultiplePatches(t *testing.T) {
-	th := kusttest_test.MakeHarness(t)
-	th.WriteK("base", `
-namePrefix: b-
-commonLabels:
-  team: foo
-resources:
-- deployment.yaml
-configMapGenerator:
-- name: configmap-in-base
-  literals:
-  - foo=bar
-`)
-	th.WriteF("base/deployment.yaml", `
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  name: nginx
-spec:
-  template:
-    spec:
-      containers:
-      - name: nginx
-        image: nginx
-        volumeMounts:
-        - name: nginx-persistent-storage
-          mountPath: /tmp/ps
-      - name: sidecar
-        image: sidecar:latest
-      volumes:
-      - name: nginx-persistent-storage
-        emptyDir: {}
-      - configMap:
-          name: configmap-in-base
-        name: configmap-in-base
-`)
-	th.WriteK("overlay", `
-namePrefix: a-
-commonLabels:
-  env: staging
-patchesStrategicMerge:
-- deployment-patch1.yaml
-- deployment-patch2.yaml
-resources:
-- ../base
-`)
-	th.WriteF("overlay/deployment-patch1.yaml", `
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  name: nginx
-spec:
-  template:
-    spec:
-      containers:
-      - name: nginx
-        image: nginx:latest
-        env:
-        - name: ENVKEY
-          value: ENVVALUE
-      volumes:
-      - name: nginx-persistent-storage
-        emptyDir: null
+`
+	// TODO(#3394)
+	th.AssertActualEqualsExpected(
+		m, opts.IfApiMachineryElseKyaml(
+			fmt.Sprintf(expFmt, `
+      - gcePersistentDisk:
+          pdName: nginx-persistent-storage`),
+			fmt.Sprintf(expFmt, `
+      - emptyDir: {}
         gcePersistentDisk:
-          pdName: nginx-persistent-storage
-`)
-	th.WriteF("overlay/deployment-patch2.yaml", `
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  name: nginx
-spec:
-  template:
-    spec:
-      containers:
-      - name: nginx
-        env:
-        - name: ANOTHERENV
-          value: FOO
-      volumes:
-      - name: nginx-persistent-storage
-`)
-	m := th.Run("overlay", th.MakeDefaultOptions())
-	th.AssertActualEqualsExpected(m, `
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  labels:
-    env: staging
-    team: foo
-  name: a-b-nginx
-spec:
-  selector:
-    matchLabels:
-      env: staging
-      team: foo
-  template:
-    metadata:
-      labels:
-        env: staging
-        team: foo
-    spec:
-      containers:
-      - env:
-        - name: ANOTHERENV
-          value: FOO
-        - name: ENVKEY
-          value: ENVVALUE
-        image: nginx:latest
-        name: nginx
-        volumeMounts:
-        - mountPath: /tmp/ps
-          name: nginx-persistent-storage
-      - image: sidecar:latest
-        name: sidecar
-      volumes:
-      - gcePersistentDisk:
-          pdName: nginx-persistent-storage
-        name: nginx-persistent-storage
-      - configMap:
-          name: a-b-configmap-in-base-798k5k7g9f
-        name: configmap-in-base
----
-apiVersion: v1
-data:
-  foo: bar
-kind: ConfigMap
-metadata:
-  labels:
-    env: staging
-    team: foo
-  name: a-b-configmap-in-base-798k5k7g9f
-`)
+          pdName: nginx-persistent-storage`),
+		))
 }
 
 func TestSimpleMultiplePatches(t *testing.T) {
@@ -476,8 +362,6 @@ spec:
       - name: sidecar
         image: sidecar:latest
       volumes:
-      - name: nginx-persistent-storage
-        emptyDir: {}
       - configMap:
           name: configmap-in-base
         name: configmap-in-base
@@ -521,7 +405,6 @@ spec:
           value: ENVVALUE
       volumes:
       - name: nginx-persistent-storage
-        emptyDir: null
         gcePersistentDisk:
           pdName: nginx-persistent-storage
       - configMap:
@@ -541,8 +424,6 @@ spec:
         env:
         - name: ANOTHERENV
           value: FOO
-      volumes:
-      - name: nginx-persistent-storage
 `)
 	m := th.Run("overlay", th.MakeDefaultOptions())
 	th.AssertActualEqualsExpected(m, `
@@ -623,7 +504,7 @@ metadata:
 `)
 }
 
-func makeCommonFileForMultiplePatchTest(th kusttest_test.Harness) {
+func makeCommonFilesForMultiplePatchTests(th kusttest_test.Harness) {
 	th.WriteK("/app/base", `
 namePrefix: team-foo-
 commonLabels:
@@ -662,8 +543,6 @@ spec:
       - name: sidecar
         image: sidecar:latest
       volumes:
-      - name: nginx-persistent-storage
-        emptyDir: {}
       - configMap:
           name: configmap-in-base
         name: configmap-in-base
@@ -699,7 +578,7 @@ configMapGenerator:
 
 func TestMultiplePatchesNoConflict(t *testing.T) {
 	th := kusttest_test.MakeHarness(t)
-	makeCommonFileForMultiplePatchTest(th)
+	makeCommonFilesForMultiplePatchTests(th)
 	th.WriteF("/app/overlay/staging/deployment-patch1.yaml", `
 apiVersion: apps/v1
 kind: Deployment
@@ -716,7 +595,6 @@ spec:
           value: ENVVALUE
       volumes:
       - name: nginx-persistent-storage
-        emptyDir: null
         gcePersistentDisk:
           pdName: nginx-persistent-storage
       - configMap:
@@ -736,8 +614,6 @@ spec:
         env:
         - name: ANOTHERENV
           value: FOO
-      volumes:
-      - name: nginx-persistent-storage
 `)
 	m := th.Run("/app/overlay/staging", th.MakeDefaultOptions())
 	th.AssertActualEqualsExpected(m, `
@@ -840,11 +716,7 @@ metadata:
 
 func TestMultiplePatchesWithConflict(t *testing.T) {
 	th := kusttest_test.MakeHarness(t)
-	opts := th.MakeDefaultOptions()
-	if opts.UseKyaml {
-		t.Skip("kyaml merging doesn't look for conflicts")
-	}
-	makeCommonFileForMultiplePatchTest(th)
+	makeCommonFilesForMultiplePatchTests(th)
 	th.WriteF("/app/overlay/staging/deployment-patch1.yaml", `
 apiVersion: apps/v1
 kind: Deployment
@@ -860,7 +732,6 @@ spec:
           value: TRUE
       volumes:
       - name: nginx-persistent-storage
-        emptyDir: null
         gcePersistentDisk:
           pdName: nginx-persistent-storage
       - configMap:
@@ -881,13 +752,114 @@ spec:
         - name: ENABLE_FEATURE_FOO
           value: FALSE
 `)
-	err := th.RunWithErr("/app/overlay/staging", opts)
-	if err == nil {
-		t.Fatalf("expected conflict")
-	}
-	if !strings.Contains(
-		err.Error(), "conflict between ") {
-		t.Fatalf("Unexpected err: %v", err)
+	opts := th.MakeDefaultOptions()
+	if opts.UseKyaml {
+		// kyaml doesn't try to detect conflicts in patches
+		// (so ENABLE_FEATURE_FOO FALSE wins).
+		m := th.Run("/app/overlay/staging", opts)
+		th.AssertActualEqualsExpected(m, `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  annotations:
+    note: This is a test annotation
+  labels:
+    app: mynginx
+    env: staging
+    org: example.com
+    team: foo
+  name: staging-team-foo-nginx
+spec:
+  selector:
+    matchLabels:
+      app: mynginx
+      env: staging
+      org: example.com
+      team: foo
+  template:
+    metadata:
+      annotations:
+        note: This is a test annotation
+      labels:
+        app: mynginx
+        env: staging
+        org: example.com
+        team: foo
+    spec:
+      containers:
+      - env:
+        - name: ENABLE_FEATURE_FOO
+          value: false
+        image: nginx
+        name: nginx
+        volumeMounts:
+        - mountPath: /tmp/ps
+          name: nginx-persistent-storage
+      - image: sidecar:latest
+        name: sidecar
+      volumes:
+      - gcePersistentDisk:
+          pdName: nginx-persistent-storage
+        name: nginx-persistent-storage
+      - configMap:
+          name: staging-configmap-in-overlay-dc6fm46dhm
+        name: configmap-in-overlay
+      - configMap:
+          name: staging-team-foo-configmap-in-base-798k5k7g9f
+        name: configmap-in-base
+---
+apiVersion: v1
+kind: Service
+metadata:
+  annotations:
+    note: This is a test annotation
+  labels:
+    app: mynginx
+    env: staging
+    org: example.com
+    team: foo
+  name: staging-team-foo-nginx
+spec:
+  ports:
+  - port: 80
+  selector:
+    app: mynginx
+    env: staging
+    org: example.com
+    team: foo
+---
+apiVersion: v1
+data:
+  foo: bar
+kind: ConfigMap
+metadata:
+  annotations:
+    note: This is a test annotation
+  labels:
+    app: mynginx
+    env: staging
+    org: example.com
+    team: foo
+  name: staging-team-foo-configmap-in-base-798k5k7g9f
+---
+apiVersion: v1
+data:
+  hello: world
+kind: ConfigMap
+metadata:
+  labels:
+    env: staging
+  name: staging-configmap-in-overlay-dc6fm46dhm
+`)
+	} else {
+		err := th.RunWithErr("/app/overlay/staging", opts)
+		if err == nil {
+			t.Fatalf("expected conflict")
+		}
+		if !strings.Contains(
+			err.Error(), "conflict between ") {
+			t.Fatalf("Unexpected err: %v", err)
+		}
 	}
 }
 
@@ -936,8 +908,7 @@ spec:
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
 			th := kusttest_test.MakeHarness(t)
-
-			makeCommonFileForMultiplePatchTest(th)
+			makeCommonFilesForMultiplePatchTests(th)
 			th.WriteF("/app/overlay/staging/deployment-patch1.yaml", c.patch1)
 			th.WriteF("/app/overlay/staging/deployment-patch2.yaml", c.patch2)
 			m := th.Run("/app/overlay/staging", th.MakeDefaultOptions())
@@ -979,8 +950,6 @@ spec:
         - mountPath: /tmp/ps
           name: nginx-persistent-storage
       volumes:
-      - emptyDir: {}
-        name: nginx-persistent-storage
       - configMap:
           name: staging-team-foo-configmap-in-base-798k5k7g9f
         name: configmap-in-base
@@ -1034,7 +1003,7 @@ metadata:
 
 func TestMultiplePatchesBothWithPatchDeleteDirective(t *testing.T) {
 	th := kusttest_test.MakeHarness(t)
-	makeCommonFileForMultiplePatchTest(th)
+	makeCommonFilesForMultiplePatchTests(th)
 	th.WriteF("/app/overlay/staging/deployment-patch1.yaml", `
 apiVersion: apps/v1
 kind: Deployment
@@ -1059,12 +1028,98 @@ spec:
       - $patch: delete
         name: nginx
 `)
-	err := th.RunWithErr("/app/overlay/staging", th.MakeDefaultOptions())
-	if err == nil {
-		t.Fatalf("Expected error")
-	}
-	if !strings.Contains(
-		err.Error(), "both containing ") {
-		t.Fatalf("Unexpected err: %v", err)
+	opt := th.MakeDefaultOptions()
+	if opt.UseKyaml {
+		// kyaml doesn't fail on conflicts in patches; both containers
+		// (nginx and sidecar) are deleted per this patching instruction.
+		m := th.Run("/app/overlay/staging", opt)
+		th.AssertActualEqualsExpected(m, `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  annotations:
+    note: This is a test annotation
+  labels:
+    app: mynginx
+    env: staging
+    org: example.com
+    team: foo
+  name: staging-team-foo-nginx
+spec:
+  selector:
+    matchLabels:
+      app: mynginx
+      env: staging
+      org: example.com
+      team: foo
+  template:
+    metadata:
+      annotations:
+        note: This is a test annotation
+      labels:
+        app: mynginx
+        env: staging
+        org: example.com
+        team: foo
+    spec:
+      containers: []
+      volumes:
+      - configMap:
+          name: staging-team-foo-configmap-in-base-798k5k7g9f
+        name: configmap-in-base
+---
+apiVersion: v1
+kind: Service
+metadata:
+  annotations:
+    note: This is a test annotation
+  labels:
+    app: mynginx
+    env: staging
+    org: example.com
+    team: foo
+  name: staging-team-foo-nginx
+spec:
+  ports:
+  - port: 80
+  selector:
+    app: mynginx
+    env: staging
+    org: example.com
+    team: foo
+---
+apiVersion: v1
+data:
+  foo: bar
+kind: ConfigMap
+metadata:
+  annotations:
+    note: This is a test annotation
+  labels:
+    app: mynginx
+    env: staging
+    org: example.com
+    team: foo
+  name: staging-team-foo-configmap-in-base-798k5k7g9f
+---
+apiVersion: v1
+data:
+  hello: world
+kind: ConfigMap
+metadata:
+  labels:
+    env: staging
+  name: staging-configmap-in-overlay-dc6fm46dhm
+`)
+	} else {
+		// No kyaml means error on a patch conflict.
+		err := th.RunWithErr("/app/overlay/staging", opt)
+		if err == nil {
+			t.Fatalf("Expected error")
+		}
+		if !strings.Contains(
+			err.Error(), "both containing ") {
+			t.Fatalf("Unexpected err: %v", err)
+		}
 	}
 }