WIP

Tool: gitpod/catfood.gitpod.cloud

Author: geropl

components/ws-daemon/cmd/content-initializer.go

@@ -5,7 +5,7 @@
 package cmd
 
 import (
-	"fmt"
+	"os"
 
 	"github.com/spf13/cobra"
 
@@ -18,13 +18,14 @@ var contentInitializerCmd = &cobra.Command{
 	Short: "fork'ed by ws-daemon to initialize content",
 	Args:  cobra.ExactArgs(0),
 	RunE: func(cmd *cobra.Command, args []string) error {
-		stats, err := content.RunInitializerChild()
+
+		statsFd := os.NewFile(content.RUN_INITIALIZER_CHILD_STATS_FD, "stats")
+
+		err := content.RunInitializerChild(statsFd)
 		if err != nil {
 			return err
 		}
 
-		fmt.Printf(content.FormatStatsBytes(stats))
-
 		return nil
 	},
 }

components/ws-daemon/cmd/content-initializer/main.go

@@ -21,13 +21,13 @@ func main() {
 	log.Init("content-initializer", "", true, false)
 	tracing.Init("content-initializer")
 
-	stats, err := content.RunInitializerChild()
+	statsFd := os.NewFile(content.RUN_INITIALIZER_CHILD_STATS_FD, "stats")
+
+	err := content.RunInitializerChild(statsFd)
 	if err != nil {
-		errfd := os.NewFile(uintptr(3), "errout")
+		errfd := os.NewFile(content.RUN_INITIALIZER_CHILD_ERROUT_FD, "errout")
 		_, _ = fmt.Fprintf(errfd, err.Error())
 
 		os.Exit(content.FAIL_CONTENT_INITIALIZER_EXIT_CODE)
 	}
-
-	fmt.Printf(content.FormatStatsBytes(stats))
 }

components/ws-daemon/debug.Dockerfile

@@ -9,7 +9,7 @@ RUN go get -u github.com/go-delve/delve/cmd/dlv
 FROM cgr.dev/chainguard/wolfi-base:latest@sha256:a7db49b55bd97c12cd686272325bbac236830111db336e084b89f5c816ab0537 as dl
 WORKDIR /dl
 RUN apk add --no-cache curl file \
-  && curl -OsSL https://github.com/opencontainers/runc/releases/download/v1.1.12/runc.amd64 \
+  && curl -OsSL https://github.com/opencontainers/runc/releases/download/v1.1.14/runc.amd64 \
   && chmod +x runc.amd64 \
   && if ! file runc.amd64 | grep -iq "ELF 64-bit LSB pie executable"; then echo "runc.amd64 is not a binary file"; exit 1;fi
 

components/ws-daemon/pkg/content/initializer.go

@@ -5,16 +5,15 @@
 package content
 
 import (
-	"bufio"
 	"bytes"
 	"context"
 	"encoding/json"
 	"errors"
-	"fmt"
-	"io/ioutil"
+	"io"
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"syscall"
 	"time"
@@ -253,37 +252,36 @@ func RunInitializer(ctx context.Context, destination string, initializer *csapi.
 		name = "init-ws-" + opts.OWI.InstanceID
 	}
 
-	args = append(args, "--log-format", "json", "run")
-	args = append(args, "--preserve-fds", "1")
-	args = append(args, name)
-
+	// pass a pipe "file" to the content init process as fd 3 to capture the error output
 	errIn, errOut, err := os.Pipe()
 	if err != nil {
 		return nil, err
 	}
-	errch := make(chan []byte, 1)
-	go func() {
-		errmsg, _ := ioutil.ReadAll(errIn)
-		errch <- errmsg
-	}()
+	defer errOut.Close()
+
+	// pass a pipe "file" to the content init process as fd 4 to capture the metrics output
+	statsIn, statsOut, err := os.Pipe()
+	if err != nil {
+		return nil, err
+	}
+	defer statsOut.Close()
+
+	args = append(args, "--log-format", "json", "run")
+	extraFiles := []*os.File{errOut, statsOut}
+	args = append(args, "--preserve-fds", strconv.Itoa(len(extraFiles)))
+	args = append(args, name)
 
 	var cmdOut bytes.Buffer
 	cmd := exec.Command("runc", args...)
 	cmd.Dir = tmpdir
 	cmd.Stdout = &cmdOut
 	cmd.Stderr = os.Stderr
 	cmd.Stdin = os.Stdin
-	cmd.ExtraFiles = []*os.File{errOut}
+	cmd.ExtraFiles = extraFiles
 	err = cmd.Run()
 	log.FromBuffer(&cmdOut, log.WithFields(opts.OWI.Fields()))
-	errOut.Close()
 
-	var errmsg []byte
-	select {
-	case errmsg = <-errch:
-	case <-time.After(1 * time.Second):
-		errmsg = []byte("failed to read content initializer response")
-	}
+	errmsg, statsBytes := waitForAndReadExtraFiles(errIn, statsIn)
 	if err != nil {
 		if exiterr, ok := err.(*exec.ExitError); ok {
 			// The program has exited with an exit code != 0. If it's FAIL_CONTENT_INITIALIZER_EXIT_CODE, it was deliberate.
@@ -296,42 +294,76 @@ func RunInitializer(ctx context.Context, destination string, initializer *csapi.
 		return nil, err
 	}
 
-	stats := parseStats(&cmdOut)
-
+	stats := parseStats(statsBytes)
 	return stats, nil
 }
 
-func parseStats(buf *bytes.Buffer) *csapi.InitializerMetrics {
-	scanner := bufio.NewScanner(buf)
-	for scanner.Scan() {
-		line := string(scanner.Bytes())
-		if !strings.HasPrefix(line, STATS_PREFIX) {
-			continue
+// waitForAndReadExtraFiles tries to read the content of the extra files passed to the content initializer, and waits up to 1s to do so
+func waitForAndReadExtraFiles(errIn *os.File, statsIn *os.File) (errmsg []byte, statsBytes []byte) {
+	// read err
+	errch := make(chan []byte, 1)
+	go func() {
+		errmsg, _ := io.ReadAll(errIn)
+		errch <- errmsg
+	}()
+
+	// read stats
+	statsCh := make(chan []byte, 1)
+	go func() {
+		statsBytes, readErr := io.ReadAll(statsIn)
+		if readErr != nil {
+			log.WithError(readErr).Error("cannot read stats")
 		}
+		log.WithField("statsBytes", log.TrustedValueWrap{Value: string(statsBytes)}).Info("read stats")
+		statsCh <- statsBytes
+	}()
 
-		b := strings.TrimSpace(strings.TrimPrefix(line, STATS_PREFIX))
-		var stats csapi.InitializerMetrics
-		err := json.Unmarshal([]byte(b), &stats)
-		if err != nil {
-			log.WithError(err).WithField("line", line).Error("cannot unmarshal stats")
-			return nil
+	readFiles := 0
+	for {
+		select {
+		case errmsg = <-errch:
+			readFiles += 1
+		case statsBytes = <-statsCh:
+			readFiles += 1
+		case <-time.After(1 * time.Second):
+			if errmsg == nil {
+				errmsg = []byte("failed to read content initializer response")
+			}
+			return
+		}
+		if readFiles == 2 {
+			return
 		}
-		return &stats
 	}
-	return nil
 }
 
+func parseStats(statsBytes []byte) *csapi.InitializerMetrics {
+	var stats csapi.InitializerMetrics
+	err := json.Unmarshal(statsBytes, &stats)
+	if err != nil {
+		log.WithError(err).WithField("bytes", log.TrustedValueWrap{Value: statsBytes}).Error("cannot unmarshal stats")
+		return nil
+	}
+	return &stats
+}
+
+// RUN_INITIALIZER_CHILD_ERROUT_FD is the fileDescriptor of the "errout" file descriptor passed to the content initializer
+const RUN_INITIALIZER_CHILD_ERROUT_FD = 3
+
+// RUN_INITIALIZER_CHILD_STATS_FD is the fileDescriptor of the "stats" file descriptor passed to the content initializer
+const RUN_INITIALIZER_CHILD_STATS_FD = 4
+
 // RunInitializerChild is the function that's expected to run when we call `/proc/self/exe content-initializer`
-func RunInitializerChild() (serializedStats []byte, err error) {
+func RunInitializerChild(statsFd *os.File) (err error) {
 	fc, err := os.ReadFile("/content.json")
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	var initmsg msgInitContent
 	err = json.Unmarshal(fc, &initmsg)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	log.Log = logrus.WithFields(initmsg.OWI)
 
@@ -348,15 +380,15 @@ func RunInitializerChild() (serializedStats []byte, err error) {
 	var req csapi.WorkspaceInitializer
 	err = proto.Unmarshal(initmsg.Initializer, &req)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	rs := &remoteContentStorage{RemoteContent: initmsg.RemoteContent}
 
 	dst := initmsg.Destination
 	initializer, err := wsinit.NewFromRequest(ctx, dst, rs, &req, wsinit.NewFromRequestOpts{ForceGitpodUserForGit: false})
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	initSource, stats, err := wsinit.InitializeWorkspace(ctx, dst, rs,
@@ -366,35 +398,46 @@ func RunInitializerChild() (serializedStats []byte, err error) {
 		wsinit.WithCleanSlate,
 	)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	// some workspace content may have a `/dst/.gitpod` file or directory. That would break
 	// the workspace ready file placement (see https://github.com/gitpod-io/gitpod/issues/7694).
 	err = wsinit.EnsureCleanDotGitpodDirectory(ctx, dst)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	// Place the ready file to make Theia "open its gates"
 	err = wsinit.PlaceWorkspaceReadyFile(ctx, dst, initSource, stats, initmsg.UID, initmsg.GID)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	// Serialize metrics, so we can pass them back to the caller
-	serializedStats, err = json.Marshal(stats)
-	if err != nil {
-		return nil, err
+	if statsFd != nil {
+		defer statsFd.Close()
+		writeStats(statsFd, &stats)
+	} else {
+		log.Warn("no stats file descriptor provided")
 	}
 
-	return serializedStats, nil
+	return nil
 }
 
-const STATS_PREFIX = "STATS:"
+func writeStats(statsFd *os.File, stats *csapi.InitializerMetrics) {
+	serializedStats, err := json.Marshal(stats)
+	if err != nil {
+		log.WithError(err).Warn("cannot serialize stats")
+		return
+	}
 
-func FormatStatsBytes(statsBytes []byte) string {
-	return fmt.Sprintf("%s %s\n", STATS_PREFIX, string(statsBytes))
+	log.WithField("stats", log.TrustedValueWrap{Value: string(serializedStats)}).Info("writing stats to fd")
+	_, writeErr := statsFd.Write(serializedStats)
+	if writeErr != nil {
+		log.WithError(writeErr).Warn("error writing stats to fd")
+		return
+	}
 }
 
 var _ storage.DirectAccess = &remoteContentStorage{}