feat(pkg/*): Set Envoy sidecar log-evel via configMap (openservicemes…

…h#1469) * feat(pkg/*): get envoy logging level from configmap Adds the ability to configure the envoy logging level via the osm configmap * feat(pkg/*): get envoy logging level from configmap Adds the ability to configure the envoy logging level via the osm configmap * feat(pkg/*): adds a comment for EnvoyLogLevel fieldname Rearranges the osmConfig struct to avoid maligned struct and adds a comment for EnvoyLogLevel fieldname * feat(pkg/configurator): adds unit tests for GetEnvoyLogLevel Adds unit tests for getting the Envoy log level from the osm configmap and fixes a unit test for GetMeshCIDRRanges Co-authored-by: Tapas Kapadia <tapaskapadia@Tapass-MacBook-Pro.local>
eduser25 · Aug 10, 2020 · 55cf1d4 · 55cf1d4
1 parent f9c2b1f
commit 55cf1d4
Show file tree

Hide file tree

Showing 12 changed files with 120 additions and 10 deletions.
diff --git a/charts/osm/templates/osm-configmap.yaml b/charts/osm/templates/osm-configmap.yaml
@@ -6,6 +6,7 @@ metadata:
 data:
   permissive_traffic_policy_mode: {{ .Values.OpenServiceMesh.enablePermissiveTrafficPolicy | default "false" | quote }}
   egress: {{ .Values.OpenServiceMesh.enableEgress | quote }}
+  envoy_log_level: {{ .Values.OpenServiceMesh.envoyLogLevel | quote }}
   prometheus_scraping: "true"
 
   zipkin_tracing: {{ .Values.OpenServiceMesh.zipkin.enable | quote }}

diff --git a/charts/osm/values.yaml b/charts/osm/values.yaml
@@ -32,6 +32,7 @@ OpenServiceMesh:
   meshName: osm
   meshCIDRRanges: 0.0.0.0/0
   useHTTPSIngress: false
+  envoyLogLevel: debug
 
   # Set deployZipkin to true to deploy a Zipkin cluster in the
   # namespace where OSM resides. Set this to false if Zipkin

diff --git a/pkg/configurator/client.go b/pkg/configurator/client.go
@@ -24,6 +24,7 @@ const (
 	zipkinPortKey                  = "zipkin_port"
 	zipkinEndpointKey              = "zipkin_endpoint"
 	defaultInMeshCIDR              = ""
+	envoyLogLevel                  = "envoy_log_level"
 )
 
 // NewConfigurator implements configurator.Configurator and creates the Kubernetes client to manage namespaces.
@@ -74,6 +75,9 @@ type osmConfig struct {
 	// PrometheusScraping is a bool toggle used to enable or disable metrics scraping by Prometheus
 	PrometheusScraping bool `yaml:"prometheus_scraping"`
 
+	// UseHTTPSIngress is a bool toggle enabling HTTPS protocol between ingress and backend pods
+	UseHTTPSIngress bool `yaml:"use_https_ingress"`
+
 	// ZipkinTracing is a bool toggle used to enable ot disable Zipkin tracing
 	ZipkinTracing bool `yaml:"zipkin_tracing"`
 
@@ -89,8 +93,8 @@ type osmConfig struct {
 	// MeshCIDRRanges is the list of CIDR ranges for in-mesh traffic
 	MeshCIDRRanges string `yaml:"mesh_cidr_ranges"`
 
-	// UseHTTPSIngress is a bool toggle enabling HTTPS protocol between ingress and backend pods
-	UseHTTPSIngress bool `yaml:"use_https_ingress"`
+	// EnvoyLogLevel is a string that defines the log level for envoy proxies
+	EnvoyLogLevel string `yaml:"envoy_log_level"`
 }
 
 func (c *Client) run(stop <-chan struct{}) {
@@ -137,6 +141,7 @@ func (c *Client) getConfigMap() *osmConfig {
 		ZipkinAddress:  getStringValueForKey(configMap, zipkinAddressKey),
 		ZipkinPort:     getIntValueForKey(configMap, zipkinPortKey),
 		ZipkinEndpoint: getStringValueForKey(configMap, zipkinEndpointKey),
+		EnvoyLogLevel:  getStringValueForKey(configMap, envoyLogLevel),
 	}
 }
 

diff --git a/pkg/configurator/client_test.go b/pkg/configurator/client_test.go
@@ -107,11 +107,12 @@ var _ = Describe("Test OSM ConfigMap parsing", func() {
 				"ZipkinEndpoint":              zipkinEndpointKey,
 				"MeshCIDRRanges":              meshCIDRRangesKey,
 				"UseHTTPSIngress":             useHTTPSIngressKey,
+				"EnvoyLogLevel":               envoyLogLevel,
 			}
 			t := reflect.TypeOf(osmConfig{})
 
 			actualNumberOfFields := t.NumField()
-			expectedNumberOfFields := 9
+			expectedNumberOfFields := 10
 			Expect(actualNumberOfFields).To(
 				Equal(expectedNumberOfFields),
 				fmt.Sprintf("Fields have been added or removed from the osmConfig struct -- expected %d, actual %d; please correct this unit test", expectedNumberOfFields, actualNumberOfFields))

diff --git a/pkg/configurator/fake.go b/pkg/configurator/fake.go
@@ -97,3 +97,8 @@ func (f FakeConfigurator) GetZipkinPort() uint32 {
 func (f FakeConfigurator) GetZipkinEndpoint() string {
 	return constants.DefaultZipkinEndpoint
 }
+
+// GetEnvoyLogLevel returns the Zipkin endpoint
+func (f FakeConfigurator) GetEnvoyLogLevel() string {
+	return constants.DefaultEnvoyLogLevel
+}
diff --git a/pkg/configurator/methods.go b/pkg/configurator/methods.go
@@ -3,10 +3,11 @@ package configurator
 import (
 	"encoding/json"
 	"fmt"
-	"github.com/openservicemesh/osm/pkg/constants"
 	"net"
 	"sort"
 	"strings"
+
+	"github.com/openservicemesh/osm/pkg/constants"
 )
 
 // The functions in this file implement the configurator.Configurator interface
@@ -116,6 +117,15 @@ func (c *Client) UseHTTPSIngress() bool {
 	return c.getConfigMap().UseHTTPSIngress
 }
 
+// GetEnvoyLogLevel returns the envoy log level
+func (c *Client) GetEnvoyLogLevel() string {
+	logLevel := c.getConfigMap().EnvoyLogLevel
+	if logLevel != "" {
+		return logLevel
+	}
+	return constants.DefaultEnvoyLogLevel
+}
+
 // GetAnnouncementsChannel returns a channel, which is used to announce when changes have been made to the OSM ConfigMap.
 func (c *Client) GetAnnouncementsChannel() <-chan interface{} {
 	return c.announcements

diff --git a/pkg/configurator/methods_test.go b/pkg/configurator/methods_test.go
@@ -13,12 +13,14 @@ import (
 
 var _ = Describe("Test Envoy configuration creation", func() {
 	testCIDRRanges := "10.2.0.0/16 10.0.0.0/16"
+	testDebugEnvoyLogLevel := "debug"
 	defaultConfigMap := map[string]string{
 		permissiveTrafficPolicyModeKey: "false",
 		egressKey:                      "true",
 		meshCIDRRangesKey:              testCIDRRanges,
 		prometheusScrapingKey:          "true",
 		zipkinTracingKey:               "true",
+		envoyLogLevel:                  testDebugEnvoyLogLevel,
 	}
 
 	Context("create OSM configurator client", func() {
@@ -59,6 +61,7 @@ var _ = Describe("Test Envoy configuration creation", func() {
 				PrometheusScraping:          true,
 				ZipkinTracing:               true,
 				MeshCIDRRanges:              testCIDRRanges,
+				EnvoyLogLevel:               testDebugEnvoyLogLevel,
 			}
 			expectedConfigBytes, err := marshalConfigToJSON(expectedConfig)
 			Expect(err).ToNot(HaveOccurred())
@@ -286,7 +289,7 @@ var _ = Describe("Test Envoy configuration creation", func() {
 		cfg := NewConfigurator(kubeClient, stop, osmNamespace, osmConfigMapName)
 
 		It("correctly retrieves the mesh CIDR ranges", func() {
-			Expect(cfg.IsZipkinTracingEnabled()).To(BeFalse())
+			Expect(cfg.GetMeshCIDRRanges()).To(BeEmpty())
 			configMap := v1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{
 					Namespace: osmNamespace,
@@ -305,4 +308,80 @@ var _ = Describe("Test Envoy configuration creation", func() {
 			Expect(cfg.GetMeshCIDRRanges()).To(Equal(expectedMeshCIDRRanges))
 		})
 	})
+
+	Context("create OSM config for the Envoy proxy log level", func() {
+		kubeClient := testclient.NewSimpleClientset()
+		stop := make(chan struct{})
+		osmNamespace := "-test-osm-namespace-"
+		osmConfigMapName := "-test-osm-config-map-"
+		testInfoEnvoyLogLevel := "info"
+		testErrorEnvoyLogLevel := "error"
+		cfg := NewConfigurator(kubeClient, stop, osmNamespace, osmConfigMapName)
+
+		It("correctly identifies that the Envoy log level is debug", func() {
+			Expect(cfg.GetEnvoyLogLevel()).To(Equal(testDebugEnvoyLogLevel))
+			configMap := v1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: osmNamespace,
+					Name:      osmConfigMapName,
+				},
+				Data: defaultConfigMap,
+			}
+			_, err := kubeClient.CoreV1().ConfigMaps(osmNamespace).Create(context.TODO(), &configMap, metav1.CreateOptions{})
+			Expect(err).ToNot(HaveOccurred())
+
+			// Wait for the config map change to propagate to the cache.
+			log.Info().Msg("Waiting for announcement")
+			<-cfg.GetAnnouncementsChannel()
+
+			Expect(cfg.GetOSMNamespace()).To(Equal(osmNamespace))
+			Expect(err).ToNot(HaveOccurred())
+
+			Expect(cfg.GetEnvoyLogLevel()).To(Equal(testDebugEnvoyLogLevel))
+		})
+
+		It("correctly identifies that Envoy log level is info", func() {
+			defaultConfigMap[envoyLogLevel] = testInfoEnvoyLogLevel
+			configMap := v1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: osmNamespace,
+					Name:      osmConfigMapName,
+				},
+				Data: defaultConfigMap,
+			}
+			_, err := kubeClient.CoreV1().ConfigMaps(osmNamespace).Update(context.TODO(), &configMap, metav1.UpdateOptions{})
+			Expect(err).ToNot(HaveOccurred())
+
+			// Wait for the config map change to propagate to the cache.
+			log.Info().Msg("Waiting for announcement")
+			<-cfg.GetAnnouncementsChannel()
+
+			Expect(cfg.GetOSMNamespace()).To(Equal(osmNamespace))
+			Expect(err).ToNot(HaveOccurred())
+
+			Expect(cfg.GetEnvoyLogLevel()).To(Equal(testInfoEnvoyLogLevel))
+		})
+
+		It("correctly identifies that Envoy log level is error", func() {
+			defaultConfigMap[envoyLogLevel] = testErrorEnvoyLogLevel
+			configMap := v1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: osmNamespace,
+					Name:      osmConfigMapName,
+				},
+				Data: defaultConfigMap,
+			}
+			_, err := kubeClient.CoreV1().ConfigMaps(osmNamespace).Update(context.TODO(), &configMap, metav1.UpdateOptions{})
+			Expect(err).ToNot(HaveOccurred())
+
+			// Wait for the config map change to propagate to the cache.
+			log.Info().Msg("Waiting for announcement")
+			<-cfg.GetAnnouncementsChannel()
+
+			Expect(cfg.GetOSMNamespace()).To(Equal(osmNamespace))
+			Expect(err).ToNot(HaveOccurred())
+
+			Expect(cfg.GetEnvoyLogLevel()).To(Equal(testErrorEnvoyLogLevel))
+		})
+	})
 })
diff --git a/pkg/configurator/types.go b/pkg/configurator/types.go
@@ -55,6 +55,9 @@ type Configurator interface {
 	// UseHTTPSIngress determines whether protocol used for traffic from ingress to backend pods should be HTTPS.
 	UseHTTPSIngress() bool
 
+	// GetEnvoyLogLevel returns the envoy log level
+	GetEnvoyLogLevel() string
+
 	// GetAnnouncementsChannel returns a channel, which is used to announce when changes have been made to the OSM ConfigMap
 	GetAnnouncementsChannel() <-chan interface{}
 }
diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go
@@ -57,6 +57,9 @@ const (
 	// DefaultZipkinPort is the Zipkin port number.
 	DefaultZipkinPort = uint32(9411)
 
+	// DefaultEnvoyLogLevel is the default envoy log level if not defined in the osm configmap
+	DefaultEnvoyLogLevel = "debug"
+
 	// EnvoyPrometheusInboundListenerPort is Envoy's inbound listener port number for prometheus
 	EnvoyPrometheusInboundListenerPort = 15010
 

diff --git a/pkg/injector/config_test.go b/pkg/injector/config_test.go
@@ -94,7 +94,8 @@ var _ = Describe("Test Envoy configuration creation", func() {
 var _ = Describe("Test Envoy sidecar", func() {
 	Context("create Envoy sidecar", func() {
 		It("creates correct Envoy sidecar spec", func() {
-			actual := getEnvoySidecarContainerSpec("a", "b", "c", "d")
+			cfg := configurator.NewFakeConfigurator()
+			actual := getEnvoySidecarContainerSpec("a", "b", "c", "d", cfg)
 			Expect(len(actual)).To(Equal(1))
 
 			expected := corev1.Container{
@@ -132,7 +133,7 @@ var _ = Describe("Test Envoy sidecar", func() {
 					"envoy",
 				},
 				Args: []string{
-					"--log-level", "debug",
+					"--log-level", cfg.GetEnvoyLogLevel(),
 					"--config-path", "/etc/envoy/bootstrap.yaml",
 					"--service-node", "c",
 					"--service-cluster", "d",

diff --git a/pkg/injector/patch.go b/pkg/injector/patch.go
@@ -81,7 +81,7 @@ func (wh *webhook) createPatch(pod *corev1.Pod, namespace string) ([]byte, error
 
 	patches = append(patches, addContainer(
 		pod.Spec.Containers,
-		getEnvoySidecarContainerSpec(envoyContainerName, wh.config.SidecarImage, envoyNodeID, envoyClusterID),
+		getEnvoySidecarContainerSpec(envoyContainerName, wh.config.SidecarImage, envoyNodeID, envoyClusterID, wh.configurator),
 		"/spec/containers")...,
 	)
 

diff --git a/pkg/injector/sidecar.go b/pkg/injector/sidecar.go
@@ -5,6 +5,7 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 
+	"github.com/openservicemesh/osm/pkg/configurator"
 	"github.com/openservicemesh/osm/pkg/constants"
 )
 
@@ -14,7 +15,7 @@ const (
 	envoyContainerName       = "envoy"
 )
 
-func getEnvoySidecarContainerSpec(containerName, envoyImage, nodeID, clusterID string) []corev1.Container {
+func getEnvoySidecarContainerSpec(containerName, envoyImage, nodeID, clusterID string, cfg configurator.Configurator) []corev1.Container {
 	container := corev1.Container{
 		Name:            containerName,
 		Image:           envoyImage,
@@ -42,7 +43,7 @@ func getEnvoySidecarContainerSpec(containerName, envoyImage, nodeID, clusterID s
 		}},
 		Command: []string{"envoy"},
 		Args: []string{
-			"--log-level", "debug", // TODO(draychev): Move to ConfigMap: Github Issue https://github.com/openservicemesh/osm/issues/1232
+			"--log-level", cfg.GetEnvoyLogLevel(),
 			"--config-path", strings.Join([]string{envoyProxyConfigPath, envoyBootstrapConfigFile}, "/"),
 			"--service-node", nodeID,
 			"--service-cluster", clusterID,