Skip to content

Appeng 4906/harbor eval task #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
GuyZivRH merged 7 commits into from
Apr 23, 2026
Merged

Appeng 4906/harbor eval task #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
941e78a
feat: add eval config generation script and tests
GuyZivRH Apr 15, 2026
88d2eca
feat: add harbor-eval Tekton task
GuyZivRH Apr 15, 2026
1d17b95
feat: add RBAC for trial Pod lifecycle
GuyZivRH Apr 15, 2026
45f18a3
docs: add harbor fork requirements and update implementation plan
GuyZivRH Apr 15, 2026
3cc9d94
fix: address review findings for harbor-eval task
GuyZivRH Apr 15, 2026
df20025
fix: unify fork URL and expand fork requirements doc
GuyZivRH Apr 15, 2026
04e81b0
docs: align harbor handoff doc with actual implementation
GuyZivRH Apr 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
187 changes: 187 additions & 0 deletions Docs/harbor_fork_requirements.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
# Harbor Fork — Integration Requirements for ABEvalFlow

> **Target repo:** [RHEcosystemAppEng/skills_eval_corrections](https://github.com/RHEcosystemAppEng/skills_eval_corrections)
> **Open PR:** [#1 — feat: add OpenShift environment backend](https://github.com/RHEcosystemAppEng/skills_eval_corrections/pull/1)
> **ABEvalFlow branch:** `APPENG-4906/harbor-eval-task`

---

## ABEvalFlow-Side Implementation (current state)

### How ABEvalFlow invokes Harbor

The `harbor-eval` Tekton task (`pipeline/tasks/harbor-eval.yaml`) runs a single
step that:

1. Installs Harbor from the fork via `pip install git+<fork-url>@<revision>`
2. Generates **two separate Harbor job configs** (one per variant) using
`scripts/generate_eval_config.py`
3. Runs `harbor run -c treatment-config.yaml` followed by
`harbor run -c control-config.yaml`
4. Parses `result.json` files from each variant's results directory to compute
pass rates

### Per-variant config structure

Each config is a standard Harbor `JobConfig` YAML with a **single task** and
the image ref set via global `environment.kwargs.image_ref`:

```yaml
# treatment-config.yaml
job_name: my-submission-treatment
jobs_dir: /workspace/eval-results/my-submission/treatment
n_attempts: 20
environment:
type: openshift
delete: true
kwargs:
image_ref: "registry/ns/my-submission@sha256:abc..."
override_cpus: 1
override_memory_mb: 2048
override_storage_mb: 10240
agents:
- {}
tasks:
- path: /workspace/tasks-treatment/my-submission
```

Control config is identical but with the control image ref and task path.

### Result directory layout

```
eval-results/<submission-name>/
treatment/
<job-name>/
<task-name>__<uuid>/result.json
<task-name>__<uuid>/result.json
... (N trials)
control/
<job-name>/
<task-name>__<uuid>/result.json
... (N trials)
```

### What ABEvalFlow reads from metadata.yaml

The config generator extracts these fields from `SubmissionMetadata`:

| Field | Maps to | Default |
|-------|---------|---------|
| `experiment.n_trials` | `n_attempts` | 20 |
| `agent_timeout_sec` | `agent_timeout_multiplier` (ratio vs 600s) | 1.0x |
| `verifier_timeout_sec` | `verifier_timeout_multiplier` (ratio vs 120s) | 1.0x |
| `agent_setup_timeout_sec` | `agent_setup_timeout_multiplier` (ratio vs 600s) | 1.0x |
| `build_timeout_sec` | `environment_build_timeout_multiplier` (ratio vs 600s) | 1.0x |
| `cpus` | `environment.override_cpus` | 1 |
| `memory_mb` | `environment.override_memory_mb` | 2048 |
| `storage_mb` | `environment.override_storage_mb` | 10240 |

### Eval modes

| Mode | `--ek image_ref` | Image source | When to use |
|------|------------------|--------------|-------------|
| `prebuilt` | Set to digest ref from build-push task | Tekton builds with Buildah, pushes to internal registry | Default pipeline flow |
| `local-build` | Not set; `force_build: true` | Harbor builds from `environment/Dockerfile` in each task dir | Local dev, or when skipping the build-push step |

### Tekton results emitted

| Result | Description |
|--------|-------------|
| `treatment-pass-rate` | Decimal string (e.g. `"0.8500"`) |
| `control-pass-rate` | Decimal string (e.g. `"0.6000"`) |
| `results-dir` | Absolute path to the results base directory |

---

## What the Harbor fork must support

### Required (blocking)

**1. `harbor run -c <config.yaml>` with OpenShift environment**

The fork's `OpenShiftEnvironment` must handle the full trial lifecycle when
invoked with `--env openshift` (or `environment.type: openshift` in config):

- Accept `image_ref` via `environment.kwargs` — verify the image is pullable,
skip building
- Create trial Pods with the pre-built image
- Execute agent + verifier inside the Pod via `exec`
- Upload/download files via tar-over-exec
- Write `result.json` with `verifier_result.reward` (float: 1.0 = pass, 0.0 = fail)
- Clean up Pods after each trial (`delete: true`)

**Status:** Implemented in PR #1. Needs merge.

**2. `environment.kwargs` passthrough in config-based invocation**

When `harbor run -c config.yaml` is used, the `environment.kwargs` dict from
the config must be passed to the environment's `__init__`. This is how
`image_ref` reaches `OpenShiftEnvironment`.

**Status:** Verify this works in `harbor jobs start` with YAML config
(vs CLI `--ek` flags). The CLI path (`--ek image_ref=...`) is tested;
the config path should behave identically but needs confirmation.

**3. `result.json` output format**

ABEvalFlow's pass-rate parser expects:

```json
{
"verifier_result": {
"reward": 1.0
}
}
```

Where `reward > 0.0` means pass. This is Harbor's standard format — no change
needed, but any deviation would break the parser.

### Implemented (fork PR #2)

**4. Per-task `environment_kwargs` support**

Implemented in [PR #2](https://github.com/RHEcosystemAppEng/skills_eval_corrections/pull/2).
`TaskConfig` now has an `environment_kwargs: dict[str, Any]` field. `Job._env_config_for_task`
merges per-task kwargs into the global `EnvironmentConfig.kwargs` (task-level overrides global).

ABEvalFlow currently runs each variant as a separate Harbor job, which works
without this feature. Per-task kwargs enables a single-job alternative for
sweep-based workflows:

```yaml
# Single-job example (supported since PR #2):
tasks:
- path: /workspace/tasks-treatment/my-submission
environment_kwargs:
image_ref: "registry/ns/my-submission@sha256:abc..."
- path: /workspace/tasks-control/my-submission
environment_kwargs:
image_ref: "registry/ns/my-submission@sha256:def..."
```

---

## Handoff Doc Alignment (WS3A)

The existing handoff doc (`Docs/harbor_openshift_backend.md`) has diverged from
the actual implementation in PR #1. These items should be updated:

| Section | Current (outdated) | Correct |
|---------|-------------------|---------|
| File path | `openshift_environment.py` | `openshift.py` |
| Build modes | `_build_and_push_image` is no-op only | Supports pre-built (`--ek image_ref=`) AND local podman build |
| Pod security | `readOnlyRootFilesystem: true` | Intentionally unset — many agent workloads need writes; `HOME=/tmp` is injected instead |
| RBAC table | ConfigMaps, Secrets, PVCs, ImageStreams | Only Pods + exec + Secrets used in practice |
| Naming | "skilled / unskilled" | "treatment / control" |
| Trial count | "20 skilled + 20 unskilled" | "20 treatment + 20 control" |
| Tekton params | `skilled-image-ref` / `unskilled-image-ref` | `treatment-image-ref` / `control-image-ref` |
| Definition of Done | "20 skilled + 20 unskilled" | "20 treatment + 20 control" |

### Additional Notes

- The OpenShift backend supports a `cpu_request` kwarg (`--ek cpu_request=<val>`)
for clusters with tight resource constraints — not documented in the handoff doc.
- The `--ek registry=<url>` kwarg enables local podman build+push to a specified
registry — also undocumented.
140 changes: 97 additions & 43 deletions Docs/harbor_openshift_backend.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,75 +1,114 @@
# Harbor OpenShift Backend — Handoff Document

> **Jira:** APPENG-4906 (Phase 4 — Harbor OpenShift Backend)
> **Target repo:** [GuyZivRH/skills_eval_corrections](https://github.com/GuyZivRH/skills_eval_corrections) (Harbor fork)
> **Full spec:** See [implementation_plan.md](./implementation_plan.md), Phase 4 (lines 270–343)
> **Target repo:** [RHEcosystemAppEng/skills_eval_corrections](https://github.com/RHEcosystemAppEng/skills_eval_corrections) (Harbor fork)
> **Fork PRs:** [#1 — OpenShift environment backend](https://github.com/RHEcosystemAppEng/skills_eval_corrections/pull/1), [#2 — per-task environment_kwargs](https://github.com/RHEcosystemAppEng/skills_eval_corrections/pull/2)
> **Full spec:** See [implementation_plan.md](./implementation_plan.md), Phase 4

---

## What to Build
## What Was Built

A new `OpenShiftEnvironment` class in the Harbor fork that enables `harbor run --env openshift`. This backend manages trial Pod lifecycle on OpenShift using pre-built container images (built by ABEvalFlow's Tekton pipeline).
An `OpenShiftEnvironment` class in the Harbor fork that enables
`harbor run --env openshift`. This backend manages trial Pod lifecycle
on OpenShift and supports two modes: pre-built container images (from
the Tekton pipeline) and local build via podman.

## Where in the Harbor Fork

| File | Action |
|---|---|
| `src/harbor/environments/openshift_environment.py` | Create — new backend |
| `src/harbor/models/environment_type.py` | Edit — add `OPENSHIFT = "openshift"` to enum |
| Backend registration (entry point or factory) | Edit — register so `--env openshift` works |
| `tests/` | Create — unit tests with mocked K8s API |
| `src/harbor/environments/openshift.py` | New backend |
| `src/harbor/environments/k8s_client_manager.py` | Shared K8s client management (independent instances per caller) |
| `src/harbor/models/environment_type.py` | `OPENSHIFT = "openshift"` added to enum |
| `src/harbor/models/trial/config.py` | `environment_kwargs` field added to `TaskConfig` (PR #2) |
| `src/harbor/job.py` | `_env_config_for_task` merges per-task kwargs into env config (PR #2) |
| `tests/unit/environments/test_openshift.py` | Unit tests with mocked K8s API |
| `tests/unit/models/test_trial_task_config.py` | Tests for per-task kwargs merge behavior |
| `examples/configs/openshift-*.yaml` | Config examples for both modes + per-task kwargs |

## Reference Implementation

Use the GKE backend as your template: `src/harbor/environments/gke.py` (~1044 lines). It implements the full `BaseEnvironment` interface from `src/harbor/environments/base.py`.
The GKE backend (`src/harbor/environments/gke.py`) implements the full
`BaseEnvironment` interface from `src/harbor/environments/base.py`.

## Key Differences from GKE

### `_init_client`
- GKE: `gcloud container clusters get-credentials` + `load_kube_config()`
- **OpenShift:** `load_incluster_config()` when running inside the cluster (Tekton), or `load_kube_config()` for local dev
- **OpenShift:** `load_incluster_config()` when running inside the cluster
(Tekton), falls back to `load_kube_config()` for local dev

### `_build_and_push_image`
- GKE: `gcloud builds submit` (Cloud Build)
- **OpenShift: No-op.** Image build/push is handled by Tekton (Phase 3). The backend receives a digest-based image reference and only verifies the image exists and is pullable.
- **OpenShift — two modes:**

**Contract:**
| Mode | Behavior | When to use |
|------|----------|-------------|
| **Prebuilt** (`image_ref` kwarg set) | Verify image is pullable, skip building | Default pipeline flow — Tekton builds with Buildah |
| **Local build** (`force_build: true`) | Build with podman, push to specified registry | Local dev, skipping the build-push Tekton step |

**Prebuilt contract:**
- **Input:** Immutable image ref (e.g., `image-registry.openshift-image-registry.svc:5000/ab-eval-flow/my-skill@sha256:abc...`)
- **Behavior:** Verify image exists using the trial ServiceAccount
- **Output:** Return the image ref unchanged
- **Failure:** Raise `ImageNotFoundError` / `ImageNotPullableError` with the image ref and SA identity
- **Failure:** Raise `ImageNotFoundError` / `ImageNotPullableError`

### `_image_exists`
- GKE: `gcloud artifacts docker images describe`
- **OpenShift:** Query internal registry API or use `skopeo inspect`
**Local build contract:**
- **Input:** `environment/Dockerfile` in the task directory, `registry` kwarg for push target
- **Behavior:** `podman build` + `podman push` to the specified registry
- **Kwargs:** `registry` (push target URL), `tls_verify` (default `"true"`)

### `start`, `stop`, `exec`, `upload_file/dir`, `download_file/dir`
- Same K8s API patterns as GKE — the `kubernetes` Python client is identical

## Pod Security Requirements
## Environment kwargs

Kwargs are passed via `environment.kwargs` in the job config YAML or
via `--ek key=value` on the CLI. Per-task kwargs (PR #2) override
global kwargs when set.

| Kwarg | Description | Default |
|-------|-------------|---------|
| `namespace` | OpenShift namespace for trial Pods | Required |
| `image_ref` | Digest-based image ref (prebuilt mode) | — |
| `registry` | Push target URL (local-build mode) | — |
| `cpu_request` | CPU request for trial Pods (e.g. `"250m"`) | — |
| `tls_verify` | TLS verification for registry (e.g. `"false"`) | `"true"` |

### Per-task environment_kwargs (PR #2)

Trial Pods must run with hardened security context:
`TaskConfig` now supports an `environment_kwargs` dict that merges into
the global `environment.kwargs` (task-level overrides global). This
enables treatment and control variants with different image refs in a
single Harbor job:

```yaml
securityContext:
runAsNonRoot: true
readOnlyRootFilesystem: true
allowPrivilegeEscalation: false
capabilities:
drop: ["ALL"]
seccompProfile:
type: RuntimeDefault
tasks:
- path: /workspace/tasks-treatment/my-submission
environment_kwargs:
image_ref: "registry/ns/my-submission@sha256:abc..."
- path: /workspace/tasks-control/my-submission
environment_kwargs:
image_ref: "registry/ns/my-submission@sha256:def..."
```

Since `readOnlyRootFilesystem: true` prevents writes, mount `emptyDir` volumes for:
- `/tmp`
- Agent cache directories (varies by agent)
ABEvalFlow currently runs each variant as a separate Harbor job (two
`harbor run` invocations). Per-task kwargs enables a single-job
alternative if needed in the future.

## Pod Security Requirements

Trial Pods run with OpenShift's default security constraints. The
`readOnlyRootFilesystem` is intentionally **not** set — many agent
workloads need filesystem writes. Instead, `HOME=/tmp` is injected to
direct writes to a writable location.

Verify the target cluster uses `restricted-v2` SCC (OpenShift 4.11+).

## Trial Execution

- **N = 20** attempts per variant (skilled + unskilled = 40 total sessions)
- **N = 20** attempts per variant (treatment + control = 40 total sessions)
- Image refs come as params from the build-push Tekton task (digest-based)
- LLM endpoint via environment variable — backend is agnostic to LLM access mode
- Configurable per-trial timeout and global evaluation timeout
Expand All @@ -82,37 +121,52 @@ The pipeline ServiceAccount in `ab-eval-flow` namespace needs:
| Resource | Verbs | Purpose |
|---|---|---|
| Pods | create, get, list, watch, delete | Trial Pod lifecycle |
| ConfigMaps | get, list | Trial configuration |
| Pods/exec | create | Agent/verifier execution inside trial Pods |
| Pods/log | get | Retrieving trial output |
| Secrets | get | LLM credentials injection |
| Events | get, list | Diagnosing hung/failed Pods |
| PVCs | get, list, create | Pipeline workspaces |
| ImageStreams | get, list | Registry access |

## K8s Client Management

The `BaseK8sClientManager` (shared by GKE and OpenShift backends) was
refactored in PR #2 to return **independent `CoreV1Api` instances** per
caller, each backed by its own `ApiClient`. This prevents concurrent
`kubernetes.stream.stream()` calls (which monkey-patch `ApiClient.call_api`)
from interfering with regular REST calls in other coroutines.

## Testing Strategy

- **Unit tests:** Mock K8s API with `pytest` + `unittest.mock`. No live cluster needed.
- **Integration tests:** Test against OpenShift developer sandbox (ROSA/OSD). Do **not** use Kind/Minikube — they won't catch SCC/Routes differences.
- **Integration tests:** Test against OpenShift developer sandbox (ROSA/OSD).
Do **not** use Kind/Minikube — they won't catch SCC/Routes differences.

## Tekton Task (in ABEvalFlow repo, not Harbor fork)
## Tekton Task (in ABEvalFlow repo)

A `pipeline/tasks/harbor-eval.yaml` Tekton Task will also be needed in ABEvalFlow to invoke Harbor. This task:
- Accepts `skilled-image-ref` and `unskilled-image-ref` as params
- Runs `harbor run --env openshift` with the image refs
- Collects results to workspace/PVC
The `harbor-eval` Tekton task (`pipeline/tasks/harbor-eval.yaml`) invokes
Harbor from the ABEvalFlow pipeline:

This can be built after the backend is functional.
- Installs Harbor from the fork via `pip install git+<fork-url>@<revision>`
- Generates two per-variant job configs using `scripts/generate_eval_config.py`
- Runs `harbor run -c treatment-config.yaml` then `harbor run -c control-config.yaml`
- Parses `result.json` files to compute pass rates as Tekton results
- Supports `prebuilt` and `local-build` eval modes via a pipeline param

## Definition of Done

- [ ] 40 trial Pods complete (20 skilled + 20 unskilled)
- [x] `OpenShiftEnvironment` backend implemented (PR #1)
- [x] Per-task `environment_kwargs` support (PR #2)
- [x] Independent K8s client instances per caller (PR #2)
- [x] Config examples for prebuilt, local-build, and per-task modes (PR #2)
- [x] Unit tests with mocked K8s API
- [ ] 40 trial Pods complete (20 treatment + 20 control) on live cluster
- [ ] Cleanup verified — no stale Pods after evaluation
- [ ] Retry behavior validated for transient failures
- [ ] Unit tests pass with mocked K8s API
- [ ] Integration test passes on OpenShift sandbox

## LLM Access Modes (for reference)

The backend doesn't need to know which mode is used — it just passes env vars to trial Pods:
The backend doesn't need to know which mode is used — it just passes
env vars to trial Pods:

| Mode | Env Var | Infrastructure |
|---|---|---|
Expand Down
Loading