Repository: linkerd/linkerd2
Pull request #4688 (2020-06-30 11:06:33)

Fix `splitStringListToPorts` port range object rendering
The `splitStringListToPorts` helm function is currently incorrectly formating a list of ports as an array of Port objects that look ike `{"port" : 555}`.  The config map protobuf representation however expects that the `ignoreOutboundPorts` and `ignoreInboundPorts` fields are are list of `PortRange` objects (`{"portRange" : 555}`). 

This was causing the injector to return an empty string when trying to parse a `PortRange` object resulting in the ports not getting set correctly when injecting workloads. Note that this is happening only with helm installations as this is when we are actually using a helm template for outputting the config map. 

To fix that the `splitStringListToPorts` helm function is changed to format the objects as the json representation of PortRange and is renamed to `splitStringListToPortRanges`

Fix: #4679

Signed-off-by: Zahari Dichev <zaharidichev@gmail.com>

zaharidichev: @alpeb how do you feel about using the helm template for creating the config map even when installing through the CLI? 
alpeb: > @alpeb how do you feel about using the helm template for creating the config map even when installing through the CLI?

Yes!
So I see that in `config.yaml` we're taking separate paths depending on the existence of `.Values.configs.global` for example, which seems is only set for the CLI. So we should stop setting that. And instead, make sure all the values referred to in `_config.tpl` are actually set by the CLI...
adleong: @zaharidichev ping :)

Patch:
diff --git a/ADOPTERS.md b/ADOPTERS.md
index 38b3ca466..d8d3f3d89 100644
--- a/ADOPTERS.md
+++ b/ADOPTERS.md
@@ -29,6 +29,7 @@
 - [Sue](https://sue.nl)
 - [Transit](https://transit.app)
 - [Vernacular.ai](https://vernacular.ai/)
+- [xCloud](https://www.xbox.com/en-US/xbox-game-streaming/project-xcloud)
 - [YouMail](https://www.youmail.com)
 
 If you're using Linkerd 2.x and aren't on this list, please [submit a pull
diff --git a/charts/linkerd2/templates/_config.tpl b/charts/linkerd2/templates/_config.tpl
index ea41d359b..fe365fb4b 100644
--- a/charts/linkerd2/templates/_config.tpl
+++ b/charts/linkerd2/templates/_config.tpl
@@ -30,10 +30,10 @@
     "port": {{.Values.global.proxy.ports.control}}
   },
   "ignoreInboundPorts":[
-    {{- include "partials.splitStringListToPorts" .Values.global.proxyInit.ignoreInboundPorts -}}
+    {{- include "partials.splitStringListToPortRanges" .Values.global.proxyInit.ignoreInboundPorts -}}
   ],
   "ignoreOutboundPorts":[
-    {{- include "partials.splitStringListToPorts" .Values.global.proxyInit.ignoreOutboundPorts -}}
+    {{- include "partials.splitStringListToPortRanges" .Values.global.proxyInit.ignoreOutboundPorts -}}
   ],
   "inboundPort":{
     "port": {{.Values.global.proxy.ports.inbound}}
diff --git a/charts/partials/templates/_helpers.tpl b/charts/partials/templates/_helpers.tpl
index 6eec943f9..928da6609 100644
--- a/charts/partials/templates/_helpers.tpl
+++ b/charts/partials/templates/_helpers.tpl
@@ -46,16 +46,16 @@ For example "11,22,55,44" will become "11","22","55","44"
 {{- end -}}
 
 {{/*
-Splits a coma separated list into a list of port objects.
-For example "11,22,55" will become{"port":11},{"port":22},
-{"port":55}
+Splits a coma separated list into a list of PortRange objects.
+For example "11,22,55" will become{"portRange":11},{"portRange":22},
+{"portRange":55}
 */}}
-{{- define "partials.splitStringListToPorts" -}}
+{{- define "partials.splitStringListToPortRanges" -}}
 {{- if gt (len .) 0 -}}
 {{- $ports := splitList "," . -}}
 {{- $last := sub (len $ports) 1 -}}
 {{- range $i,$port := $ports -}}
-{"port":{{$port}}}{{ternary "," "" (ne $i $last)}}
+{"portRange":"{{$port}}"}{{ternary "," "" (ne $i $last)}}
 {{- end -}}
 {{- end -}}
 {{- end -}}
diff --git a/controller/api/util/api_utils.go b/controller/api/util/api_utils.go
index a934a7574..38d30da1b 100644
--- a/controller/api/util/api_utils.go
+++ b/controller/api/util/api_utils.go
@@ -594,6 +594,11 @@ func K8sPodToPublicPod(pod corev1.Pod, ownerKind string, ownerName string) *pb.P
 	if pod.DeletionTimestamp != nil {
 		status = "Terminating"
 	}
+
+	if pod.Status.Reason == "Evicted" {
+		status = "Evicted"
+	}
+
 	controllerComponent := pod.Labels[k8s.ControllerComponentLabel]
 	controllerNS := pod.Labels[k8s.ControllerNSLabel]
 
diff --git a/controller/proxy-injector/webhook_ops.go b/controller/proxy-injector/webhook_ops.go
deleted file mode 100644
index e2e5b932f..000000000
--- a/controller/proxy-injector/webhook_ops.go
+++ /dev/null
@@ -1,53 +0,0 @@
-package injector
-
-import (
-	"bytes"
-
-	k8sPkg "github.com/linkerd/linkerd2/pkg/k8s"
-	log "github.com/sirupsen/logrus"
-	arv1beta1 "k8s.io/api/admissionregistration/v1beta1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	clientArv1beta1 "k8s.io/client-go/kubernetes/typed/admissionregistration/v1beta1"
-	"sigs.k8s.io/yaml"
-)
-
-// Ops satisfies the ConfigOps interface for managing MutatingWebhook configs
-type Ops struct{}
-
-// Create persists the Mutating webhook config and returns its SelfLink
-func (*Ops) Create(
-	client clientArv1beta1.AdmissionregistrationV1beta1Interface,
-	buf *bytes.Buffer,
-) (string, error) {
-	var config arv1beta1.MutatingWebhookConfiguration
-	if err := yaml.Unmarshal(buf.Bytes(), &config); err != nil {
-		log.Infof("failed to unmarshal mutating webhook configuration: %s\n%s\n", err, buf.String())
-		return "", err
-	}
-
-	obj, err := client.MutatingWebhookConfigurations().Create(&config)
-	if err != nil {
-		return "", err
-	}
-	return obj.ObjectMeta.SelfLink, nil
-}
-
-// Exists returns an error if the Mutating webhook doesn't exist
-func (o *Ops) Exists(client clientArv1beta1.AdmissionregistrationV1beta1Interface) error {
-	_, err := client.
-		MutatingWebhookConfigurations().
-		Get(o.Name(), metav1.GetOptions{})
-	return err
-}
-
-// Delete removes the Mutating webhook from the cluster
-func (o *Ops) Delete(client clientArv1beta1.AdmissionregistrationV1beta1Interface) error {
-	return client.
-		MutatingWebhookConfigurations().
-		Delete(o.Name(), &metav1.DeleteOptions{})
-}
-
-// Name returns name for this webhook configuration resource
-func (*Ops) Name() string {
-	return k8sPkg.ProxyInjectorWebhookConfigName
-}
diff --git a/controller/sp-validator/webhook_ops.go b/controller/sp-validator/webhook_ops.go
deleted file mode 100644
index f584a5805..000000000
--- a/controller/sp-validator/webhook_ops.go
+++ /dev/null
@@ -1,53 +0,0 @@
-package validator
-
-import (
-	"bytes"
-
-	k8sPkg "github.com/linkerd/linkerd2/pkg/k8s"
-	log "github.com/sirupsen/logrus"
-	arv1beta1 "k8s.io/api/admissionregistration/v1beta1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	clientArv1beta1 "k8s.io/client-go/kubernetes/typed/admissionregistration/v1beta1"
-	"sigs.k8s.io/yaml"
-)
-
-// Ops satisfies the ConfigOps interface for managing ValidatingWebhook configs
-type Ops struct{}
-
-// Create persists the Validating webhook config and returns its SelfLink
-func (*Ops) Create(
-	client clientArv1beta1.AdmissionregistrationV1beta1Interface,
-	buf *bytes.Buffer,
-) (string, error) {
-	var config arv1beta1.ValidatingWebhookConfiguration
-	if err := yaml.Unmarshal(buf.Bytes(), &config); err != nil {
-		log.Infof("failed to unmarshal validating webhook configuration: %s\n%s\n", err, buf.String())
-		return "", err
-	}
-
-	obj, err := client.ValidatingWebhookConfigurations().Create(&config)
-	if err != nil {
-		return "", err
-	}
-	return obj.ObjectMeta.SelfLink, nil
-}
-
-// Exists returns an error if the Validating webhook doesn't exist
-func (o *Ops) Exists(client clientArv1beta1.AdmissionregistrationV1beta1Interface) error {
-	_, err := client.
-		ValidatingWebhookConfigurations().
-		Get(o.Name(), metav1.GetOptions{})
-	return err
-}
-
-// Delete removes the Validating webhook from the cluster
-func (o *Ops) Delete(client clientArv1beta1.AdmissionregistrationV1beta1Interface) error {
-	return client.
-		ValidatingWebhookConfigurations().
-		Delete(o.Name(), &metav1.DeleteOptions{})
-}
-
-// Name returns name for this webhook configuration resource
-func (*Ops) Name() string {
-	return k8sPkg.SPValidatorWebhookConfigName
-}
diff --git a/pkg/healthcheck/healthcheck.go b/pkg/healthcheck/healthcheck.go
index 40072bbf1..e76d2fe7b 100644
--- a/pkg/healthcheck/healthcheck.go
+++ b/pkg/healthcheck/healthcheck.go
@@ -2132,7 +2132,7 @@ func validateDataPlanePods(pods []*pb.Pod, targetNamespace string) error {
 	}
 
 	for _, pod := range pods {
-		if pod.Status != "Running" {
+		if pod.Status != "Running" && pod.Status != "Evicted" {
 			return fmt.Errorf("The \"%s\" pod is not running",
 				pod.Name)
 		}


Test patch:
diff --git a/cli/cmd/install_helm_test.go b/cli/cmd/install_helm_test.go
index e2462c383..dc248acfc 100644
--- a/cli/cmd/install_helm_test.go
+++ b/cli/cmd/install_helm_test.go
@@ -6,6 +6,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/linkerd/linkerd2/pkg/charts"
 	l5dcharts "github.com/linkerd/linkerd2/pkg/charts/linkerd2"
 	"github.com/linkerd/linkerd2/pkg/k8s"
 	"k8s.io/helm/pkg/chartutil"
@@ -230,6 +231,7 @@ func chartControlPlane(t *testing.T, ha bool, addOnConfig string, ignoreOutbound
 }
 
 func buildAddOnChart(t *testing.T, addon l5dcharts.AddOn, chartPartials *pb.Chart) *pb.Chart {
+	rawValues := readValuesFile(t, filepath.Join("add-ons", addon.Name()))
 	addOnChart := pb.Chart{
 		Metadata: &pb.Metadata{
 			Name: addon.Name(),
@@ -240,6 +242,9 @@ func buildAddOnChart(t *testing.T, addon l5dcharts.AddOn, chartPartials *pb.Char
 		Dependencies: []*pb.Chart{
 			chartPartials,
 		},
+		Values: &pb.Config{
+			Raw: string(rawValues),
+		},
 	}
 
 	for _, filepath := range append(addon.ConfigStageTemplates(), addon.ControlPlaneStageTemplates()...) {
@@ -291,3 +296,17 @@ func readTestValues(t *testing.T, ha bool, ignoreOutboundPorts string, ignoreInb
 
 	return yaml.Marshal(values)
 }
+
+// readValues reads values.yaml file from the given path
+func readValuesFile(t *testing.T, path string) []byte {
+
+	valuesFiles := []*chartutil.BufferedFile{
+		{Name: chartutil.ValuesfileName},
+	}
+
+	if err := charts.FilesReader(path+"/", valuesFiles); err != nil {
+		t.Fatal("Unexpected error", err)
+	}
+
+	return valuesFiles[0].Data
+}
diff --git a/cli/cmd/testdata/install_helm_output.golden b/cli/cmd/testdata/install_helm_output.golden
index 953b2a6c4..150affdd7 100644
--- a/cli/cmd/testdata/install_helm_output.golden
+++ b/cli/cmd/testdata/install_helm_output.golden
@@ -902,8 +902,8 @@ data:
       "controlPort":{
         "port": 4190
       },
-      "ignoreInboundPorts":[{"port":222}],
-      "ignoreOutboundPorts":[{"port":111}],
+      "ignoreInboundPorts":[{"portRange":"222"}],
+      "ignoreOutboundPorts":[{"portRange":"111"}],
       "inboundPort":{
         "port": 4143
       },
diff --git a/cli/cmd/testdata/install_helm_output_addons.golden b/cli/cmd/testdata/install_helm_output_addons.golden
index a8253d435..a0ffddf71 100644
--- a/cli/cmd/testdata/install_helm_output_addons.golden
+++ b/cli/cmd/testdata/install_helm_output_addons.golden
@@ -902,8 +902,8 @@ data:
       "controlPort":{
         "port": 4190
       },
-      "ignoreInboundPorts":[{"port":222}],
-      "ignoreOutboundPorts":[{"port":111}],
+      "ignoreInboundPorts":[{"portRange":"222"}],
+      "ignoreOutboundPorts":[{"portRange":"111"}],
       "inboundPort":{
         "port": 4143
       },
diff --git a/cli/cmd/testdata/install_helm_output_ha.golden b/cli/cmd/testdata/install_helm_output_ha.golden
index 8afbbbbc6..77a47c933 100644
--- a/cli/cmd/testdata/install_helm_output_ha.golden
+++ b/cli/cmd/testdata/install_helm_output_ha.golden
@@ -902,8 +902,8 @@ data:
       "controlPort":{
         "port": 4190
       },
-      "ignoreInboundPorts":[{"port":222}],
-      "ignoreOutboundPorts":[{"port":111}],
+      "ignoreInboundPorts":[{"portRange":"222"}],
+      "ignoreOutboundPorts":[{"portRange":"111"}],
       "inboundPort":{
         "port": 4143
       },
diff --git a/controller/api/util/api_utils_test.go b/controller/api/util/api_utils_test.go
index 2d2ed4db0..708d88c5f 100644
--- a/controller/api/util/api_utils_test.go
+++ b/controller/api/util/api_utils_test.go
@@ -460,6 +460,26 @@ func TestK8sPodToPublicPod(t *testing.T) {
 					PodIP:               "pod-ip",
 				},
 			},
+			{
+				k8sPod: corev1.Pod{
+					Status: corev1.PodStatus{
+						Phase:  "Failed",
+						Reason: "Evicted",
+						ContainerStatuses: []corev1.ContainerStatus{
+							{
+								Name:  k8s.ProxyContainerName,
+								Ready: true,
+							},
+						},
+					},
+				},
+				ownerName: "owner-name",
+				publicPod: &pb.Pod{
+					Name:       "/",
+					Status:     "Evicted",
+					ProxyReady: true,
+				},
+			},
 		}
 
 		for _, exp := range expectations {
diff --git a/pkg/healthcheck/healthcheck_test.go b/pkg/healthcheck/healthcheck_test.go
index 684a7edcb..9607610f7 100644
--- a/pkg/healthcheck/healthcheck_test.go
+++ b/pkg/healthcheck/healthcheck_test.go
@@ -2160,6 +2160,17 @@ func TestValidateDataPlanePods(t *testing.T) {
 		}
 	})
 
+	t.Run("Does not return an error if the pod is Evicted", func(t *testing.T) {
+		pods := []*pb.Pod{
+			{Name: "emoji-d9c7866bb-7v74n", Status: "Evicted", ProxyReady: true},
+		}
+
+		err := validateDataPlanePods(pods, "emojivoto")
+		if err != nil {
+			t.Fatalf("Expected no error, got %s", err)
+		}
+	})
+
 	t.Run("Returns an error if the proxy container is not ready", func(t *testing.T) {
 		pods := []*pb.Pod{
 			{Name: "emoji-d9c7866bb-7v74n", Status: "Running", ProxyReady: true},