Revert "Fix container stats baseline for recycled runners (#7080)" (#…

…7123)

enterprise/server/remote_execution/container/container.go

@@ -213,13 +213,14 @@ type UsageStats struct {
 	baselineCPUPressure, baselineMemoryPressure, baselineIOPressure *repb.PSI
 }
 
-// SetBaseline records the current stats readings just before executing a new
-// task. It should be called at the beginning of Exec() in the container
-// lifecycle to ensure that the reported stats are only accounting for the
-// container's resource usage during the Exec() call itself.
-func (s *UsageStats) SetBaseline(lifetimeStats *repb.UsageStats) {
-	s.Update(lifetimeStats)
-
+// Reset resets resource usage counters in preparation for a new task, so that
+// the new task's resource usage can be accounted for. It should be called
+// at the beginning of Exec() in the container lifecycle.
+func (s *UsageStats) Reset() {
+	if s.last == nil {
+		// No observations yet; nothing to do.
+		return
+	}
 	s.last.MemoryBytes = 0
 	s.baselineCPUNanos = s.last.GetCpuNanos()
 	s.baselineCPUPressure = s.last.GetCpuPressure()

enterprise/server/remote_execution/container/container_test.go

@@ -266,7 +266,7 @@ func TestUsageStats(t *testing.T) {
 	s := &container.UsageStats{}
 	require.Empty(t, cmp.Diff(&repb.UsageStats{}, s.TaskStats(), protocmp.Transform()))
 
-	s.SetBaseline(&repb.UsageStats{})
+	s.Reset()
 	require.Empty(t, cmp.Diff(&repb.UsageStats{}, s.TaskStats(), protocmp.Transform()))
 
 	// Observe some cgroup usage.
@@ -306,16 +306,10 @@ func TestUsageStats(t *testing.T) {
 		IoPressure:      makePSI(30_000, 3_000),
 	}, s.TaskStats(), protocmp.Transform()))
 
-	// Start a new task, setting the baseline to the last observed stats. The
-	// cgroup accumulated CPU etc. will not be reset, but the metrics that we
-	// report should appear as though they were.
-	s.SetBaseline(&repb.UsageStats{
-		CpuNanos:       2e9,
-		MemoryBytes:    45 * 1024 * 1024,
-		CpuPressure:    makePSI(150, 10),
-		MemoryPressure: makePSI(2_000, 200),
-		IoPressure:     makePSI(30_000, 3_000),
-	})
+	// Start a new task, using the same UsageStats instance. The cgroup
+	// accumulated CPU etc. will not be reset, but the metrics that we report
+	// should appear as though they were.
+	s.Reset()
 	require.Empty(t, cmp.Diff(&repb.UsageStats{
 		CpuPressure:    makePSI(0, 0),
 		MemoryPressure: makePSI(0, 0),

enterprise/server/remote_execution/containers/ociruntime/ociruntime.go

@@ -352,14 +352,8 @@ func (c *ociContainer) Create(ctx context.Context, workDir string) error {
 }
 
 func (c *ociContainer) Exec(ctx context.Context, cmd *repb.Command, stdio *interfaces.Stdio) *interfaces.CommandResult {
-	// Set the baseline for stats to ensure that we only report the usage
-	// incurred during this Exec() call.
-	s, err := c.Stats(ctx)
-	if err != nil {
-		return commandutil.ErrorResult(status.UnavailableErrorf("failed to get baseline stats for execution: %s", err))
-	}
-	c.stats.SetBaseline(s)
-
+	// Reset CPU usage and peak memory since we're starting a new task.
+	c.stats.Reset()
 	args := []string{"exec", "--cwd=" + execrootPath}
 	// Respect command env. Note, when setting any --env vars at all, it
 	// completely overrides the env from the bundle, rather than just adding
@@ -372,7 +366,7 @@ func (c *ociContainer) Exec(ctx context.Context, cmd *repb.Command, stdio *inter
 	if !ok {
 		return commandutil.ErrorResult(status.UnavailableError("exec called before pulling image"))
 	}
-	cmd, err = withImageConfig(cmd, image)
+	cmd, err := withImageConfig(cmd, image)
 	if err != nil {
 		return commandutil.ErrorResult(status.UnavailableErrorf("apply image config: %s", err))
 	}

enterprise/server/remote_execution/containers/podman/podman.go

@@ -480,14 +480,10 @@ func (c *podmanCommandContainer) Create(ctx context.Context, workDir string) err
 }
 
 func (c *podmanCommandContainer) Exec(ctx context.Context, cmd *repb.Command, stdio *interfaces.Stdio) *interfaces.CommandResult {
-	// Set the baseline for stats to ensure that we only report the usage
-	// incurred during this Exec() call.
-	s, err := c.Stats(ctx)
-	if err != nil {
-		return commandutil.ErrorResult(status.UnavailableErrorf("failed to get baseline stats for execution: %s", err))
-	}
-	c.stats.SetBaseline(s)
-
+	// Reset usage stats since we're running a new task. Note: This throws away
+	// any resource usage between the initial "Create" call and now, but that's
+	// probably fine for our needs right now.
+	c.stats.Reset()
 	podmanRunArgs := make([]string, 0, 2*len(cmd.GetEnvironmentVariables())+len(cmd.Arguments)+1)
 	for _, envVar := range cmd.GetEnvironmentVariables() {
 		podmanRunArgs = append(podmanRunArgs, "--env", fmt.Sprintf("%s=%s", envVar.GetName(), envVar.GetValue()))