Address comments.

tools/cmd/runner/main.go

@@ -78,7 +78,7 @@ func main() {
 	log.Printf("Queue concurrency levels: %v", c)
 	log.Printf("Output directories: %v", outputDirMap)
 	if logURLPrefix != "" {
-		log.Printf("Prefix for url for saved logs: %s", logURLPrefix)
+		log.Printf("Prefix for log urls: %s", logURLPrefix)
 	}
 
 	r := runner.NewRunner(runner.NewLoadTestGetter(), runner.AfterIntervalFunction(p), retries, deleteSuccessfulTests, runner.NewPodsGetter(), logURLPrefix)

tools/runner/logsaver.go

@@ -29,11 +29,15 @@ import (
 	corev1types "k8s.io/client-go/kubernetes/typed/core/v1"
 )
 
-// SaveLogs saves logs to files, the name of the files are in format
-// pod-name-container-name.log.
-// This function returns a list of pointers of LogInfo.
-func SaveLogs(ctx context.Context, loadTest *grpcv1.LoadTest, pods []*corev1.Pod, podsGetter corev1types.PodsGetter, podLogDir string) ([]*LogInfo, error) {
-	logInfos := []*LogInfo{}
+// SaveAllLogs saves all logs to files and return their LogInfo.
+//
+// SaveAllLogs goes through all the containers in each pod and write
+// the log to the a given path if the container have logs. The name
+// of the files are in format pod-name-container-name.log. After
+// writing the logs the function returns a list of pointers of
+// LogInfo objects containing the log's information.
+func SaveAllLogs(ctx context.Context, loadTest *grpcv1.LoadTest, podsGetter corev1types.PodsGetter, pods []*corev1.Pod, podLogDir string) ([]*LogInfo, error) {
+	var logInfos []*LogInfo
 
 	// Attempt to create directory. Will not error if directory already exists
 	err := os.MkdirAll(podLogDir, os.ModePerm)
@@ -44,33 +48,26 @@ func SaveLogs(ctx context.Context, loadTest *grpcv1.LoadTest, pods []*corev1.Pod
 	// Write logs to files
 	for _, pod := range pods {
 		for _, container := range pod.Spec.Containers {
-			logBuffer, err := GetLogBuffer(ctx, pod, podsGetter, container.Name)
-
-			if err != nil {
-				return logInfos, fmt.Errorf("could not get log from pod: %s", err)
-			}
-
-			if logBuffer.Len() == 0 {
-				continue
-			}
-
 			logFilePath := filepath.Join(podLogDir, fmt.Sprintf("%s-%s.log", pod.Name, container.Name))
-			logInfo := NewLogInfo(PodNameElement(pod.Name, loadTest.Name), container.Name, logFilePath)
 
-			err = writeBufferToFile(logBuffer, logFilePath)
+			logInfo, err := SaveLog(ctx, pod, podsGetter, PodNameElem(pod.Name, loadTest.Name), container.Name, logFilePath)
 			if err != nil {
-				return logInfos, fmt.Errorf("could not write %s container in %s pod log buffer to file: %s", logInfo.containerName, pod.Name, err)
+				return logInfos, fmt.Errorf("could not get log from container: %v", err)
+			}
+			if logInfo != nil {
+				logInfos = append(logInfos, logInfo)
 			}
-
-			logInfos = append(logInfos, logInfo)
 		}
 	}
 	return logInfos, nil
 }
 
-// GetLogBuffer retrieves logs from a specific container
-// in the given pod and return the log buffer.
-func GetLogBuffer(ctx context.Context, pod *corev1.Pod, podsGetter corev1types.PodsGetter, containerName string) (*bytes.Buffer, error) {
+// SaveLog retrieves and save logs for a specific container.
+//
+// SaveLog retrieves logs from a container under given container
+// name within given pod,then save the logs to the given file
+// path, if there are logs to save.
+func SaveLog(ctx context.Context, pod *corev1.Pod, podsGetter corev1types.PodsGetter, podNameElem string, containerName string, filePath string) (*LogInfo, error) {
 	req := podsGetter.Pods(pod.Namespace).GetLogs(pod.Name, &corev1.PodLogOptions{Container: containerName})
 	containerLogs, err := req.Stream(ctx)
 	if err != nil {
@@ -79,28 +76,27 @@ func GetLogBuffer(ctx context.Context, pod *corev1.Pod, podsGetter corev1types.P
 	defer containerLogs.Close()
 	logBuffer := new(bytes.Buffer)
 	logBuffer.ReadFrom(containerLogs)
-	return logBuffer, nil
-}
 
-func writeBufferToFile(buffer *bytes.Buffer, filePath string) error {
 	// Don't write empty buffers
-	if buffer.Len() == 0 {
-		return nil
+	if logBuffer.Len() == 0 {
+		return nil, nil
 	}
 
 	// Open output file
 	file, err := os.Create(filePath)
 	if err != nil {
-		return fmt.Errorf("could not open %s for writing", filePath)
+		return nil, fmt.Errorf("could not open %s for writing", filePath)
 	}
 	defer file.Close()
 
 	// Write log to output file
-	_, err = io.Copy(file, buffer)
+	_, err = io.Copy(file, logBuffer)
 	file.Sync()
 	if err != nil {
-		return fmt.Errorf("error writing to %s: %v", filePath, err)
+		return nil, fmt.Errorf("error writing to %s: %v", filePath, err)
 	}
 
-	return nil
+	logInfo := &LogInfo{PodNameElem: podNameElem, ContainerName: containerName, LogPath: filePath}
+
+	return logInfo, nil
 }

tools/runner/properties.go

@@ -23,52 +23,60 @@ import (
 	corev1 "k8s.io/api/core/v1"
 )
 
-// LogInfo contains infomation for a log entry.
+// LogInfo contains infomation for each log file.
 type LogInfo struct {
-	podNameElement string
-	containerName  string
-	logPath        string
+	// PodNameElem is a part of the pod name.
+	// PodNameElem is the remaining part of the pod name after the
+	// subtraction of the LoadTest name. Examples of the PodNameElem
+	// are: client-0, driver-0 and server-0.
+	PodNameElem string
+	// ContainerName is the container's name where the log comes from.
+	ContainerName string
+	// LogPath is where the log is saved.
+	LogPath string
 }
 
-// NewLogInfo creates a pointer of new LogInfo object.
-func NewLogInfo(podNameElement string, containerName string, logPath string) *LogInfo {
-	return &LogInfo{
-		podNameElement: podNameElement,
-		containerName:  containerName,
-		logPath:        logPath,
-	}
-}
-
-// PodLogProperties creates container log property name to
-// container log link map.
-func PodLogProperties(logInfos []*LogInfo, logURLPrefix string) map[string]string {
+// PodLogProperties creates log property name to property value map.
+func PodLogProperties(logInfos []*LogInfo, logURLPrefix string, prefix ...string) map[string]string {
 	properties := make(map[string]string)
 	for _, logInfo := range logInfos {
-		prefix := []string{logInfo.podNameElement, "name", "log", logInfo.containerName}
-		podLogPropertyKey := strings.Join(prefix, ".")
-		url := logURLPrefix + logInfo.logPath
-		properties[podLogPropertyKey] = url
+		podLogPropertyKey := PodLogPropertyKey(logInfo, prefix...)
+		logURL := logURLPrefix + logInfo.LogPath
+		properties[podLogPropertyKey] = logURL
 	}
 	return properties
 }
 
-// PodNameElement trim off the loadtest name from the pod name,
-// and return only the element of pod name such as client-0,
+// PodNameElem generate the pod name element.
+//
+// PodNameElem trims off the given LoadTest name and "-" from the
+// given pod name,returns remaining part such as client-0,
 // driver-0 and server-0.
-func PodNameElement(podName, loadTestName string) string {
+func PodNameElem(podName, loadTestName string) string {
 	prefix := fmt.Sprintf("%s-", loadTestName)
 	podNameElement := strings.TrimPrefix(podName, prefix)
 	return podNameElement
 }
 
+// PodLogPropertyKey generates the key for a pod log property.
+func PodLogPropertyKey(logInfo *LogInfo, prefix ...string) string {
+	key := strings.Join(append(prefix, logInfo.PodNameElem, "log", logInfo.ContainerName), ".")
+	return key
+}
+
 // PodNameProperties creates pod property name to pod name map.
-func PodNameProperties(pods []*corev1.Pod, loadTestName string) map[string]string {
+func PodNameProperties(pods []*corev1.Pod, loadTestName string, prefix ...string) map[string]string {
 	properties := make(map[string]string)
 	for _, pod := range pods {
-		prefix := []string{PodNameElement(pod.Name, loadTestName), "name"}
-		podNamePropertyKey := strings.Join(prefix, ".")
+		podNamePropertyKey := PodNamePropertyKey(PodNameElem(pod.Name, loadTestName), prefix...)
 		properties[podNamePropertyKey] = pod.Name
 	}
 
 	return properties
 }
+
+// PodNamePropertyKey generates the key for a pod name property.
+func PodNamePropertyKey(podNameElem string, prefix ...string) string {
+	key := strings.Join(append(prefix, podNameElem, "name"), ".")
+	return key
+}

tools/runner/runner.go

@@ -42,6 +42,9 @@ type Runner struct {
 	// loadTestGetter interacts with the cluster to create, get and delete
 	// LoadTests.
 	loadTestGetter clientset.LoadTestGetter
+	// podsGetter has a method to return a PodInterface which provide access
+	// to work with Pod resources.
+	corev1types.PodsGetter
 	// afterInterval stops for a set time interval before returning.
 	// It is used to set a polling interval.
 	afterInterval func()
@@ -51,8 +54,6 @@ type Runner struct {
 	// deleteSuccessfulTests determines whether tests that terminate without
 	// errors should be deleted immediately.
 	deleteSuccessfulTests bool
-	// podsLister obtain a list of pods
-	podsGetter corev1types.PodsGetter
 	// logURLPrefix is used to calculate the link to the log saved in placer
 	logURLPrefix string
 }
@@ -64,7 +65,7 @@ func NewRunner(loadTestGetter clientset.LoadTestGetter, afterInterval func(), re
 		afterInterval:         afterInterval,
 		retries:               retries,
 		deleteSuccessfulTests: deleteSuccessfulTests,
-		podsGetter:            podsGetter,
+		PodsGetter:            podsGetter,
 		logURLPrefix:          logURLPrefix,
 	}
 }
@@ -145,20 +146,20 @@ func (r *Runner) runTest(ctx context.Context, config *grpcv1.LoadTest, reporter
 		status = statusString(config)
 		switch {
 		case loadTest.Status.State.IsTerminated():
-			pods, err := GetTestPods(ctx, loadTest, r.podsGetter)
+			pods, err := GetTestPods(ctx, loadTest, r.PodsGetter)
 			if err != nil {
 				reporter.Error("Could not list all pods belongs to %s: %s", loadTest.Name, err)
 			}
-			savedLogInfos, err := SaveLogs(ctx, loadTest, pods, r.podsGetter, outputDir)
+			savedLogInfos, err := SaveAllLogs(ctx, loadTest, r.PodsGetter, pods, outputDir)
 			if err != nil {
 				reporter.Error("Could not save pod logs: %s", err)
 			}
 			reporter.AddProperty("name", loadTest.Name)
-			for property, value := range PodNameProperties(pods, loadTest.Name) {
+			for property, value := range PodNameProperties(pods, loadTest.Name, "pod") {
 				reporter.AddProperty(property, value)
 			}
 
-			for property, value := range PodLogProperties(savedLogInfos, r.logURLPrefix) {
+			for property, value := range PodLogProperties(savedLogInfos, r.logURLPrefix, "pod") {
 				reporter.AddProperty(property, value)
 			}