Extract dummy CPI from bosh-director into integration_support#2756
Merged
Conversation
The dummy CPI (Bosh::Clouds::Dummy, DummyV2, and the dummy_cpi binary) is
only used by integration specs, not by the bosh-director production code.
Move these files out of src/bosh-director/ into src/spec/integration_support/
where they belong:
src/bosh-director/bin/dummy_cpi
→ src/spec/integration_support/bin/dummy_cpi
(update BUNDLE_GEMFILE path: ../../ → ../../../)
(update require_relative: ../lib/clouds/ → ../clouds/)
src/bosh-director/lib/clouds/dummy.rb
→ src/spec/integration_support/clouds/dummy.rb
(update require_relative '../clouds/errors' → require 'clouds/errors'
since errors.rb stays in bosh-director as production code)
src/bosh-director/lib/clouds/dummy_v2.rb
→ src/spec/integration_support/clouds/dummy_v2.rb
(same errors.rb require update)
Update sandbox.rb to reflect the new locations:
- require 'clouds/dummy' → require 'integration_support/clouds/dummy'
- exec_path: now points to spec/integration_support/bin/dummy_cpi
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe dummy CPI script now uses the integration support Gemfile and loads dummy implementations from 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@src/spec/integration_support/bin/dummy_cpi`:
- Line 3: The dummy CPI boot path is missing the dependency needed for require
'cloud', so update the bundle setup in dummy_cpi to point at the correct bundle
root and ensure the cloud load-path is available, either by adding the missing
local dependency to the Gemfile used here or by changing the require to a
provided module. Use the dummy_cpi entry point and the Gemfile/bundle
initialization as the place to fix it.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4d60d802-e0bb-4af2-93d7-e83b945071f7
📒 Files selected for processing (4)
src/spec/integration_support/bin/dummy_cpisrc/spec/integration_support/clouds/dummy.rbsrc/spec/integration_support/clouds/dummy_v2.rbsrc/spec/integration_support/sandbox.rb
- Add comprehensive unit specs for Bosh::Clouds::Dummy and DummyV2 covering: initialization, VM/stemcell/disk/snapshot/network lifecycles, metadata operations, CommandTransport, invocation recording, and error scenarios (IP collision, configured failures, NotImplemented variants) - dummy.rb: add explicit requires for all standard-library and gem dependencies (ipaddr, json, logging, membrane, securerandom, socket, yaml) so the file is self-contained and loadable without first requiring sandbox.rb or bosh-director - dummy_v2.rb: replace redundant top-level requires with a single require_relative 'dummy', since DummyV2 depends entirely on Dummy
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@src/spec/integration_support/spec/clouds/dummy_spec.rb`:
- Around line 238-243: The `#reboot_vm` spec is asserting the wrong exception
type; update it to expect the shared `Bosh::Clouds::NotImplemented` error
instead of `Bosh::Clouds::Dummy::NotImplemented` so it matches the contract in
`errors.rb`. Use the `dummy.reboot_vm` example in `dummy_spec.rb` and keep the
existing message match, but change the raised class reference to the shared
error namespace.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 79da7e3d-a6b7-4cb1-8ac0-968baa1dcbfd
📒 Files selected for processing (3)
src/spec/integration_support/clouds/dummy.rbsrc/spec/integration_support/clouds/dummy_v2.rbsrc/spec/integration_support/spec/clouds/dummy_spec.rb
- dummy.rb: remove require 'logging' — the real logging gem overrides
the hand-rolled Logging module in the dummy_cpi subprocess, breaking
subprocess execution; logging is always available via bosh/director
in the test context
- dummy_spec.rb: three correctness fixes
1. invocations tests: CPI method names are stored and returned as
Strings after JSON round-trip, not Symbols — update assertions
to compare against strings ('create_disk', not :create_disk)
2. vm_cids test: stub spawn_agent_process with distinct return values
(10001, 10002) so each VM gets a unique cid; a shared stub value
of 99999 caused the second VM to overwrite the first in the VMRepo
3. Add require 'stringio' and require 'logging' at top of spec for
explicit documentation of dependencies
The real logging gem (used in the test process) maintains a global
registry of appenders by name. Multiple Bosh::Clouds::Dummy instances
created within the same process (e.g. across spec examples) all tried
to register an IO appender named 'DummyCPIIO' and a logger named
'DummyCPI', causing a registration conflict from the second instance
onward.
Use object_id to make each instance's logger and appender names unique:
"DummyCPI_#{object_id}" / "DummyCPIIO_#{object_id}"
This has no effect in the dummy_cpi subprocess context — the custom
Logging module defined there does not maintain a global registry, so
unique vs. shared names are irrelevant.
reboot_vm was calling validate_and_record_inputs(__method__, vm_cid) but the method signature is (schema, the_method, *args). Without the schema, the_method received the vm_cid string, causing Kernel#method to be called with the CID value instead of a method name, raising NameError.
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
github-project-automation
Bot
moved this from Inbox
to Waiting for Changes | Open for Contribution
in Foundational Infrastructure Working Group
- Remove two useless local variable assignments in dummy_spec.rb that were flagged by github-code-quality (vm_cid and snapshot_id were assigned but never read) - Make reboot_vm raise Bosh::Clouds::NotImplemented for consistency with all other unimplemented CPI methods (attach_disk, detach_disk, resize_disk, update_disk); update the spec to match
github-project-automation
Bot
moved this from Waiting for Changes | Open for Contribution
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
ystros
previously approved these changes
Jun 26, 2026
- run release specs in parallel
ystros
approved these changes
Jun 26, 2026
github-project-automation
Bot
moved this from Pending Merge | Prioritized
to Done
in Foundational Infrastructure Working Group
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
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.
Summary
Changes
`clouds/errors.rb` stays in `src/bosh-director/lib/clouds/` — it is production code used by `external_cpi.rb` and loaded by the director at runtime.
Unit spec coverage
`spec/integration_support/spec/clouds/dummy_spec.rb` covers:
Test plan