kubectl apply admits Deployment with duplicate volumes then fails when renaming duplicate

[dfjones]

What happened:

kubectl apply allows Deployments with volumes with the same name to be created. If the duplicate volume is then removed through a subsequent kubectl apply, this fails with an error.

What you expected to happen:

kubectly apply should reject the Deployment with duplicate volume names.

How to reproduce it (as minimally and precisely as possible):

I've created a repo that has a full reproduction: https://github.com/dfjones/kubectl-apply-volume-bug

Environment:

• Kubernetes version (use kubectl version):

kubectl version
Client Version: version.Info{Major:"1", Minor:"14", GitVersion:"v1.14.2", GitCommit:"66049e3b21efe110454d67df4fa62b08ea79a19b", GitTreeState:"clean", BuildDate:"2019-05-16T18:55:03Z", GoVersion:"go1.12.5", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"12", GitVersion:"v1.12.4", GitCommit:"f49fa022dbe63faafd0da106ef7e05a29721d3f1", GitTreeState:"clean", BuildDate:"2018-12-14T06:59:37Z", GoVersion:"go1.10.4", Compiler:"gc", Platform:"linux/amd64"}

• Cloud provider or hardware configuration:
  self hosted

[dfjones]

/sig cli

[vasrem]

I tried to investigate a little bit as well since I had the same issue. I used your reproduction gitrepo.

I get this when I apply for the first time to good.yaml:

I0524 15:43:13.677174   24285 request.go:942] Request Body: {"apiVersion":"apps/v1","kind":"Deployment","metadata":{"annotations":{"kubectl.kubernetes.io/last-applied-configuration":"{\"apiVersion\":\"apps/v1\",\"kind\":\"Deployment\",\"metadata\":{\"annotations\":{},\"labels\":{\"service\":\"bad-volumes-test\"},\"name\":\"bad-volumes-test\",\"namespace\":\"bad-volumes-test\"},\"spec\":{\"replicas\":1,\"selector\":{\"matchLabels\":{\"service\":\"bad-volumes-test\"}},\"template\":{\"metadata\":{\"labels\":{\"service\":\"bad-volumes-test\"},\"name\":\"bad-volumes-test\",\"namespace\":\"bad-volumes-test\"},\"spec\":{\"containers\":[{\"args\":[\"-f\",\"/dev/null\"],\"command\":[\"tail\"],\"image\":\"bash:latest\",\"name\":\"noop\",\"resources\":{\"limits\":{\"cpu\":\"5m\",\"memory\":\"10Mi\"},\"requests\":{\"cpu\":\"5m\",\"memory\":\"10Mi\"}},\"volumeMounts\":[{\"mountPath\":\"/service/test\",\"name\":\"config\"}]}],\"volumes\":[{\"configMap\":{\"items\":[{\"key\":\"a.txt\",\"path\":\"a.txt\"}],\"name\":\"test\"},\"name\":\"config\"}]}}}}\n"},"labels":{"service":"bad-volumes-test"},"name":"bad-volumes-test","namespace":"bad-volumes-test"},"spec":{"replicas":1,"selector":{"matchLabels":{"service":"bad-volumes-test"}},"template":{"metadata":{"labels":{"service":"bad-volumes-test"},"name":"bad-volumes-test","namespace":"bad-volumes-test"},"spec":{"containers":[{"args":["-f","/dev/null"],"command":["tail"],"image":"bash:latest","name":"noop","resources":{"limits":{"cpu":"5m","memory":"10Mi"},"requests":{"cpu":"5m","memory":"10Mi"}},"volumeMounts":[{"mountPath":"/service/test","name":"config"}]}],"volumes":[{"configMap":{"items":[{"key":"a.txt","path":"a.txt"}],"name":"test"},"name":"config"}]}}}}
I0524 15:43:13.664253   24285 round_trippers.go:419] curl -k -v -XPOST  -H "Accept: application/json" -H "Content-Type: application/json" -H "User-Agent: kubectl/v1.14.2 (darwin/amd64) kubernetes/66049e3" 'https://localhost:55042/apis/apps/v1/namespaces/bad-volumes-test/deployments'
I0524 15:43:13.677193   24285 round_trippers.go:438] POST https://localhost:55042/apis/apps/v1/namespaces/bad-volumes-test/deployments 201 Created in 12 milliseconds
I0524 15:43:13.677231   24285 round_trippers.go:444] Response Headers:
I0524 15:43:13.677244   24285 round_trippers.go:447]     Date: Fri, 24 May 2019 13:43:13 GMT
I0524 15:43:13.677254   24285 round_trippers.go:447]     Content-Type: application/json
I0524 15:43:13.677262   24285 round_trippers.go:447]     Content-Length: 2329
I0524 15:43:13.677311   24285 request.go:942] Response Body: {"kind":"Deployment","apiVersion":"apps/v1","metadata":{"name":"bad-volumes-test","namespace":"bad-volumes-test","selfLink":"/apis/apps/v1/namespaces/bad-volumes-test/deployments/bad-volumes-test","uid":"e082875b-7e29-11e9-87ef-0242ac110003","resourceVersion":"13867","generation":1,"creationTimestamp":"2019-05-24T13:43:13Z","labels":{"service":"bad-volumes-test"},"annotations":{"kubectl.kubernetes.io/last-applied-configuration":"{\"apiVersion\":\"apps/v1\",\"kind\":\"Deployment\",\"metadata\":{\"annotations\":{},\"labels\":{\"service\":\"bad-volumes-test\"},\"name\":\"bad-volumes-test\",\"namespace\":\"bad-volumes-test\"},\"spec\":{\"replicas\":1,\"selector\":{\"matchLabels\":{\"service\":\"bad-volumes-test\"}},\"template\":{\"metadata\":{\"labels\":{\"service\":\"bad-volumes-test\"},\"name\":\"bad-volumes-test\",\"namespace\":\"bad-volumes-test\"},\"spec\":{\"containers\":[{\"args\":[\"-f\",\"/dev/null\"],\"command\":[\"tail\"],\"image\":\"bash:latest\",\"name\":\"noop\",\"resources\":{\"limits\":{\"cpu\":\"5m\",\"memory\":\"10Mi\"},\"requests\":{\"cpu\":\"5m\",\"memory\":\"10Mi\"}},\"volumeMounts\":[{\"mountPath\":\"/service/test\",\"name\":\"config\"}]}],\"volumes\":[{\"configMap\":{\"items\":[{\"key\":\"a.txt\",\"path\":\"a.txt\"}],\"name\":\"test\"},\"name\":\"config\"}]}}}}\n"}},"spec":{"replicas":1,"selector":{"matchLabels":{"service":"bad-volumes-test"}},"template":{"metadata":{"name":"bad-volumes-test","namespace":"bad-volumes-test","creationTimestamp":null,"labels":{"service":"bad-volumes-test"}},"spec":{"volumes":[{"name":"config","configMap":{"name":"test","items":[{"key":"a.txt","path":"a.txt"}],"defaultMode":420}}],"containers":[{"name":"noop","image":"bash:latest","command":["tail"],"args":["-f","/dev/null"],"resources":{"limits":{"cpu":"5m","memory":"10Mi"},"requests":{"cpu":"5m","memory":"10Mi"}},"volumeMounts":[{"name":"config","mountPath":"/service/test"}],"terminationMessagePath":"/dev/termination-log","terminationMessagePolicy":"File","imagePullPolicy":"Always"}],"restartPolicy":"Always","terminationGracePeriodSeconds":30,"dnsPolicy":"ClusterFirst","securityContext":{},"schedulerName":"default-scheduler"}},"strategy":{"type":"RollingUpdate","rollingUpdate":{"maxUnavailable":"25%","maxSurge":"25%"}},"revisionHistoryLimit":10,"progressDeadlineSeconds":600},"status":{}}
deployment.apps/bad-volumes-test created

This looks fine! When I apply the bad.yaml on top I get this:

I0524 15:45:09.613869   24301 request.go:942] Request Body: {"metadata":{"annotations":{"kubectl.kubernetes.io/last-applied-configuration":"{\"apiVersion\":\"apps/v1\",\"kind\":\"Deployment\",\"metadata\":{\"annotations\":{},\"labels\":{\"service\":\"bad-volumes-test\"},\"name\":\"bad-volumes-test\",\"namespace\":\"bad-volumes-test\"},\"spec\":{\"replicas\":1,\"selector\":{\"matchLabels\":{\"service\":\"bad-volumes-test\"}},\"template\":{\"metadata\":{\"labels\":{\"service\":\"bad-volumes-test\"},\"name\":\"bad-volumes-test\",\"namespace\":\"bad-volumes-test\"},\"spec\":{\"containers\":[{\"args\":[\"-f\",\"/dev/null\"],\"command\":[\"tail\"],\"image\":\"bash:latest\",\"name\":\"noop\",\"resources\":{\"limits\":{\"cpu\":\"5m\",\"memory\":\"10Mi\"},\"requests\":{\"cpu\":\"5m\",\"memory\":\"10Mi\"}},\"volumeMounts\":[{\"mountPath\":\"/service/test\",\"name\":\"config\"}]}],\"volumes\":[{\"configMap\":{\"items\":[{\"key\":\"a.txt\",\"path\":\"a.txt\"}],\"name\":\"test\"},\"name\":\"config\"},{\"configMap\":{\"items\":[{\"key\":\"b.txt\",\"path\":\"b.txt\"}],\"name\":\"foo\"},\"name\":\"config\"}]}}}}\n"}},"spec":{"template":{"spec":{"$setElementOrder/volumes":[{"name":"config"},{"name":"config"}],"volumes":[{"$retainKeys":["configMap","name"],"configMap":{"items":[{"key":"b.txt","path":"b.txt"}],"name":"foo"},"name":"config"},{"configMap":{"items":[{"key":"a.txt","path":"a.txt"}],"name":"test"},"name":"config"}]}}}}
I0524 15:45:09.614007   24301 round_trippers.go:419] curl -k -v -XPATCH  -H "Accept: application/json" -H "Content-Type: application/strategic-merge-patch+json" -H "User-Agent: kubectl/v1.14.2 (darwin/amd64) kubernetes/66049e3" 'https://localhost:55042/apis/apps/v1/namespaces/bad-volumes-test/deployments/bad-volumes-test'
I0524 15:45:09.627218   24301 round_trippers.go:438] PATCH https://localhost:55042/apis/apps/v1/namespaces/bad-volumes-test/deployments/bad-volumes-test 200 OK in 13 milliseconds
I0524 15:45:09.627243   24301 round_trippers.go:444] Response Headers:
I0524 15:45:09.627251   24301 round_trippers.go:447]     Content-Type: application/json
I0524 15:45:09.627259   24301 round_trippers.go:447]     Content-Length: 3031
I0524 15:45:09.627265   24301 round_trippers.go:447]     Date: Fri, 24 May 2019 13:45:09 GMT
I0524 15:45:09.628174   24301 request.go:942] Response Body: {"kind":"Deployment","apiVersion":"apps/v1","metadata":{"name":"bad-volumes-test","namespace":"bad-volumes-test","selfLink":"/apis/apps/v1/namespaces/bad-volumes-test/deployments/bad-volumes-test","uid":"e082875b-7e29-11e9-87ef-0242ac110003","resourceVersion":"14028","generation":2,"creationTimestamp":"2019-05-24T13:43:13Z","labels":{"service":"bad-volumes-test"},"annotations":{"deployment.kubernetes.io/revision":"1","kubectl.kubernetes.io/last-applied-configuration":"{\"apiVersion\":\"apps/v1\",\"kind\":\"Deployment\",\"metadata\":{\"annotations\":{},\"labels\":{\"service\":\"bad-volumes-test\"},\"name\":\"bad-volumes-test\",\"namespace\":\"bad-volumes-test\"},\"spec\":{\"replicas\":1,\"selector\":{\"matchLabels\":{\"service\":\"bad-volumes-test\"}},\"template\":{\"metadata\":{\"labels\":{\"service\":\"bad-volumes-test\"},\"name\":\"bad-volumes-test\",\"namespace\":\"bad-volumes-test\"},\"spec\":{\"containers\":[{\"args\":[\"-f\",\"/dev/null\"],\"command\":[\"tail\"],\"image\":\"bash:latest\",\"name\":\"noop\",\"resources\":{\"limits\":{\"cpu\":\"5m\",\"memory\":\"10Mi\"},\"requests\":{\"cpu\":\"5m\",\"memory\":\"10Mi\"}},\"volumeMounts\":[{\"mountPath\":\"/service/test\",\"name\":\"config\"}]}],\"volumes\":[{\"configMap\":{\"items\":[{\"key\":\"a.txt\",\"path\":\"a.txt\"}],\"name\":\"test\"},\"name\":\"config\"},{\"configMap\":{\"items\":[{\"key\":\"b.txt\",\"path\":\"b.txt\"}],\"name\":\"foo\"},\"name\":\"config\"}]}}}}\n"}},"spec":{"replicas":1,"selector":{"matchLabels":{"service":"bad-volumes-test"}},"template":{"metadata":{"name":"bad-volumes-test","namespace":"bad-volumes-test","creationTimestamp":null,"labels":{"service":"bad-volumes-test"}},"spec":{"volumes":[{"name":"config","configMap":{"name":"test","items":[{"key":"a.txt","path":"a.txt"}],"defaultMode":420}}],"containers":[{"name":"noop","image":"bash:latest","command":["tail"],"args":["-f","/dev/null"],"resources":{"limits":{"cpu":"5m","memory":"10Mi"},"requests":{"cpu":"5m","memory":"10Mi"}},"volumeMounts":[{"name":"config","mountPath":"/service/test"}],"terminationMessagePath":"/dev/termination-log","terminationMessagePolicy":"File","imagePullPolicy":"Always"}],"restartPolicy":"Always","terminationGracePeriodSeconds":30,"dnsPolicy":"ClusterFirst","securityContext":{},"schedulerName":"default-scheduler"}},"strategy":{"type":"RollingUpdate","rollingUpdate":{"maxUnavailable":"25%","maxSurge":"25%"}},"revisionHistoryLimit":10,"progressDeadlineSeconds":600},"status":{"observedGeneration":1,"replicas":1,"updatedReplicas":1,"readyReplicas":1,"availableReplicas":1,"conditions":[{"type":"Available","status":"True","lastUpdateTime":"2019-05-24T13:43:22Z","lastTransitionTime":"2019-05-24T13:43:22Z","reason":"MinimumReplicasAvailable","message":"Deployment has minimum availability."},{"type":"Progressing","status":"True","lastUpdateTime":"2019-05-24T13:43:22Z","lastTransitionTime":"2019-05-24T13:43:13Z","reason":"NewReplicaSetAvailable","message":"ReplicaSet \"bad-volumes-test-88d66c855\" has successfully progressed."}]}}
deployment.apps/bad-volumes-test configured

Which seems weird because I get a 200 but the response body doesn't contain the volume using the ConfigMap named foo. Also, the deployment definition stored in the etcd remains the same after applying the bad.yaml (just last-applied-config, generation and observedGeneration are changed).

If I just try to apply the bad.yaml without applying the good.yaml before, I get the correct error:

The Deployment "bad-volumes-test" is invalid: spec.template.spec.volumes[1].name: Duplicate value: "config"

Which leads to the fact that most likely the problem occurs when we do update of a Resource(I only tested with your repo, so in this case Deployment). I defined some tests to find out if the validator is working correctly, and I couldn't find any problem there.

diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go
index f3523c5ada..a930a04648 100644
--- a/pkg/api/pod/util_test.go
+++ b/pkg/api/pod/util_test.go
@@ -1660,7 +1660,54 @@ func TestDropPodSysctls(t *testing.T) {
                }
        }
 }
+func TestDropDisabledTemplateFields(t *testing.T) {
+       old := func() *api.PodTemplateSpec {
+               return &api.PodTemplateSpec{
+                       Spec: api.PodSpec{
+                               Containers: []api.Container{{Name: "container1", Image: "testimage", VolumeMounts: []api.VolumeMount{{Name: "config", MountPath: "/service/test"}}}},
+                               Volumes: []api.Volume{
+                                       {Name: "config", VolumeSource: api.VolumeSource{ConfigMap: &api.ConfigMapVolumeSource{Items: []api.KeyToPath{{Key: "a.txt", Path: "a.txt"}}}}},
+                               },
+                       },
+               }
+       }
+       new := func() *api.PodTemplateSpec {
+               return &api.PodTemplateSpec{
+                       Spec: api.PodSpec{
+                               Containers: []api.Container{{Name: "container1", Image: "testimage", VolumeMounts: []api.VolumeMount{{Name: "config", MountPath: "/service/test"}}}},
+                               Volumes: []api.Volume{
+                                       {Name: "config", VolumeSource: api.VolumeSource{ConfigMap: &api.ConfigMapVolumeSource{Items: []api.KeyToPath{{Key: "a.txt", Path: "a.txt"}}}}},
+                                       {Name: "config", VolumeSource: api.VolumeSource{ConfigMap: &api.ConfigMapVolumeSource{Items: []api.KeyToPath{{Key: "b.txt", Path: "b.txt"}}}}},
+                               },
+                       },
+               }
+       }

+       tests := []struct {
+               oldPod func() *api.PodTemplateSpec
+               newPod func() *api.PodTemplateSpec
+               expectedOldPodVolumes int
+               expectedNewPodVolumes int
+       }{
+               {
+                       oldPod: old,
+                       newPod: new,
+                       expectedOldPodVolumes: 1,
+                       expectedNewPodVolumes: 2,
+               },
+       }
+       for _, test := range tests {
+               newPod := test.newPod()
+               oldPod := test.oldPod()
+               DropDisabledTemplateFields(newPod, oldPod)
+               if test.expectedNewPodVolumes != len(newPod.Spec.Volumes){
+                       t.Errorf("new pod has wrong number of volumes, expected = %d, actual = %d", test.expectedNewPodVolumes, len(newPod.Spec.Volumes))
+               }
+               if test.expectedOldPodVolumes != len(oldPod.Spec.Volumes){
+                       t.Errorf("old pod has wrong number of volumes, expected = %d, actual = %d", test.expectedNewPodVolumes, len(newPod.Spec.Volumes))
+               }
+       }
+}
 func TestDropSubPathExpr(t *testing.T) {
        podWithSubpaths := func() *api.Pod {
                return &api.Pod{

After this I tried to find if there is a problem in the ValidatePodSpec function using the following test in order to get the output of the function:

diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go
index 2feccddb9b..1947280dea 100644
--- a/pkg/apis/core/validation/validation_test.go
+++ b/pkg/apis/core/validation/validation_test.go
@@ -6333,6 +6333,15 @@ func TestValidatePodSpec(t *testing.T) {
                        DNSPolicy:        core.DNSClusterFirst,
                        RuntimeClassName: utilpointer.StringPtr("valid-sandbox"),
                },
+               {
+                       Containers:    []core.Container{{Name: "container1", Image: "testimage", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File", VolumeMounts: []core.VolumeMount{{Name: "config", MountPath: "/service/test"}}}},
+                       RestartPolicy: core.RestartPolicyAlways,
+                       DNSPolicy:     core.DNSClusterFirst,
+                       Volumes: []core.Volume{
+                               {Name: "config", VolumeSource: core.VolumeSource{ConfigMap: &core.ConfigMapVolumeSource{LocalObjectReference: core.LocalObjectReference{Name: "foo"}, Items: []core.KeyToPath{{Key: "a.txt", Path: "a.txt"}}}}},
+                               {Name: "config", VolumeSource: core.VolumeSource{ConfigMap: &core.ConfigMapVolumeSource{LocalObjectReference: core.LocalObjectReference{Name: "bar"}, Items: []core.KeyToPath{{Key: "b.txt", Path: "b.txt"}}}}},
+                       },
+               },
        }
        for i := range successCases {
                if errs := ValidatePodSpec(&successCases[i], field.NewPath("field")); len(errs) != 0 {

And there I get:

--- FAIL: TestValidatePodSpec (0.00s)
    /Users/v.remmas/work/k8s/kubernetes/pkg/apis/core/validation/validation_test.go:6348: expected success: [field.volumes[1].name: Duplicate value: "config"]

which is correct.

So as far as I can see, there is no problem at the validator. My idea now is that the object might reach the BeforeUpdate function "corrupted". Unfortunately, I don't have much experience of the codebase so this was what I could get after some time of debugging.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

[k8s-ci-robot]

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.