Skip to content

Rewrite the driver for modern Test Kitchen - #46

Merged
tas50 merged 1 commit into
mainfrom
modernize-transport-api
Aug 22, 2026
Merged

Rewrite the driver for modern Test Kitchen#46
tas50 merged 1 commit into
mainfrom
modernize-transport-api

Conversation

@tas50

@tas50 tas50 commented Aug 22, 2026

Copy link
Copy Markdown
Member

Why

The driver subclassed Kitchen::Driver::SSHBase, which Test Kitchen removed in 4.0. The gemspec allowed test-kitchen < 5, so the gem installed happily against a current Test Kitchen and then failed at load time:

NameError: uninitialized constant Kitchen::Driver::SSHBase

That made it unusable with current Cinc Workstation and Chef Workstation. This ports it onto Kitchen::Driver::Base and the transport API.

What changed

The driver no longer connects to instances. It works out an address and credentials, puts them into instance state, and lets the configured transport connect. wait_for_sshd, the direct Fog::SSH usage, and deploy_private_key are all gone — the last of those copied your ~/.ssh/id_rsa.pub into every instance it built, purely so SSHBase's later commands would authenticate.

Windows/WinRM works by setting transport: name: winrm. Port forwarding and firewall rules follow the transport's port instead of a hardcoded 22, and the password CloudStack generates for a password-enabled template is handed to WinRM the same way it is to SSH.

kitchen list reports real state. Driver#status is implemented, so an instance destroyed outside Test Kitchen is reported accurately instead of assumed.

Split into focused units, largely because the async-job polling was copy-pasted six times:

File Responsibility
cloudstack.rb create/destroy/status, transport handoff
cloudstack/client.rb API connection + async job protocol
cloudstack/server_options.rb deploy parameters, naming, userdata
cloudstack/networking.rb public addresses, port forwards, firewall rules
cloudstack/credentials.rb keypair/password resolution

Bugs fixed along the way

Consolidating the six duplicated job loops surfaced several real defects:

  • Four of six async job checks were inverted. They tested jobstatus == 0, but CloudStack uses 0=running, 1=success, 2=failed — so they logged an error on success and stayed silent on failure. And since they queried once without polling, they usually read 0 and said nothing at all. Failures to create port forwards, create firewall rules, and release public addresses were silently ignored.
  • Teardown's rescue clauses referenced a constant that does not exist. Fog::Compute::Cloudstack::BadRequest — the real one is Fog::Cloudstack::Compute::BadRequest. Any BadRequest during teardown raised NameError from inside the rescue.
  • Name generation could hang forever. The loop ran until joined.length <= 64, but each branch stopped shortening at floors totalling 67 characters. A login longer than 16 characters hung kitchen create indefinitely. Verified: the old algorithm doesn't terminate, stuck at [16, 16, 24, 8] = 67.
  • associate_public_ip returned a variable only assigned on the success path, raising NameError when allocation failed.
  • Job ids now go to fog as strings. Fog merge!s a hash argument in place; the old code worked around that by re-cloning the hash on every poll.

Breaking changes

  • Requires test-kitchen >= 3.0.
  • No longer copies ~/.ssh/id_rsa.pub into instances. Use cloudstack_userdata or a CloudStack keypair if you relied on it.
  • username/port are no longer defaulted to root/22 by the driver. This fixes a silent bug: state overrides transport config, so the old driver default beat an explicit transport: username: setting. Set on the driver they behave as before; left unset your transport: config now applies.
  • The name option is removed — it never had any effect, as the driver set it but read server_name when deploying.

Version bumped to 1.0.0 accordingly.

Testing

The gem had no tests. This adds 63 examples:

  • Unit specs per component, with a fake client injected at the API seam
  • An integration spec exercising the full lifecycle through real Test Kitchen and real fog, stubbing only the HTTP boundary — so request signing, response parsing, and plugin wiring all actually run

Verified locally: 63 examples / 0 failures (stable across randomized seeds), cookstyle --chefstyle clean, markdownlint clean, and the driver resolves by name through Test Kitchen's plugin loader reporting api_version 2.

Not verified against a live CloudStack — I don't have one to point at. The API-shape assumptions are drawn from the previous implementation and fog-cloudstack, so a review from someone with a real deployment would be valuable, particularly on the WinRM path.

🤖 Generated with Claude Code

The driver subclassed Kitchen::Driver::SSHBase, removed in Test Kitchen
4.0, so the gem installed against a current Test Kitchen and then failed
at load time with a NameError. It is now built on Kitchen::Driver::Base
and the transport API.

The driver no longer opens its own SSH connections. It works out an
address and credentials, puts them into instance state, and lets the
configured transport connect. That removes wait_for_sshd, the Fog::SSH
usage, and deploy_private_key, which copied ~/.ssh/id_rsa.pub into every
instance so that SSHBase's later commands would authenticate.

Because the transport is now the thing that connects, Windows works by
setting `transport: name: winrm`. Port forwarding and firewall rules use
the transport's port instead of a hardcoded 22, and the password
CloudStack generates is handed to WinRM the same way it is to SSH.

Split into focused units: Client owns the API connection and the async
job protocol, ServerOptions builds the deploy parameters, Networking
manages public addresses and rules, and Credentials resolves how to log
in. Driver#status is implemented, so `kitchen list` reports state from
CloudStack rather than assuming.

username and port are no longer defaulted by the driver. State overrides
transport configuration, so defaulting them meant a driver default of
"root" silently beat an explicit `transport: username:` setting. They are
now only sent when set on the driver.

Fixes found while consolidating the duplicated code:

- Four of the six async job checks tested `jobstatus == 0`, treating
  "still running" as success and reporting an error on the successful
  result. Failures to create port forwards and firewall rules, and to
  release public addresses, were silently ignored.
- Teardown rescued Fog::Compute::Cloudstack::BadRequest, which does not
  exist; it is Fog::Cloudstack::Compute::BadRequest. An error during
  teardown raised NameError from the rescue clause itself.
- Instance name generation could loop forever. It shortened until the
  name fit 64 characters, but the per-part floors totalled 67, so a login
  longer than 16 characters hung kitchen create.
- associate_public_ip returned a variable only assigned on the success
  path, raising NameError when allocation failed.
- Job ids are passed to fog as strings. Fog mutates a hash argument in
  place, which the old code worked around by re-cloning it every poll.

Adds an RSpec suite; the gem previously had no tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@tas50
tas50 merged commit 38e5a2d into main Aug 22, 2026
8 checks passed
@tas50
tas50 deleted the modernize-transport-api branch August 22, 2026 20:50
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.

1 participant