Repository navigation
test: OSAC-3153 ui-design.md — should NOT trigger review - #45
Open
ItzikEzra-rh wants to merge 225 commits into
Open
ItzikEzra-rh wants to merge 225 commits into
ItzikEzra-rh wants to merge 225 commits into
Conversation
Replace user stories to match the defined OSAC personas (Cloud Provider Admin, Tenant Admin, Tenant User). The previous stories focused on implementation details (pre-define fields, prevent overrides) rather than the persona-driven workflows (create, publish, browse, provision). Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Elad Tabak <etabak@redhat.com>
* OSAC-46: Add VM instance types enhancement proposal
- Phase 1 MVP: Provider-controlled global instance types
- Strict mode: instance_type field required, cores/memory_gib removed
- Enables/disables control visibility to org users
- VMaaS only (ComputeInstance), CaaS uses HostType for bare metal
- GPU support deferred to Phase 2
* Replace boolean enabled field with InstanceTypeState enum
Changes based on PR discussion with mhrivnak and ygalblum:
State management:
- Replace `enabled` boolean with `InstanceTypeState` enum (ACTIVE, DEPRECATED, OBSOLETE)
- Follow GCP deprecation model for lifecycle management
- ACTIVE: Fully available for new VM creation
- DEPRECATED: Available with warnings, includes optional replacement field
- OBSOLETE: Cannot create new VMs (409 Conflict), still visible via GetInstanceType
API behavior:
- ListInstanceTypes: Returns ACTIVE and DEPRECATED by default, supports filter for OBSOLETE
- GetInstanceType: Returns any state (for viewing details of existing VMs)
- CreateComputeInstance: ACTIVE succeeds, DEPRECATED succeeds with warning, OBSOLETE rejected with 409
Summary updates:
- Replace long "In the initial implementation..." with "All instance types are global"
- Highlight that instance types exist only at gRPC API layer, not CRD layer
Other changes:
- Use osac-admin CLI for instance type management operations
- Simplify Proposal section (just mention deprecation mechanism)
- Remove parenthetical visibility qualifications
- Add filter parameter support to ListInstanceTypes
- Update all examples, validation rules, and test cases
Addresses all review feedback from mhrivnak and ygalblum.
Signed-off-by: Avishay Traeger <atraeger@redhat.com>
Assisted-by: Claude Code <noreply@anthropic.com>
* clarify name as primary identifier and uniqueness model
Address review feedback from ygalblum about ID vs name usage:
- Standardize on 'name' as the primary identifier for InstanceTypes
- Clarify that instance type names are globally unique (Phase 1)
- Update database schema to include UNIQUE constraint on name column
- Update proto definitions to use 'name' field instead of 'id'
- Update API paths to use {name} instead of {id}
- Add new section explaining naming and uniqueness guarantees
- Document future Phase 2 approach for tenant-specific types (prefixed names)
- Update all validation rules to reference "name" consistently
- Clarify that id field is internal database primary key (UUID)
The name field serves as both the user-visible identifier and the
reference used in ComputeInstance.spec.instance_type (e.g., "standard-4-16").
For future tenant-specific types, the proposal recommends a prefixed
naming convention (e.g., "org-name/type-name") to maintain global
uniqueness while supporting organizational customization.
Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>
* docs(instance-types): fix troubleshooting terminology to use OBSOLETE and 409
Update troubleshooting section to use consistent lifecycle terminology:
- Change "instance type disabled" to "instance type is obsolete"
- Update HTTP status from 400 Bad Request to 409 Conflict
- Use OBSOLETE/DEPRECATED/ACTIVE terminology consistently
- Update resolution guidance to reference state transitions
- Clarify deletion protection references instance type by name
Addresses CodeRabbit review feedback.
Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>
* docs(instance-types): add GCP-style deprecation metadata with timestamps
Add comprehensive deprecation metadata following GCP's machine type
deprecation model to support transparent migration timelines:
Proto Changes:
- Add InstanceTypeDeprecation message with state, replacement, deprecated,
and obsolete timestamps
- Replace standalone replacement field with deprecation sub-message
- Auto-populate timestamps on state transitions
API Changes:
- Support --obsolete-at flag when deprecating instance types
- Include deprecation metadata in List/Get responses
- Warning messages include obsolete date when creating VMs with DEPRECATED types
- Validate obsolete timestamp is in the future when transitioning to DEPRECATED
Database Changes:
- Store full deprecation metadata in JSONB data field
- Add examples for both ACTIVE and DEPRECATED instance types
Workflow Changes:
- Admin can specify future obsolete date when deprecating
- Users see migration timeline in warnings and list responses
- Deprecation timestamps provide audit trail for lifecycle changes
Testing:
- Validate deprecation metadata in unit and integration tests
- Test warning messages include obsolete dates
- Verify auto-population of timestamps on state transitions
Non-Goals (Phase 1):
- Automatic state transitions based on timestamps (scheduled job deferred)
Reference: https://cloud.google.com/compute/docs/reference/rest/v1/machineTypes
Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>
* docs(instance-types): clarify name uniqueness semantics (never reusable)
Fix contradictory soft-delete uniqueness policy in database schema:
- Remove redundant partial unique index
- Clarify that PRIMARY KEY on name prevents all duplicates
- Document that instance type names are never reusable, even after soft deletion
The PRIMARY KEY constraint already enforces global uniqueness across all rows,
making the conditional unique index redundant and confusing.
Addresses CodeRabbit review feedback.
Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>
* docs(instance-types): clarify deprecation timestamp validation rules
Address ygalblum's question about timestamp enforcement:
Deprecation Timestamp Validation:
- deprecated timestamp: Can be past or future (records when deprecation was/will be announced)
- obsolete timestamp: Can be past or future for OBSOLETE state, must be future for DEPRECATED state
- API behavior: Based on state field, not timestamps (future timestamps don't affect current behavior)
Auto-population:
- deprecated: Set to current time when transitioning to DEPRECATED (if not provided)
- obsolete: Set to current time when transitioning to OBSOLETE (if not provided)
Key Design Decision:
State transitions are manual (admin-initiated), not automatic based on timestamps.
Timestamps are for communication and planning, not enforcement.
This allows admins to:
- Backdate deprecated timestamp when recording historical decisions
- Set future obsolete dates for migration planning
- Manually control all state transitions
Addresses review comment on line 288.
Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>
* docs(instance-types): clarify manual state transitions in Phase 1
Fix workflow contradiction - remove misleading "(Future)" note that suggested
automatic transitions might happen:
- Change workflow step 6 from "(Future) Scheduled job automatically transitions"
to explicit note that Phase 1 requires manual transitions
- Keep Non-Goals bullet consistent: automatic transitions are out of scope
- Clarify that Cloud Provider Admin must manually transition to OBSOLETE
Phase 1 Behavior:
- Timestamps communicate migration timeline to users
- Admins manually trigger DEPRECATED → OBSOLETE transitions
- No scheduled jobs or automatic state changes
Future Phase:
- Scheduled job could auto-transition based on obsolete timestamp
- Remains explicitly out of scope for Phase 1
Addresses CodeRabbit review feedback about contradictory statements.
Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>
* docs(instance-types): add warnings field to CreateComputeInstanceResponse
Address CodeRabbit review feedback - specify API contract for DEPRECATED warnings:
Proto Changes:
- Add CreateComputeInstanceResponse message with warnings field
- warnings: repeated string field containing deprecation messages
- Example: "Instance type 'standard-2-4' is deprecated and will become obsolete on 2026-12-31. Consider migrating to 'standard-2-8'."
Updated Documentation:
- Workflow: Show CreateComputeInstanceResponse with warnings array
- Validation: Reference CreateComputeInstanceResponse.warnings field explicitly
- API behavior: Specify DEPRECATED returns warning in response.warnings field
Warning Format:
- Empty array for ACTIVE instance types
- Contains deprecation message with replacement and obsolete timestamp for DEPRECATED types
This provides a clear API contract for returning warnings consistently across
all CreateComputeInstance operations.
Addresses review feedback on lines 202-203.
Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>
* docs(instance-types): update last-updated date to 2026-05-17
Signed-off-by: Avishay Traeger <atraeger@redhat.com>
* docs(instance-types): address reviewer feedback
- Add update_timestamp field for auditing (ygalblum)
- Clarify soft-delete pattern used by DeleteInstanceType (ygalblum)
- Change deletion error to not leak VM details, just indicate in-use (ygalblum, mhrivnak)
- Add Private API List/Get operations for completeness (ygalblum)
- Document labels/annotations reserved for future extensibility (ygalblum)
- Explain JSONB usage follows OSAC standard pattern (ygalblum)
- Add note about bidirectional state transitions (ygalblum)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>
* docs(instance-types): document bidirectional state transitions
Update lifecycle descriptions to show ACTIVE ↔ DEPRECATED ↔ OBSOLETE
with bidirectional arrows and add Reactivating workflow section.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>
---------
Signed-off-by: Avishay Traeger <atraeger@redhat.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
## What Add a ComputeImage API resource for managing VM images as first-class entities in OSAC. ## Why Today, ComputeInstance creation accepts arbitrary image URLs with no access control, discoverability, or governance. Every major cloud provider treats VM images as managed resources — OSAC needs the same to be a credible VM-as-a-Service platform. ## Changes - Introduce ComputeImage resource with CRUD API for registering, listing, and managing VM image metadata - Support two-tier visibility: provider-global images (all tenants) and tenant-scoped images - Define image lifecycle states: AVAILABLE, DEPRECATED (with warnings), and OBSOLETE (blocks new VMs) - Update ComputeInstance to reference a registered ComputeImage by ID instead of accepting arbitrary URLs - Include deletion protection preventing removal of images referenced by active ComputeInstances - Define authorization model for provider admins, tenant admins, and tenant users - Specify database schema, list filtering, deprecation metadata, and migration strategy ## Testing Comprehensive test plan covering unit tests for CRUD and validation, integration tests for multi-tenant visibility and lifecycle transitions, and end-to-end tests for the full image registration through VM provisioning workflow. Assisted-by: Claude Code <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ygal Blum <ygal.blum@gmail.com>
This update provides a more detailed architecture that more closely matches k8s patterns.
* OSAC-1118: Add BareMetal Instance API enhancement proposal Signed-off-by: Adrien Gentil <agentil@redhat.com> * Address review: refine user stories and workflow descriptions Signed-off-by: Adrien Gentil <agentil@redhat.com> * Address CodeRabbit review comments Signed-off-by: Adrien Gentil <agentil@redhat.com> * Address review: immutable ssh_key/user_data, quota as non-goal Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1118: update initial backend from OpenStack to BCM Scope change: BCM (NVIDIA Base Command Manager) is the first supported backend, replacing OpenStack/MOC references throughout the EP. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1118: keep API backend-agnostic, BCM only in admin/impl context Public API (proto field comments, tenant user stories, non-goals) must not leak BCM as the backend. BCM is referenced only where relevant: Cloud Infrastructure Admin user story, workflow actors, risks, support procedures, and infrastructure section. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1118: simplify proto field comments in BaremetalInstanceTemplateSpecDefaults Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1118: address CodeRabbit review comments - Non-goals: document interim workarounds for deferred networking (user_data/cloud-init) and custom profile deferral - Pluggable provider: add HostLease → BaremetalInstanceStatus mapping table - Test plan: remove quota enforcement from unit tests (quota is non-goal) - Credential risk: defer to fulfillment-service cross-cutting policy instead of prescribing storage mechanisms Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1118: defer credential management to a dedicated security enhancement Credential storage is undefined and not owned by fulfillment-service. Only constraint stated: no credentials in CRDs or API responses. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1118: remove HostLease→BaremetalInstance mapping table (HostLease WIP) Mapping will be defined once the HostLease CRD schema is finalized. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1118: fix networking non-goal — network config owned by admin, not tenant Tenants have no mechanism to configure networking; it is defined by the Cloud Provider Admin in the BaremetalInstanceTemplate. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1118: remove Pluggable Provider Architecture section Pluggable backend interface definition is owned by OSAC-1032. This EP covers BCM integration against that interface, not the interface itself. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1118: clarify networking non-goal is phase-scoped, not permanent Network config via VirtualNetwork/Subnet/SecurityGroup is deferred to a dedicated networking enhancement, not removed from scope entirely. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1118: close open question on BaremetalInstance vs HostLease coupling HostLease remains distinct from BaremetalInstance because it serves cases outside fulfillment-service scope (e.g. cluster BM nodes). Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1118: empty BaremetalInstanceTemplateSpecDefaults, no fields yet spec_defaults follows template convention but no tenant-overridable fields are defined in this initial version. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1118: clarify BaremetalPool+HostLease may both be used initially Making pool optional is not a prerequisite; both CRs may exist in the initial implementation as internal details of the BMF component. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1118: rearchitect provisioning flow — FS creates HostLease directly - fulfillment-service creates HostLease CR directly (no BareMetalPool, no osac-operator CR creation step) - baremetal-fulfillment-operator owns inventory assignment and AAP provisioning trigger - osac-operator watches HostLease for status and signals back via Signal RPC - Add provisioning sequence diagram - Update actors, workflow steps, drawbacks, alternatives, version skew, and support procedures to match Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1118: address carbonin review comments - Remove pluggable provider abstraction from Goals (implementation concern of baremetal fulfillment component, not API scope) - Remove "no intermediate CRD" callout per carbonin's suggestion - Fix step numbering gap in provisioning sequence (9→8) - Note that delete+recreate retry may assign a different host - Remove ip_address from BaremetalInstanceStatus and mermaid diagram; deferred until networking integration is scoped - Simplify Drawbacks: drop consolidation language (BaremetalInstance and HostLease would never be the same object) - Simplify Alternatives: same reasoning, remove "future consolidation breaking change" framing Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1118: clarify run_strategy field comment Explain that run_strategy controls power state and note why an enum is used over a boolean, per tzumainn's review feedback. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1118: describe BMF/osac-operator division of responsibility Per carbonin's feedback, add a note near the non-goals clarifying the intended pattern: baremetal-fulfillment-operator handles host assignment and provisioning; osac-operator handles status feedback via Signal RPC. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1118: fix non-goals formatting, inline BMF/operator note Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1118: remove osac-operator non-goal bullet and inline note Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1118: clear drawbacks section Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1118: remove HostLease-as-API alternative Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> --------- Signed-off-by: Adrien Gentil <agentil@redhat.com>
Introduce a Snapshot API for OSAC ComputeInstance resources, enabling tenants to create, list, get, delete, and restore from point-in-time snapshots of their VMs through the fulfillment-service API. Key design decisions: - New Snapshot resource (proto + CRD + dual controller) - Both online and offline snapshot modes via KubeVirt - In-place restore via declarative signal on ComputeInstance - Cascade deletion when parent ComputeInstance is deleted Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Ygal Blum <ygal.blum@gmail.com>
* OSAC-1317: rename BaremetalInstance → BareMetalInstance (case fix) BaremetalInstance → BareMetalInstance BaremetalInstanceTemplate → BareMetalInstanceTemplate BAREMETAL_INSTANCE_* → BARE_METAL_INSTANCE_* (enum values) Mechanical rename only; no API or semantic changes. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1317: adopt BareMetalInstanceCatalogItem API pattern Replace BareMetalInstanceTemplate with BareMetalInstanceCatalogItem as the tenant-facing resource, following the ComputeInstanceCatalogItem pattern. - BareMetalInstanceCatalogItem gains: template (ref to private resource), published, tenant, field_definitions fields - BareMetalInstanceSpec.catalog_item replaces spec.template - Private admin CRUD service added (BareMetalInstanceCatalogItems) - Public API restricted to List/Get on published catalog items - Private REST routes added under /api/private/v1/ - Workflow, proto definitions, mermaid diagram, test plan, non-goals, support procedures, and alignment section updated throughout - see-also: /enhancements/catalog-items added to frontmatter Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1317: add log hygiene note to Support Procedures Address CodeRabbit No-Sensitive-Data-In-Logs check: operators must redact ssh_key, user_data, and backend credentials before inspecting or sharing logs. Treat any such exposure as a security incident. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1317: remove stale non-goal about tenant scoping The tenant field is fully defined in the BareMetalInstanceCatalogItem proto (empty = global, set = tenant-scoped) and relied upon in the proposal, test plan, and support procedures. The non-goal claiming it was deferred contradicted the rest of the document. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1317: add BareMetalInstanceTemplate proto and private service Both BareMetalInstanceTemplate and BareMetalInstanceCatalogItem are now fully specified: - Add BareMetalInstanceTemplate proto (id, metadata, title, description, spec with host_type and os_image) - Add BareMetalInstanceTemplates private gRPC service (full CRUD) - Add private REST routes for baremetal_instance_templates Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1317: fix summary and consolidate non-goals - Correct template/catalog-item relationship: each catalog item is backed by a BareMetalInstanceTemplate (not the reverse) - Clarify that templates are created via osac-aap; catalog items are managed through the OSAC private API - Consolidate four "covered in companion work" non-goals into one line Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1317: align BareMetalInstance API with ComputeInstance pattern - Add Tenant Admin user story for tenant-scoped catalog items - Make BareMetalInstanceTemplate and BareMetalInstanceCatalogItem fully public (full CRUD), consistent with ComputeInstanceTemplate and ComputeInstanceCatalogItem - Public BareMetalInstanceCatalogItem omits tenant field (server-managed) - Show public/private proto variants for BareMetalInstanceCatalogItem - Add explicit consistency goal instead of scattered pattern references - Update API Extensions, REST routes, actors, and test plan accordingly Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1317: show catalog item resolution in provisioning sequence The fulfillment service resolves catalog_item to templateID and derives templateParameters from field_definitions internally before creating the HostLease CR. Update sequence diagram and provisioning step 4 to reflect this — HostLease carries templateID + templateParameters, not catalogItemID. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1317: fix mermaid syntax in provisioning sequence diagram Replace <br/> with a single-line note — GitHub's mermaid renderer does not support HTML self-closing tags in Note blocks. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> * OSAC-1317: restrict public BareMetalInstanceTemplates to List/Get only Templates are managed through the private API by Cloud Provider Admins; tenants can discover available templates via public List/Get only. This matches the ComputeInstanceTemplates authconfig pattern. Update Goals, Proposal, Actors, Provisioning step 1, API Extensions, REST routes, and Alignment section accordingly. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> --------- Signed-off-by: Adrien Gentil <agentil@redhat.com>
Introduces the split PRD + design doc format for the TenantStorage CRD and OSAC Storage Controller extraction from the Tenant controller. Includes the AAP playbook split (OSAC-1145) as part of the delivery scope. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Zoltan Szabo <zszabo@redhat.com>
PRD should be created, reviewed, and merged before the design doc. Design will come in a follow-up PR. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Zoltan Szabo <zszabo@redhat.com>
- Storage Controller watches Tenant CRs and creates TenantStorage (not Tenant controller) - Controller-driven deletion (no owner reference GC) - ClusterOrder watch included in v0.1 scope - Tenant CRD cleanup (remove storage fields) in scope - Phase 1/Phase 2 terminology defined explicitly - Remove internal workflow markers ([Clarify: D1] etc.) Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Zoltan Szabo <zszabo@redhat.com>
Storage Controller sets StorageReady condition on the Tenant CR (and future ClusterOrder CR) so storage readiness is visible through the primary resources users interact with. The Tenant controller still runs no storage logic. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Zoltan Szabo <zszabo@redhat.com>
Reorganize requirements into Controller/API Foundation and Storage Onboarding Workflow sections to make the dependency explicit: storage must be extracted from the Tenant controller before onboarding workflows can run independently. Introduce a two-stage model for tenant storage provisioning. Stage 1 (backend setup) creates tenant credentials and sets StorageBackendReady on the Tenant. Stage 2 (cluster-side setup) discovers StorageClasses on target clusters. For VMaaS, both stages run at tenant onboarding. For CaaS, Stage 2 runs after ClusterOrder reaches Ready. Add per-cluster entries array to TenantStorage status so each cluster tracks its own storage readiness and delivery model. Add migration risk for existing tenants and open questions for delivery model detection and migration procedure. Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> Assisted-by: Cursor/Claude Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Remove verbose phrasing, clarify the storage controller scope (clusters, compute instances, PVCs), and tighten wording throughout for readability. Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> Assisted-by: Cursor/Claude Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
The OSAC Storage Controller manages storage conditions and status fields directly on the Tenant CR instead of introducing a separate TenantStorage CRD. This follows the condition ownership pattern where multiple controllers manage distinct conditions on a shared resource. Key changes from the previous PRD revision: - StorageBackendReady and StorageClassReady conditions on Tenant CR - status.storageClasses and status.jobs ownership moves to storage controller - ComputeInstance controller unchanged (reads from Tenant as before) - Finalizer on Tenant CR for deletion ordering - AAP playbooks renamed: osac-create-tenant-storage-backend, osac-create-tenant-storage-class, osac-delete-tenant-storage-class, osac-delete-tenant-storage-backend - StorageClasses created by AAP (not manually), controller discovers them via osac.openshift.io/tenant label - VMaaS target cluster is remote or hub (statically configured) - Removed Risk 7.2 (migration): pre-GA, no existing tenants - Resolved all open questions - Added NFR for credential-safe logging - Added E2E testing acceptance criterion Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> Assisted-by: Cursor/Claude Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Clarify that status.storageClasses uses the same tier resolution algorithm (tenant-specific with Default fallback) previously owned by the Tenant controller. Tighten FR-5 to require the hub Secret exists and is valid before marking StorageBackendReady=True. Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> Assisted-by: Cursor/Claude Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
- Remove Risk 7.1 (pre-GA, breaking changes are expected) - Move API field names from FR-1 to design doc scope - Fix NFR-1: admin credentials are in a pre-configured Secret, not "never persisted to Kubernetes" Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Zoltan Szabo <zszabo@redhat.com>
FR-1 now requires per-backend detail (name, provider, status, error) and per-cluster detail (cluster name, readiness, reason) in the Tenant status. Clarifies that status.jobs is a shared field used by multiple controllers, not owned by the storage controller. For this PRD, cluster storage readiness covers the VMaaS target cluster only. CaaS clusters are covered by a separate PRD. Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> Assisted-by: Cursor/Claude Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Rename osac-create-tenant-storage-class to osac-create-tenant-cluster-storage and osac-delete-tenant-storage-class to osac-delete-tenant-cluster-storage. "cluster-storage" better describes the scope (CSI operator, CSI Secrets, StorageClasses) than "storage-class" which only captures one artifact. Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> Assisted-by: Cursor/Claude Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Add "Tier addition and removal workflows" to non-goals per Avishay's feedback. Scope the management-state Unmanaged check to steady-state reconciliation only so that finalizer and deletion handling still run, preventing Tenants from being stuck in Terminating. Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> Assisted-by: Cursor/Claude Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
* OSAC-1111: Add StorageBackend enhancement proposal Signed-off-by: Roy Golan <rgolan@redhat.com> Assisted-by: Claude Code <noreply@anthropic.com> * OSAC-1111: Address EP review findings - Add README.md index with YAML frontmatter and stub section headers - Clarify PENDING→READY state transition: manual via Signal RPC (Phase 1), not READY-on-create (NetworkClass) or full reconciler (PublicIPPool) - Explain Signal RPC divergence from NetworkClass (status payload needed because fulfillment-service has no direct knowledge of array state) - Fix description field: mark as `optional string` in proto appendix - Document soft-delete name reuse as intentional (tiers reference by ID) - Fix copyright year: 2025 → 2026 - Resolve Open Question 1: hub = management cluster in OSAC HCP model - Standardize "management cluster" terminology throughout design.md - Fix tracking-link to issues.redhat.com Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com> * remove design.md, it will come later Signed-off-by: Roy Golan <rgolan@redhat.com> * OSAC-1111: Rework PRD to structured requirement format Restructure the StorageBackend PRD from narrative format to the numbered section template with FR-N/NFR-N requirement IDs, acceptance criteria checkboxes, and structured risks/assumptions/dependencies sections. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com> * OSAC-1111: Remove implementation details from PRD Reframe requirements in terms of product capabilities rather than implementation choices (PostgreSQL, FieldMask, buf lint, SQL indexes). Addresses review feedback to keep the PRD scoped to product requirements. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com> * OSAC-1111: Address review feedback round 2 - Remove Risk 7.1 (proto appendix divergence) — coding concern, not product requirement - Clarify CSI drivers and StorageClasses install on target clusters (VMaaS/CaaS), not management cluster - Renumber remaining risks Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com> * OSAC-1111: Clarify FR-4 and FR-8 per review feedback - FR-4: Note that pagination/filtering/ordering follows the established OSAC List API pattern used by all existing entities - FR-8: Define each state meaning (UNSPECIFIED, PENDING, READY, FAILED) and explain why Phase 1 transitions are manual (no reconciler) Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com> * OSAC-1111: Simplify state lifecycle to UNSPECIFIED and READY only Remove PENDING and FAILED states — they will be introduced when reconciliation or health probing is added in a future phase. Backends are created with initial state READY. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com> * OSAC-1111: Remove UNSPECIFIED state, keep READY only Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com> --------- Signed-off-by: Roy Golan <rgolan@redhat.com>
…permissions (#38) * Update organizations & authentication enhancement Key updates: - Clarify multi-cluster scope (infrastructure clusters, not CaaS) - Add project-scoped resource access pattern (project switcher) - Defer cross-project views to future enhancement - Answer deletion lifecycle semantics (block by default, optional cascade) - Define hierarchical project permissions policy - Add Keycloak Authorization Services integration details (local RPT validation) - Clarify project permission model and resource listing - Align scope examples with OSAC resources (CREATE_COMPUTE_INSTANCE) - Note that specific scopes are implementation details - Update last-updated date to 2026-05-06 Addresses feedback from Crystal Chun and Juan Antonio Hernandez Fernandez on multi-cluster architecture, project permissions, and resource visibility. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Avishay Traeger <atraeger@redhat.com> * Address CodeRabbit review comments - Split long summary paragraph into 5 focused paragraphs for readability - Clarify that local RPT validation is the recommended approach (not token introspection) - Add reference link to detailed RPT validation section Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Avishay Traeger <atraeger@redhat.com> * Fix CodeRabbit review findings - Fix HTTP code block formatting (add Content-Type header, use http syntax) - Clarify hierarchical permission propagation is consistent across document - Expand RPT validation to include full JWT validation details (JWKS/JWS, claims, time checks) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Avishay Traeger <atraeger@redhat.com> * Address CrystalChun's review comments - Clarify hierarchical permissions: MANAGE_PROJECT and VIEW_PROJECT cascade from parent to child projects, but resource-specific permissions do not (line 248) - Clarify that the 'default' project does not grant automatic access to all org users - permissions must be explicitly granted (line 165) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Avishay Traeger <atraeger@redhat.com> * Address mhrivnak's scalability concerns - Add implementation note about on-demand OpenShift Project creation to avoid N×M scaling issues - Note that database queries need efficient indexing on project_id and organization_id - Clarify that OpenShift Projects can be created on-demand or upfront as implementation choice Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Avishay Traeger <atraeger@redhat.com> --------- Signed-off-by: Avishay Traeger <atraeger@redhat.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
* OSAC-23: add design document for tenant storage onboarding Introduces a dedicated OSAC Storage Controller that extracts all storage provisioning logic from the Tenant controller. The controller manages a two-stage onboarding workflow (backend setup, then cluster-side StorageClass installation) and a two-step ordered teardown, using condition ownership on the Tenant CR. Key design decisions: - Two conditions on Tenant CR: StorageBackendReady, ClusterStorageReady - Two ProvisioningProvider instances (backend + cluster-storage) - Tenant Phase=Ready decoupled from storage readiness - Four AAP playbook lifecycle actions - Per-backend and per-cluster status detail structs Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> Assisted-by: Cursor/Claude Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> * OSAC-23: use annotation-based ClusterOrder watch mapping ClusterOrder and Tenant live in different namespaces, so ownerReferences cannot be used. Replace EnqueueRequestForOwner with EnqueueRequestsFromMapFunc that reads the osac.openshift.io/tenant annotation from ClusterOrder to enqueue the correct Tenant. This is consistent with how ComputeInstance and all other OSAC resources associate with Tenants via the same annotation, set by the fulfillment-service at CR creation time. Also incorporates review feedback: management state scoping (FR-4 steady-state only), label-based hub Secret lookup, no-auto-retry on failure, operator-first deployment order, priority=1 print columns, per-tenant management-state in support procedures, and tier addition workflows as non-goal. Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> Assisted-by: Cursor/Claude Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> * OSAC-23: add extension points for multi-backend and tier source Frame the reconciliation loop around two pluggable sources (backend list and tier source) so the controller logic can evolve from single VAST backend to multiple backends (OSAC-1111) and from STORAGE_TIERS env var to StorageTier CRs (OSAC-1110) without rewriting the core loop or changing the status schema. Clarify that v0.1 storageBackends and clusterStorage lists each have a single entry, with the list structures anticipating multi-backend and multi-cluster support. Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> Assisted-by: Cursor/Claude Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> * OSAC-23: update last-updated date to 2026-06-15 Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> Assisted-by: Cursor/Claude Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> * OSAC-23: clarify implicit stage derivation in reconciliation steps Add parenthetical notes to make the stage sequencing explicit for code reviewers: Secret absence indicates Stage 1 has not run, StorageBackendReady=True indicates Stage 1 is complete. Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> Assisted-by: Cursor/Claude Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> * OSAC-23: tighten design document prose Remove source markers, cut redundant sentences, shorten failure table, simplify alternatives and boilerplate sections, remove inline Go comments from self-documenting structs. Preserves all technical content in a more concise form. Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> Assisted-by: Cursor/Claude Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> * OSAC-23: rename design document from README.md to design.md Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> Assisted-by: Cursor/Claude Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> --------- Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
- Replace credentials_ref (K8s Secret reference) with inline credentials (username/password) stored in JSONB, consistent with all existing OSAC entities - Remove Signal RPC — no reconciler, no consumer - Update PRD to match (FR-3, FR-10, open questions) Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
Tenants don't need direct access to StorageBackend — they interact with storage through StorageTier (OSAC-1110). Removed public API proto, server, tests, and all references. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
…dated Add name validation constraints to StorageBackend design: - metadata.name is required on Create (must be non-empty) - metadata.name is immutable on Update (cannot be changed after creation) - DNS label format enforced by generic server (RFC 1035) - Uniqueness already covered by the partial index (FR-9) Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
Replace StorageBackendEndpoint message (host + port) with a plain string field that accepts a URL or host:port. Simpler and more flexible. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
…rd-delete) The generic DAO uses soft-delete by default. Switching to hard-delete requires DAO extension or custom server logic. Mark as OQ-1 for review. Also adds referential integrity check: delete is rejected if any StorageTier references the backend. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
…ategy OQ-1 resolution: soft-delete by default (standard DAO), hard-delete when purge=true on the request. Both modes enforce referential integrity (reject if StorageTiers reference the backend). Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
Replace soft-delete/archive as user-facing decommission with state-based lifecycle: - READY: operational (phase 1) - MAINTENANCE: temporary unavailability, reversible (phase 0.2) - DECOMMISSIONED: terminal, backend retired but visible (phase 0.2) Delete RPC permanently removes the record. In phase 0.2, only allowed on DECOMMISSIONED backends with no StorageTier references. The archived table remains as DAO infrastructure but is not user-facing. Resolves OQ-1. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
The generate agent should ONLY generate the test plan. Scoring is a separate process via /test-plan-score. Changes: - Remove self-scoring instruction from generate prompt - load_verdict returns a stub for generate mode (no verdict.json needed) - Generate agent just writes testplan-output.md, nothing else This was causing the PR creation to be skipped — agentic-ci expected verdict.json but the agent didn't write it. Assisted-by: Claude Code <noreply@anthropic.com>
Fix: separate generation from scoring
The Podman container only sees /workspace/ (the work_dir). Repos
at /opt/osac-workspace/ were invisible. Fix: copy all git repos
from the workspace into work_dir so they appear at /workspace/{repo}/
Also fix the prompt to tell the agent repos are in the working dir.
Assisted-by: Claude Code <noreply@anthropic.com>
Fix: copy repos into container work_dir
The agentic-ci apply_labels hook wasn't being called after generation. Instead of debugging the hook pipeline, do the PR creation directly in tp_generate.py after run_skill() returns — copy testplan-output.md to the EP repo, create branch, commit, push, open PR. Assisted-by: Claude Code <noreply@anthropic.com>
Fix: create test plan PR directly after generation
Assisted-by: Claude Code <noreply@anthropic.com>
Fix: handle existing branch in PR creation
Assisted-by: Claude Code <noreply@anthropic.com>
Fix: git identity + error handling for PR creation
agentic-ci calls sys.exit() after run_skill(), so Python code after it never executes. Move PR creation to a shell step that runs even if the agent step fails (|| true). The shell step checks for testplan-output.md and creates the branch/commit/PR directly. Assisted-by: Claude Code <noreply@anthropic.com>
Fix: PR creation as separate workflow step
The agent writes ep_slug from the design title (storage-tier-api) but the directory is storage-tier-OSAC-1110. Write ep-slug.txt from the detection step. Also simplify commit message to avoid YAML issues. Assisted-by: Claude Code <noreply@anthropic.com>
Fix: EP slug from directory name, not agent YAML
Assisted-by: Claude Code <noreply@anthropic.com>
Fix: checkout main before branch cleanup
Same pattern as test-plan-generate PR creation — agentic-ci apply_labels hook doesn't fire, so post the scoring comment directly in a shell step that reads verdict.json from the work_dir. Adds labels: test-plan-scored, test-plan-ready or test-plan-rubric-fail. Assisted-by: Claude Code <noreply@anthropic.com>
Fix: post score comment as separate workflow step
New workflow test-plan-generate-odh.yml triggered by /test-plan-create-odh slash command. Clones opendatahub-io/odh-test-gen, runs its test-plan-create skill via agentic-ci, and opens a PR with TestPlan-odh.md for side-by-side comparison with our /test-plan-create output. Usage: comment /test-plan-create-odh OSAC-1123 on a design PR Requires secrets: JIRA_URL, JIRA_USER, JIRA_TOKEN (for Jira fetch) Assisted-by: Claude Code <noreply@anthropic.com>
Add odh-test-gen comparison workflow
jq on GH runners doesn't support named capture groups (?P<name>).
Use split("/")[1] instead to extract EP slug from file paths.
Assisted-by: Claude Code <noreply@anthropic.com>
Fix: jq regex for EP slug extraction
Agent writes to /workspace/ inside container which maps to work_dir on the host, not /opt/test-plans/ (host-only path). Assisted-by: Claude Code <noreply@anthropic.com>
Fix: search work_dir for odh-test-gen output
Parses api_request events from claude-otel.jsonl, sums input/output/cache tokens, and appends a collapsible cost details block to the PR comment. Assisted-by: Claude Code <noreply@anthropic.com>
Add cost summary to score comment
detect_skills() used endswith("design.md") which incorrectly matched
ui-design.md and ux-design.md. Switch to os.path.basename exact match.
Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Itzik Ezra <iezra@redhat.com>
Signed-off-by: Itzik Ezra <iezra@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Test PR for OSAC-3153. Expected: EP Review does NOT trigger at all.