Skip to content

Commit

Permalink
Set envoy log level as error to default in OSM (openservicemesh#1679)
Browse files Browse the repository at this point in the history 
This PR : 
- sets the default value of the log level for the envoy to error when OSM is installed
- exposes envoy-log-level as an install feature flag so that the demo and cli can run in the debug mode for envoy
- updates the demo and .env file to have the log level as debug
  • Loading branch information
@snehachhabria
snehachhabria authored Sep 10, 2020
1 parent f543ce2 commit 76147c6
Show file tree
Hide file tree
Showing 8 changed files with 95 additions and 13 deletions.
4 changes: 4 additions & 0 deletions .env.example
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ export BOOKWAREHOUSE_NAMESPACE=bookwarehouse
# optional: Whether to deploy traffic split policy or not
# Default: true
# export DEPLOY_TRAFFIC_SPLIT=true

# optional: specify the log level for the enovy's
# Default: debug
# export ENVOY_LOG_LEVEL=debug
#--------------------------------------

### The section below configures certificates management
Expand Down
2 changes: 1 addition & 1 deletion charts/osm/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ A Helm chart to install the OSM control plane on Kubernetes
| OpenServiceMesh.enableEgress | bool | `false` | |
| OpenServiceMesh.enableMetricsStack | bool | `true` | |
| OpenServiceMesh.enablePermissiveTrafficPolicy | bool | `false` | |
| OpenServiceMesh.envoyLogLevel | string | `"debug"` | |
| OpenServiceMesh.envoyLogLevel | string | `"error"` | |
| OpenServiceMesh.grafana.port | int | `3000` | |
| OpenServiceMesh.image.pullPolicy | string | `"IfNotPresent"` | |
| OpenServiceMesh.image.registry | string | `"openservicemesh"` | |
Expand Down
2 changes: 1 addition & 1 deletion charts/osm/values.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ OpenServiceMesh:
meshName: osm
meshCIDRRanges: 0.0.0.0/0
useHTTPSIngress: false
envoyLogLevel: debug
envoyLogLevel: error

# Set deployJaeger to true to deploy a Jaeger cluster in the
# namespace where OSM resides.
Expand Down
19 changes: 19 additions & 0 deletions cmd/cli/install.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ type installCmd struct {
vaultProtocol string
vaultToken string
vaultRole string
envoyLogLevel string
serviceCertValidityMinutes int
enableDebugServer bool
enableEgress bool
Expand Down Expand Up @@ -152,6 +153,7 @@ func newInstallCmd(config *helm.Configuration, out io.Writer) *cobra.Command {
f.BoolVar(&inst.enableGrafana, "enable-grafana", false, "Enable Grafana installation and deployment")
f.StringVar(&inst.meshName, "mesh-name", defaultMeshName, "name for the new control plane instance")
f.BoolVar(&inst.deployJaeger, "deploy-jaeger", true, "Deploy Jaeger in the namespace of the OSM controller")
f.StringVar(&inst.envoyLogLevel, "envoy-log-level", "error", "Envoy log level is used to specify the level of logs collected from envoy and needs to be one of these (trace, debug, info, warning, warn, error, critical, off)")

return cmd
}
Expand Down Expand Up @@ -218,6 +220,7 @@ func (i *installCmd) resolveValues() (map[string]interface{}, error) {
fmt.Sprintf("OpenServiceMesh.enableEgress=%t", i.enableEgress),
fmt.Sprintf("OpenServiceMesh.meshCIDRRanges=%s", strings.Join(i.meshCIDRRanges, " ")),
fmt.Sprintf("OpenServiceMesh.deployJaeger=%t", i.deployJaeger),
fmt.Sprintf("OpenServiceMesh.envoyLogLevel=%s", strings.ToLower(i.envoyLogLevel)),
}

if i.containerRegistrySecret != "" {
Expand Down Expand Up @@ -290,9 +293,25 @@ func (i *installCmd) validateOptions() error {
}
}

//validate the envoy log level type
if err := isValidEnvoyLogLevel(i.envoyLogLevel); err != nil {
return err
}

return nil
}

func isValidEnvoyLogLevel(envoyLogLevel string) error {
// allowedLogLevels referenced from : https://github.com/envoyproxy/envoy/blob/release/v1.15/test/server/options_impl_test.cc#L373
allowedLogLevels := []string{"trace", "debug", "info", "warning", "warn", "error", "critical", "off"}
for _, logLevel := range allowedLogLevels {
if strings.EqualFold(envoyLogLevel, logLevel) {
return nil
}
}
return errors.Errorf("Invalid envoy log level.\n A valid envoy log level must be one from the following : %v", allowedLogLevels)
}

func isValidMeshName(meshName string) error {
meshNameErrs := validation.IsValidLabelValue(meshName)
if len(meshNameErrs) != 0 {
Expand Down
56 changes: 56 additions & 0 deletions cmd/cli/install_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ var (
testRetentionTime = "5d"
testMeshCIDR = "10.20.0.0/16"
testMeshCIDRRanges = []string{testMeshCIDR}
testEnvoyLogLevel = "error"
)

var _ = Describe("Running the install command", func() {
Expand Down Expand Up @@ -82,6 +83,7 @@ var _ = Describe("Running the install command", func() {
enableGrafana: true,
meshCIDRRanges: testMeshCIDRRanges,
clientSet: fakeClientSet,
envoyLogLevel: testEnvoyLogLevel,
}

err = installCmd.run(config)
Expand Down Expand Up @@ -143,6 +145,7 @@ var _ = Describe("Running the install command", func() {
"enablePrometheus": true,
"enableGrafana": true,
"deployJaeger": false,
"envoyLogLevel": testEnvoyLogLevel,
}}))
})

Expand Down Expand Up @@ -194,6 +197,7 @@ var _ = Describe("Running the install command", func() {
enablePrometheus: true,
enableGrafana: true,
clientSet: fakeClientSet,
envoyLogLevel: testEnvoyLogLevel,
}

err = installCmd.run(config)
Expand Down Expand Up @@ -260,6 +264,7 @@ var _ = Describe("Running the install command", func() {
"enablePrometheus": true,
"enableGrafana": true,
"deployJaeger": false,
"envoyLogLevel": testEnvoyLogLevel,
}}))
})

Expand Down Expand Up @@ -318,6 +323,7 @@ var _ = Describe("Running the install command", func() {
enablePrometheus: true,
enableGrafana: true,
clientSet: fakeClientSet,
envoyLogLevel: testEnvoyLogLevel,
}

err = installCmd.run(config)
Expand Down Expand Up @@ -385,6 +391,7 @@ var _ = Describe("Running the install command", func() {
"enablePrometheus": true,
"enableGrafana": true,
"deployJaeger": false,
"envoyLogLevel": testEnvoyLogLevel,
}}))
})

Expand Down Expand Up @@ -485,6 +492,7 @@ var _ = Describe("Running the install command", func() {
enablePrometheus: true,
enableGrafana: true,
clientSet: fakeClientSet,
envoyLogLevel: testEnvoyLogLevel,
}

err = installCmd.run(config)
Expand Down Expand Up @@ -552,6 +560,7 @@ var _ = Describe("Running the install command", func() {
"enablePrometheus": true,
"enableGrafana": true,
"deployJaeger": false,
"envoyLogLevel": testEnvoyLogLevel,
}}))
})

Expand Down Expand Up @@ -774,6 +783,7 @@ var _ = Describe("Resolving values for install command with vault parameters", f
meshCIDRRanges: testMeshCIDRRanges,
enablePrometheus: true,
enableGrafana: true,
envoyLogLevel: testEnvoyLogLevel,
}

vals, err = installCmd.resolveValues()
Expand Down Expand Up @@ -823,6 +833,7 @@ var _ = Describe("Resolving values for install command with vault parameters", f
"enablePrometheus": true,
"enableGrafana": true,
"deployJaeger": false,
"envoyLogLevel": testEnvoyLogLevel,
}}))
})
})
Expand Down Expand Up @@ -854,6 +865,7 @@ var _ = Describe("Ensure that grafana is disabled when flag is set to false", fu
meshCIDRRanges: testMeshCIDRRanges,
enablePrometheus: true,
enableGrafana: false,
envoyLogLevel: testEnvoyLogLevel,
}

vals, err = installCmd.resolveValues()
Expand Down Expand Up @@ -903,6 +915,7 @@ var _ = Describe("Ensure that grafana is disabled when flag is set to false", fu
"enablePrometheus": true,
"enableGrafana": false,
"deployJaeger": false,
"envoyLogLevel": testEnvoyLogLevel,
}}))
})

Expand Down Expand Up @@ -935,6 +948,7 @@ var _ = Describe("Ensure that grafana is enabled when flag is set to true", func
meshCIDRRanges: testMeshCIDRRanges,
enablePrometheus: true,
enableGrafana: true,
envoyLogLevel: testEnvoyLogLevel,
}

vals, err = installCmd.resolveValues()
Expand Down Expand Up @@ -984,6 +998,7 @@ var _ = Describe("Ensure that grafana is enabled when flag is set to true", func
"enablePrometheus": true,
"enableGrafana": true,
"deployJaeger": false,
"envoyLogLevel": testEnvoyLogLevel,
}}))
})

Expand Down Expand Up @@ -1016,6 +1031,7 @@ var _ = Describe("Ensure that prometheus is disabled when flag is set to false",
meshCIDRRanges: testMeshCIDRRanges,
enablePrometheus: false,
enableGrafana: true,
envoyLogLevel: testEnvoyLogLevel,
}

vals, err = installCmd.resolveValues()
Expand Down Expand Up @@ -1065,6 +1081,7 @@ var _ = Describe("Ensure that prometheus is disabled when flag is set to false",
"enablePrometheus": false,
"enableGrafana": true,
"deployJaeger": false,
"envoyLogLevel": testEnvoyLogLevel,
}}))
})
})
Expand Down Expand Up @@ -1096,6 +1113,7 @@ var _ = Describe("Resolving values for install command with cert-manager paramet
meshCIDRRanges: testMeshCIDRRanges,
enablePrometheus: true,
enableGrafana: true,
envoyLogLevel: testEnvoyLogLevel,
}

vals, err = installCmd.resolveValues()
Expand Down Expand Up @@ -1145,6 +1163,7 @@ var _ = Describe("Resolving values for install command with cert-manager paramet
"enablePrometheus": true,
"enableGrafana": true,
"deployJaeger": false,
"envoyLogLevel": testEnvoyLogLevel,
}}))
})
})
Expand Down Expand Up @@ -1216,6 +1235,43 @@ var _ = Describe("Test mesh CIDR ranges", func() {
})
})

var _ = Describe("Test envoy log level types", func() {
Context("Test envoyLogLevel chart value with install cli option", func() {
It("Should correctly resolve envoyLogLevel when specified", func() {
installCmd := &installCmd{
envoyLogLevel: testEnvoyLogLevel,
}

vals, err := installCmd.resolveValues()
Expect(err).NotTo(HaveOccurred())

logLevel := vals["OpenServiceMesh"].(map[string]interface{})["envoyLogLevel"]
Expect(logLevel).To(Equal(testEnvoyLogLevel))
})
})

Context("Test isValidEnvoyLogLevel", func() {
It("Should validate if the specified envoy log level is supported", func() {
err := isValidEnvoyLogLevel("error")
Expect(err).NotTo(HaveOccurred())

err = isValidEnvoyLogLevel("off")
Expect(err).NotTo(HaveOccurred())

err = isValidEnvoyLogLevel("warn")
Expect(err).NotTo(HaveOccurred())
})

It("Should correctly error for invalid envoy log level", func() {
err := isValidEnvoyLogLevel("tracing")
Expect(err).To(HaveOccurred())

err = isValidEnvoyLogLevel("warns")
Expect(err).To(HaveOccurred())
})
})
})

var _ = Describe("Test osm image pull policy cli option", func() {
It("Should correctly resolve the pull policy option to chart values", func() {
installCmd := &installCmd{
Expand Down
3 changes: 3 additions & 0 deletions demo/run-osm-demo.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ ENABLE_EGRESS="${ENABLE_EGRESS:-false}"
ENABLE_GRAFANA="${ENABLE_GRAFANA:-false}"
MESH_CIDR=$(./scripts/get_mesh_cidr.sh)
DEPLOY_WITH_SAME_SA="${DEPLOY_WITH_SAME_SA:-false}"
ENVOY_LOG_LEVEL="${ENVOY_LOG_LEVEL:-debug}"

# For any additional installation arguments. Used heavily in CI.
optionalInstallArgs=$*
Expand Down Expand Up @@ -115,6 +116,7 @@ if [ "$CERT_MANAGER" = "vault" ]; then
--enable-egress="$ENABLE_EGRESS" \
--enable-grafana="$ENABLE_GRAFANA" \
--mesh-cidr "$MESH_CIDR" \
--envoy-log-level "$ENVOY_LOG_LEVEL" \
$optionalInstallArgs
else
# shellcheck disable=SC2086
Expand All @@ -130,6 +132,7 @@ else
--enable-egress="$ENABLE_EGRESS" \
--enable-grafana="$ENABLE_GRAFANA" \
--mesh-cidr "$MESH_CIDR" \
--envoy-log-level "$ENVOY_LOG_LEVEL" \
$optionalInstallArgs
fi

Expand Down
20 changes: 10 additions & 10 deletions pkg/configurator/methods_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ import (

var _ = Describe("Test Envoy configuration creation", func() {
testCIDRRanges := "10.2.0.0/16 10.0.0.0/16"
testDebugEnvoyLogLevel := "debug"
testErrorEnvoyLogLevel := "error"
defaultConfigMap := map[string]string{
permissiveTrafficPolicyModeKey: "false",
egressKey: "true",
meshCIDRRangesKey: testCIDRRanges,
prometheusScrapingKey: "true",
tracingEnableKey: "true",
envoyLogLevel: testDebugEnvoyLogLevel,
envoyLogLevel: testErrorEnvoyLogLevel,
}

Context("create OSM configurator client", func() {
Expand Down Expand Up @@ -61,7 +61,7 @@ var _ = Describe("Test Envoy configuration creation", func() {
PrometheusScraping: true,
TracingEnable: true,
MeshCIDRRanges: testCIDRRanges,
EnvoyLogLevel: testDebugEnvoyLogLevel,
EnvoyLogLevel: testErrorEnvoyLogLevel,
}
expectedConfigBytes, err := marshalConfigToJSON(expectedConfig)
Expect(err).ToNot(HaveOccurred())
Expand Down Expand Up @@ -315,11 +315,11 @@ var _ = Describe("Test Envoy configuration creation", func() {
osmNamespace := "-test-osm-namespace-"
osmConfigMapName := "-test-osm-config-map-"
testInfoEnvoyLogLevel := "info"
testErrorEnvoyLogLevel := "error"
testDebugEnvoyLogLevel := "debug"
cfg := NewConfigurator(kubeClient, stop, osmNamespace, osmConfigMapName)

It("correctly identifies that the Envoy log level is debug", func() {
Expect(cfg.GetEnvoyLogLevel()).To(Equal(testDebugEnvoyLogLevel))
It("correctly identifies that the Envoy log level is error", func() {
Expect(cfg.GetEnvoyLogLevel()).To(Equal(testErrorEnvoyLogLevel))
configMap := v1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Namespace: osmNamespace,
Expand All @@ -337,7 +337,7 @@ var _ = Describe("Test Envoy configuration creation", func() {
Expect(cfg.GetOSMNamespace()).To(Equal(osmNamespace))
Expect(err).ToNot(HaveOccurred())

Expect(cfg.GetEnvoyLogLevel()).To(Equal(testDebugEnvoyLogLevel))
Expect(cfg.GetEnvoyLogLevel()).To(Equal(testErrorEnvoyLogLevel))
})

It("correctly identifies that Envoy log level is info", func() {
Expand All @@ -362,8 +362,8 @@ var _ = Describe("Test Envoy configuration creation", func() {
Expect(cfg.GetEnvoyLogLevel()).To(Equal(testInfoEnvoyLogLevel))
})

It("correctly identifies that Envoy log level is error", func() {
defaultConfigMap[envoyLogLevel] = testErrorEnvoyLogLevel
It("correctly identifies that Envoy log level is debug", func() {
defaultConfigMap[envoyLogLevel] = testDebugEnvoyLogLevel
configMap := v1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Namespace: osmNamespace,
Expand All @@ -381,7 +381,7 @@ var _ = Describe("Test Envoy configuration creation", func() {
Expect(cfg.GetOSMNamespace()).To(Equal(osmNamespace))
Expect(err).ToNot(HaveOccurred())

Expect(cfg.GetEnvoyLogLevel()).To(Equal(testErrorEnvoyLogLevel))
Expect(cfg.GetEnvoyLogLevel()).To(Equal(testDebugEnvoyLogLevel))
})
})
})
2 changes: 1 addition & 1 deletion pkg/constants/constants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const (
DefaultTracingPort = uint32(9411)

// DefaultEnvoyLogLevel is the default envoy log level if not defined in the osm configmap
DefaultEnvoyLogLevel = "debug"
DefaultEnvoyLogLevel = "error"

// EnvoyPrometheusInboundListenerPort is Envoy's inbound listener port number for prometheus
EnvoyPrometheusInboundListenerPort = 15010
Expand Down

0 comments on commit 76147c6

Please sign in to comment.