Fix python log config file (#822)

* Fix python log config file

* fix ci

* fix ci

Author: jiangpengcheng

.ci/helm.sh

@@ -588,7 +588,6 @@ function ci::verify_log_topic() {
 
   sleep "$timesleep"
   MESSAGE=$(kubectl exec -n ${NAMESPACE} ${CLUSTER}-pulsar-broker-0 -- bin/pulsar-client consume -n 1 -s "sub" --subscription-position Earliest "${logTopic}")
-  echo "$MESSAGE"
   if [[ "$MESSAGE" == *"$message"* ]]; then
     return 0
   fi

.ci/tests/integration/cases/py-download-function/verify.sh

@@ -44,7 +44,7 @@ if [ $? -ne 0 ]; then
 fi
 
 if [ $USE_TLS == "false" ]; then
-  verify_log_topic=$(ci::verify_log_topic persistent://public/default/py-function-logs-sidecar "python_instance_main.py: Starting Python instance with Namespace" 10 2>&1)
+  verify_log_topic=$(ci::verify_log_topic persistent://public/default/py-function-logs-sidecar "Starting Python instance with Namespace" 10 2>&1)
   if [ $? -ne 0 ]; then
     echo "$verify_log_topic"
     kubectl delete -f "${BASE_DIR}"/.ci/tests/integration/cases/py-download-function/manifests.yaml > /dev/null 2>&1 || true

.ci/tests/integration/cases/python-log-format-json/manifests.yaml

@@ -11,6 +11,7 @@ spec:
   replicas: 1
   maxReplicas: 1
   logTopic: persistent://public/default/py-function-logs
+  logTopicAgent: sidecar
   input:
     topics:
       - persistent://public/default/input-python-log-format-json-topic

.ci/tests/integration/e2e.yaml

@@ -28,7 +28,7 @@ setup:
       command: |
         helm repo add cowboysysop https://cowboysysop.github.io/charts/
         helm repo update
-        helm install vpa cowboysysop/vertical-pod-autoscaler --version 6.0.3
+        helm install vpa cowboysysop/vertical-pod-autoscaler --version 11.1.1
       wait:
         - namespace: default
           resource: pod

controllers/spec/common.go

@@ -169,6 +169,25 @@ type TLSConfig interface {
 	GetMountPath() string
 }
 
+type RollingCfg struct {
+	Enabled bool
+	Type    string // "size" or "time"
+	File    string
+	// size-based
+	MaxBytes int
+	Backups  int
+	// time-based
+	When     string // e.g. "D", "W0"
+	Interval int
+}
+
+type LogCfg struct {
+	Level    string // INFO/DEBUG/...
+	Format   string // "json" or "text"
+	Handlers string // computed: "stream_handler[,rotating_file_handler|,timed_rotating_file_handler]"
+	Rolling  RollingCfg
+}
+
 func IsManaged(object metav1.Object) bool {
 	managed, exists := object.GetAnnotations()[AnnotationManaged]
 	return !exists || managed != "false"
@@ -1104,88 +1123,89 @@ func generatePythonLogConfigCommand(name string, runtime *v1alpha1.PythonRuntime
 
 func renderPythonInstanceLoggingINITemplate(name string, runtime *v1alpha1.PythonRuntime, agent v1alpha1.LogTopicAgent) (string, error) {
 	tmpl := template.Must(template.New("spec").Parse(pythonLoggingINITemplate))
-	var tpl bytes.Buffer
-	type logConfig struct {
-		RollingEnabled bool
-		Level          string
-		Policy         template.HTML
-		Handlers       string
-		Format         string
+
+	lc := LogCfg{
+		Level:  "INFO",
+		Format: "text",
+		Rolling: RollingCfg{
+			Enabled: false,
+			Backups: 5,
+		},
 	}
-	lc := &logConfig{}
-	lc.Level = "INFO"
-	lc.Format = "text"
-	lc.Handlers = "stream_handler"
+
+	// level
 	if runtime.Log != nil && runtime.Log.Level != "" {
 		if level := parsePythonLogLevel(runtime); level != "" {
 			lc.Level = level
 		}
 	}
-	if runtime.Log != nil && runtime.Log.Format != nil {
-		lc.Format = string(*runtime.Log.Format)
+	// format
+	if runtime.Log != nil && runtime.Log.Format != nil && strings.ToLower(string(*runtime.Log.Format)) == "json" {
+		lc.Format = "json"
 	}
+
+	// default handler
+	lc.Handlers = "stream_handler"
+
+	// log file path
+	logFile := fmt.Sprintf("logs/functions/%s.log", name)
+
+	// rolling policy
 	if runtime.Log != nil && runtime.Log.RotatePolicy != nil {
-		lc.RollingEnabled = true
-		logFile := fmt.Sprintf("logs/functions/%s-${%s}.log", name, EnvShardID)
+		lc.Rolling.Enabled = true
 		switch *runtime.Log.RotatePolicy {
+		case v1alpha1.SizedPolicyWith10MB:
+			lc.Rolling.Type = "size"
+			lc.Rolling.File = logFile
+			lc.Rolling.MaxBytes = 10 * 1024 * 1024
+			lc.Handlers = "stream_handler,rotating_file_handler"
+		case v1alpha1.SizedPolicyWith50MB:
+			lc.Rolling.Type = "size"
+			lc.Rolling.File = logFile
+			lc.Rolling.MaxBytes = 50 * 1024 * 1024
+			lc.Handlers = "stream_handler,rotating_file_handler"
+		case v1alpha1.SizedPolicyWith100MB:
+			lc.Rolling.Type = "size"
+			lc.Rolling.File = logFile
+			lc.Rolling.MaxBytes = 100 * 1024 * 1024
+			lc.Handlers = "stream_handler,rotating_file_handler"
 		case v1alpha1.TimedPolicyWithDaily:
+			lc.Rolling.Type = "time"
+			lc.Rolling.File = logFile
+			lc.Rolling.When = "D"
+			lc.Rolling.Interval = 1
 			lc.Handlers = "stream_handler,timed_rotating_file_handler"
-			lc.Policy = template.HTML(fmt.Sprintf(`[handler_timed_rotating_file_handler]
-args=(\"%s\", 'D', 1, 5,)
-class=handlers.TimedRotatingFileHandler
-level=%s
-formatter=formatter`, logFile, lc.Level))
 		case v1alpha1.TimedPolicyWithWeekly:
+			lc.Rolling.Type = "time"
+			lc.Rolling.File = logFile
+			lc.Rolling.When = "W0" // every monday
+			lc.Rolling.Interval = 1
 			lc.Handlers = "stream_handler,timed_rotating_file_handler"
-			lc.Policy = template.HTML(fmt.Sprintf(`[handler_timed_rotating_file_handler]
-args=(\"%s\", 'W0', 1, 5,)
-class=handlers.TimedRotatingFileHandler
-level=%s
-formatter=formatter`, logFile, lc.Level))
 		case v1alpha1.TimedPolicyWithMonthly:
+			lc.Rolling.Type = "time"
+			lc.Rolling.File = logFile
+			lc.Rolling.When = "D" // day
+			lc.Rolling.Interval = 30
 			lc.Handlers = "stream_handler,timed_rotating_file_handler"
-			lc.Policy = template.HTML(fmt.Sprintf(`[handler_timed_rotating_file_handler]
-args=(\"%s\", 'D', 30, 5,)
-class=handlers.TimedRotatingFileHandler
-level=%s
-formatter=formatter`, logFile, lc.Level))
-		case v1alpha1.SizedPolicyWith10MB:
-			lc.Handlers = "stream_handler,rotating_file_handler"
-			lc.Policy = template.HTML(fmt.Sprintf(`[handler_rotating_file_handler]
-args=(\"%s\", 'a', 10485760, 5,)
-class=handlers.RotatingFileHandler
-level=%s
-formatter=formatter`, logFile, lc.Level))
-		case v1alpha1.SizedPolicyWith50MB:
-			lc.Handlers = "handler_stream_handler,rotating_file_handler"
-			lc.Policy = template.HTML(fmt.Sprintf(`[handler_rotating_file_handler]
-args=(%s, 'a', 52428800, 5,)
-class=handlers.RotatingFileHandler
-level=%s
-formatter=formatter`, logFile, lc.Level))
-		case v1alpha1.SizedPolicyWith100MB:
-			lc.Handlers = "handler_stream_handler,rotating_file_handler"
-			lc.Policy = template.HTML(fmt.Sprintf(`[handler_rotating_file_handler]
-args=(%s, 'a', 104857600, 5,)
-class=handlers.RotatingFileHandler
-level=%s
-formatter=formatter`, logFile, lc.Level))
-		}
-	} else if agent == v1alpha1.SIDECAR { // sidecar mode needs the rotated log file
-		lc.RollingEnabled = true
-		logFile := fmt.Sprintf("logs/functions/%s-${%s}.log", name, EnvShardID)
+		}
+	} else if agent == v1alpha1.SIDECAR {
+		// sidecar mode enables rolling by default, using size policy with 10MB
+		lc.Rolling = RollingCfg{
+			Enabled:  true,
+			Type:     "size",
+			File:     logFile,
+			MaxBytes: 10 * 1024 * 1024,
+			Backups:  5,
+		}
 		lc.Handlers = "stream_handler,rotating_file_handler"
-		lc.Policy = template.HTML(fmt.Sprintf(`[handler_rotating_file_handler]
-args=(\"%s\", 'a', 10485760, 5,)
-class=handlers.RotatingFileHandler
-level=%s
-formatter=formatter`, logFile, lc.Level))
 	}
-	if err := tmpl.Execute(&tpl, lc); err != nil {
+
+	var buf bytes.Buffer
+	if err := tmpl.Execute(&buf, &lc); err != nil {
 		log.Error(err, "failed to render python instance logging template")
 		return "", err
 	}
-	return tpl.String(), nil
+	return buf.String(), nil
 }
 
 func parseJavaLogLevel(runtime *v1alpha1.JavaRuntime) string {