Skip to content

tests: shared integration harness and single launcher layout - #120

Merged
aishukamal merged 3 commits into
mainfrom
tests/integ-framework
Jul 30, 2026
Merged

tests: shared integration harness and single launcher layout#120
aishukamal merged 3 commits into
mainfrom
tests/integ-framework

Conversation

@aishukamal

@aishukamal aishukamal commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

Stacked on #119. Pure refactor — no behavior change.

  • tests/integration becomes the home of the integration suites with the launcher at the top: tests/integration/{README.md, run.sh, runner.yaml, harness/, snapshot-agent/}. run.sh stays the single entrypoint (paths updated; the gate workflow's README pointer follows the move).
  • New tests/integration/harness package: the generic pieces extracted from the snapshot-agent harness — in-cluster client setup, node selection (TEST_NODE / first-free-GPU pick), pod lifecycle (ready-wait, force-delete, exec with stdin), HTTP and VRAM helpers, and attach-by-label for chart-deployed pods. snapshot-agent/ keeps only the agent specifics (agent deployment, engine specs, the agentctl.py call path) and composes the shared Cluster.

Verification

Cloud Build green (build, go vet -tags integration ./tests/..., unit tests, lint). Pure refactor with no runtime change, so no live cluster run.

Summary by CodeRabbit

  • Documentation

    • Updated integration test documentation, repository layout guidance, and command examples.
    • Improved failure messages to reference the current documentation location.
  • Integration Testing

    • Added a Kubernetes-based integration test runner and shared test utilities.
    • Expanded test execution to cover the full integration test suite.
    • Improved support for node and GPU selection, pod readiness, command execution, and resource verification.
    • Simplified snapshot-agent integration test setup through shared testing capabilities.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@aishukamal, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 18 seconds

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d10c15b1-b926-4759-b813-6b52f8b7f401

📥 Commits

Reviewing files that changed from the base of the PR and between 1328f9e and 9012a82.

📒 Files selected for processing (6)
  • .github/workflows/label-integration-test.yaml
  • tests/integration/README.md
  • tests/integration/harness/harness.go
  • tests/integration/run.sh
  • tests/integration/runner.yaml
  • tests/integration/snapshot-agent/harness.go
📝 Walkthrough

Walkthrough

Changes

Integration test harness

Layer / File(s) Summary
Shared cluster and pod helpers
tests/integration/harness/harness.go
Adds reusable Kubernetes configuration, GPU node selection, pod readiness/deletion, HTTP polling, pod exec, and VRAM helpers.
Runner permissions and test entrypoint
tests/integration/runner.yaml, tests/integration/run.sh
Adds the Kubernetes runner pod and RBAC, and expands execution to all integration packages.
Snapshot-agent harness integration
tests/integration/snapshot-agent/harness.go
Refactors snapshot-agent tests to use the shared cluster and pod helpers.
Integration documentation and workflow guidance
tests/integration/README.md, .github/workflows/label-integration-test.yaml
Updates documentation links, layout references, and test commands.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

Suggested reviewers: lynnl0927

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly reflects the main refactor: a shared integration harness and a single launcher layout.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Fix failing CI checks
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch tests/integ-framework

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@aishukamal
aishukamal force-pushed the tests/helm-deploy branch 2 times, most recently from 62e3fef to 5b49591 Compare July 24, 2026 05:16
@aishukamal
aishukamal force-pushed the tests/integ-framework branch from 91864f0 to 46aa01d Compare July 24, 2026 05:23
@aishukamal aishukamal changed the title tests: shared integration framework and rlts scenario extraction tests: consolidate integration tests on a shared framework and the rlts scenarios Jul 24, 2026
@aishukamal
aishukamal force-pushed the tests/integ-framework branch from 46aa01d to f4db72b Compare July 24, 2026 05:40
@aishukamal
aishukamal force-pushed the tests/helm-deploy branch 2 times, most recently from 2ea7fb7 to e51f15e Compare July 24, 2026 06:20
@aishukamal
aishukamal force-pushed the tests/integ-framework branch 2 times, most recently from 38d3c9b to d41d81d Compare July 24, 2026 17:33
@aishukamal aishukamal changed the title tests: consolidate integration tests on a shared framework and the rlts scenarios tests: shared integration harness and single launcher layout Jul 24, 2026
@aishukamal
aishukamal force-pushed the tests/integ-framework branch from d41d81d to fca1d00 Compare July 24, 2026 18:39
@aishukamal
aishukamal marked this pull request as draft July 24, 2026 19:54
Base automatically changed from tests/helm-deploy to main July 28, 2026 00:05
@aishukamal
aishukamal marked this pull request as ready for review July 28, 2026 00:16
@aishukamal
aishukamal force-pushed the tests/integ-framework branch 2 times, most recently from 0cb2e0a to 88a6133 Compare July 28, 2026 03:46
@aishukamal
aishukamal requested a review from jessicaochen July 28, 2026 21:05
@jessicaochen

Copy link
Copy Markdown
Collaborator

I suppose a nit is that it should probably be test not tests following https://github.com/golang-standards/project-layout/blob/master/test/README.md

@jessicaochen

Copy link
Copy Markdown
Collaborator

so I know with what expectation to review this with, is the orchestrator integegraion supposed to run with the code in this PR or is it purely moving the agent harness to the shared location before making it work with both agent & orchestrator e2e?

Comment thread tests/integration/run.sh
Comment thread tests/integration/snapshot-agent/harness.go
Comment thread tests/integration/snapshot-agent/harness.go
@github-project-automation github-project-automation Bot moved this from Todo to In Progress in Time-Slicing Project Tracker Jul 28, 2026
@aishukamal

Copy link
Copy Markdown
Collaborator Author

so I know with what expectation to review this with, is the orchestrator integegraion supposed to run with the code in this PR or is it purely moving the agent harness to the shared location before making it work with both agent & orchestrator e2e?

No this PR only extracts the shared harness and moves the launcher up to test/integration/. The orchestrator integration suite is in #129 (draft), which builds on this shared infrastructure.

I suppose a nit is that it should probably be test not tests following https://github.com/golang-standards/project-layout/blob/master/test/README.md

Ack, I'll do this in a separate PR once this chain is merged to avoid having to update 4 other PRs in the queue.

@aishukamal
aishukamal force-pushed the tests/integ-framework branch 2 times, most recently from 13e0d5e to d6f1fbd Compare July 29, 2026 03:25

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/integration/harness/harness.go`:
- Around line 124-126: Update GPUsRequestedOnNode to calculate each pod’s
scheduler-effective GPU request instead of summing only pod.Spec.Containers: use
the greater of the non-init container request sum and init-container peak,
account for restartable init sidecars and GPU PodOverhead according to
Kubernetes scheduling rules, then add that effective value to the node total
used by PickGPUNode.

In `@tests/integration/runner.yaml`:
- Around line 11-23: The snapshot-agent-test ClusterRoleBinding currently grants
the broad cluster-wide view role. Replace roleRef.name with a dedicated
least-privilege ClusterRole and define that role with only core pod get, list,
and watch permissions required by the agent’s Kubernetes watcher, preserving the
existing ServiceAccount subject and binding scope.
- Around line 88-101: Harden the runner container in the Pod manifest by adding
a securityContext with runAsNonRoot, allowPrivilegeEscalation disabled, all
Linux capabilities dropped, and seccompProfile set to RuntimeDefault. Update the
runner image or container user configuration so the golang:1.25-bookworm-based
container runs as a non-root user while preserving its existing test command.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9be23374-4257-4cec-bd03-dc262a0dab98

📥 Commits

Reviewing files that changed from the base of the PR and between c48a169 and 1328f9e.

📒 Files selected for processing (6)
  • .github/workflows/label-integration-test.yaml
  • tests/integration/README.md
  • tests/integration/harness/harness.go
  • tests/integration/run.sh
  • tests/integration/runner.yaml
  • tests/integration/snapshot-agent/harness.go

Comment on lines +124 to +126
for j := range pod.Spec.Containers {
req := pod.Spec.Containers[j].Resources.Requests[corev1.ResourceName("nvidia.com/gpu")]
used += req.Value()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate files =="
git ls-files | rg '(^|/)harness\.go$|pkg/|cmd/|Makefile|README|go\.mod|test' | head -200

echo
echo "== target snippet with context =="
if [ -f tests/integration/harness/harness.go ]; then
  nl -ba tests/integration/harness/harness.go | sed -n '90,155p'
fi

echo
echo "== search PickGPUNode and GPU request usage =="
rg -n "PickGPUNode|Resources\.Requests|nvidia\.com/gpu|Overhead|InitContainers|Containers|pod" tests/integration/harness/harness.go . --glob '!vendor/**' --glob '!testdata/**' | head -300

Repository: llm-d-incubation/llm-d-rl-time-slicing

Length of output: 5142


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== target file size and imports/outline-ish context =="
wc -l tests/integration/harness/harness.go
echo
seq 1 220 > /tmp/indices
xargs -n1 < /tmp/indices | parallel -q -I {} sh -c 'printf "%7d: " "{}"; sed -n "{}p" tests/integration/harness/harness.go'

echo
echo "== PickGPUNode region =="
grep -n "PickGPUNode\\|used += req\\.Value\\|PodSpec\\|Containers\\|InitContainers" tests/integration/harness/harness.go

echo
echo "== repository GPU/scheduler/accounting related search =="
grep -RIn "PickGPUNode\\|nvidia\\.com/gpu\\|InitContainers\\|PodOverhead\\|Overhead\\|scheduler\\|scheduler semantics" . --exclude-dir=.git --exclude-dir=vendor --exclude-dir=dist --exclude-dir=build | head -300

Repository: llm-d-incubation/llm-d-rl-time-slicing

Length of output: 314


🏁 Script executed:

#!/bin/bash
set -euo pipefail
shopt -s extglob

echo "== target file lines =="
python3 - <<'PY'
from pathlib import Path
p = Path('tests/integration/harness/harness.go')
lines = p.read_text().splitlines()
for i in range(1, len(lines)+1):
    print(f"{i:5d}: {lines[i-1]}")
PY

echo
echo "== relevant symbols =="
grep -n "PickGPUNode\\|used += req\\.Value\\|Containers\\|InitContainers\\|Overhead\\|pod" tests/integration/harness/harness.go || true

echo
echo "== GPU / scheduler references =="
grep -RInE "PickGPUNode|nvidia\.com/gpu|InitContainers|PodOverhead|scheduler|scheduler semantics|overhead" . --exclude-dir=.git --exclude-dir=vendor --exclude-dir=dist --exclude-dir=build | head -300 || true

Repository: llm-d-incubation/llm-d-rl-time-slicing

Length of output: 18350


🌐 Web query:

Kubernetes init containers resource requests PodOverhead scheduler reserves maximum init request non-init request documentation

💡 Result:

In Kubernetes, the scheduler calculates a Pod's total resource requirements by combining the effective container resource requests with the PodOverhead [1][2]. The effective resource request for a Pod (before adding PodOverhead) is defined as the maximum of [3][4]: 1. The sum of all non-init containers' resource requests [3]. 2. The maximum resource request among all individual init containers [3]. If a Pod includes sidecar containers (which are a special type of init container), the calculation becomes more complex as they do not terminate, and their resource requirements must be aggregated into the effective calculation [4]. PodOverhead is then added to this effective container request sum to determine the final resource requirement used by the scheduler, resource quotas, and Kubelet [1][2][5]. The final formula for the Pod's effective resource request is: Effective Pod Request = Max(Sum of App Containers, Max of Init Containers) + PodOverhead [1][4]. This total value is what the scheduler uses to determine if a node has sufficient capacity to host the Pod [1]. This calculation remains constant throughout the Pod's lifecycle; the scheduler does not reduce the Pod's resource request on a node after init containers have successfully finished execution [6]. This ensures that sufficient resources are reserved to accommodate the init containers if they need to restart or re-execute [6].

Citations:


🌐 Web query:

Kubernetes sidecar containers initContainers resources additive Max of other init containers

💡 Result:

In Kubernetes, native sidecar containers (init containers with restartPolicy: Always) are included in the resource calculation for the Pod in a way that differs from traditional "regular" init containers [1][2][3]. Effective Pod resource requests and limits are calculated as the higher of two values [1][4][5]: 1. The sum of all non-init containers (including all application containers AND all sidecar containers) [1][6][5]. 2. The effective init request/limit [1][4][5]. The effective init request/limit is defined by the maximum of the resource requirements among all init containers, but with a modification to account for sidecars that start sequentially [6][7]. Specifically, because sidecars (which have restartPolicy: Always) run for the entire lifetime of the Pod and exist alongside other containers, their resource usage is aggregated into the "init container usage" calculation for any init containers that start after them [6]. The formula for the effective Pod resource request is generally [6][8]: Max ( Max( each InitContainerUse), Sum(Sidecar Containers) + Sum(App Containers)) + Pod Overhead [1][6] Where InitContainerUse(i) accounts for the sidecar containers that have already started by that point [6]: InitContainerUse(i) = Sum(sidecar containers with index < i) + Max(Spec.InitContainers[i].Resources, Status.InitContainerStatuses[i].ResourcesAllocated) [6] In practice, this means: - Sidecar containers are treated as part of the "sum of all non-init containers" when compared against the peak requirements of the initialization phase [1][2][9]. - Unlike regular init containers, which only compete for resources during the startup phase, sidecar containers reserve their resources for the duration of the Pod's lifecycle [1][2]. - The Kubernetes scheduler and kubelet use this effective aggregate calculation to ensure sufficient resources are available on the node, preventing overcommitment [1][7][10]. For developers, this means that while sidecars provide a defined startup order and stable lifecycle, their resource requests and limits are additive to the total Pod footprint, just like application containers [2][9].

Citations:


Account for scheduler-effective GPU reservations.

GPUsRequestedOnNode() only sums pod.Spec.Containers, so pods with restartable init sidecars, overlapping regular init requests, or GPU PodOverhead can leave PickGPUNode thinking there is capacity when the Kubernetes scheduler has already reserved enough GPUs to keep another workload Pending. Use the scheduler’s effective pod GPU request: the greater of non-init request sum and init request peak, with sidecar and overhead rules applied.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/harness/harness.go` around lines 124 - 126, Update
GPUsRequestedOnNode to calculate each pod’s scheduler-effective GPU request
instead of summing only pod.Spec.Containers: use the greater of the non-init
container request sum and init-container peak, account for restartable init
sidecars and GPU PodOverhead according to Kubernetes scheduling rules, then add
that effective value to the node total used by PickGPUNode.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/integration/harness/harness.go`:
- Around line 124-126: Update GPUsRequestedOnNode to calculate each pod’s
scheduler-effective GPU request instead of summing only pod.Spec.Containers: use
the greater of the non-init container request sum and init-container peak,
account for restartable init sidecars and GPU PodOverhead according to
Kubernetes scheduling rules, then add that effective value to the node total
used by PickGPUNode.

In `@tests/integration/runner.yaml`:
- Around line 11-23: The snapshot-agent-test ClusterRoleBinding currently grants
the broad cluster-wide view role. Replace roleRef.name with a dedicated
least-privilege ClusterRole and define that role with only core pod get, list,
and watch permissions required by the agent’s Kubernetes watcher, preserving the
existing ServiceAccount subject and binding scope.
- Around line 88-101: Harden the runner container in the Pod manifest by adding
a securityContext with runAsNonRoot, allowPrivilegeEscalation disabled, all
Linux capabilities dropped, and seccompProfile set to RuntimeDefault. Update the
runner image or container user configuration so the golang:1.25-bookworm-based
container runs as a non-root user while preserving its existing test command.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9be23374-4257-4cec-bd03-dc262a0dab98

📥 Commits

Reviewing files that changed from the base of the PR and between c48a169 and 1328f9e.

📒 Files selected for processing (6)
  • .github/workflows/label-integration-test.yaml
  • tests/integration/README.md
  • tests/integration/harness/harness.go
  • tests/integration/run.sh
  • tests/integration/runner.yaml
  • tests/integration/snapshot-agent/harness.go
🛑 Comments failed to post (2)
tests/integration/runner.yaml (2)

11-23: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate runner.yaml =="
fd -a 'runner\.yaml$' . || true

echo "== runner.yaml contents =="
if [ -f tests/integration/runner.yaml ]; then
  cat -n tests/integration/runner.yaml
fi

echo "== git diff/stat for context =="
git diff --stat || true

echo "== search snapshot-agent and pods/resource usage in repo =="
rg -n "snapshot-agent|pod|namespace|ClusterRole|view|get|list|watch|verbs|resources" -S . \
  --glob '!**/.git/**' --glob '!node_modules/**' \
  | head -n 250

Repository: llm-d-incubation/llm-d-rl-time-slicing

Length of output: 31173


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== inspect agent pod template and k8s test harness =="
cat -n tests/integration/snapshot-agent/engines.go | sed -n '1,145p'
echo
cat -n pkg/snapshot-agent/utils/pod-utils.go | sed -n '70,160p'

echo "== check whether snapshot-agent-test is used by chart agent SA in manifests =="
fd -a 'templates' deploy tests 2>/dev/null || true
repo="$(pwd)"
for f in $(git ls-files deploy tests); do
  case "$f" in
    *.yaml|*.yml|*.tpl|*.yaml.gotmpl)
      if rg -n "snapshot-agent-test|SASelector|ServiceAccount|ClusterRoleBinding|ClusterRole|rbac.authorization.k8s.io" "$f" >/dev/null 2>&1; then
        echo "--- $f"
        rg -n -C 3 "snapshot-agent-test|SASelector|ServiceAccount|ClusterRoleBinding|ClusterRole|rbac.authorization.k8s.io" "$f"
      fi
      ;;
  esac
done

Repository: llm-d-incubation/llm-d-rl-time-slicing

Length of output: 14989


Replace the cluster-wide view binding with least-privilege RBAC.

snapshot-agent-test is the test harness ServiceAccount used by the agent pod, and its K8s code only reads/listes pods by node and labels. Binding Kubernetes’ built-in view role cluster-wide exposes many read-only resources in every namespace; use a dedicated ClusterRole with only core pods: get/list/watch or whichever resources are actually required.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/runner.yaml` around lines 11 - 23, The snapshot-agent-test
ClusterRoleBinding currently grants the broad cluster-wide view role. Replace
roleRef.name with a dedicated least-privilege ClusterRole and define that role
with only core pod get, list, and watch permissions required by the agent’s
Kubernetes watcher, preserving the existing ServiceAccount subject and binding
scope.

88-101: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Files:"
git ls-files | rg '(^|/)runner\.yaml$|(^|/)test-runner|snapshot-agent|integration|ServiceAccount' || true

echo
echo "runner.yaml surrounding lines:"
if [ -f tests/integration/runner.yaml ]; then
  nl -ba tests/integration/runner.yaml | sed -n '70,115p'
else
  fd -a 'runner\.yaml' . | sed -n '1,50p'
fi

echo
echo "Related YAML/security-relevant references (limited):"
rg -n "securityContext|runAsNonRoot|allowPrivilegeEscalation|capabilities|seccompProfile|snapshot-agent-test-runner|test-runner" tests .github -g '*.yaml' -g '*.yml' || true

echo
echo "Relevant manifests:"
for f in $(git ls-files | rg '\.ya?ml$'); do
  if rg -q "test-runner|snapshot-agent-test-runner|securityContext|runAsNonRoot|allowPrivilegeEscalation|capabilities|seccompProfile" "$f"; then
    echo "--- $f"
    nl -ba "$f" | sed -n '1,180p'
  fi
done

Repository: llm-d-incubation/llm-d-rl-time-slicing

Length of output: 2007


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "runner.yaml with line numbers:"
awk '{printf "%6d\t%s\n", NR, $0}' tests/integration/runner.yaml | sed -n '70,115p'

echo
echo "ServiceAccount/runner references:"
awk '{printf "%6d\t%s\n", NR, $0}' tests/integration/runner.yaml | sed -n '1,140p'
for f in deploy/snapshot-agent/templates/serviceaccount.yaml tests/integration/README.md tests/integration/run.sh; do
  if [ -f "$f" ]; then
    echo "--- $f"
    awk '{printf "%6d\t%s\n", NR, $0}' "$f" | sed -n '1,220p'
  fi
done

echo
echo "Relevant references in tracked YAML/Go files:"
python3 - <<'PY'
import subprocess, pathlib, re
files=subprocess.check_output(['git','ls-files'], text=True).splitlines()
patterns=['securityContext','runAsNonRoot','allowPrivilegeEscalation','capabilities','seccompProfile','snapshot-agent-test-runner','test-runner']
for f in files:
    p=pathlib.Path(f)
    if p.suffix not in {'.yaml','.yml','.go'} and f != 'tests/integration/runner.yaml':
        continue
    txt=p.read_text(errors='ignore')
    if any(re.search(patt, txt) for patt in patterns):
        print(f'--- {f}')
        lines=txt.splitlines()
        for i,l in enumerate(lines,1):
            if any(re.search(patt, l) for patt in patterns):
                lo=max(1,i-3); hi=min(len(lines),i+5)
                for j in range(lo,hi+1):
                    print(f'{j}: {lines[j-1]}')
                print()
PY

Repository: llm-d-incubation/llm-d-rl-time-slicing

Length of output: 27659


🌐 Web query:

golang:1.25-bookworm Docker image default USER non-root

💡 Result:

The golang:1.25-bookworm Docker image does not specify a USER instruction in its Dockerfile, which means it runs as the root user by default [1][2]. While the image runs as root, it is specifically designed to support non-root execution [3][4]. The default GOPATH directory (/go) is set to be world-writable (mode 777) [3][5]. This configuration is intended to provide flexibility, allowing users to run containers as a non-root user (e.g., using the --user flag) without encountering permission errors when Go attempts to write to the GOPATH [3][4]. If you require your application to run as a non-root user, you can: 1. Specify the --user flag when running the container (e.g., docker run --user 1000:1000...) [3][4]. 2. Create a custom Dockerfile that extends the official golang image and explicitly adds a USER instruction to switch to a non-root user [2].

Citations:


Run the test runner as a non-root, non-escalating container.

This pod runs go test against the copied workspace and deploys agent/engine pods itself, but it has no container security hardening. Add a securityContext with runAsNonRoot: true, allowPrivilegeEscalation: false, dropped capabilities, and seccompProfile: RuntimeDefault; use a non-root image/override since golang:1.25-bookworm defaults to root.

🧰 Tools
🪛 Checkov (3.3.8)

[medium] 88-101: Containers should not run with allowPrivilegeEscalation

(CKV_K8S_20)


[medium] 88-101: Minimize the admission of root containers

(CKV_K8S_23)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/runner.yaml` around lines 88 - 101, Harden the runner
container in the Pod manifest by adding a securityContext with runAsNonRoot,
allowPrivilegeEscalation disabled, all Linux capabilities dropped, and
seccompProfile set to RuntimeDefault. Update the runner image or container user
configuration so the golang:1.25-bookworm-based container runs as a non-root
user while preserving its existing test command.

Source: Linters/SAST tools

@aishukamal

Copy link
Copy Markdown
Collaborator Author

Integration suite rerun from the refactored layout on a dedicated H100 node: 9/9 pass.

PASS TestK8s (414.4s):        CUDAWatcherDiscoveredPIDs, VLLMSleepWake, SGLangReleaseResume
PASS TestStandalone (162.2s): CUDACheckpointRestore, VLLMSleepWake, VLLMCompound,
                              VLLMSuspendDiscard, SGLangReleaseResume, SGLangCompound
ok   tests/integration/snapshot-agent   576.7s

The k8s phase ran against the official chart (installed by run.sh); the standalone phase built from make standalone artifacts. The shared harness extraction introduced no behavioral change.

Comment thread tests/integration/README.md
Restructure tests/integration as the home of the integration suites,
with run.sh as the single entrypoint at the top:

  tests/integration/{README.md, run.sh, runner.yaml,
                     harness/, snapshot-agent/}

The new harness package holds the generic pieces extracted from the
snapshot-agent harness — in-cluster client setup, node selection
(TEST_NODE / free-GPU pick), pod lifecycle, exec (with stdin) / HTTP /
VRAM helpers, and attach-by-label for chart-deployed pods. The
snapshot-agent suite keeps its specifics (agent deployment, engine
specs, agentctl call path) and composes the shared Cluster.

Pure refactor: no behavior change.

Signed-off-by: Aishu Kamal <aishuk@google.com>
Signed-off-by: Aishu Kamal <aishuk@google.com>
Add a note to the README explaining that the standalone phase simulates
a bare-metal host via a privileged pod (hostPID + hostNetwork) on the
test node. Rename deployAgent to deployStandaloneAgent for clarity.

Signed-off-by: Aishu Kamal <aishuk@google.com>
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Time-Slicing Project Tracker Jul 30, 2026
@aishukamal
aishukamal merged commit aab76aa into main Jul 30, 2026
11 of 12 checks passed
@aishukamal
aishukamal deleted the tests/integ-framework branch July 30, 2026 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants