Handle missing pids during sidecar teardown

Make the sidecar container script tolerate missing pids during cleanup to prevent spurious errors on shutdown.
openshift · Dec 4, 2019 · 52362da · 52362da
1 parent 6e9e960
commit 52362da
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 5 deletions.
diff --git a/assets/dns/daemonset.yaml b/assets/dns/daemonset.yaml
@@ -75,7 +75,7 @@ spec:
           #!/bin/bash
           set -uo pipefail
 
-          trap 'kill $(jobs -p); exit 0' TERM
+          trap 'jobs -p | xargs kill || true; wait; exit 0' TERM
 
           OPENSHIFT_MARKER="openshift-generated-node-resolver"
           HOSTS_FILE="/etc/hosts"

diff --git a/pkg/manifests/bindata.go b/pkg/manifests/bindata.go
diff --git a/pkg/operator/controller/controller_dns_daemonset.go b/pkg/operator/controller/controller_dns_daemonset.go
@@ -194,6 +194,21 @@ func daemonsetConfigChanged(current, expected *appsv1.DaemonSet) (bool, *appsv1.
 		changed = true
 	}
 
+	// Detect changes to container commands
+	if len(current.Spec.Template.Spec.Containers) != len(expected.Spec.Template.Spec.Containers) {
+		updated.Spec.Template.Spec.Containers = expected.Spec.Template.Spec.Containers
+		changed = true
+	} else {
+		for i, a := range current.Spec.Template.Spec.Containers {
+			b := expected.Spec.Template.Spec.Containers[i]
+			if !cmp.Equal(a.Command, b.Command, cmpopts.EquateEmpty()) {
+				updated.Spec.Template.Spec.Containers = expected.Spec.Template.Spec.Containers
+				changed = true
+				break
+			}
+		}
+	}
+
 	if !changed {
 		return false, nil
 	}

diff --git a/pkg/operator/controller/controller_dns_daemonset_test.go b/pkg/operator/controller/controller_dns_daemonset_test.go
@@ -104,6 +104,20 @@ func TestDaemonsetConfigChanged(t *testing.T) {
 			},
 			expect: true,
 		},
+		{
+			description: "if a container command length changed",
+			mutate: func(daemonset *appsv1.DaemonSet) {
+				daemonset.Spec.Template.Spec.Containers[1].Command = append(daemonset.Spec.Template.Spec.Containers[1].Command, "--foo")
+			},
+			expect: true,
+		},
+		{
+			description: "if a container command contents changed",
+			mutate: func(daemonset *appsv1.DaemonSet) {
+				daemonset.Spec.Template.Spec.Containers[0].Command[1] = "c"
+			},
+			expect: true,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -120,10 +134,18 @@ func TestDaemonsetConfigChanged(t *testing.T) {
 							{
 								Name:  "dns",
 								Image: "openshift/origin-coredns:v4.0",
+								Command: []string{
+									"a",
+									"b",
+								},
 							},
 							{
 								Name:  "dns-node-resolver",
 								Image: "openshift/origin-cli:v4.0",
+								Command: []string{
+									"c",
+									"d",
+								},
 							},
 						},
 						NodeSelector: map[string]string{