Repository: linkerd/linkerd2
Pull request #5532 (2021-01-13 20:04:48)

Cherry-picks and release notes for stable-2.9.2
## stable-2.9.2

This stable release fixes an issue that stops traffic to a pod when there is an
IP address conflict with another pod that is not in a running state.

It also fixes an upgrade issue when using HA that would lead to values being
overridden.

Signed-off-by: Kevin Leimkuhler <kevin@kleimkuhler.com>


kleimkuhler: I needed to force push to fix one of the cherry picked `Signed off by` trailers.

Patch:
diff --git a/CHANGES.md b/CHANGES.md
index 6bc053308..72c549d29 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -1,5 +1,13 @@
 # Changes
 
+## stable-2.9.2
+
+This stable release fixes an issue that stops traffic to a pod when there is an
+IP address conflict with another pod that is not in a running state.
+
+It also fixes an upgrade issue when using HA that would lead to values being
+overridden.
+
 ## stable-2.9.1
 
 This stable release contains a number of proxy enhancements: better support for
diff --git a/cli/cmd/check.go b/cli/cmd/check.go
index 556051228..6b25025e8 100644
--- a/cli/cmd/check.go
+++ b/cli/cmd/check.go
@@ -369,7 +369,7 @@ func runChecksJSON(wout io.Writer, werr io.Writer, hc *healthcheck.HealthChecker
 }
 
 func renderInstallManifest(ctx context.Context) (string, error) {
-	values, err := charts.NewValues(false)
+	values, err := charts.NewValues()
 	if err != nil {
 		return "", err
 	}
diff --git a/cli/cmd/inject.go b/cli/cmd/inject.go
index 16bfa284f..03c07cf9f 100644
--- a/cli/cmd/inject.go
+++ b/cli/cmd/inject.go
@@ -48,7 +48,7 @@ func runInjectCmd(inputs []io.Reader, errWriter, outWriter io.Writer, transforme
 }
 
 func newCmdInject() *cobra.Command {
-	defaults, err := charts.NewValues(false)
+	defaults, err := charts.NewValues()
 	if err != nil {
 		fmt.Fprint(os.Stderr, err.Error())
 		os.Exit(1)
diff --git a/cli/cmd/install.go b/cli/cmd/install.go
index b0316d308..f2a44530d 100644
--- a/cli/cmd/install.go
+++ b/cli/cmd/install.go
@@ -209,7 +209,7 @@ control plane. It should be run after "linkerd install config".`,
 }
 
 func newCmdInstall() *cobra.Command {
-	values, err := l5dcharts.NewValues(false)
+	values, err := l5dcharts.NewValues()
 
 	allStageFlags, allStageFlagSet := makeAllStageFlags(values)
 	installOnlyFlags, installOnlyFlagSet := makeInstallFlags(values)
@@ -407,7 +407,7 @@ func render(w io.Writer, values *l5dcharts.Values, stage string) error {
 // resource is not part of the Helm chart and will not be present when installing
 // with Helm.
 func renderOverrides(values *l5dcharts.Values, namespace string) ([]byte, error) {
-	defaults, err := l5dcharts.NewValues(false)
+	defaults, err := l5dcharts.NewValues()
 	if err != nil {
 		return nil, err
 	}
diff --git a/cli/cmd/options.go b/cli/cmd/options.go
index 8bda7042c..5dd50718b 100644
--- a/cli/cmd/options.go
+++ b/cli/cmd/options.go
@@ -54,12 +54,7 @@ func makeInstallUpgradeFlags(defaults *l5dcharts.Values) ([]flag.Flag, *pflag.Fl
 			func(values *l5dcharts.Values, value bool) error {
 				values.GetGlobal().HighAvailability = value
 				if value {
-					haValues, err := l5dcharts.NewValues(true)
-					if err != nil {
-						return err
-					}
-					*values, err = values.Merge(*haValues)
-					if err != nil {
+					if err := l5dcharts.MergeHAValues(values); err != nil {
 						return err
 					}
 				}
diff --git a/cli/cmd/upgrade.go b/cli/cmd/upgrade.go
index 69427c79f..e26ca39b0 100644
--- a/cli/cmd/upgrade.go
+++ b/cli/cmd/upgrade.go
@@ -114,7 +114,7 @@ install command. It should be run after "linkerd upgrade config".`,
 }
 
 func newCmdUpgrade() *cobra.Command {
-	values, err := l5dcharts.NewValues(false)
+	values, err := l5dcharts.NewValues()
 	if err != nil {
 		fmt.Fprint(os.Stderr, err.Error())
 		os.Exit(1)
@@ -290,7 +290,7 @@ func upgrade(ctx context.Context, k *k8s.KubernetesAPI, flags []flag.Flag, stage
 
 func loadStoredValues(ctx context.Context, k *k8s.KubernetesAPI) (*charts.Values, error) {
 	// Load the default values from the chart.
-	values, err := charts.NewValues(false)
+	values, err := charts.NewValues()
 	if err != nil {
 		return nil, err
 	}
@@ -361,7 +361,7 @@ func ensureIssuerCertWorksWithAllProxies(ctx context.Context, k *k8s.KubernetesA
 }
 
 func clearAddonOverrides(values *l5dcharts.Values) error {
-	defaults, err := l5dcharts.NewValues(false)
+	defaults, err := l5dcharts.NewValues()
 	if err != nil {
 		return err
 	}
diff --git a/cli/cmd/upgrade_legacy.go b/cli/cmd/upgrade_legacy.go
index 759cbdb1e..bb71e909c 100644
--- a/cli/cmd/upgrade_legacy.go
+++ b/cli/cmd/upgrade_legacy.go
@@ -32,7 +32,7 @@ func loadStoredValuesLegacy(ctx context.Context, k *k8s.KubernetesAPI) (*charts.
 	}
 	repairConfigs(configs)
 
-	values, err := charts.NewValues(false)
+	values, err := charts.NewValues()
 	if err != nil {
 		return nil, err
 	}
diff --git a/controller/api/destination/watcher/ip_watcher.go b/controller/api/destination/watcher/ip_watcher.go
index d3028f577..4451fac43 100644
--- a/controller/api/destination/watcher/ip_watcher.go
+++ b/controller/api/destination/watcher/ip_watcher.go
@@ -154,10 +154,23 @@ func (iw *IPWatcher) GetPod(podIP string) (*corev1.Pod, error) {
 }
 
 func getResource(ip string, informer cache.SharedIndexInformer) (interface{}, error) {
-	objs, err := informer.GetIndexer().ByIndex(podIPIndex, ip)
+	matchingObjs, err := informer.GetIndexer().ByIndex(podIPIndex, ip)
 	if err != nil {
 		return nil, status.Error(codes.Unknown, err.Error())
 	}
+
+	objs := make([]interface{}, 0)
+	for _, obj := range matchingObjs {
+		// Ignore terminated pods,
+		// their IPs can be reused for new Running pods
+		pod, ok := obj.(*corev1.Pod)
+		if ok && podTerminated(pod) {
+			continue
+		}
+
+		objs = append(objs, obj)
+	}
+
 	if len(objs) > 1 {
 		return nil, status.Errorf(codes.FailedPrecondition, "IP address conflict: %v, %v", objs[0], objs[1])
 	}
@@ -167,6 +180,11 @@ func getResource(ip string, informer cache.SharedIndexInformer) (interface{}, er
 	return nil, nil
 }
 
+func podTerminated(pod *corev1.Pod) bool {
+	phase := pod.Status.Phase
+	return phase == corev1.PodSucceeded || phase == corev1.PodFailed
+}
+
 func (iw *IPWatcher) addService(obj interface{}) {
 	service := obj.(*corev1.Service)
 	if service.Namespace == kubeSystem || service.Spec.ClusterIP == "None" {
@@ -285,10 +303,18 @@ func (iw *IPWatcher) getOrNewServiceSubscriptions(clusterIP string) *serviceSubs
 			pods := []*corev1.Pod{}
 			for _, obj := range objs {
 				if pod, ok := obj.(*corev1.Pod); ok {
-					// Skip pods with HostNetwork.
-					if !pod.Spec.HostNetwork {
-						pods = append(pods, pod)
+					// Skip pods with HostNetwork
+					if pod.Spec.HostNetwork {
+						continue
 					}
+
+					// Ignore terminated pods,
+					// their IPs can be reused for new Running pods
+					if podTerminated(pod) {
+						continue
+					}
+
+					pods = append(pods, pod)
 				}
 			}
 			if len(pods) > 1 {
diff --git a/pkg/charts/linkerd2/values.go b/pkg/charts/linkerd2/values.go
index a425191a6..a79ecd240 100644
--- a/pkg/charts/linkerd2/values.go
+++ b/pkg/charts/linkerd2/values.go
@@ -256,9 +256,8 @@ type (
 )
 
 // NewValues returns a new instance of the Values type.
-func NewValues(ha bool) (*Values, error) {
-	chartDir := fmt.Sprintf("%s/", helmDefaultChartDir)
-	v, err := readDefaults(chartDir, ha)
+func NewValues() (*Values, error) {
+	v, err := readDefaults(false)
 	if err != nil {
 		return nil, err
 	}
@@ -275,38 +274,34 @@ func NewValues(ha bool) (*Values, error) {
 	return v, nil
 }
 
-// readDefaults read all the default variables from the values.yaml file.
-// chartDir is the root directory of the Helm chart where values.yaml is.
-func readDefaults(chartDir string, ha bool) (*Values, error) {
-	valuesFiles := []*chartutil.BufferedFile{
-		{Name: chartutil.ValuesfileName},
+// MergeHAValues retrieves the default HA values and merges them into the received values
+func MergeHAValues(values *Values) error {
+	haValues, err := readDefaults(true)
+	if err != nil {
+		return err
 	}
+	*values, err = values.Merge(*haValues)
+	return err
+}
 
+// readDefaults read all the default variables from the values.yaml file.
+func readDefaults(ha bool) (*Values, error) {
+	var valuesFile *chartutil.BufferedFile
 	if ha {
-		valuesFiles = append(valuesFiles, &chartutil.BufferedFile{
-			Name: helmDefaultHAValuesFile,
-		})
+		valuesFile = &chartutil.BufferedFile{Name: helmDefaultHAValuesFile}
+	} else {
+		valuesFile = &chartutil.BufferedFile{Name: chartutil.ValuesfileName}
 	}
 
-	if err := charts.FilesReader(chartDir, valuesFiles); err != nil {
+	chartDir := fmt.Sprintf("%s/", helmDefaultChartDir)
+	if err := charts.ReadFile(chartDir, valuesFile); err != nil {
 		return nil, err
 	}
 
-	values := Values{}
-	for _, valuesFile := range valuesFiles {
-		var v Values
-		if err := yaml.Unmarshal(charts.InsertVersion(valuesFile.Data), &v); err != nil {
-			return nil, err
-		}
-
-		var err error
-		values, err = values.Merge(v)
-		if err != nil {
-			return nil, err
-		}
-	}
+	var values Values
+	err := yaml.Unmarshal(charts.InsertVersion(valuesFile.Data), &values)
 
-	return &values, nil
+	return &values, err
 }
 
 // Merge merges the non-empty properties of src into v.


Test patch:
diff --git a/cli/cmd/inject_test.go b/cli/cmd/inject_test.go
index ba2f79db1..312d1c802 100644
--- a/cli/cmd/inject_test.go
+++ b/cli/cmd/inject_test.go
@@ -587,7 +587,7 @@ func TestWalk(t *testing.T) {
 }
 
 func TestProxyConfigurationAnnotations(t *testing.T) {
-	baseValues, err := linkerd2.NewValues(false)
+	baseValues, err := linkerd2.NewValues()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -643,7 +643,7 @@ func TestProxyConfigurationAnnotations(t *testing.T) {
 }
 
 func TestProxyImageAnnotations(t *testing.T) {
-	baseValues, err := linkerd2.NewValues(false)
+	baseValues, err := linkerd2.NewValues()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -669,7 +669,7 @@ func TestProxyImageAnnotations(t *testing.T) {
 }
 
 func TestProxyInitImageAnnotations(t *testing.T) {
-	baseValues, err := linkerd2.NewValues(false)
+	baseValues, err := linkerd2.NewValues()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -693,7 +693,7 @@ func TestProxyInitImageAnnotations(t *testing.T) {
 }
 
 func TestNoAnnotations(t *testing.T) {
-	baseValues, err := linkerd2.NewValues(false)
+	baseValues, err := linkerd2.NewValues()
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/cli/cmd/install_helm_test.go b/cli/cmd/install_helm_test.go
index 2d410b246..1d9c62694 100644
--- a/cli/cmd/install_helm_test.go
+++ b/cli/cmd/install_helm_test.go
@@ -319,10 +319,15 @@ func chartPartials(t *testing.T, paths []string) *pb.Chart {
 }
 
 func readTestValues(ha bool, ignoreOutboundPorts string, ignoreInboundPorts string) (*l5dcharts.Values, error) {
-	values, err := l5dcharts.NewValues(ha)
+	values, err := l5dcharts.NewValues()
 	if err != nil {
 		return nil, err
 	}
+	if ha {
+		if err = l5dcharts.MergeHAValues(values); err != nil {
+			return nil, err
+		}
+	}
 	values.GetGlobal().ProxyInit.IgnoreOutboundPorts = ignoreOutboundPorts
 	values.GetGlobal().ProxyInit.IgnoreInboundPorts = ignoreInboundPorts
 
diff --git a/cli/cmd/install_test.go b/cli/cmd/install_test.go
index b98c20378..eb841dec3 100644
--- a/cli/cmd/install_test.go
+++ b/cli/cmd/install_test.go
@@ -319,10 +319,15 @@ func testInstallOptionsHA(ha bool) (*charts.Values, error) {
 }
 
 func testInstallOptionsNoCerts(ha bool) (*charts.Values, error) {
-	values, err := charts.NewValues(ha)
+	values, err := charts.NewValues()
 	if err != nil {
 		return nil, err
 	}
+	if ha {
+		if err = charts.MergeHAValues(values); err != nil {
+			return nil, err
+		}
+	}
 
 	values.GetGlobal().Proxy.Image.Version = installProxyVersion
 	values.DebugContainer.Image.Version = installDebugVersion
@@ -333,7 +338,7 @@ func testInstallOptionsNoCerts(ha bool) (*charts.Values, error) {
 }
 
 func testInstallValues() (*charts.Values, error) {
-	values, err := charts.NewValues(false)
+	values, err := charts.NewValues()
 	if err != nil {
 		return nil, err
 	}
diff --git a/cli/cmd/testdata/install_addon.golden b/cli/cmd/testdata/install_addon.golden
index f6d98698b..775e42356 100644
--- a/cli/cmd/testdata/install_addon.golden
+++ b/cli/cmd/testdata/install_addon.golden
@@ -2048,6 +2048,7 @@ spec:
   template:
     metadata:
       annotations:
+        checksum/config: 906e23f11c1c920abd60ae9cce09ca9f53673544c9c587ef390110c4a4bfe60d
         linkerd.io/created-by: linkerd/cli dev-undefined
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: install-proxy-version
@@ -2303,6 +2304,7 @@ spec:
   template:
     metadata:
       annotations:
+        checksum/config: 82e78e61c12a83a1c769a6a0b19b1567b0f73e5ebe240277970a23d83de6fe22
         linkerd.io/created-by: linkerd/cli dev-undefined
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: install-proxy-version
@@ -2539,6 +2541,7 @@ spec:
   template:
     metadata:
       annotations:
+        checksum/config: 51c6f0865aca4a1b4e25e619385ff0d9f95683e27c4d29c3072eac39b3fa7220
         linkerd.io/created-by: linkerd/cli dev-undefined
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: install-proxy-version
diff --git a/cli/cmd/testdata/install_tracing.golden b/cli/cmd/testdata/install_tracing.golden
index f6d98698b..775e42356 100644
--- a/cli/cmd/testdata/install_tracing.golden
+++ b/cli/cmd/testdata/install_tracing.golden
@@ -2048,6 +2048,7 @@ spec:
   template:
     metadata:
       annotations:
+        checksum/config: 906e23f11c1c920abd60ae9cce09ca9f53673544c9c587ef390110c4a4bfe60d
         linkerd.io/created-by: linkerd/cli dev-undefined
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: install-proxy-version
@@ -2303,6 +2304,7 @@ spec:
   template:
     metadata:
       annotations:
+        checksum/config: 82e78e61c12a83a1c769a6a0b19b1567b0f73e5ebe240277970a23d83de6fe22
         linkerd.io/created-by: linkerd/cli dev-undefined
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: install-proxy-version
@@ -2539,6 +2541,7 @@ spec:
   template:
     metadata:
       annotations:
+        checksum/config: 51c6f0865aca4a1b4e25e619385ff0d9f95683e27c4d29c3072eac39b3fa7220
         linkerd.io/created-by: linkerd/cli dev-undefined
         linkerd.io/identity-mode: default
         linkerd.io/proxy-version: install-proxy-version
diff --git a/cli/cmd/uninject_test.go b/cli/cmd/uninject_test.go
index bb957894f..ae1312dfc 100644
--- a/cli/cmd/uninject_test.go
+++ b/cli/cmd/uninject_test.go
@@ -93,7 +93,7 @@ func TestUninjectYAML(t *testing.T) {
 		},
 	}
 
-	values, err := charts.NewValues(false)
+	values, err := charts.NewValues()
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/cli/cmd/upgrade_test.go b/cli/cmd/upgrade_test.go
index 7f07633da..c7075c5a3 100644
--- a/cli/cmd/upgrade_test.go
+++ b/cli/cmd/upgrade_test.go
@@ -628,7 +628,7 @@ func TestUpgradeOverwriteRemoveAddonKeys(t *testing.T) {
 /* Helpers */
 
 func testUpgradeOptions() ([]flag.Flag, *pflag.FlagSet, error) {
-	defaults, err := charts.NewValues(false)
+	defaults, err := charts.NewValues()
 	if err != nil {
 		return nil, nil, err
 	}
diff --git a/controller/api/destination/server_test.go b/controller/api/destination/server_test.go
index ec1580ac8..e844e1eb9 100644
--- a/controller/api/destination/server_test.go
+++ b/controller/api/destination/server_test.go
@@ -73,6 +73,24 @@ metadata:
     phase: Running
     podIP: 172.17.0.12`,
 		`
+apiVersion: v1
+kind: Pod
+metadata:
+  name: name2-2
+  namespace: ns
+status:
+  phase: Succeeded
+  podIP: 172.17.0.13`,
+		`
+apiVersion: v1
+kind: Pod
+metadata:
+  name: name2-3
+  namespace: ns
+status:
+  phase: Failed
+  podIP: 172.17.0.13`,
+		`
 apiVersion: linkerd.io/v1alpha2
 kind: ServiceProfile
 metadata:
diff --git a/controller/proxy-injector/webhook_test.go b/controller/proxy-injector/webhook_test.go
index 20e803632..e6a9e3298 100644
--- a/controller/proxy-injector/webhook_test.go
+++ b/controller/proxy-injector/webhook_test.go
@@ -21,7 +21,7 @@ import (
 type unmarshalledPatch []map[string]interface{}
 
 var (
-	values, _ = linkerd2.NewValues(false)
+	values, _ = linkerd2.NewValues()
 )
 
 func confNsEnabled() *inject.ResourceConfig {
diff --git a/pkg/charts/linkerd2/values_test.go b/pkg/charts/linkerd2/values_test.go
index 4dc6b3f3a..b8a568c3f 100644
--- a/pkg/charts/linkerd2/values_test.go
+++ b/pkg/charts/linkerd2/values_test.go
@@ -10,7 +10,7 @@ import (
 )
 
 func TestNewValues(t *testing.T) {
-	actual, err := NewValues(false)
+	actual, err := NewValues()
 	if err != nil {
 		t.Fatalf("Unexpected error: %v\n", err)
 	}
@@ -172,7 +172,7 @@ func TestNewValues(t *testing.T) {
 	}
 
 	t.Run("HA", func(t *testing.T) {
-		actual, err := NewValues(true)
+		err := MergeHAValues(actual)
 
 		if err != nil {
 			t.Fatalf("Unexpected error: %v\n", err)
diff --git a/pkg/healthcheck/healthcheck_test.go b/pkg/healthcheck/healthcheck_test.go
index 1bd2398f8..c5ffb3dd5 100644
--- a/pkg/healthcheck/healthcheck_test.go
+++ b/pkg/healthcheck/healthcheck_test.go
@@ -2546,7 +2546,7 @@ data:
 }
 
 func TestFetchCurrentConfiguration(t *testing.T) {
-	defaultValues, err := linkerd2.NewValues(false)
+	defaultValues, err := linkerd2.NewValues()
 
 	if err != nil {
 		t.Fatalf("Unexpected error validating options: %v", err)
diff --git a/pkg/inject/inject_test.go b/pkg/inject/inject_test.go
index 1c1648e81..39fca138d 100644
--- a/pkg/inject/inject_test.go
+++ b/pkg/inject/inject_test.go
@@ -23,7 +23,7 @@ func TestGetOverriddenValues(t *testing.T) {
 		pullPolicy           = "Always"
 	)
 
-	testConfig, err := l5dcharts.NewValues(false)
+	testConfig, err := l5dcharts.NewValues()
 	if err != nil {
 		t.Fatalf("Unexpected error: %v", err)
 	}
@@ -72,7 +72,7 @@ func TestGetOverriddenValues(t *testing.T) {
 				},
 			},
 			expected: func() *l5dcharts.Values {
-				values, _ := l5dcharts.NewValues(false)
+				values, _ := l5dcharts.NewValues()
 
 				values.GetGlobal().Proxy.Cores = 2
 				values.GetGlobal().Proxy.DisableIdentity = true
@@ -122,7 +122,7 @@ func TestGetOverriddenValues(t *testing.T) {
 				},
 			},
 			expected: func() *l5dcharts.Values {
-				values, _ := l5dcharts.NewValues(false)
+				values, _ := l5dcharts.NewValues()
 				return values
 			},
 		},
@@ -160,7 +160,7 @@ func TestGetOverriddenValues(t *testing.T) {
 				},
 			},
 			expected: func() *l5dcharts.Values {
-				values, _ := l5dcharts.NewValues(false)
+				values, _ := l5dcharts.NewValues()
 
 				values.GetGlobal().Proxy.Cores = 2
 				values.GetGlobal().Proxy.DisableIdentity = true
@@ -212,7 +212,7 @@ func TestGetOverriddenValues(t *testing.T) {
 				},
 			},
 			expected: func() *l5dcharts.Values {
-				values, _ := l5dcharts.NewValues(false)
+				values, _ := l5dcharts.NewValues()
 				return values
 			},
 		},
@@ -229,7 +229,7 @@ func TestGetOverriddenValues(t *testing.T) {
 				},
 			},
 			expected: func() *l5dcharts.Values {
-				values, _ := l5dcharts.NewValues(false)
+				values, _ := l5dcharts.NewValues()
 				values.GetGlobal().Proxy.OutboundConnectTimeout = "6005ms"
 				values.GetGlobal().Proxy.InboundConnectTimeout = "2005ms"
 				return values
@@ -259,7 +259,7 @@ func TestGetOverriddenValues(t *testing.T) {
 				},
 			},
 			expected: func() *l5dcharts.Values {
-				values, _ := l5dcharts.NewValues(false)
+				values, _ := l5dcharts.NewValues()
 				values.GetGlobal().Proxy.OpaquePorts = "3306"
 				return values
 			},