Add OCI Compute and Block Volume: instances, volumes, images, pools

[arunesh-j]

Summary

• Implements OCI Compute and Block Volume against the existing portable compute driver.
• Closes the topology gap deferred from the VCN review (Add OCI VCN: networks, subnets, gateways, security #368) — CanConnect now evaluates OCI security lists, not just NSGs.
• Nothing added to services/compute/driver; OCI-only behaviour is a consumer-side Extras interface, per Move OCI-only capabilities out of shared driver packages #393.

Closes #411. Part of #376. Depends on OCI VCN (#379), merged.

Changes

• providers/oci/compute/ — Mock over memstore implementing driver.Compute, single sync.RWMutex.
• server/oci/compute/ — the /20160918/ Core Compute surface.
• features/topology/ — the deferred security-list fix (see below).
• Wiring is one line each in providers/oci/oci.go and server/oci/oci.go.

Operations: instances (launch/get/list/update/terminate, ?action=START|STOP|SOFTSTOP|RESET|SOFTRESET), shapes, images, volumes and attachments, boot volumes and attachments, volume and boot-volume backups, volume groups, VNIC attachments, instance configurations (+ actions/launch), instance pools (+ actions and /instances), and changeCompartment on every collection. Preemptible → spot, pools → auto-scaling groups, configurations → launch templates, backups → snapshots.

The deferred topology gap

CanConnect reached only DescribeSecurityGroups, so an OCI instance with no NSG false-DENYed. Fixed in two halves:

• providers/oci/compute records an instance's effective rule collections at launch — its VNIC's NSGs plus its subnet's security lists (falling back to the VCN's default list).
• features/topology's findMatchingSGRule now resolves each attached id as a security group and, for ids that are not, as a network ACL (allow rules only) — the union real OCI applies.

AWS is provably unaffected: its instances carry only security groups, so every id resolves and the ACL branch is never reached. The only other edit in that package is extracting "allow" into an actionAllow constant. All existing topology tests pass unchanged.

New tests: TestCanConnectHonoursOCISecurityLists (SSH allowed by the default list, HTTP denied), TestCanConnectHonoursOCINetworkSecurityGroups (NSG path still wins and is attributed), and TestSubnetSecurityListsReachTheInstance pinning the subnet coupling end-to-end through both handlers.

VCN wiring

providers/oci/oci.go holds the concrete *vcn.Mock and calls computeMock.SetNetworking(vcnMock) — the providers/aws/aws.go SetSubnetResolver pattern. A narrow Networking interface wraps VCN's existing CreateNetworkInterface, DescribeSubnets, DescribeVNICs, UpdateVNIC, DeleteNetworkInterface, Defaults. A launch creates the VNIC through VCN and records the OCI vnicAttachment around VCN's attachment id. No VNIC is modelled in compute.

No key pair resource

OCI has none — keys are instance metadata. CreateKeyPair/DeleteKeyPair/DescribeKeyPairs return Unimplemented naming ssh_authorized_keys, RunInstances rejects a KeyName rather than dropping it, and there is no /keyPairs collection.

Matches is disjoint from VCN's

Tested both directions: 16 positive collections, plus explicit negatives for vcns, subnets, vnics, securityLists, networkSecurityGroups, routeTables, publicIps, workRequests and other API versions. TestVCNTrafficIsNotSwallowed runs both handlers behind one mux in registration order and confirms VCN still answers.

Provider Coverage

• AWS
• Azure
• GCP
• OCI

Checklist

• All tests pass (go test ./...) — exit 0, 272 packages
• Linter passes (golangci-lint run --timeout=9m) — 0 issues
• Every provider the change applies to implements the same behavior — OCI-only except the additive topology fix
• Integration tests added to cloudemu_test.go — driver + handler + topology tests instead
• Unit tests added to provider test files

Test Plan

go build ./...                                                       clean
go test ./...                                                        exit 0, 272 packages
go test -race ./providers/oci/... ./server/oci/... ./features/topology/...   12/12 ok
golangci-lint run --timeout=9m (same three trees)                    0 issues
go generate ./...                                                    docs/coverage committed

git diff development -- services/ empty; no non-OCI coverage page changed.

Lock sweep: every exported Mock method locks, none calls another. The three cross-boundary paths release first — RunInstances calls into VCN before taking m.mu; syncPool computes its delta under RLock, releases, then launches/terminates; emitMetrics runs after unlock. No store.Update closure re-enters its store. race_test.go drives 16 goroutines through launch/attach/detach/tag/stop/terminate plus pool resize — it would hang, not merely fail, if that were violated.

End-to-end on a running server (port 4612):

create VCN -> subnet (securityListIds set)   -> AVAILABLE
list images (Oracle Linux)                   -> Oracle-Linux-9.4
launch instance                              -> 200 + Opc-Work-Request-Id, RUNNING
                                                shapeConfig{ocpus:2, memoryInGBs:32}
                                                metadata.ssh_authorized_keys round-trips
                                                freeformTags clean — no cloudemu: keys leak
vnicAttachments?instanceId=                  -> ATTACHED, nicIndex 0, subnet matches
create volume -> attach                      -> paravirtualized, /dev/oracleoci/oraclevdb
instance action STOP                         -> STOPPED
list work requests                           -> LAUNCH_INSTANCE / CREATE_VOLUME /
                                                ATTACH_VOLUME / INSTANCE_ACTION_STOP all SUCCEEDED
terminate instance                           -> 204 + work request; GET -> 404
volume after termination                     -> AVAILABLE (detached, survives)

One thing worth a follow-up

docs/coverage/oci/compute.md lists CreateKeyPair/DeleteKeyPair/DescribeKeyPairs even though OCI returns Unimplemented for them, because coveragegen renders every driver method for every provider implementing the service. I deliberately did not add a docs/coverage/nongoals/compute.md fragment: those are per-service, so the note would also render on the AWS, Azure and GCP compute pages, which do implement key pairs.

The gap is documented in docs/services.md and in the driver instead. Making the coverage doc accurate needs a per-provider non-goals hook in coveragegen — same family as #394, and worth its own issue.

[NitinKumar004]

Review notes

Real data-plane engine (per #427): MISSING. See finding below.

CI: CodeQL fails — high go/uncontrolled-allocation-size at providers/oci/compute/instance.go:67. Guard the size before allocating.

Findings

Medium · real-engine — OCI Compute does not route its data plane through the shared ComputeEngine helper (#427 divergence)
providers/oci/compute/instance.go:60
If a user does cloudemu.NewOCI(config.WithComputeEngine(dockerEngine)) then LaunchInstance -> no real backing is provisioned and the instance keeps its synthetic 10.0.x private IP, because RunInstances never invokes computeengine.Provision — the opt-in real engine silently works for AWS/Azure/GCP compute but is a no-op for OCI, breaking cross-cloud parity.

RunInstances (instance.go:60) and TerminateInstance (instance.go:214) never call services/compute/computeengine.Provision/Deprovision, and the mock never reads o.ComputeEngine. AWS ec2 (providers/aws/ec2/ec2.go:301,341,452) wires all three hooks. #427 is merged on development; this PR branched from f1bc63d which predates it, so it compiles+merges green without the wiring.

Low · structure — Service branch modifies the shared topology engine beyond the three-slot scope
features/topology/connectivity.go:152
If an AWS/Azure/GCP instance references a security-group id the provider mock no longer returns (e.g. a deleted SG) -> topology now issues an extra DescribeNetworkACLs lookup for that id where before it was ignored; benign today because sg/acl id namespaces are disjoint so the lookup returns nothing, but it widens an OCI branch's blast radius into an engine every provider's connectivity path depends on, with no non-OCI regression test.

findMatchingSGRule now falls through to findMatchingACLRule for any instance security id the VCN/VPC mock's DescribeSecurityGroups did not return (connectivity.go:186 unresolved()). This is a shared cross-service engine used by CanConnect for ALL providers, changed in an OCI service branch and tested only via features/topology/oci_security_list_test.go. Conventions say a service branch touches only the three slot files + providers/oci/ + server/oci/, and not to modify the shared founda

Low · correctness — RunInstances count>1 has no rollback on a mid-loop failure
providers/oci/compute/instance.go:69
If a user calls RunInstances with count>1 and placement fails after the first instance is created -> the caller gets an error but the already-created instances are orphaned in the mock. Low impact in practice: all instances share one InstanceConfig, so a placement error (bad/absent subnet) fails on the first iteration and creates nothing; only a transient VCN error (which the mock does not generate) could partially create.

The launch loop (instance.go:69-81) creates each instance's VNIC (place) then stores it; on an error from place() for a later iteration it returns nil,err while earlier instances and their VNICs/boot volumes remain in the stores.

[NitinKumar004]

Review — OCI Compute + Block Volume

Verdict: Request changes (merge-after-rebase). The compute/state-machine substance is strong, but the PR can't merge as-is: CI is red, mergeable: CONFLICTING, and the merged tree neither compiles nor passes the persistence gate.

On the red CI

The single red check is CodeQL, not any Go gate (Build/Vet/Test/Lint/Structure/Tidy/Format all pass):

• CodeQL go/uncontrolled-allocation-size — providers/oci/compute/instance.go:67 (make([]driver.Instance, 0, count)). count is bounded — validateLaunch rejects count > maxLaunchCount (1000) earlier in the flow — so this is an inter-procedural false-positive CodeQL can't see across the function boundary. It still reddens CI and blocks merge. Fix: make the bound visible to data-flow (clamp the make capacity, or inline the count > maxLaunchCount check immediately before the make), or dismiss the alert.

Blocking (surface on rebase onto current development)

• Merged tree does not compile — stale driver signature — providers/oci/compute/volume.go:259. development evolved compute/driver.Compute.DetachVolume to (ctx, volumeID, instanceID, device string); this PR implements the old 2-arg DetachVolume(ctx, volumeID), so *compute.Mock no longer satisfies driver.Compute (the compute.go:70 compile-assert fails). Fix: update to the 3-arg signature.
• Missing snapshot.go — persist completeness gate fails — providers/oci/compute/ (no snapshot.go). *compute.Mock holds 16 memstore.Store fields but implements no snapshot.Snapshottable, while siblings vcn/identity/monitoring all ship snapshot.go. On the merged tree TestSnapshotCompleteness/oci errors *"field Compute (compute.Mock) holds a memstore.Store but is not Snapshottable". The PR's own CI was green only because it branched before that guard landed. Fix: add snapshot.go covering all 16 stores (instances, details, images, volumes, volAttach, bootVolumes, bootAttach, backups, volGroups, vnicAttach, pools, configs, spot, shapes, scopes, created). Consequence: a snapshot/restore silently drops all OCI compute + block-volume state.
• Merge conflicts — providers/oci/oci.go, features/topology/connectivity.go, generated docs/coverage/*. oci.go and docs are mechanical, but connectivity.go is a real code overlap: development refactored the same matchRules region (value-loop → pointer-loop, new targetIP param) while this PR added a found map for OCI security-list connectivity — reconcile by hand.

Should fix

• STOP on an already-STOPPED instance returns 200 idempotent — providers/oci/compute/instance.go (StopInstances). Real OCI InstanceAction STOP on a STOPPED instance returns 409 IncorrectState. Defensible EC2-style choice, but a genuine OCI deviation.
• Error message leaks the internal cerrors prefix — double-attach returned message: "AlreadyExists: volume ... is already attached"; the AlreadyExists: prefix is CloudEmu-internal (real OCI omits it). The code ("Conflict") is correct. Shared ocirest behavior — cosmetic.

Non-blocking

• Batch applyTransition partial application without rollback — instance.go. In a multi-ID batch, IDs processed before a failing one stay mutated (metrics aren't emitted, limiting impact). Matches AWS EC2 batch semantics; minor.

Verified good

State machine correct (§19): Start(Stopped→Running), Stop(Running→Stopped), Reset, idempotent + FailedPrecondition on illegal states; terminated instances → 404 on get/action; volume attach rejects double-attach, detach rejects unattached. Live e2e over the wire: VCN→subnet→launch(RUNNING)→STOP→START→GET(stable id/timeCreated)→volume(AVAILABLE)→attach(ATTACHED)→attach-again(409)→detach(204)→detach-again(404)→terminate(204)→get-terminated(404). Pool/launch sizes bounded (maxLaunchCount/maxPoolSize=1000, enforced through the scaling-policy path too — no unbounded alloc). Reads deep-copy; creates mint fresh OCIDs (no check-then-set race); -race clean. Wiring complete (from_provider.go, handler on VCN's shared prefix with work-request registry, auto-metrics on launch/lifecycle, monitoring wired); naming conformant.

Path to merge

1. Resolve the CodeQL alloc alert (make the bound data-flow-visible).
2. Rebase onto development; fix DetachVolume to the 3-arg signature.
3. Add providers/oci/compute/snapshot.go (+ test) for all 16 stores.
4. Hand-merge the connectivity.go / oci.go / docs conflicts.

[arunesh-j]

Rebased onto a45ac909 and worked the path to merge in 2d029237.

1. CodeQL — you were right that it's a false positive

count was already bounded by validateLaunch, so a second runtime check would have been redundant code that didn't clear the alert. Instead the bound now dominates the allocation: count <= 0 and count > maxLaunchCount are inline in RunInstances immediately before make([]driver.Instance, 0, count), with validateLaunch keeping only the cfg rules.

TestLaunchRejectsOversizedCount covers 1001 and 1<<40 and asserts nothing was created. Removing the guard gives panic: makeslice: cap out of range at instance.go:79.

2. snapshot.go — all 16 stores, and both traps were real

Mirrors providers/oci/vcn/snapshot.go with the shared storeDump table so Snapshot and Restore can't drift. Two things bit exactly as feared:

• instanceData.engineBacked was unexported → exported as EngineBacked. Without it, engine-backed instances restore unbacked and terminate never tears the real backing down.
• poolData.policies is a nested *memstore.Store → JSON dropped it and left it nil. Added MarshalJSON/UnmarshalJSON on poolData carrying policies as a map and rebuilding the store. Without them the round-trip panics on a nil deref.
• Also snapshotted ipCounter (monotonic on restore) so a restore doesn't re-issue private IPs already in use.

go test ./persist/... passes.

3. DetachVolume — confirmed 3-arg after the rebase

(ctx, volumeID, instanceID, device), with the narrow-by-instance/device check mirroring EC2.

4. STOP on STOPPED → 409

StopInstances no longer treats stopped as idempotent; START stays a no-op. Pool STOP filters to running members via membersInState, so a pool action isn't failed by one already-stopped member. Live: step 11 below.

5. The AlreadyExists: prefix — not reproducible on the current tree

ocirest.WriteDriverError already goes through cerrors.Message, which returns e.Message without the code. Verified over the wire — double-attach returns {"code":"Conflict","message":"volume ... is already attached to ..."}, no prefix. Your observation predates the current tree.

I deliberately did not touch shared ocirest on that basis. Pinned it instead with TestAttachVolumeTwiceIsAConflictWithACleanMessage (asserts NotContains "AlreadyExists:") plus the same assertion on the FailedPrecondition: STOP path, so a future regression is caught here rather than rediscovered.

#427 ComputeEngine wiring

RunInstances→Provision (engine IP overrides the synthetic one), TerminateInstance→Deprovision, GetConsoleOutput via driver.ConsoleReader with a compile-time assert. Batch rollback tears down instances, VNICs and boot volumes on any mid-loop failure — removing rollbackLaunch orphans 3 instances, 3 boot volumes and 3 VNICs.

Topology pin (your Low from the previous round)

aws_absent_security_group_test.go wraps the VPC mock to count DescribeNetworkACLs calls. An AWS instance with a deleted SG is denied with the same "no ingress rule" reason, nil IngressMatch, and zero ACL lookups — AWS's DescribeSecurityGroups errors on an unknown id, so the OCI fall-through is unreachable there. The pin bites: making that lookup swallow its error gives Should be zero, but was 1.

Two bugs found that weren't on the list

• Launch children landed in the wrong compartment. VNIC attachment, boot volume and boot-volume attachment were recorded in the mock's default compartment, so ListVnicAttachments?compartmentId=<caller> returned []. Found because the e2e transcript step 6 came back empty. Added LaunchedResourceIDs + placeInstance (also used for pool members).
• Pool shrink TOCTOU. syncPool reads delta.Newest under RLock and releases before terminating (as the cross-service rule requires), so two concurrent resizes picked the same instance and the loser failed the whole resize with NotFound — 1 flake in 5. A vanished instance is now treated as the shrink that iteration wanted. 12/12 green after.

Verification

go build ./...                                    clean
go test ./...                                     exit 0, 451 packages, completed
go test ./persist/...                             ok
go test -race providers/oci + server/oci + topology   12/12 ok
golangci-lint (both compute packages)             0 issues
go generate ./...                                 committed; re-running gives no diff
git diff origin/development -- services/compute/  empty (shared driver untouched)
no OCI op in docs/coverage/{aws,azure,gcp}/*.md   confirmed

Lock sweep over all 102 locking *Mock methods: no locking method calls another. The three cross-boundary paths still release first — RunInstances→VCN, syncPool, emitMetrics. New code follows suit: provision snapshots under RLock, calls the engine unlocked, re-locks to write.

Two things I did not meet — flagging rather than burying

• Coverage: providers/oci/compute 68.5%, server/oci/compute 54.2%. For context I measured the merged siblings on development: providers/oci/vcn 81.8%, server/oci/vcn 66.6%, providers/oci/identity 91.7% — so 90% isn't currently met by merged OCI code, and CI has no coverage gate (golangci-lint runs report-only). New code is well covered per-function (RunInstances, provision, StopInstances, TerminateInstance, GetConsoleOutput, MarshalJSON, membersInState at 100%; rollbackLaunch 85.7%). Raising the package figure means testing pre-existing untested branches across ~15 files — happy to do it as a separate pass if you want it before merge.
• 9 gocritic findings remain in features/topology/resolve.go, providers/oci/identity/portable.go and {providers,server}/oci/vcn/nsg.go. git diff origin/development on those files is empty — pre-existing, left alone rather than a drive-by.

Batch applyTransition partial application left alone per your non-blocking note.

E2E, port 4612

#	Step	Result
4	LaunchInstance	RUNNING + Opc-Work-Request-Id
6	ListVnicAttachments	1 ATTACHED in the caller's compartment (was [] before the fix)
8/9	AttachVolume, again	200 ATTACHED, then 409 Conflict, message clean
10/11	STOP, STOP again	200 STOPPED, then 409 IncorrectState
12	ListWorkRequests	4 SUCCEEDED
13/14	Terminate, Get	204, then 404