Skip to content

feat: run classic Harbor on stock 0.20.0 + OpenShift custom env #67

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

Open
ikrispin wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: run classic Harbor on stock 0.20.0 + OpenShift custom env #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
140 changes: 140 additions & 0 deletions Docs/harbor_custom_env.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
# Harbor Custom Environment — Integration Contract for ABEvalFlow

> **Status:** Current (replaces the skills_eval_corrections fork runtime dependency)
> **Related:** [harbor_openshift_backend.md](./harbor_openshift_backend.md)

---

## Runtime contract

Classic Harbor A/B (`eval_engine: harbor`) and AEH Harbor runs share:

| Dependency | Pin / source |
|---|---|
| Harbor | `harbor==0.20.0` (PyPI; stock upstream) |
| Kubernetes client | `kubernetes>=32.0.0` |
| AEH K8s env | `agent_eval.harbor.kubernetes.KubernetesEnvironment` on `PYTHONPATH` (pinned AEH SHA in eval-base) |
| ABEvalFlow plugin | `abevalflow.harbor_extensions.openshift_environment:OpenShiftEnvironment` on `PYTHONPATH` |

**Custom env selection** (prebuilt / OpenShift CI only):

```text
abevalflow.harbor_extensions.openshift_environment:OpenShiftEnvironment
```

via JobConfig `environment.import_path` and/or:

```bash
harbor run -c <config.yaml> \
--environment-import-path abevalflow.harbor_extensions.openshift_environment:OpenShiftEnvironment \
-y
```

**Do not** use stock Harbor `environment.type: openshift` (oc CLI / custom SCC model) for shared OpenShift CI.

---

## How ABEvalFlow invokes Harbor

The composite evaluate task (`pipeline/tasks/phases/evaluate.yaml`) Harbor path:

1. Uses `eval-base:local-env` (stock Harbor + AEH + baked `abevalflow`)
2. Generates **two** Harbor job configs via `scripts/generate_eval_config.py`
3. Writes treatment/control digests into each task’s `task.toml` as `docker_image`
4. Runs `harbor run -c treatment-config.yaml` then `harbor run -c control-config.yaml` with `--environment-import-path`

### Per-variant config (prebuilt)

```yaml
job_name: my-submission-treatment
jobs_dir: /workspace/eval-results/my-submission/treatment
n_attempts: 20
environment:
import_path: abevalflow.harbor_extensions.openshift_environment:OpenShiftEnvironment
delete: true
override_memory_mb: 2048
override_storage_mb: 10240
kwargs:
cpu_request: "100m"
memory_limit_multiplier: 1.5
agents:
- name: claude-code
model_name: claude-sonnet
tasks:
- path: /workspace/tasks-treatment/my-submission
```

Corresponding `task.toml` fragment:

```toml
[environment]
docker_image = "registry/ns/my-submission@sha256:abc..."
cpus = 1
memory_mb = 2048
```

### Local / dev (`eval_mode=local-build`)

```yaml
environment:
type: docker
force_build: true
delete: true
```

No OpenShift import path; Harbor builds from each task’s Dockerfile.

### Result directory layout

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

Analyze/publish remain compatible as long as this `jobs/` shape holds.

---

## Cluster RBAC (unchanged)

Pipeline ServiceAccount needs: create/get/delete pods, exec, pull secrets.
**No** `harbor-task-scc` / root-capable custom SCC.

Trial pods target OpenShift `restricted-v2` SCC. The custom env adds emptyDir mounts for `/workspace` and `/tmp` and ensures Harbor EnvironmentPaths exist before verifier redirect.

---

## Eval-base image

Built from [`templates/Dockerfile.base`](../templates/Dockerfile.base):

```bash
./scripts/build_base_image.sh --tag latest
./scripts/build_base_image.sh --tag local-env
```

Pins: `HARBOR_VERSION=0.20.0`, `AEH_SHA` (default AEH v1.20.0).

---

## Historical note

The `skills_eval_corrections` Harbor fork (`environment.type: openshift` + `kwargs.image_ref`) is **no longer a required runtime dependency**. See [harbor_openshift_backend.md](./harbor_openshift_backend.md) for the historical fork handoff.

---

## Restricted-v2 smoke checklist

After rebuilding `eval-base:latest` and `eval-base:local-env`:

1. Trigger one Harbor submission (treatment+control), small `n_trials`, restricted-v2 namespace.
2. Confirm evaluate logs: no `git+…/skills_eval_corrections` install; `harbor==0.20.0` / import path present.
3. Confirm trial pods start and finish; `result.json` under `eval-results/.../treatment|control`.
4. Confirm analyze/publish still consume the result layout.
5. Optional: compare mean-reward shape vs a prior fork-path baseline run.
191 changes: 7 additions & 184 deletions Docs/harbor_fork_requirements.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,187 +1,10 @@
# Harbor Fork — Integration Requirements for ABEvalFlow
# Harbor Fork Requirements — Redirect

> **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`
> **Superseded.** Classic Harbor A/B no longer installs
> `skills_eval_corrections` at runtime.

---
See **[harbor_custom_env.md](./harbor_custom_env.md)** for the current contract
(stock Harbor + OpenShift custom env via import path + `docker_image`).

## 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.
Historical fork OpenShift backend notes:
**[harbor_openshift_backend.md](./harbor_openshift_backend.md)**.
Loading
Loading