Appeng 4906/harbor eval task#8
Conversation
Python script that reads metadata.yaml and generates a Harbor job config YAML for A/B evaluation. Supports two modes: prebuilt (digest image refs from build-push) and local-build (Harbor builds from Dockerfiles). Extracts n_trials, timeout multipliers, and resource overrides from the submission's ExperimentConfig and SubmissionMetadata schemas.
Three-step Tekton task that installs Harbor from the fork, generates a job config from submission metadata, and runs treatment + control trials. Parses Harbor result.json files to emit pass rates as Tekton results.
Add pipeline-trial-manager Role and RoleBinding granting the pipeline ServiceAccount permissions for trial Pod lifecycle management: pods, pods/exec, pods/log (create/get/list/watch/delete), events (get/list), and secrets (get for LLM credentials).
Document the per-task environment_kwargs change needed in the Harbor fork's TaskConfig for per-variant image refs in a single sweep config. Also document WS3A doc alignment items (naming, RBAC, security context). Update Phase 4 checkboxes: mark fork integration (PR #1), Tekton task, eval config script, RBAC, and dual eval mode support as done.
- Collapse three Tekton steps into one to fix step isolation (harbor binary and config file were not shared across containers) - Split evaluation into separate Harbor jobs per variant with dedicated results directories, eliminating fragile trial classification heuristic - Add validation guard in build_variant_config for prebuilt mode - Remove dead compute_pass_rate function from inline parser - Pin ubi9/python-311 image to :9.6 - Split RBAC verbs per subresource (pods/exec -> create, pods/log -> get) - Remove --quiet flag to preserve diagnostic output - Add comments for n_concurrent_trials and agents defaults - Fix fork URL to GuyZivRH/skills_eval_corrections - Downgrade per-task environment_kwargs to nice-to-have in docs
- Use VARIANTS constant in generate_eval_configs loop - Unify Harbor fork URL to RHEcosystemAppEng/skills_eval_corrections across all docs (README, implementation_plan, harbor_openshift_backend, harbor_fork_requirements) - Expand harbor_fork_requirements.md with ABEvalFlow-side implementation details: per-variant config structure, result directory layout, metadata field mapping, eval modes, and Tekton results emitted - Document three blocking fork requirements (OpenShift env, config-based kwargs passthrough, result.json format) with current status
RoddieKieley
left a comment
RoddieKieley
left a comment
There was a problem hiding this comment.
I have not as of yet built and utilized the project locally, only cloned and analysed, and thus feedback is non gating and high level.
Some inconsistency was noted between the branch and main in relation to what the docs state and what the code does, but without auto updated docs that's not unusual. For example the harbor_openshift_backend.md file only discusses pre-built container images although elsewhere it discusses local dev but not locally built container images. Looking at the workstreams_roadmap.md however it appears that the doc work is set for the following workstream item which means this would be covered by plan in the future.
In the same harbor_openshift_backend.md I see that the SCC requirements are called out such that trial pods must run with hardened security context which is good, as OpenShift is more specific than "vanilla" kubernetes in that regard.
|
Thanks for the review Roddie — both observations are spot on. Doc inconsistency ( You're right that the handoff doc only describes pre-built mode. The actual implementation (in fork PR #1) supports both pre-built and local-build (podman) modes. Additionally, fork PR #2 has since added per-task I've updated
Also updated WS3A doc work planned: As you noted from SCC requirements: Thanks for calling this out positively. The |
Update harbor_openshift_backend.md to match the current state of the Harbor fork (PRs #1 and #2): - Document both eval modes (prebuilt + local-build via podman) - Fix file path: openshift.py, not openshift_environment.py - Correct Pod security: readOnlyRootFilesystem intentionally unset, HOME=/tmp injected instead - Narrow RBAC table to actual usage (Pods, exec, log, Secrets, Events) - Update naming to treatment/control throughout - Add environment kwargs table (namespace, image_ref, registry, cpu_request, tls_verify) - Document per-task environment_kwargs (fork PR #2) - Document K8s client manager refactor for concurrent safety (fork PR #2) - Update Definition of Done with completed items Update harbor_fork_requirements.md to reflect per-task environment_kwargs as implemented (no longer nice-to-have).
2 participants
Summary
Changes
Test plan
Related