Skip to content

fix: libvirt backend failures (#425, #427, #430, #431, #432)#484

Merged
poyrazK merged 1 commit intomainfrom
fix/libvirt-backend-failures-425-427-430-431-432
May 7, 2026
Merged

fix: libvirt backend failures (#425, #427, #430, #431, #432)#484
poyrazK merged 1 commit intomainfrom
fix/libvirt-backend-failures-425-427-430-431-432

Conversation

@poyrazK
Copy link
Copy Markdown
Owner

@poyrazK poyrazK commented May 7, 2026

Summary

  • Remove Docker-specific compute.Type() == "docker" checks from cache and database services that broke libvirt backend
  • Make instance stop error messages backend-agnostic ("failed to stop {backend} instance" instead of "docker container")
  • Skip OVS bridge creation in VPC service when COMPUTE_BACKEND=libvirt (libvirt manages its own networking)
  • Document that ResilientCompute circuit breaker is per-backend, not per-operation

Test plan

  • go build ./... passes
  • go test ./internal/core/services/... passes (VPC, cache, database, instance tests)

Fixes #425, #427, #430, #431, #432

Copilot AI review requested due to automatic review settings May 7, 2026 17:23
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 7, 2026

Review Change Stack

Warning

Rate limit exceeded

@poyrazK has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 46 minutes and 18 seconds before requesting another review.

To continue reviewing without waiting, purchase usage credits in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c5fddb5a-8a6b-4fbd-a97b-2abd2e0f4633

📥 Commits

Reviewing files that changed from the base of the PR and between 685daeb and c112970.

📒 Files selected for processing (6)
  • internal/api/setup/dependencies.go
  • internal/core/services/cache.go
  • internal/core/services/database.go
  • internal/core/services/instance.go
  • internal/core/services/vpc.go
  • internal/platform/resilient_compute.go
📝 Walkthrough

Walkthrough

VPC service initialization now accepts and stores the compute backend type. OVS bridge creation and NetworkID assignment are now conditional on the backend: Docker backends create bridges and populate NetworkID; libvirt backends skip bridge creation and leave NetworkID empty. Docker+OVS special-case logic is removed from network-resolution paths. Error logging is made backend-agnostic.

Changes

VPC Networking Backend Abstraction

Layer / File(s) Summary
Data Contract
internal/core/services/vpc.go
VpcServiceParams gains ComputeBackend field; VpcService stores it internally during construction.
VPC Creation with Conditional Bridge Setup
internal/core/services/vpc.go
CreateVPC conditionally creates OVS bridge only for non-libvirt backends. NetworkID is set to bridge name for Docker or left empty for libvirt. Bridge-creation state is tracked to enable proper rollback on database failure.
VPC Deletion with Backend-Aware Cleanup
internal/core/services/vpc.go
DeleteVPC skips bridge deletion when NetworkID is empty (libvirt). Route-table failure rollback respects the bridge-creation flag.
Network Resolution Simplification
internal/core/services/cache.go, internal/core/services/database.go
Remove conditional logic that returned empty network IDs for Docker backends with br-vpc- prefixed networks. Both now consistently return vpc.NetworkID on successful VPC lookup.
Backend-Agnostic Error Logging
internal/core/services/instance.go
StopInstance error messages report compute backend type and use generic format (failed to stop <type> instance) instead of docker-specific wording.
Dependency Wiring
internal/api/setup/dependencies.go
InitServices passes ComputeBackend from config to NewVpcService constructor.
Documentation
internal/platform/resilient_compute.go
Comments clarify circuit breaker is scoped per compute backend instance and reset behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • poyrazK/thecloud#127: Both PRs modify network-resolution behavior for Docker/OVS VPCs; the related PR introduced the Docker/br-vpc- special-case logic that this PR removes and replaces with compute-backend-aware configuration.

Poem

🐰 A rabbit's ode to backend abstraction

Docker dances with bridges bright,
Libvirt walks a different path—
One codebase, backends unified,
No special cases, clean and apt!
🌉✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: fixing libvirt backend failures across multiple issues by removing Docker-specific checks and making the VPC/instance services backend-agnostic.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/libvirt-backend-failures-425-427-430-431-432

Warning

Review ran into problems

🔥 Problems

Timed out fetching pipeline failures after 30000ms


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@poyrazK poyrazK force-pushed the fix/libvirt-backend-failures-425-427-430-431-432 branch from 98bb0a0 to 685daeb Compare May 7, 2026 17:37
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/core/services/vpc.go`:
- Around line 114-117: CreateVPC currently calls s.network.CreateBridge in the
non-libvirt branch without checking s.network, which can cause a nil-pointer
panic; update CreateVPC to guard s.network before using it: if s.network is nil
return an appropriate domain/internal error (wrap with errors.Wrap like other
failures) instead of calling s.network.CreateBridge, and only call
s.network.CreateBridge(ctx, bridgeName, vxlanID) when s.network != nil; ensure
the error message references the bridge creation step so callers can diagnose
the missing network implementation.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3f294833-a1c4-4392-a9e7-e7757946730b

📥 Commits

Reviewing files that changed from the base of the PR and between decac3e and 685daeb.

📒 Files selected for processing (6)
  • internal/api/setup/dependencies.go
  • internal/core/services/cache.go
  • internal/core/services/database.go
  • internal/core/services/instance.go
  • internal/core/services/vpc.go
  • internal/platform/resilient_compute.go
💤 Files with no reviewable changes (2)
  • internal/core/services/cache.go
  • internal/core/services/database.go

Comment thread internal/core/services/vpc.go
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

- Remove hardcoded compute.Type()=="docker" checks in cache.go and
  database.go that broke libvirt backend. These OVS bridge checks
  were Docker-specific and never triggered for libvirt anyway.

- Make instance stop error messages backend-agnostic in instance.go.
  Error now says "failed to stop {backend} instance" instead of
  hardcoded "failed to stop container".

- Add ComputeBackend field to VpcServiceParams. When libvirt is the
  compute backend, VPC creation skips OVS bridge creation since
  libvirt manages its own networking.

- Document in ResilientCompute godoc that the circuit breaker is
  per-backend (not per-operation), explaining the design trade-off.

Fixes: #425, #427, #430, #431, #432
@poyrazK poyrazK force-pushed the fix/libvirt-backend-failures-425-427-430-431-432 branch from 685daeb to c112970 Compare May 7, 2026 17:51
Copy link
Copy Markdown
Owner Author

@poyrazK poyrazK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's okay to merge

@poyrazK poyrazK merged commit 5faabcc into main May 7, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

VPC creation fails with libvirt backend - 'failed to create OVS bridge'

2 participants