diff --git a/.gitignore b/.gitignore index ef442c35..228344f7 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,8 @@ Gemfile.lock InstalledFiles _yardoc coverage +.rspec_status +vendor/bundle doc/ lib/bundler/man pkg diff --git a/.rspec b/.rspec new file mode 100644 index 00000000..16005547 --- /dev/null +++ b/.rspec @@ -0,0 +1,3 @@ +--require spec_helper +--color +--format progress diff --git a/.yardopts b/.yardopts new file mode 100644 index 00000000..e70a9286 --- /dev/null +++ b/.yardopts @@ -0,0 +1,10 @@ +--markup markdown +--private +--protected +--readme README.md +--output-dir doc +--title "kitchen-openstack" +lib/**/*.rb +- +CHANGELOG.md +LICENSE.txt diff --git a/AGENTS.md b/AGENTS.md index 1b3f5307..7b0b2071 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -28,10 +28,12 @@ kitchen-openstack is a Test Kitchen driver for OpenStack. It provisions and dest ```bash bundle install -bundle exec rake # runs tests + style + stats (default) -bundle exec rake test # unit tests only (RSpec) -bundle exec rake style # Cookstyle lint -bundle exec rake quality # style + stats +bundle exec rake # runs tests + style (default) +bundle exec rake test # unit tests only (RSpec) +bundle exec rake style # Cookstyle lint +bundle exec rake quality # style +bundle exec rake yard # render YARD docs to doc/ (not CI-gated) +bundle exec rake yard_stats # list undocumented methods ``` ## Conventions @@ -39,5 +41,10 @@ bundle exec rake quality # style + stats - Use `Fog::OpenStack::Compute` and `Fog::OpenStack::Network` for cloud interactions - Thread safety: use `Mutex` for shared resource pools (e.g., floating IP allocation) - Resource finders (`find_image`, `find_flavor`, `find_network`) support regex matching via `/pattern/` syntax -- Test with RSpec 3 using `let` fixtures, `double` mocks, and `allow_any_instance_of` for Kitchen internals +- Every method in `lib/` carries YARD tags (`@param`/`@return`/`@raise`). Keep new ones documented; `rake yard_stats` reports gaps but nothing enforces it +- Specs mirror `lib/` one-to-one: `spec/kitchen/driver/openstack/_spec.rb` +- Shared spec setup lives in `spec/support/`: the `"with a configured driver"` shared context builds the driver and stubs `instance`; `FogDoubles` builds the Fog stand-ins +- `verify_partial_doubles` is on. Fog *model* classes take `instance_double`; Fog *service* objects cannot, because Fog defines their methods dynamically at instantiation +- Unit tests never sleep, hit the network, or read outside a `Dir.mktmpdir`. The one exemption is `openstack_version_spec.rb`, which reads the gemspec and the Release Please manifest to catch version drift, and skips when they are absent. `ENV` is replaced with an `OS_*`-free hash, and the clouds.yaml specs additionally pin `Dir.pwd`, `Dir.home` and `/etc/openstack`, so neither a developer's OpenStack environment nor their real `clouds.yaml`/`secure.yaml` can leak in +- SimpleCov reports to `coverage/` with no enforced threshold - Release automation via Release Please — version bumps go in `lib/kitchen/driver/openstack_version.rb` diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1bb97441..f653a0e8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,8 +1,8 @@ # Contributing to kitchen-openstack -Thanks for your interest in improving kitchen-openstack. Bug reports, feature requests, and pull requests are all welcome. +Pull requests are welcome. Please make sure your patches are tested. -This project is actively maintained by the [OSU Open Source Lab](https://osuosl.org/). +This project is maintained by the [OSU Open Source Lab](https://osuosl.org/). ## Reporting issues @@ -18,43 +18,70 @@ its config during `finalize_config!`, so diagnose shows the settings actually in effect after `clouds.yaml`, the `OS_*` environment variables, and `kitchen.yml` have been combined. -## Development setup +## Getting set up -Clone the repository and install the dependencies: - -```sh -git clone https://github.com/test-kitchen/kitchen-openstack.git +```bash +git clone https://github.com/test-kitchen/kitchen-openstack cd kitchen-openstack bundle install ``` -## Running the tests +## Rake tasks -```sh -bundle exec rake spec # RSpec unit tests -bundle exec rake rubocop # Cookstyle / RuboCop +```bash +bundle exec rake # tests + lint (default) +bundle exec rake test # unit tests only +bundle exec rake style # Cookstyle lint +bundle exec rake yard # render docs to doc/ +bundle exec rake yard_stats # list undocumented methods ``` +`rake test` runs the `unit` task, and `rake quality` runs `style`. + To run a single spec file: -```sh +```bash bundle exec rspec spec/kitchen/driver/openstack_spec.rb ``` -Many style offenses can be corrected automatically: +## How the tests work -```sh -bundle exec cookstyle -a -``` +Unit tests live in `spec/`, one file per file in `lib/`, and the whole suite +runs in well under a second. Nothing in it touches the network or the clock. + +Nothing reads the filesystem outside a temp directory either, with one +deliberate exception: `spec/kitchen/driver/openstack_version_spec.rb` reads the +gemspec and the Release Please manifest to check that the version number agrees +in all four places it is written down. Those two examples skip themselves when +the files are not present. + +Your own OpenStack setup cannot change the result. `ENV` is replaced with an +`OS_*`-free hash, and the `clouds.yaml` specs additionally pin `Dir.pwd`, +`Dir.home` and `/etc/openstack`, so a real `clouds.yaml` or `secure.yaml` on +your machine is never read — which also means a real password can never end up +in an RSpec diff. + +Fog *model* objects use `instance_double`. Fog *service* objects cannot, because +Fog defines their methods dynamically at instantiation — `spec/support/fog_doubles.rb` +explains this. + +Coverage instrumentation is off by default so a single-file run stays fast. Use +`bundle exec rake coverage` (or set `COVERAGE=1`) to write a report to +`coverage/`; CI always collects it. It is informational and never fails the run. + +## Documentation + +`lib/` is documented with YARD. `bundle exec rake yard_stats` lists anything +undocumented. Neither is enforced in CI, but new methods should come with docs. -The unit tests stub Fog, so they neither build instances nor require an -OpenStack account. +When you add or change a configuration option, update the configuration +reference in `README.md` as well. ## Manual testing -Changes that touch instance creation, networking, or credential resolution -should also be exercised against a real cloud, since the stubbed tests cannot -catch API-level regressions. +The unit tests never contact a cloud, so changes that touch instance creation, +networking, or credential resolution should also be exercised against a real +OpenStack deployment. Worth exercising separately, since they take different paths: @@ -66,18 +93,16 @@ Worth exercising separately, since they take different paths: Confirm after `kitchen destroy` that no instances remain, and that any floating IP allocated by the run was released. -## Submitting changes +## Opening a pull request -1. Fork the repository. -2. Create a feature branch off `main`. -3. Make your change, adding or updating tests to cover it. -4. Run the tests and the linter: `bundle exec rake spec` and - `bundle exec rake rubocop`. -5. Push the branch to your fork and open a pull request. +1. Fork the repo +2. Create a topic branch (`git checkout -b my-new-feature`) +3. Make your change, with tests +4. Run `bundle exec rake` +5. Push and open a pull request Please keep pull requests focused on a single change — it makes review much -faster. Update the documentation in `README.md` when you add or change a -configuration option. +faster. ## Release process diff --git a/Gemfile b/Gemfile index 293fa0df..02e13cfa 100644 --- a/Gemfile +++ b/Gemfile @@ -8,6 +8,11 @@ group :test do gem "rake" gem "kitchen-inspec" gem "rspec", "~> 3.2" + gem "simplecov", "~> 0.22" +end + +group :docs do + gem "yard" end group :debug do diff --git a/README.md b/README.md index a9ed2fa8..ed2fd36d 100644 --- a/README.md +++ b/README.md @@ -1,178 +1,214 @@ -# Kitchen::OpenStack +# kitchen-openstack ![Gem Version](https://img.shields.io/gem/v/kitchen-openstack.svg) ![CI](https://github.com/test-kitchen/kitchen-openstack/actions/workflows/lint.yml/badge.svg) -A Test Kitchen Driver for OpenStack. +A [Test Kitchen](https://kitchen.ci/) driver for OpenStack. -This driver uses the fog gem to provision and destroy nova instances. Use an OpenStack cloud for your infrastructure testing! +Test Kitchen builds a throwaway machine, converges your configuration code on +it, runs your tests, and destroys it. This driver makes that throwaway machine +a Nova instance in an OpenStack cloud, so you can test against the same +platform you deploy to. -Shamelessly copied from [Fletcher Nichol](https://github.com/fnichol)'s awesome work on an [EC2 driver](https://github.com/test-kitchen/kitchen-ec2), and [Adam Leff](https://github.com/adamleff)'s amazing work on an [VRO driver](https://github.com/chef-partners/kitchen-vro). +Maintained by the [OSU Open Source Lab](https://osuosl.org/). -## Status +> This documentation uses [Cinc Workstation](https://cinc.sh/) and the `cinc` +> commands throughout. Everything here works identically with Chef Workstation — +> see [Using with Chef](#using-with-chef). -This software project is actively maintained by the [OSU Open Source Lab](https://osuosl.org/). +--- -## Requirements +## Contents + +- [Requirements](#requirements) +- [Installation](#installation) +- [Quick start](#quick-start) +- [Authentication](#authentication) +- [Configuration reference](#configuration-reference) +- [Common setups](#common-setups) +- [Troubleshooting](#troubleshooting) +- [Using with Chef](#using-with-chef) +- [Contributing](#contributing) + +--- -There are **no** external system requirements for this driver. However you will need access to an OpenStack cloud. +## Requirements -## Installation and Setup +- Ruby 3.1 or newer +- Access to an OpenStack cloud, and credentials for it +- An SSH keypair uploaded to that cloud (Nova calls this a "keypair"; you + reference it by name as `key_name`) -This plugin ships out of the box with [Cinc Workstation](https://cinc.sh/start/workstation/), which is the easiest -way to make sure you always have the latest testing dependencies in a single package. If you have Cinc Workstation -installed, there is nothing else to install. +There are no other system requirements. The driver talks to OpenStack over +HTTPS using the [fog-openstack](https://github.com/fog/fog-openstack) library. -The examples below use the `cinc` commands. Everything here works identically with Chef Workstation — see -[Using with Chef](#using-with-chef). +## Installation -### Manual Installation +This driver ships with [Cinc +Workstation](https://cinc.sh/start/workstation/), which is the simplest way to +get Test Kitchen and its plugins in one package. It also ships with +[Chef Workstation](https://www.chef.io/downloads/tools/workstation). -Add this line to your application's Gemfile: +To install it yourself, add it to your `Gemfile`: ```ruby -gem 'kitchen-openstack' +gem "kitchen-openstack" ``` -And then execute: +then `bundle install`. Or install the gem directly: ```bash -bundle +gem install kitchen-openstack ``` -Or install it yourself as: +Confirm Test Kitchen can see it: ```bash -gem install kitchen-openstack +cinc kitchen driver discover | grep openstack ``` -## Quick Start +## Quick start -The least you need is credentials, an image, a flavor, and a keypair. If you already use the `openstack` CLI, source -your `openrc` and the driver picks the credentials up automatically: +This walks from nothing to a running instance. It assumes you already have a +project with a `kitchen.yml`. + +### 1. Get your credentials into your shell + +If you use the `openstack` CLI, you already have what you need. Most clouds +give you an `openrc` file to source, or a `clouds.yaml` in +`~/.config/openstack/`. Either works — this driver reads both, the same way +the CLI does: ```bash -source openrc.sh +source openrc.sh # sets OS_AUTH_URL, OS_USERNAME, OS_PASSWORD, ... +# or +export OS_CLOUD=mycloud # selects an entry from your existing clouds.yaml +``` + +Check that it works before involving Test Kitchen: + +```bash +openstack server list +``` + +If that fails, fix it first. Test Kitchen will fail the same way, with less +helpful output. + +### 2. Find an image and a flavor + +You need to tell the driver what to boot and how big: + +```bash +openstack image list +openstack flavor list +openstack keypair list ``` +### 3. Write your `kitchen.yml` + ```yaml --- driver: name: openstack - image_ref: ubuntu-22.04 + image_ref: ubuntu-24.04 # name, ID, or /regex/ flavor_ref: m1.small - key_name: my-keypair + key_name: my-keypair # a keypair already uploaded to OpenStack + +transport: + username: ubuntu # the image's default login user + ssh_key: ~/.ssh/my-keypair # the *private* half of key_name provisioner: name: cinc_infra verifier: - name: cinc_auditor + name: inspec platforms: - - name: ubuntu-22.04 + - name: ubuntu-24.04 suites: - name: default - run_list: - - recipe[my_cookbook::default] ``` -Then run the full test cycle: +Two things new users most often get wrong here: -```bash -cinc kitchen test -``` +- **`username` and `ssh_key` go under `transport:`, not `driver:`.** The driver + creates the machine; the transport logs into it. They are separate. +- **`key_name` is the name OpenStack knows; `ssh_key` is the private key file + on your disk.** They must be two halves of the same pair. -Or step through it: +### 4. Run it ```bash -cinc kitchen create # build the nova instance -cinc kitchen converge # apply your cookbook -cinc kitchen verify # run your tests -cinc kitchen destroy # delete the instance +cinc kitchen create # boot the instance +cinc kitchen converge # apply your configuration code +cinc kitchen verify # run your tests +cinc kitchen destroy # tear it down + +cinc kitchen test # all four, from scratch ``` -## Credentials +`cinc kitchen list` shows the state of each suite. If something goes wrong, jump to +[Troubleshooting](#troubleshooting). -There are three ways to give the driver credentials, described in full under -[Using `clouds.yaml`](#using-cloudsyaml) below: +## Authentication -- a `clouds.yaml` file, the same one the `openstack` CLI uses -- the standard `OS_*` environment variables, e.g. from an `openrc` file -- explicit `openstack_*` options in `kitchen.yml` +You can supply credentials three ways. You do not need to pick one globally — +they layer, and the precedence is fixed: -They combine, in that order of increasing precedence. +1. **`kitchen.yml`** — anything set explicitly here always wins +2. **`OS_*` environment variables** — override `clouds.yaml` +3. **`clouds.yaml`** (merged with `secure.yaml`) — the base -### Using `clouds.yaml` +This is the order the upstream OpenStack SDK uses, so it should match what the +`openstack` CLI does. -This driver supports OpenStack's standard +### Using `clouds.yaml` (recommended) + +OpenStack's standard [`clouds.yaml`](https://docs.openstack.org/python-openstackclient/latest/configuration/index.html) -client configuration file. This allows you to use the same credentials and -endpoint configuration that other OpenStack tools (like the `openstack` CLI) -already use, instead of duplicating them in `kitchen.yml`. +keeps credentials in one place shared by every OpenStack tool. If you have one, +use it — there is nothing to copy into `kitchen.yml`. -The driver searches for `clouds.yaml` in the standard locations: +The driver searches these locations and uses the first file it finds: -1. `OS_CLIENT_CONFIG_FILE` environment variable (if set) -2. `clouds_yaml_path` driver config option (if set) -3. Current directory (`./clouds.yaml`) +1. `$OS_CLIENT_CONFIG_FILE` +2. the `clouds_yaml_path` driver option +3. `./clouds.yaml` 4. `~/.config/openstack/clouds.yaml` 5. `/etc/openstack/clouds.yaml` -The first file found is used. A `secure.yaml` file in the same search -locations is also loaded and merged, so you can split secrets out of -`clouds.yaml` following the -[standard convention](https://docs.openstack.org/openstacksdk/latest/user/config/configuration.html#splitting-secrets). - -#### Selecting a cloud - -Specify which cloud entry to use in one of two ways: - -- Set `openstack_cloud` in `kitchen.yml` (takes precedence) -- Set the `OS_CLOUD` environment variable +A `secure.yaml` is searched for in the same locations — with +`$OS_CLIENT_SECURE_FILE` in place of `$OS_CLIENT_CONFIG_FILE` — and merged on +top, so you can keep secrets in a separate file following the [standard +convention](https://docs.openstack.org/openstacksdk/latest/user/config/configuration.html#splitting-secrets). -#### Example `kitchen.yml` +Select which cloud entry to use with either `OS_CLOUD` or the +`openstack_cloud` driver option: ```yaml driver: name: openstack openstack_cloud: mycloud - image_ref: ubuntu-22.04 + image_ref: ubuntu-24.04 flavor_ref: m1.small key_name: my-keypair ``` -Or, relying entirely on `OS_CLOUD`: - -```bash -export OS_CLOUD=mycloud -``` - -```yaml -driver: - name: openstack - image_ref: ubuntu-22.04 - flavor_ref: m1.small - key_name: my-keypair -``` - -Settings specified in `kitchen.yml` always take precedence over values from -`clouds.yaml`. For example, you can override just the region: +Because `kitchen.yml` wins, you can adopt a cloud entry and override one piece +of it: ```yaml driver: name: openstack openstack_cloud: mycloud - openstack_region: RegionTwo + openstack_region: RegionTwo # everything else still comes from clouds.yaml ``` -#### Using `OS_*` environment variables +### Using `OS_*` environment variables -The driver recognizes the standard OpenStack `OS_*` environment variables -(e.g. from an `openrc` file). This means you can source your OpenStack -credentials and use them directly without any extra configuration in -`kitchen.yml`: +Sourcing an `openrc` file is enough on its own: ```bash source openrc.sh @@ -181,15 +217,16 @@ source openrc.sh ```yaml driver: name: openstack - image_ref: ubuntu-22.04 + image_ref: ubuntu-24.04 flavor_ref: m1.small key_name: my-keypair ``` -The supported environment variables are: +Recognized variables: -| Env var | Maps to | +| Environment variable | Driver option | | --- | --- | +| `OS_CLOUD` | `openstack_cloud` | | `OS_AUTH_URL` | `openstack_auth_url` | | `OS_USERNAME` | `openstack_username` | | `OS_PASSWORD` | `openstack_api_key` | @@ -208,184 +245,311 @@ The supported environment variables are: | `OS_APPLICATION_CREDENTIAL_SECRET` | `openstack_application_credential_secret` | | `OS_CACERT` | `ssl_ca_file` | -#### Configuration precedence +An exported-but-empty variable is ignored, so `OS_REGION_NAME=""` will not +shadow a region set in `clouds.yaml`. -The driver follows the upstream OpenStack SDK precedence order: +### Putting credentials in `kitchen.yml` -1. **`kitchen.yml`** — explicit driver config always wins -2. **`OS_*` env vars** — override `clouds.yaml` values -3. **`clouds.yaml`** (merged with `secure.yaml`) — base configuration +Supported, but avoid committing secrets. Prefer reading them from the +environment: -#### New driver config options +```yaml +driver: + name: openstack + openstack_username: <%= ENV["OS_USERNAME"] %> + openstack_api_key: <%= ENV["OS_PASSWORD"] %> + openstack_auth_url: https://keystone.example.com:5000/v3 + openstack_domain_id: default + openstack_project_name: my-project +``` -| Option | Default | Description | -| --- | --- | --- | -| `openstack_cloud` | `nil` | Name of the cloud entry in `clouds.yaml`. Falls back to the `OS_CLOUD` env var. | -| `clouds_yaml_path` | `nil` | Explicit path to a `clouds.yaml` file, inserted into the search path. | +### Application credentials -## Configuration +Preferred over a password where your cloud supports them: -All options below are set under the `driver:` key in `kitchen.yml`, or per platform under `platforms[].driver:`. +```yaml +driver: + name: openstack + openstack_auth_url: https://keystone.example.com:5000/v3 + openstack_application_credential_id: <%= ENV["OS_APPLICATION_CREDENTIAL_ID"] %> + openstack_application_credential_secret: <%= ENV["OS_APPLICATION_CREDENTIAL_SECRET"] %> +``` -Credential options (`openstack_auth_url`, `openstack_username`, `openstack_api_key`, `openstack_project_name`, -and the rest of the `openstack_*` family) are listed in -[Using `OS_*` environment variables](#using-os_-environment-variables) above, alongside the environment variable -each one maps to. +## Configuration reference -### Image and flavor +Everything below goes under `driver:` in `kitchen.yml`. -Give **either** the `_ref` or the `_id` form of each, never both — the driver raises `ActionFailed` if you set both. +### Choosing what to boot | Option | Default | Description | | --- | --- | --- | -| `image_ref` | *none* | Image to boot, by name, ID, or regular expression. | -| `image_id` | *none* | Image to boot, by exact ID. Cannot be combined with `image_ref`. | -| `flavor_ref` | *none* | Flavor to use, by name, ID, or regular expression. | -| `flavor_id` | *none* | Flavor to use, by exact ID. Cannot be combined with `flavor_ref`. | +| `image_ref` | — | Image to boot, by name, ID, or `/regex/`. Mutually exclusive with `image_id`. | +| `image_id` | — | Image UUID, used verbatim with no lookup. | +| `flavor_ref` | — | Flavor, by name, ID, or `/regex/`. Mutually exclusive with `flavor_id`. | +| `flavor_id` | — | Flavor UUID, used verbatim with no lookup. | +| `key_name` | `nil` | Name of an SSH keypair already uploaded to OpenStack. | +| `availability_zone` | `nil` | Availability zone to boot into. | +| `security_groups` | `nil` | List of security group names. **Must be a list**, even for one group. | +| `metadata` | `nil` | Hash of Nova instance metadata. | + +Set exactly one of `image_ref`/`image_id`, and exactly one of +`flavor_ref`/`flavor_id`. Setting both members of a pair is an error. + +The `_ref` options accept three forms: + +```yaml +image_ref: 8a8c0f4d-... # exact ID (checked first) +image_ref: ubuntu-24.04 # exact name +image_ref: /^ubuntu-24\.04/ # regex, first match wins +``` -### Instance +### Naming the instance | Option | Default | Description | | --- | --- | --- | -| `server_name` | *generated* | Name of the instance. Generated from the suite and platform if unset. | -| `server_name_prefix` | `nil` | Prefix for the generated name. Ignored when `server_name` is set. | -| `availability_zone` | *scheduler chooses* | Availability zone to launch into. | -| `security_groups` | *project default* | Array of security group names to apply. | -| `key_name` | `nil` | Name of an existing OpenStack keypair to inject. | -| `metadata` | `nil` | Hash of instance metadata. | -| `block_device_mapping` | `nil` | Hash describing a block device to boot from or attach. See [Block device mapping](#block-device-mapping). | -| `user_data` | *unset* | Path to a user data file passed to the instance. Cannot be combined with `cloud_config`. | -| `cloud_config` | *unset* | Inline cloud-init configuration. Cannot be combined with `user_data`. | +| `server_name` | generated | Exact instance name. Overrides everything below. | +| `server_name_prefix` | `nil` | Prefix, plus a random 8-character suffix. | + +With neither set, the name is `---`, +truncated to OpenStack's 63-character limit. Non-word characters are stripped. ### Networking | Option | Default | Description | | --- | --- | --- | -| `openstack_network_name` | `nil` | Name of the network to attach to, and to take the instance's address from. | -| `network_ref` | `nil` | Network to attach, by name, ID, or an array of either, for multiple NICs. | -| `network_id` | `nil` | Network to attach, by exact ID. | -| `floating_ip` | `nil` | Specific floating IP to associate with the instance. | -| `floating_ip_pool` | `nil` | Pool to allocate a floating IP from. | -| `allocate_floating_ip` | `false` | Allocate a new floating IP, and release it again on destroy. | -| `use_ipv6` | `false` | Connect over the instance's IPv6 address. | -| `public_ip_order` | `0` | Index of the public address to use when the instance has several. | -| `private_ip_order` | `0` | Index of the private address to use when the instance has several. | -| `port` | `"22"` | SSH port to connect to. | - -### Waiting and timeouts +| `network_ref` | `nil` | Network(s) to attach, by name, ID, or `/regex/`. A string or a list. Mutually exclusive with `network_id`. | +| `network_id` | `nil` | Network UUID(s), used verbatim. A string or a list. | +| `openstack_network_name` | `nil` | Which network's address Test Kitchen should connect to. | +| `floating_ip` | `nil` | A specific floating IP to attach. | +| `floating_ip_pool` | `nil` | Pool (external network) to take a floating IP from. | +| `allocate_floating_ip` | `false` | Allocate a *new* floating IP rather than reusing a free one. Released on `destroy`. | +| `public_ip_order` | `0` | Index into the public addresses when several exist. | +| `private_ip_order` | `0` | Index into the private addresses when several exist. | +| `use_ipv6` | `false` | Connect over IPv6 instead of IPv4. | + +Address selection, in order: `floating_ip` if set, then +`openstack_network_name` if set, then the public addresses at +`public_ip_order`, then the private addresses at `private_ip_order`. + +### Storage | Option | Default | Description | | --- | --- | --- | -| `server_wait` | *unset* | Seconds to sleep after the instance is active, before connecting. Useful when an image needs time to finish booting. | -| `no_ssh_tcp_check` | `false` | Skip the TCP check on the SSH port. Use when a firewall makes the check unreliable. | -| `no_ssh_tcp_check_sleep` | `120` | Seconds to sleep instead of checking, when `no_ssh_tcp_check` is enabled. | -| `glance_cache_wait_timeout` | `600` | Seconds to wait while Glance caches the image before the instance can boot. | -| `connect_timeout` | `60` | Seconds to wait when opening an API connection. | -| `read_timeout` | `60` | Seconds to wait for an API read. | -| `write_timeout` | `60` | Seconds to wait for an API write. | +| `block_device_mapping` | `nil` | Boot from, or attach, a Cinder volume. See [below](#booting-from-a-volume). | -### API and endpoints +### Instance customization | Option | Default | Description | | --- | --- | --- | -| `openstack_region` | `$OS_REGION_NAME` | Region to operate in. | -| `openstack_service_name` | `nil` | Compute service name in the catalog, when the deployment uses a non-standard one. | -| `disable_ssl_validation` | `false` | Skip TLS certificate validation. Only use this against a deployment with an invalid certificate. | -| `openstack_cloud` | `nil` | Name of the cloud entry in `clouds.yaml`. Falls back to `OS_CLOUD`. | -| `clouds_yaml_path` | `nil` | Explicit path to a `clouds.yaml` file, inserted into the search path. | +| `user_data` | `nil` | Path to a cloud-init file. **Must exist**, or `create` fails. | +| `cloud_config` | `nil` | Inline cloud-config as YAML, rendered for you. Mutually exclusive with `user_data`. | +| `config_drive` | `nil` | Attach a config drive. | + +### Connection and timeouts -## Block device mapping +| Option | Default | Description | +| --- | --- | --- | +| `connect_timeout` | `60` | Seconds to wait establishing an API connection. | +| `read_timeout` | `60` | Seconds to wait reading an API response. | +| `write_timeout` | `60` | Seconds to wait writing an API request. | +| `glance_cache_wait_timeout` | `600` | Seconds to wait for the instance to reach `ACTIVE`. Raise it if your cloud caches images slowly on first boot. | +| `server_wait` | `nil` | Extra seconds to sleep after boot before trying SSH. A blunt instrument; try it if your instances need a moment before accepting connections. | +| `disable_ssl_validation` | `false` | Skip TLS verification. Prefer `ssl_ca_file`. | +| `ssl_ca_file` | `nil` | Path to a CA bundle, passed to the HTTP connection. Set by `OS_CACERT` or a `cacert` entry in `clouds.yaml`. | -`block_device_mapping` boots the instance from a volume rather than the image directly, or attaches an extra volume: +### Credentials and endpoints + +| Option | Default | Description | +| --- | --- | --- | +| `openstack_cloud` | `nil` | Cloud entry to read from `clouds.yaml`. Falls back to `OS_CLOUD`. | +| `clouds_yaml_path` | `nil` | Explicit `clouds.yaml` path, inserted into the search path. | +| `openstack_auth_url` | `nil` | Keystone endpoint. | +| `openstack_username` | `nil` | Username. | +| `openstack_api_key` | `nil` | Password. | +| `openstack_project_name` | `nil` | Project (tenant) name. | +| `openstack_project_id` | `nil` | Project ID. | +| `openstack_domain_id` | `nil` | Domain ID. Usually `default`. | +| `openstack_domain_name` | `nil` | Domain name. | +| `openstack_user_domain` | `nil` | User domain name. | +| `openstack_user_domain_id` | `nil` | User domain ID. | +| `openstack_project_domain` | `nil` | Project domain name. | +| `openstack_project_domain_id` | `nil` | Project domain ID. | +| `openstack_region` | `nil` | Region name. | +| `openstack_endpoint_type` | `nil` | Endpoint interface: `public`, `internal`, or `admin`. | +| `openstack_identity_api_version` | `nil` | Keystone API version. | +| `openstack_service_name` | `nil` | Compute service name. | +| `openstack_application_credential_id` | `nil` | Application credential ID. | +| `openstack_application_credential_secret` | `nil` | Application credential secret. | +| `openstack_tenant` | `nil` | Tenant name. The Keystone v2 name for a project; use `openstack_project_name` on v3. | +| `openstack_tenant_id` | `nil` | Tenant ID. The Keystone v2 name for a project ID. | +| `openstack_service_type` | `nil` | Compute service type to look up in the catalog. | + +Any other `openstack_*` option that fog-openstack recognizes is forwarded as +well, including `openstack_auth_token`, `openstack_identity_endpoint`, +`openstack_management_url`, and `openstack_cache_ttl`. To see the full list +your installed version supports: + +```bash +ruby -r fog/openstack -e 'puts Fog::OpenStack::Compute.recognized.grep(/^openstack/).sort' +``` + +### Settings that are not driver options + +Commonly mistaken for driver options: + +- **`username`, `ssh_key`, `port`, `connection_timeout`** belong under + `transport:`. The driver builds the instance; the transport connects to it. +- **`no_ssh_tcp_check` and `no_ssh_tcp_check_sleep`** are accepted but have no + effect. They are leftovers from an earlier version and are not read anywhere + in the driver. +- **`pre_create_command`** is declared by Test Kitchen's base driver, but this + driver overrides `create` without invoking it, so setting it does nothing + here. + +## Common setups + +### Attaching a floating IP + +Reuse an already-allocated but unattached address from a pool: ```yaml driver: name: openstack - image_ref: ubuntu-22.04 - flavor_ref: m1.small - block_device_mapping: - make_volume: true - snapshot_id: 5e4e5d5e-1f1f-4b4b-9c9c-2d2d3e3e4f4f - device_name: vda - volume_size: 20 - volume_id: null - availability_zone: nova - delete_on_termination: true + floating_ip_pool: public ``` -## Examples - -### Allocating a floating IP +Or allocate a fresh one, which is released again on `kitchen destroy`: ```yaml driver: name: openstack - image_ref: ubuntu-22.04 - flavor_ref: m1.small - key_name: my-keypair floating_ip_pool: public allocate_floating_ip: true ``` -### Several networks +Or pin a specific address: + +```yaml +driver: + name: openstack + floating_ip: 203.0.113.10 +``` + +### Choosing a network + +```yaml +driver: + name: openstack + network_ref: my-private-net # name, ID, or /regex/ +``` + +Attach several: ```yaml driver: name: openstack - image_ref: ubuntu-22.04 - flavor_ref: m1.small network_ref: - - management - - storage - openstack_network_name: management + - my-private-net + - my-storage-net + openstack_network_name: my-private-net # which one to connect over ``` -### cloud-init +### Running cloud-init + +Inline, which is usually easier to read: ```yaml driver: name: openstack - image_ref: ubuntu-22.04 - flavor_ref: m1.small cloud_config: packages: - htop + runcmd: + - [systemctl, restart, sshd] ``` -### A slow image +Or from a file, which must exist: ```yaml driver: name: openstack - image_ref: ubuntu-22.04 - flavor_ref: m1.small - server_wait: 60 - glance_cache_wait_timeout: 1200 - no_ssh_tcp_check: true + user_data: files/cloud-init.yml ``` -### Per-platform images +### Booting from a volume ```yaml driver: name: openstack - flavor_ref: m1.small - key_name: my-keypair + block_device_mapping: + make_volume: true + volume_size: 20 + device_name: vda + delete_on_termination: true + creation_timeout: 60 # seconds to wait for the volume to be available + attach_timeout: 5 # extra seconds before attaching +``` -platforms: - - name: ubuntu-22.04 - driver: - image_ref: ubuntu-22.04 - - name: rockylinux-9 - driver: - image_ref: rocky-9 +`make_volume: true` creates a new volume; source it from `snapshot_id`, +`imageRef`, or `source_volid`. To attach a volume you already have, drop +`make_volume` and give `volume_id`. + +### A cloud with a private CA + +Point at your CA bundle rather than turning verification off: + +```bash +export OS_CACERT=/etc/ssl/certs/my-ca.pem +``` + +or in `clouds.yaml`: + +```yaml +clouds: + mycloud: + cacert: /etc/ssl/certs/my-ca.pem ``` +Either is passed through to the HTTP connection. `verify: false` in +`clouds.yaml` disables verification entirely, equivalent to setting +`disable_ssl_validation: true`. + +## Troubleshooting + +Start with `kitchen diagnose`, which prints the fully resolved driver config +after `clouds.yaml` and `OS_*` have been merged in. If a credential is not +there, the driver never saw it. + +```bash +kitchen diagnose --all +kitchen create --log-level=debug +``` + +| Symptom | Likely cause | +| --- | --- | +| `Image not found` / `Flavor not found` | The `_ref` matched nothing. Check `openstack image list`. A `/regex/` needs the surrounding slashes. | +| `Cannot specify both image_ref and image_id` | Set one, not both. Same for flavor and network. | +| `Could not find an IP` | The instance has no address of the family you asked for. Check `use_ipv6`, and whether you need a floating IP. | +| `Server is not attached to network ` | `openstack_network_name` does not match any network on the instance. | +| `Floating IP pool not found` | The pool name is wrong; it is the external *network* name. | +| `No available IPs in pool ` | Every address is in use. Set `allocate_floating_ip: true` to make a new one. | +| `The user_data file does not exist` | The path is wrong. It is resolved relative to where you run `kitchen`. | +| `The security_groups config must be an array` | Use a list, even for a single group. | +| Hangs at "Waiting for server to be ready" | The instance booted but SSH is unreachable. Check the security group allows port 22, that a floating IP is attached if you need one, and that `transport: username:` matches the image's default user. | +| Times out reaching `ACTIVE` | Raise `glance_cache_wait_timeout`. First boot of a large image can be slow. | +| TLS errors | Set `ssl_ca_file` (or `OS_CACERT`) to your CA bundle. | + +The instance is destroyed automatically if it never becomes reachable, so a +failed `kitchen create` should not leak a server. If one does leak, `kitchen +destroy` or `openstack server delete` will clear it. + ## Using with Chef -This driver is not tied to Cinc. The examples above use Cinc Workstation and the `cinc_infra` provisioner, but the -driver works exactly the same with [Chef Workstation](https://www.chef.io/downloads/tools/workstation) — run -`kitchen` instead of `cinc kitchen`, and use `chef_infra` instead of `cinc_infra`: +This driver is not tied to Cinc. The examples above use Cinc Workstation and the +`cinc_infra` provisioner, but the driver works exactly the same with +[Chef Workstation](https://www.chef.io/downloads/tools/workstation) — run +`kitchen` instead of `cinc kitchen`, and use `chef_infra` instead of +`cinc_infra`: ```yaml provisioner: @@ -399,13 +563,22 @@ No driver configuration changes are needed. ## Contributing -Pull requests are very welcome on [GitHub](https://github.com/test-kitchen/kitchen-openstack). See -[CONTRIBUTING.md](CONTRIBUTING.md) for development setup, how to run the tests, and the release process. +Pull requests are welcome. See [CONTRIBUTING.md](CONTRIBUTING.md) for +development setup, how to run the tests, how the suite is built, and the +release process. + +## Credits + +Originally created by Jonathan Hartman. -## Authors +Structure borrowed from [Fletcher Nichol](https://github.com/fnichol)'s +[kitchen-ec2](https://github.com/test-kitchen/kitchen-ec2) and [Adam +Leff](https://github.com/adamleff)'s +[kitchen-vro](https://github.com/chef-partners/kitchen-vro). -Created by Jonathan Hartman +Further reference documentation is at +. ## License -Apache 2.0 (see LICENSE.txt file) +Apache 2.0. See [LICENSE.txt](LICENSE.txt). diff --git a/Rakefile b/Rakefile index 10cbfa5f..18cf778b 100644 --- a/Rakefile +++ b/Rakefile @@ -6,6 +6,12 @@ RSpec::Core::RakeTask.new(:unit) desc "Run all test suites" task test: [:unit] +desc "Run the unit tests with coverage reporting to coverage/" +task :coverage do + ENV["COVERAGE"] = "1" + Rake::Task[:unit].invoke +end + begin require "cookstyle" desc "Run cookstyle with chefstyle rules" @@ -16,6 +22,18 @@ rescue LoadError puts "cookstyle is not available. gem install cookstyle to do style checking." end +begin + require "yard" + YARD::Rake::YardocTask.new(:yard) + + desc "List methods missing YARD documentation" + task :yard_stats do + sh "yard stats --list-undoc" + end +rescue LoadError + puts "yard is not available. gem install yard to generate documentation." +end + desc "Run all quality tasks" task quality: %i{style} diff --git a/lib/kitchen/driver/openstack.rb b/lib/kitchen/driver/openstack.rb index e0003f06..922ee242 100755 --- a/lib/kitchen/driver/openstack.rb +++ b/lib/kitchen/driver/openstack.rb @@ -34,8 +34,17 @@ module Kitchen module Driver - # This takes from the Base Class and creates the OpenStack driver. + # Test Kitchen driver for OpenStack Nova. + # + # Creates and destroys Nova instances, optionally attaching floating IPs, + # Cinder volumes and specific Neutron networks. Credentials come from + # kitchen.yml, `OS_*` environment variables, or a standard + # `clouds.yaml` -- see {Clouds} for the precedence rules. class Openstack < Kitchen::Driver::Base + # Settings Fog requires as Strings. Fog re-coerces anything that looks + # numeric back to an Integer, so these are stringified on the way in. + # + # @return [Array] FOG_STRING_SETTINGS = %i{ openstack_username openstack_api_key @@ -70,13 +79,6 @@ class Openstack < Kitchen::Driver::Base default_config :clouds_yaml_path, nil default_config :server_name, nil - # Merge clouds.yaml values into config so they are visible in - # `kitchen diagnose` and available to all driver methods. - def finalize_config!(instance) - super - apply_clouds_config - self - end default_config :server_name_prefix, nil default_config :key_name, nil default_config :port, "22" @@ -103,6 +105,28 @@ def finalize_config!(instance) default_config :write_timeout, 60 default_config :metadata, nil + # Merges clouds.yaml and `OS_*` values into the config hash. + # + # Done at finalize time rather than lazily so the resolved values show up + # in `kitchen diagnose` and are available to every driver method. + # + # @param instance [Kitchen::Instance] the instance this driver serves + # @return [self] + def finalize_config!(instance) + super + apply_clouds_config + self + end + + # Creates a Nova instance and waits until it is reachable. + # + # Idempotent: returns immediately if `state` already names a server. + # + # @param state [Hash] mutable instance state; gains `:server_id` and + # `:hostname` + # @return [void] + # @raise [Kitchen::ActionFailed] on any Fog or Excon failure + # @raise [Kitchen::InstanceFailure] if the server builds to ERROR state def create(state) config_server_name if state[:server_id] @@ -138,6 +162,14 @@ def create(state) raise ActionFailed, e.message end + # Destroys the Nova instance named by `state`, releasing its floating IP + # first when this driver allocated one. + # + # Safe to call when the server is already gone. + # + # @param state [Hash] mutable instance state; loses `:server_id` and + # `:hostname` + # @return [void] def destroy(state) return if state[:server_id].nil? @@ -150,12 +182,7 @@ def destroy(state) pub, priv = get_public_private_ips(server) pub, = parse_ips(pub, priv) pub_ip = pub[config[:public_ip_order].to_i] || nil - if pub_ip - info "Retrieve the ID of floating IP <#{pub_ip}>" - floating_ip_id = network.list_floating_ips(floating_ip_address: pub_ip).body["floatingips"][0]["id"] - network.delete_floating_ip(floating_ip_id) - info "OpenStack Floating IP <#{pub_ip}> released." - end + release_floating_ip(pub_ip) if pub_ip end server.destroy end @@ -166,6 +193,29 @@ def destroy(state) private + # Releases a floating IP back to its pool. + # + # A floating IP that Neutron no longer knows about is not an error worth + # failing a destroy over, so an unknown address is logged and skipped. + # + # @param pub_ip [String] the floating IP to release + # @return [void] + def release_floating_ip(pub_ip) + info "Retrieve the ID of floating IP <#{pub_ip}>" + net = network + floating_ips = net.list_floating_ips(floating_ip_address: pub_ip).body["floatingips"] + if floating_ips.nil? || floating_ips.empty? + warn "No floating IP found matching <#{pub_ip}>; nothing to release." + return + end + + net.delete_floating_ip(floating_ips[0]["id"]) + info "OpenStack Floating IP <#{pub_ip}> released." + end + + # Builds the settings hash handed to every Fog service constructor. + # + # @return [Hash] Fog connection settings def openstack_server server_def = { connection_options: {}, @@ -176,36 +226,61 @@ def openstack_server server_def end + # Settings always sent to Fog, even when nil. + # + # @return [Array] def required_server_settings %i{openstack_username openstack_api_key openstack_auth_url openstack_domain_id} end + # Every other `openstack_*` setting Fog recognizes, sent only when set. + # + # @return [Array] def optional_server_settings Fog::OpenStack::Compute.recognized.select do |k| k.to_s.start_with?("openstack") end - required_server_settings end + # Settings passed through to Excon rather than to Fog itself. + # + # `ssl_ca_file` belongs here, not in the Fog settings: Fog does not + # recognize it, so a CA bundle from `OS_CACERT` or a clouds.yaml + # `cacert` entry would otherwise be parsed and then silently dropped. + # + # @return [Array] def connection_options - %i{read_timeout write_timeout connect_timeout} + %i{read_timeout write_timeout connect_timeout ssl_ca_file} end + # @return [Fog::OpenStack::Network] a Neutron connection def network Fog::OpenStack::Network.new(openstack_server) end + # @return [Fog::OpenStack::Compute] a Nova connection def compute Fog::OpenStack::Compute.new(openstack_server) end + # @return [Kitchen::Driver::Openstack::Volume] a Cinder helper def volume Volume.new(logger) end + # Resolves the block device mapping to send to Nova. + # + # @param config [Hash] the driver config + # @return [Hash] a Nova block device mapping def get_bdm(config) volume.get_bdm(config, openstack_server) end + # Coerces one setting to the type Fog expects. + # + # @param setting [Symbol] the Fog config key + # @param value [Object] the configured value + # @return [Object] the coerced value def normalize_fog_setting(setting, value) return value if value.nil? return normalize_identity_api_version(value) if setting == :openstack_identity_api_version @@ -214,11 +289,20 @@ def normalize_fog_setting(setting, value) value.to_s end + # Normalizes the identity API version into a form Fog will not mangle. + # # Fog::Service#coerce_options re-coerces any value where # `value.to_s.to_i.to_s == value.to_s` back to an Integer, which # then breaks Fog::OpenStack::Auth::Token.build (it calls `=~` # on the value). Prefixing with "v" keeps Fog from coercing and # still satisfies Token.build's `/(v)*2(\.0)*/i` regex check. + # + # @example + # normalize_identity_api_version(3) #=> "v3" + # normalize_identity_api_version("2.0") #=> "v2.0" + # + # @param value [String, Integer] the configured identity API version + # @return [String] a version string prefixed with "v" def normalize_identity_api_version(value) str = value.to_s.strip return str if str.empty? diff --git a/lib/kitchen/driver/openstack/clouds.rb b/lib/kitchen/driver/openstack/clouds.rb index 9be10814..de614e9a 100644 --- a/lib/kitchen/driver/openstack/clouds.rb +++ b/lib/kitchen/driver/openstack/clouds.rb @@ -76,9 +76,12 @@ module Clouds private # Merges external config sources into the driver config hash. - # Precedence: kitchen.yml > OS_* env vars > clouds.yaml - # Only sets keys that are currently nil so that kitchen.yml - # values always take precedence. + # + # Precedence, highest first: kitchen.yml, `OS_*` environment + # variables, then clouds.yaml. Only keys that are currently nil are + # written, so anything set explicitly in kitchen.yml always wins. + # + # @return [void] def apply_clouds_config cc = load_clouds_config env = load_env_vars @@ -91,15 +94,23 @@ def apply_clouds_config config[key] = value if config[key].nil? end - # Apply SSL settings: env vars or clouds.yaml disabling verification - ssl_verify = env.key?(:ssl_verify_peer) ? env[:ssl_verify_peer] : cc[:ssl_verify_peer] - return unless ssl_verify == false && !config[:disable_ssl_validation] + # `verify: false` in clouds.yaml is how operators opt out of TLS + # verification. openstacksdk documents no environment variable for it + # (though its env loader does sweep up any OS_* name into an implicit + # cloud), and ENV_VAR_MAP deliberately mirrors the documented set -- + # so for this driver clouds.yaml is the only source. + return unless cc[:ssl_verify_peer] == false && !config[:disable_ssl_validation] config[:disable_ssl_validation] = true end - # Reads OS_* environment variables and maps them to Fog config keys. - # Returns a hash of fog config symbols for any set env vars. + # Reads `OS_*` environment variables and maps them to Fog config keys. + # + # Empty variables are ignored, so `OS_REGION_NAME=""` does not shadow + # a region set in clouds.yaml. + # + # @return [Hash{Symbol => Object}] Fog config keys for the variables + # that are set def load_env_vars result = {} ENV_VAR_MAP.each do |env_var, fog_key| @@ -109,57 +120,136 @@ def load_env_vars result end - # Resolves the cloud name from config or the OS_CLOUD environment variable + # Resolves which named cloud to read out of clouds.yaml. + # + # @return [String, nil] the cloud name, or nil if none is configured def cloud_name config[:openstack_cloud] || ENV["OS_CLOUD"] end # Loads and merges clouds.yaml with secure.yaml, then translates the # named cloud entry into Fog-compatible config keys. - # Returns a hash of fog config symbols, or empty hash if no cloud configured. + # + # secure.yaml wins over clouds.yaml, matching openstacksdk. + # + # @return [Hash{Symbol => Object}] Fog config keys, or an empty hash + # when no cloud is configured or the named cloud is absent def load_clouds_config name = cloud_name return {} unless name - clouds_data = load_yaml_file("clouds.yaml", "OS_CLIENT_CONFIG_FILE") - secure_data = load_yaml_file("secure.yaml", "OS_CLIENT_SECURE_FILE") + clouds_data, clouds_path = load_yaml_file("clouds.yaml", "OS_CLIENT_CONFIG_FILE") + secure_data, secure_path = load_yaml_file("secure.yaml", "OS_CLIENT_SECURE_FILE") - cloud = extract_cloud(clouds_data, name) - secure = extract_cloud(secure_data, name) + cloud = extract_cloud(clouds_data, name, clouds_path) + secure = extract_cloud(secure_data, name, secure_path) cloud = deep_merge(cloud, secure) translate_cloud_config(cloud) end - # Search standard OpenStack config file locations for the given filename + # Loads the first of the standard OpenStack config locations that + # exists. + # + # @param filename [String] `"clouds.yaml"` or `"secure.yaml"` + # @param env_var [String] the environment variable that overrides the + # search path for this file + # @return [Array(Hash, String), Array(Hash, nil)] the parsed document + # and the path it came from; an empty hash and nil if no file was + # found + # @raise [Kitchen::ActionFailed] if the file exists but is not valid + # YAML, or does not parse to a mapping def load_yaml_file(filename, env_var) paths = clouds_yaml_search_paths(filename, env_var) path = paths.find { |p| File.exist?(p) } - return {} unless path + return [{}, nil] unless path debug "Loading #{filename} from #{path}" - YAML.safe_load(File.read(path), permitted_classes: [Date]) || {} # rubocop: disable Style/YAMLFileRead + data = parse_yaml(path) + + # A clouds.yaml that parses to a list or a scalar is a mistake worth + # naming, rather than a NoMethodError three frames later. + raise ActionFailed, "#{path} must contain a YAML mapping" unless data.is_a?(Hash) + + [data, path] + end + + # Parses one YAML document, turning a syntax error into a message that + # names the offending file. + # + # @param path [String] the file to parse + # @return [Object] the parsed document + # @raise [Kitchen::ActionFailed] if the file is not valid YAML + def parse_yaml(path) + YAML.safe_load_file(path, permitted_classes: [Date]) || {} + rescue Psych::Exception => e + raise ActionFailed, "Could not parse #{path}: #{e.message}" end + # Standard OpenStack client config search locations, highest priority + # first. + # + # @param filename [String] the file being looked for + # @param env_var [String] the environment variable that overrides it + # @return [Array] candidate paths def clouds_yaml_search_paths(filename, env_var) paths = [] paths << ENV[env_var] if ENV[env_var] paths << config[:clouds_yaml_path] if config[:clouds_yaml_path] && filename == "clouds.yaml" paths << File.join(Dir.pwd, filename) - paths << File.join(Dir.home, ".config", "openstack", filename) + home = user_home + paths << File.join(home, ".config", "openstack", filename) if home paths << File.join("/etc/openstack", filename) paths end - def extract_cloud(data, name) - clouds = data["clouds"] || {} + # The user's home directory, or nil if there isn't one. + # + # `Dir.home` raises when HOME is unset and the uid has no passwd entry, + # which is the ordinary state inside a container running as an arbitrary + # uid. That must not take out the whole search -- /etc/openstack is + # still worth trying. + # + # @return [String, nil] the home directory, or nil if it cannot be + # determined + def user_home + Dir.home + rescue ArgumentError + nil + end + + # Pulls one named cloud entry out of a parsed clouds/secure document. + # + # An absent entry is an empty hash: clouds.yaml and secure.yaml are + # merged, and it is normal for a cloud to appear in only one of them. + # An entry that is *present but not a mapping* is always a mistake, and + # is reported rather than silently discarded -- otherwise the driver + # carries on with nil credentials and the user sees an opaque Keystone + # auth failure that never mentions their config file. + # + # @param data [Hash] the parsed document + # @param name [String] the cloud name to extract + # @param source [String] the file the document came from, for errors + # @return [Hash] the cloud entry, or an empty hash if it is absent + # @raise [Kitchen::ActionFailed] if `clouds` or the named entry is + # present but is not a mapping + def extract_cloud(data, name, source) + clouds = data["clouds"] + return {} if clouds.nil? + raise ActionFailed, "The clouds section of #{source} must be a YAML mapping" unless clouds.is_a?(Hash) + cloud = clouds[name] - return {} unless cloud + return {} if cloud.nil? + raise ActionFailed, "Cloud <#{name}> in #{source} must be a YAML mapping" unless cloud.is_a?(Hash) cloud end - # Deep merge two hashes (secure overrides clouds) + # Recursively merges `override` onto `base` without mutating either. + # + # @param base [Hash] the lower-priority hash + # @param override [Hash] the higher-priority hash + # @return [Hash] a new merged hash def deep_merge(base, override) result = base.dup override.each do |key, value| @@ -172,7 +262,10 @@ def deep_merge(base, override) result end - # Convert a clouds.yaml cloud entry into Fog-compatible config keys + # Converts a clouds.yaml cloud entry into Fog-compatible config keys. + # + # @param cloud [Hash] a single cloud entry + # @return [Hash{Symbol => Object}] Fog config keys def translate_cloud_config(cloud) result = {} @@ -196,6 +289,14 @@ def translate_cloud_config(cloud) result end + # Coerces values Fog insists on receiving as strings. + # + # YAML parses `identity_api_version: 3` and `project_id: 12345` as + # Integers, which Fog then fails on. + # + # @param fog_key [Symbol] the Fog config key + # @param value [Object] the raw value from YAML or the environment + # @return [Object] the value, stringified when the key requires it def normalize_config_value(fog_key, value) return value unless STRING_CONFIG_KEYS.include?(fog_key) diff --git a/lib/kitchen/driver/openstack/config.rb b/lib/kitchen/driver/openstack/config.rb index 6674c545..9e83ec1d 100644 --- a/lib/kitchen/driver/openstack/config.rb +++ b/lib/kitchen/driver/openstack/config.rb @@ -21,14 +21,66 @@ # See the License for the specific language governing permissions and # limitations under the License. +require "etc" unless defined?(Etc) +require "socket" unless defined?(Socket) + module Kitchen module Driver class Openstack < Kitchen::Driver::Base # Server naming and configuration helpers module Config - # Set the proper server name in the config + # Longest server name OpenStack will accept without truncating. + # + # Every other length in this file is derived from it, so the limit is + # actually maintained by the code rather than merely documented here. + # + # @return [Integer] + MAX_SERVER_NAME_LENGTH = 63 + + # Number of random characters appended to a user-supplied prefix. + # + # @return [Integer] + PREFIX_SUFFIX_LENGTH = 8 + + # Longest user-supplied prefix that still leaves room for the + # separator and the random suffix. + # + # @return [Integer] + MAX_PREFIX_LENGTH = MAX_SERVER_NAME_LENGTH - PREFIX_SUFFIX_LENGTH - 1 + + # Character budget for the instance name in a fully generated name. + # + # @return [Integer] + NAME_INSTANCE_LENGTH = 15 + + # Character budget for the username in a fully generated name. + # + # @return [Integer] + NAME_USERNAME_LENGTH = 15 + + # Number of random characters in a fully generated name. + # + # @return [Integer] + NAME_RANDOM_LENGTH = 7 + + # Character budget for the hostname in a fully generated name. + # + # Whatever is left once the other three components and the three + # separators are accounted for. + # + # @return [Integer] + NAME_HOSTNAME_LENGTH = + MAX_SERVER_NAME_LENGTH - NAME_INSTANCE_LENGTH - NAME_USERNAME_LENGTH - NAME_RANDOM_LENGTH - 3 + + # Sets `config[:server_name]` unless the user already supplied one. + # + # Called at the top of {Kitchen::Driver::Openstack#create} rather than + # at config-finalize time so that the random suffix is generated once + # per converge instead of once per `kitchen` invocation. + # + # @return [String] the resolved server name def config_server_name - return if config[:server_name] + return config[:server_name] if config[:server_name] config[:server_name] = if config[:server_name_prefix] server_name_prefix(config[:server_name_prefix]) @@ -39,46 +91,61 @@ def config_server_name private - # Generate what should be a unique server name up to 63 total chars - # Base name: 15 - # Username: 15 - # Hostname: 23 - # Random string: 7 - # Separators: 3 - # ================ - # Total: 63 + # Generates a unique server name of at most 63 characters. + # + # --- + # 15 15 23 7 + 3 separators = 63 + # + # @return [String] the generated server name def default_name [ - instance.name.gsub(/\W/, "")[0..14], - ((Etc.getpwuid ? Etc.getpwuid.name : Etc.getlogin) || "nologin").gsub(/\W/, "")[0..14], - Socket.gethostname.gsub(/\W/, "")[0..22], - Array.new(7) { rand(36).to_s(36) }.join, + instance.name.gsub(/\W/, "")[0, NAME_INSTANCE_LENGTH], + current_username.gsub(/\W/, "")[0, NAME_USERNAME_LENGTH], + Socket.gethostname.gsub(/\W/, "")[0, NAME_HOSTNAME_LENGTH], + Array.new(NAME_RANDOM_LENGTH) { rand(36).to_s(36) }.join, ].join("-") end + # Best-effort lookup of the name of the user running Test Kitchen. + # + # `Etc.getpwuid` raises `ArgumentError` rather than returning nil when + # the effective uid has no passwd entry, which is routine inside + # containers, so both failure modes fall through to `Etc.getlogin` and + # finally to a placeholder. + # + # @return [String] a username, or `"nologin"` if none can be determined + def current_username + (Etc.getpwuid&.name || Etc.getlogin || "nologin") + rescue ArgumentError + Etc.getlogin || "nologin" + end + + # Generates a unique server name from a user-supplied prefix. + # + # - + # max 54 8 + 1 separator = 63 + # + # Falls back to {#default_name} when the prefix contains nothing usable + # once non-word characters are stripped. + # + # @param server_name_prefix [String] the configured prefix; never + # mutated, so the caller's config survives intact + # @return [String] the generated server name def server_name_prefix(server_name_prefix) - # Generate what should be a unique server name with given prefix - # of up to 63 total chars - # - # Provided prefix: variable, max 54 - # Separator: 1 - # Random string: 8 - # =================== - # Max: 63 - # - if server_name_prefix.length > 54 - warn "Server name prefix too long, truncated to 54 characters" - server_name_prefix = server_name_prefix[0..53] + prefix = server_name_prefix.to_s + if prefix.length > MAX_PREFIX_LENGTH + warn "Server name prefix too long, truncated to #{MAX_PREFIX_LENGTH} characters" + prefix = prefix[0, MAX_PREFIX_LENGTH] end - server_name_prefix.gsub!(/\W/, "") + prefix = prefix.gsub(/\W/, "") - if server_name_prefix.empty? + if prefix.empty? warn "Server name prefix empty or invalid; using fully generated name" default_name else - random_suffix = ("a".."z").to_a.sample(8).join - server_name_prefix + "-" + random_suffix + random_suffix = ("a".."z").to_a.sample(PREFIX_SUFFIX_LENGTH).join + "#{prefix}-#{random_suffix}" end end end diff --git a/lib/kitchen/driver/openstack/helpers.rb b/lib/kitchen/driver/openstack/helpers.rb index 0edfec0d..46971f19 100644 --- a/lib/kitchen/driver/openstack/helpers.rb +++ b/lib/kitchen/driver/openstack/helpers.rb @@ -28,8 +28,20 @@ module Driver class Openstack < Kitchen::Driver::Base # Ohai hints, SSL handling, and server wait helpers module Helpers + # Seconds between progress dots while counting down. + # + # @return [Integer] + COUNTDOWN_TICK = 10 + private + # Drops an empty `openstack.json` ohai hint on the new instance so + # that Chef Infra's ohai run picks up OpenStack metadata. + # + # Does nothing on platforms that are neither Bourne-shell nor Windows. + # + # @param state [Hash] instance state, used to open a transport session + # @return [void] def add_ohai_hint(state) if bourne_shell? info "Adding OpenStack hint for ohai" @@ -45,18 +57,40 @@ def add_ohai_hint(state) instance.transport.connection(state).execute( "#{touch_cmd} #{touch_cmd_args}" ) + else + debug "Unknown platform shell; skipping the OpenStack ohai hint" end end + # The directory ohai reads hint files from. + # + # @return [String] the first configured ohai hints path def hints_path Ohai.config[:hints_path][0] end + # Turns off TLS peer verification for every subsequent Excon request in + # this process. + # + # Called when the `disable_ssl_validation` config key is set, which the + # user sets directly in kitchen.yml or which is inferred from a + # `verify: false` entry in clouds.yaml. + # + # @return [void] def disable_ssl_validation require "excon" unless defined?(Excon) Excon.defaults[:ssl_verify_peer] = false end + # Blocks until the transport reports the instance is reachable, + # destroying the instance if it never comes up. + # + # A server we cannot reach is a server we cannot clean up later, so the + # failure path tears it down before re-raising rather than leaking it. + # + # @param state [Hash] instance state, containing `:hostname` + # @return [void] + # @raise [StandardError] whatever the transport raised, after cleanup def wait_for_server(state) if config[:server_wait] info "Sleeping for #{config[:server_wait]} seconds to let your server start up..." @@ -64,17 +98,22 @@ def wait_for_server(state) end info "Waiting for server to be ready..." instance.transport.connection(state).wait_until_ready - rescue - error "Server #{state[:hostname]} (#{state[:server_id]}) not reachable. Destroying server..." + rescue => e + error "Server #{state[:hostname]} (#{state[:server_id]}) not reachable: #{e.message}. Destroying server..." destroy(state) raise end + # Prints a progress dot every {COUNTDOWN_TICK} seconds for the given + # duration. + # + # @param seconds [Integer] how long to wait + # @return [void] def countdown(seconds) - date1 = Time.now + seconds - while Time.now < date1 + finish_at = Time.now + seconds + while Time.now < finish_at Kernel.print "." - sleep 10 + sleep COUNTDOWN_TICK end end end diff --git a/lib/kitchen/driver/openstack/networking.rb b/lib/kitchen/driver/openstack/networking.rb index affd5bb5..c7827ac0 100644 --- a/lib/kitchen/driver/openstack/networking.rb +++ b/lib/kitchen/driver/openstack/networking.rb @@ -28,47 +28,100 @@ module Driver class Openstack < Kitchen::Driver::Base # Floating IP allocation and IP address resolution module Networking + # Serializes floating IP selection across the driver instances that + # Test Kitchen runs in parallel. Without it, two concurrent converges + # can pick the same free address out of the pool and one of them fails + # to associate it. + # + # @return [Mutex] IP_POOL_LOCK = Mutex.new private + # Attaches a floating IP from the named pool to the server, allocating + # a fresh one first when `:allocate_floating_ip` is set. + # + # Held under {IP_POOL_LOCK} for the whole select-then-attach sequence, + # so a concurrent converge cannot claim the same address in between. + # + # @param server [Fog::OpenStack::Compute::Server] the server to attach to + # @param pool [String] name of the floating IP pool / external network + # @return [void] + # @raise [Kitchen::ActionFailed] if the pool does not exist, or holds no + # free addresses def attach_ip_from_pool(server, pool) IP_POOL_LOCK.synchronize do info "Attaching floating IP from <#{pool}> pool" - if config[:allocate_floating_ip] - network_id = network - .list_networks( - name: pool - ).body["networks"][0]["id"] - resp = network.create_floating_ip(network_id) - ip = resp.body["floatingip"]["floating_ip_address"] - info "Created floating IP <#{ip}> from <#{pool}> pool" - config[:floating_ip] = ip - else - free_addrs = compute.addresses.map do |i| - i.ip if i.fixed_ip.nil? && i.instance_id.nil? && i.pool == pool - end.compact - if free_addrs.empty? - raise ActionFailed, "No available IPs in pool <#{pool}>" - end - - config[:floating_ip] = free_addrs[0] - end + config[:floating_ip] = if config[:allocate_floating_ip] + allocate_ip_from_pool(pool) + else + free_ip_from_pool(pool) + end attach_ip(server, config[:floating_ip]) end end + # Asks Neutron for a brand new floating IP on the named external + # network. + # + # @param pool [String] name of the external network + # @return [String] the newly allocated floating IP + # @raise [Kitchen::ActionFailed] if no network matches `pool` + def allocate_ip_from_pool(pool) + net = network + networks = net.list_networks(name: pool).body["networks"] + if networks.nil? || networks.empty? + raise ActionFailed, "Floating IP pool <#{pool}> not found" + end + + resp = net.create_floating_ip(networks[0]["id"]) + ip = resp.body["floatingip"]["floating_ip_address"] + info "Created floating IP <#{ip}> from <#{pool}> pool" + ip + end + + # Picks an already-allocated but unattached floating IP out of the pool. + # + # @param pool [String] name of the floating IP pool + # @return [String] a free floating IP + # @raise [Kitchen::ActionFailed] if every address in the pool is in use + def free_ip_from_pool(pool) + # `find`, not `map`+`compact`: this runs while holding IP_POOL_LOCK, + # which serializes parallel converges, so it should stop at the first + # usable address rather than building a throwaway list of all of them. + free = compute.addresses.find do |i| + i.fixed_ip.nil? && i.instance_id.nil? && i.pool == pool + end + raise ActionFailed, "No available IPs in pool <#{pool}>" if free.nil? + + free.ip + end + + # Associates a floating IP with a server. + # + # @param server [Fog::OpenStack::Compute::Server] the server + # @param ip [String] the floating IP to attach + # @return [void] def attach_ip(server, ip) info "Attaching floating IP <#{ip}>" server.associate_address ip end + # Reads the server's public and private addresses. + # + # Deployments without the floating IP extension answer the dedicated + # accessors with 404/403, so fall back to picking the lists out of the + # generic addresses hash. + # + # @see https://github.com/fog/fog/issues/2160 + # @param server [Fog::OpenStack::Compute::Server] the server + # @return [Array(Array, Array)] public and private + # addresses, either of which may be nil def get_public_private_ips(server) begin pub = server.public_ip_addresses priv = server.private_ip_addresses rescue Fog::OpenStack::Compute::NotFound, Excon::Errors::Forbidden - # See Fog issue: https://github.com/fog/fog/issues/2160 addrs = server.addresses addrs["public"] && pub = addrs["public"].map { |i| i["addr"] } addrs["private"] && priv = addrs["private"].map { |i| i["addr"] } @@ -76,6 +129,19 @@ def get_public_private_ips(server) [pub, priv] end + # Determines the address Test Kitchen should connect to. + # + # Resolution order: + # + # 1. an explicitly configured `:floating_ip` + # 2. the first address on `:openstack_network_name`, if configured + # 3. `:public_ip_order` into the public addresses + # 4. `:private_ip_order` into the private addresses + # + # @param server [Fog::OpenStack::Compute::Server] the server + # @return [String] the address to connect to + # @raise [Kitchen::ActionFailed] if network information never arrives, + # or no address of the requested family can be found def get_ip(server) if config[:floating_ip] debug "Using floating ip: #{config[:floating_ip]}" @@ -91,11 +157,7 @@ def get_ip(server) raise ActionFailed, "Could not get network information (timed out)" end - # should also work for private networks - if config[:openstack_network_name] - debug "Using configured net: #{config[:openstack_network_name]}" - return filter_ips(server.addresses[config[:openstack_network_name]]).first["addr"] - end + return ip_from_named_network(server) if config[:openstack_network_name] pub, priv = get_public_private_ips(server) priv = server.ip_addresses if Array(pub).empty? && Array(priv).empty? @@ -105,6 +167,33 @@ def get_ip(server) raise(ActionFailed, "Could not find an IP") end + # Picks the first usable address off the network named by + # `:openstack_network_name`. + # + # @param server [Fog::OpenStack::Compute::Server] the server + # @return [String] the address + # @raise [Kitchen::ActionFailed] if the server is not on that network, + # or has no address there of the configured IP family + def ip_from_named_network(server) + name = config[:openstack_network_name] + debug "Using configured net: #{name}" + + addresses = server.addresses[name] + raise ActionFailed, "Server is not attached to network <#{name}>" if addresses.nil? + + matching = filter_ips(addresses) + if matching.empty? + raise ActionFailed, + "No #{config[:use_ipv6] ? "IPv6" : "IPv4"} address found on network <#{name}>" + end + + matching.first["addr"] + end + + # Keeps only the addresses matching the configured IP family. + # + # @param addresses [Array] address hashes, each with an `"addr"` key + # @return [Array] the matching subset def filter_ips(addresses) if config[:use_ipv6] addresses.select { |i| IPAddr.new(i["addr"]).ipv6? } @@ -113,15 +202,21 @@ def filter_ips(addresses) end end + # Normalizes and filters public/private address lists to the configured + # IP family. + # + # @param pub [Array, String, nil] public addresses + # @param priv [Array, String, nil] private addresses + # @return [Array(Array, Array)] filtered public and + # private address lists def parse_ips(pub, priv) - pub = Array(pub) - priv = Array(priv) - if config[:use_ipv6] - [pub, priv].each { |n| n.select! { |i| IPAddr.new(i).ipv6? } } - else - [pub, priv].each { |n| n.select! { |i| IPAddr.new(i).ipv4? } } + # `select`, not `select!`: Array(x) returns x itself when x is already + # an Array, so filtering in place would edit the caller's list -- and + # the caller's list here is the Fog server model's own address data. + wanted = config[:use_ipv6] ? :ipv6? : :ipv4? + [Array(pub), Array(priv)].map do |addrs| + addrs.select { |i| IPAddr.new(i).public_send(wanted) } end - [pub, priv] end end end diff --git a/lib/kitchen/driver/openstack/server_helper.rb b/lib/kitchen/driver/openstack/server_helper.rb index d05c0b3d..3b8f0586 100644 --- a/lib/kitchen/driver/openstack/server_helper.rb +++ b/lib/kitchen/driver/openstack/server_helper.rb @@ -26,8 +26,29 @@ module Driver class Openstack < Kitchen::Driver::Base # Server creation and resource finders (image, flavor, network) module ServerHelper + # Config keys copied onto the server definition when set, each routed + # through {#optional_config}. + # + # @return [Array] + OPTIONAL_SERVER_KEYS = %i{ + security_groups + key_name + user_data + config_drive + metadata + }.freeze + private + # Builds the Nova server definition and submits it. + # + # Fog's `bootstrap`/`setup` helpers are deliberately not used: they + # require a public IP address, which is not guaranteed to exist on + # every OpenStack deployment. + # + # @return [Fog::OpenStack::Compute::Server] the newly created server + # @raise [Kitchen::ActionFailed] on mutually exclusive or unresolvable + # configuration def create_server server_def = init_configuration raise(ActionFailed, "Cannot specify both network_ref and network_id") if config[:network_id] && config[:network_ref] @@ -44,17 +65,7 @@ def create_server end end - if config[:block_device_mapping] - server_def[:block_device_mapping] = get_bdm(config) - end - - %i{ - security_groups - key_name - user_data - config_drive - metadata - }.each do |c| + OPTIONAL_SERVER_KEYS.each do |c| server_def[c] = optional_config(c) if config[c] end @@ -64,12 +75,25 @@ def create_server server_def[:user_data] = YAML.dump(Kitchen::Util.stringified_hash(config[:cloud_config])).gsub(/^---\n/, "#cloud-config\n") end - # Can't use the Fog bootstrap and/or setup methods here; they require a - # public IP address that can't be guaranteed to exist across all - # OpenStack deployments (e.g. TryStack ARM only has private IPs). + # Last, because this is the only step that creates a resource. Every + # check above is local config validation, and running them first means + # a bad security_groups or user_data value cannot strand a Cinder + # volume whose id exists only in the server_def about to be discarded. + if config[:block_device_mapping] + server_def[:block_device_mapping] = get_bdm(config) + end + compute.servers.create(server_def) end + # Builds the mandatory part of the server definition. + # + # `*_id` and `*_ref` are mutually exclusive: an id is used verbatim, a + # ref is resolved by name, id or regex through {#find_matching}. + # + # @return [Hash] name, image, flavor and availability zone + # @raise [Kitchen::ActionFailed] if both an id and a ref are given for + # the image or the flavor, or if either cannot be resolved def init_configuration raise(ActionFailed, "Cannot specify both image_ref and image_id") if config[:image_id] && config[:image_ref] raise(ActionFailed, "Cannot specify both flavor_ref and flavor_id") if config[:flavor_id] && config[:flavor_ref] @@ -82,17 +106,39 @@ def init_configuration } end + # Resolves one optional server setting to the value Nova expects. + # + # @param c [Symbol] the config key + # @return [Object] the resolved value + # @raise [Kitchen::ActionFailed] if `:security_groups` is not a list, or + # if the `:user_data` file does not exist def optional_config(c) case c when :security_groups - config[c] if config[c].is_a?(Array) + unless config[c].is_a?(Array) + raise ActionFailed, "The security_groups config must be an array, got #{config[c].class}" + end + + config[c] when :user_data - File.read(config[c]) if File.exist?(config[c]) + # Booting without the user_data the user asked for produces a + # server that looks fine and behaves wrongly, so a missing file is + # fatal rather than silently ignored. + unless File.exist?(config[c]) + raise ActionFailed, "The user_data file <#{config[c]}> does not exist" + end + + File.read(config[c]) else config[c] end end + # Finds a Glance image by id, name or regex. + # + # @param image_ref [String] id, name, or `/regex/` + # @return [Object] the matching image + # @raise [Kitchen::ActionFailed] if nothing matches def find_image(image_ref) image = find_matching(compute.images, image_ref) raise(ActionFailed, "Image not found") unless image @@ -101,6 +147,11 @@ def find_image(image_ref) image end + # Finds a Nova flavor by id, name or regex. + # + # @param flavor_ref [String] id, name, or `/regex/` + # @return [Object] the matching flavor + # @raise [Kitchen::ActionFailed] if nothing matches def find_flavor(flavor_ref) flavor = find_matching(compute.flavors, flavor_ref) raise(ActionFailed, "Flavor not found") unless flavor @@ -109,6 +160,11 @@ def find_flavor(flavor_ref) flavor end + # Finds a Neutron network by id, name or regex. + # + # @param network_ref [String] id, name, or `/regex/` + # @return [Object] the matching network + # @raise [Kitchen::ActionFailed] if nothing matches def find_network(network_ref) net = find_matching(network.networks.all, network_ref) raise(ActionFailed, "Network not found") unless net @@ -117,15 +173,31 @@ def find_network(network_ref) net end + # Picks a resource out of a Fog collection. + # + # A ref wrapped in forward slashes is treated as a regular expression + # matched against the resource name; anything else is compared against + # the id first and then the name, so an exact id always wins. + # + # @example an exact name + # find_matching(compute.images, "ubuntu-24.04") + # @example a regular expression + # find_matching(compute.images, "/^ubuntu-24\\.04/") + # + # @param collection [Enumerable] the Fog collection to search + # @param name [String] id, name, or `/regex/` + # @return [Object, nil] the first match, or nil def find_matching(collection, name) name = name.to_s if name.start_with?("/") && name.end_with?("/") regex = Regexp.new(name[1...-1]) - # check for regex name match - collection.each { |single| return single if regex&.match?(single.name) } + # check for regex name match, skipping unnamed resources; Neutron + # networks in particular are allowed to have no name + collection.each { |single| return single if single.name && regex.match?(single.name) } else - # check for exact id match - collection.each { |single| return single if single.id == name } + # check for exact id match; ids come back as integers on some + # deployments, so compare as strings + collection.each { |single| return single if single.id.to_s == name } # check for exact name match collection.each { |single| return single if single.name == name } end diff --git a/lib/kitchen/driver/openstack/volume.rb b/lib/kitchen/driver/openstack/volume.rb index 853b3dd6..0e4d423c 100644 --- a/lib/kitchen/driver/openstack/volume.rb +++ b/lib/kitchen/driver/openstack/volume.rb @@ -26,28 +26,70 @@ class Openstack < Kitchen::Driver::Base # A class to allow the Kitchen Openstack driver # to use Openstack volumes # + # Instances of this class translate a `block_device_mapping` config hash + # into the shape Nova expects, creating a Cinder volume first when the + # mapping asks for one. + # # @author Liam Haworth class Volume + # Seconds to wait for a newly created volume to become available when + # the block device mapping does not specify `creation_timeout`. + # + # @return [Integer] DEFAULT_CREATION_TIMEOUT = 60 + # Block device mapping keys forwarded verbatim to Cinder's create call. + # + # @return [Array] + VANILLA_VOLUME_OPTIONS = %i{ + snapshot_id + imageRef + volume_type + source_volid + availability_zone + }.freeze + + # @param logger [Kitchen::Logger] logger to report volume progress to def initialize(logger) @logger = logger end + # Builds a Cinder connection. + # + # @param openstack_server [Hash] Fog connection settings + # @return [Fog::OpenStack::Volume] a Cinder service object def volume(openstack_server) Fog::OpenStack::Volume.new(openstack_server) end + # Creates a Cinder volume and blocks until it is available. + # + # @param config [Hash] the driver config, read for `:server_name` and + # `:block_device_mapping` + # @param os [Hash] Fog connection settings + # @return [String] the id of the newly created volume + # @raise [Kitchen::ActionFailed] if a timeout is not a number, if the + # volume cannot be found after creation, or if it enters an `error` + # state + # @raise [Fog::Errors::TimeoutError] if the volume is not available + # before `:creation_timeout` elapses def create_volume(config, os) - opt = {} bdm = config[:block_device_mapping] - vanilla_options = %i{snapshot_id imageRef volume_type - source_volid availability_zone} - vanilla_options.select { |o| bdm[o] }.each do |key| - opt[key] = bdm[key] - end + + # Read both timeouts before anything is created. They are pure config + # parsing, so a bad value should cost the user an error message, not + # an orphaned volume that `kitchen destroy` cannot see. + creation_timeout = timeout_value(bdm, :creation_timeout, DEFAULT_CREATION_TIMEOUT) + attach_timeout = timeout_value(bdm, :attach_timeout, 0) + + opt = VANILLA_VOLUME_OPTIONS.select { |o| bdm[o] }.to_h { |key| [key, bdm[key]] } + + # Build the Cinder connection once and reuse it for the readiness + # lookup, rather than authenticating to Keystone a second time. + volume_service = volume(os) + @logger.info "Creating Volume..." - resp = volume(os) + resp = volume_service .create_volume( "#{config[:server_name]}-volume", "#{config[:server_name]} volume", @@ -56,41 +98,95 @@ def create_volume(config, os) ) vol_id = resp[:body]["volume"]["id"] - # Get Volume Model to make waiting for ready easy - vol_model = volume(os).volumes.first { |x| x.id == vol_id } + wait_for_volume(volume_service, vol_id, creation_timeout, attach_timeout) - # Use default creation timeout or user supplied - creation_timeout = DEFAULT_CREATION_TIMEOUT - if bdm.key?(:creation_timeout) - creation_timeout = bdm[:creation_timeout] - end + vol_id + end + + # Resolves the block device mapping to hand to Nova, creating the + # backing volume first if `:make_volume` is set. + # + # The returned hash is a copy: the driver's own config is never + # mutated, so a retried `create` sees the same input it did the first + # time. + # + # @param config [Hash] the driver config + # @param os [Hash] Fog connection settings + # @return [Hash] a block device mapping suitable for Nova + def get_bdm(config, os) + bdm = config[:block_device_mapping].dup + bdm[:volume_id] = create_volume(config, os) if bdm[:make_volume] + bdm.delete(:make_volume) + bdm.delete(:snapshot_id) + bdm + end + + private + + # Blocks until the named volume reports ready, then honours any + # additional `:attach_timeout` grace period. + # + # @param volume_service [Fog::OpenStack::Volume] an established Cinder + # connection + # @param vol_id [String] id of the volume to wait on + # @param creation_timeout [Integer] seconds to wait for readiness + # @param attach_timeout [Integer] extra seconds to sleep once ready + # @return [void] + # @raise [Kitchen::ActionFailed] if the volume cannot be fetched back + # or enters an `error` state + def wait_for_volume(volume_service, vol_id, creation_timeout, attach_timeout) + # Fetch the volume by id rather than scanning the collection: a list + # call returns a single page (Cinder caps it at osapi_max_limit), so + # in a project with more volumes than that the one just created may + # not appear on it. `get` is a direct GET and returns nil on 404. + vol_model = volume_service.volumes.get(vol_id) + raise(ActionFailed, "Volume #{vol_id} disappeared after creation") if vol_model.nil? @logger.debug "Waiting for volume to be ready for #{creation_timeout} seconds" vol_model.wait_for(creation_timeout) do sleep(1) - raise("Failed to make volume") if status.casecmp("error".downcase) == 0 + raise(ActionFailed, "Failed to make volume #{vol_id}") if status.casecmp("error") == 0 ready? end - attach_timeout = bdm.key?(:attach_timeout) ? bdm[:attach_timeout] : 0 - if attach_timeout > 0 @logger.debug "Sleeping for an additional #{attach_timeout} seconds before attaching volume to wait for Openstack to finish disk creation process.." sleep(attach_timeout) end @logger.debug "Volume Ready" - - vol_id end - def get_bdm(config, os) - bdm = config[:block_device_mapping] - bdm[:volume_id] = create_volume(config, os) if bdm[:make_volume] - bdm.delete_if { |k, _| k == :make_volume } - bdm.delete_if { |k, _| k == :snapshot_id } - bdm + # Reads a timeout out of the block device mapping as an Integer. + # + # YAML happily parses `attach_timeout: 5` as an Integer but + # `attach_timeout: "5"` as a String, and comparing a String to 0 raises. + # Coerce so both spellings work. + # + # Base 10 is explicit: a bare `Integer("010")` would read the leading + # zero as octal and quietly wait 8 seconds instead of 10, and + # `Integer("08")` would raise outright. + # + # @param bdm [Hash] the block device mapping + # @param key [Symbol] the timeout key to read + # @param default [Integer] value to use when the key is absent or empty + # @return [Integer] the timeout in seconds + # @raise [Kitchen::ActionFailed] if the value is present but not a + # number + def timeout_value(bdm, key, default) + value = bdm[key] + # `attach_timeout:` with nothing after it parses to nil. + return default if value.nil? || value.to_s.strip.empty? + + begin + # The base argument is only legal for a String; passing one + # alongside an Integer raises "base specified for non string value". + value.is_a?(String) ? Integer(value, 10) : Integer(value) + rescue ArgumentError, TypeError + raise(ActionFailed, + "The block_device_mapping #{key} must be a number, got #{value.inspect}") + end end end end diff --git a/lib/kitchen/driver/openstack_version.rb b/lib/kitchen/driver/openstack_version.rb index 9b3e1636..28ec8f36 100644 --- a/lib/kitchen/driver/openstack_version.rb +++ b/lib/kitchen/driver/openstack_version.rb @@ -23,6 +23,12 @@ module Kitchen # # @author Jonathan Hartman module Driver + # The kitchen-openstack gem version. + # + # Read by the gemspec and bumped by Release Please, so it must stay a + # plain string literal on a single line. + # + # @return [String] OPENSTACK_VERSION = "7.0.1" end end diff --git a/spec/kitchen/driver/openstack/clouds_spec.rb b/spec/kitchen/driver/openstack/clouds_spec.rb index c4653221..9bf0a249 100644 --- a/spec/kitchen/driver/openstack/clouds_spec.rb +++ b/spec/kitchen/driver/openstack/clouds_spec.rb @@ -1,43 +1,52 @@ # frozen_string_literal: true -require_relative "../../../spec_helper" -require_relative "../../../../lib/kitchen/driver/openstack" - -require "logger" -require "stringio" unless defined?(StringIO) -require "rspec" -require "kitchen" -require "kitchen/driver/openstack" -require "kitchen/provisioner/dummy" -require "kitchen/transport/dummy" -require "kitchen/verifier/dummy" - -describe Kitchen::Driver::Openstack do - let(:logged_output) { StringIO.new } - let(:logger) { Logger.new(logged_output) } - let(:config) { {} } - let(:instance_name) { "potatoes" } - let(:transport) { Kitchen::Transport::Dummy.new } - let(:platform) { Kitchen::Platform.new(name: "fake_platform") } - let(:driver) { described_class.new(config) } - - let(:instance) do - double( - name: instance_name, - transport: transport, - logger: logger, - platform: platform, - to_str: "instance" - ) +require "fileutils" +require "tmpdir" +require "yaml" + +RSpec.describe Kitchen::Driver::Openstack::Clouds do + include_context "with a configured driver" + + # A directory that is never created, used to pin the search path away from + # the developer's real files. + def nowhere + File.join(Dir.tmpdir, "kitchen-openstack-does-not-exist") + end + + # Pin every location clouds_yaml_search_paths consults. + # + # Scrubbing OS_* is not sufficient on its own: the search falls through to + # ./{clouds,secure}.yaml, ~/.config/openstack/ and /etc/openstack/ whenever + # the pinned path does not exist, so on an operator's machine a real + # secure.yaml naming a cloud these fixtures also use would fail examples + # here -- and RSpec's diff would print their real password into the terminal + # and the CI log. + before do + allow(Dir).to receive_messages(pwd: nowhere, home: nowhere) + allow(File).to receive(:exist?).with(a_string_starting_with("/etc/openstack/")).and_return(false) + end + + # Real files on disk rather than a stubbed File.exist?: these examples are + # the only place the driver touches the filesystem, and stubbing it out was + # hiding whether the search-path logic worked at all. + # + # Created lazily and torn down after: most examples in this file never touch + # the filesystem, and building then recursively removing a tmpdir for each of + # them was the largest source of dead I/O in the suite. + def tmpdir + @tmpdir ||= Dir.mktmpdir("kitchen-openstack-clouds") end - before(:each) do - allow_any_instance_of(described_class).to receive(:instance) - .and_return(instance) - allow(File).to receive(:exist?).and_call_original + after { FileUtils.remove_entry(@tmpdir) if @tmpdir } + + # Writes a YAML document into the sandbox and returns its path. + def write_yaml(filename, content) + path = File.join(tmpdir, filename) + File.write(path, content.is_a?(String) ? content : YAML.dump(content)) + path end - let(:clouds_yaml_content) do + let(:clouds_yaml) do { "clouds" => { "mycloud" => { @@ -52,714 +61,639 @@ }, "region_name" => "RegionOne", "interface" => "public", - "identity_api_version" => "3", + "identity_api_version" => 3, }, "minimal" => { "auth" => { "auth_url" => "https://minimal.example.com:5000/v3", "username" => "minuser", - "password" => "minpass", - "domain_id" => "default", - }, - }, - "appcred" => { - "auth" => { - "auth_url" => "https://appcred.example.com:5000/v3", - "application_credential_id" => "abc123", - "application_credential_secret" => "secret456", - "domain_id" => "default", }, - "auth_type" => "v3applicationcredential", - }, - "sslcloud" => { - "auth" => { - "auth_url" => "https://ssl.example.com:5000/v3", - "username" => "ssluser", - "password" => "sslpass", - "domain_id" => "default", - }, - "verify" => false, - "cacert" => "/path/to/ca.crt", }, }, } end - let(:secure_yaml_content) do - { - "clouds" => { - "mycloud" => { - "auth" => { - "password" => "secure_password_override", - }, - }, - }, + # Point the loader at the sandbox and return the clouds.yaml path. + # + # OS_CLIENT_SECURE_FILE is always pinned, at a file that only exists when the + # example asked for one, so no example can silently pick up a real + # secure.yaml. + def use_clouds_file(content = clouds_yaml, secure: nil, env: {}) + path = write_yaml("clouds.yaml", content) + vars = { + "OS_CLIENT_CONFIG_FILE" => path, + "OS_CLIENT_SECURE_FILE" => secure ? write_yaml("secure.yaml", secure) : File.join(tmpdir, "no-secure.yaml"), } + stub_env(vars.merge(env)) + path end describe "#cloud_name" do - context "when openstack_cloud is set in config" do - let(:config) { { openstack_cloud: "mycloud" } } + it "reads openstack_cloud from kitchen.yml" do + config[:openstack_cloud] = "mycloud" - it "returns the config value" do - expect(driver.send(:cloud_name)).to eq("mycloud") - end + expect(driver.send(:cloud_name)).to eq("mycloud") end - context "when OS_CLOUD env var is set" do - before { allow(ENV).to receive(:[]).and_call_original } - before { allow(ENV).to receive(:[]).with("OS_CLOUD").and_return("envcloud") } + it "falls back to OS_CLOUD" do + stub_env("OS_CLOUD" => "envcloud") - it "returns the env var value" do - expect(driver.send(:cloud_name)).to eq("envcloud") - end + expect(driver.send(:cloud_name)).to eq("envcloud") end - context "when openstack_cloud config takes precedence over OS_CLOUD" do - let(:config) { { openstack_cloud: "configcloud" } } + it "prefers kitchen.yml over OS_CLOUD" do + stub_env("OS_CLOUD" => "envcloud") + config[:openstack_cloud] = "mycloud" - before { allow(ENV).to receive(:[]).and_call_original } - before { allow(ENV).to receive(:[]).with("OS_CLOUD").and_return("envcloud") } + expect(driver.send(:cloud_name)).to eq("mycloud") + end - it "returns the config value" do - expect(driver.send(:cloud_name)).to eq("configcloud") - end + it "is nil when neither is set" do + expect(driver.send(:cloud_name)).to be_nil end + end - context "when neither is set" do - before { allow(ENV).to receive(:[]).and_call_original } - before { allow(ENV).to receive(:[]).with("OS_CLOUD").and_return(nil) } + describe "#clouds_yaml_search_paths" do + before { stub_env } - it "returns nil" do - expect(driver.send(:cloud_name)).to be_nil - end + it "searches cwd, then the user config dir, then /etc/openstack" do + allow(Dir).to receive_messages(pwd: "/work", home: "/home/me") + + expect(driver.send(:clouds_yaml_search_paths, "clouds.yaml", "OS_CLIENT_CONFIG_FILE")).to eq( + [ + "/work/clouds.yaml", + "/home/me/.config/openstack/clouds.yaml", + "/etc/openstack/clouds.yaml", + ] + ) end - end - describe "#load_clouds_config" do - before do - allow(ENV).to receive(:[]).and_call_original - allow(ENV).to receive(:[]).with("OS_CLIENT_CONFIG_FILE").and_return(nil) - allow(ENV).to receive(:[]).with("OS_CLIENT_SECURE_FILE").and_return(nil) - allow(ENV).to receive(:[]).with("OS_CLOUD").and_return(nil) + # Dir.home raises ArgumentError when HOME is unset and the uid has no + # passwd entry -- the ordinary state in a container running as an arbitrary + # uid. With OS_CLOUD set that used to crash the driver before /etc/openstack + # was ever tried, which is the one location such a container is likely to + # have. + it "skips the user config dir when there is no home directory" do + allow(Dir).to receive(:pwd).and_return("/work") + allow(Dir).to receive(:home).and_raise(ArgumentError, "couldn't find HOME environment -- expanding `~'") + + expect(driver.send(:clouds_yaml_search_paths, "clouds.yaml", "OS_CLIENT_CONFIG_FILE")).to eq( + [ + "/work/clouds.yaml", + "/etc/openstack/clouds.yaml", + ] + ) end - context "when no cloud name is configured" do - it "returns an empty hash" do - expect(driver.send(:load_clouds_config)).to eq({}) - end + it "puts the env var override first" do + stub_env("OS_CLIENT_CONFIG_FILE" => "/custom/clouds.yaml") + + expect(driver.send(:clouds_yaml_search_paths, "clouds.yaml", "OS_CLIENT_CONFIG_FILE").first) + .to eq("/custom/clouds.yaml") end - context "when a cloud name is set and clouds.yaml exists" do - let(:config) { { openstack_cloud: "mycloud" } } + it "honours clouds_yaml_path from kitchen.yml" do + config[:clouds_yaml_path] = "/kitchen/clouds.yaml" - before do - allow(File).to receive(:exist?).and_return(false) - allow(File).to receive(:exist?) - .with(File.join(Dir.pwd, "clouds.yaml")).and_return(true) - allow(File).to receive(:read) - .with(File.join(Dir.pwd, "clouds.yaml")) - .and_return(YAML.dump(clouds_yaml_content)) - end + expect(driver.send(:clouds_yaml_search_paths, "clouds.yaml", "OS_CLIENT_CONFIG_FILE")) + .to include("/kitchen/clouds.yaml") + end - it "returns translated fog config" do - result = driver.send(:load_clouds_config) - expect(result[:openstack_auth_url]).to eq("https://keystone.example.com:5000/v3") - expect(result[:openstack_username]).to eq("testuser") - expect(result[:openstack_api_key]).to eq("testpass") - expect(result[:openstack_project_name]).to eq("testproject") - expect(result[:openstack_user_domain]).to eq("Default") - expect(result[:openstack_project_domain]).to eq("Default") - expect(result[:openstack_domain_id]).to eq("default") - expect(result[:openstack_region]).to eq("RegionOne") - expect(result[:openstack_endpoint_type]).to eq("public") - expect(result[:openstack_identity_api_version]).to eq("3") - end + it "ranks the env var above clouds_yaml_path" do + stub_env("OS_CLIENT_CONFIG_FILE" => "/custom/clouds.yaml") + config[:clouds_yaml_path] = "/kitchen/clouds.yaml" + + paths = driver.send(:clouds_yaml_search_paths, "clouds.yaml", "OS_CLIENT_CONFIG_FILE") + + expect(paths.index("/custom/clouds.yaml")).to be < paths.index("/kitchen/clouds.yaml") end - context "when secure.yaml provides password override" do - let(:config) { { openstack_cloud: "mycloud" } } - - before do - allow(File).to receive(:exist?).and_return(false) - allow(File).to receive(:exist?) - .with(File.join(Dir.pwd, "clouds.yaml")).and_return(true) - allow(File).to receive(:exist?) - .with(File.join(Dir.pwd, "secure.yaml")).and_return(true) - allow(File).to receive(:read) - .with(File.join(Dir.pwd, "clouds.yaml")) - .and_return(YAML.dump(clouds_yaml_content)) - allow(File).to receive(:read) - .with(File.join(Dir.pwd, "secure.yaml")) - .and_return(YAML.dump(secure_yaml_content)) - end + it "does not apply clouds_yaml_path to secure.yaml" do + config[:clouds_yaml_path] = "/kitchen/clouds.yaml" - it "merges secure.yaml values over clouds.yaml" do - result = driver.send(:load_clouds_config) - expect(result[:openstack_api_key]).to eq("secure_password_override") - expect(result[:openstack_username]).to eq("testuser") - end + expect(driver.send(:clouds_yaml_search_paths, "secure.yaml", "OS_CLIENT_SECURE_FILE")) + .not_to include("/kitchen/clouds.yaml") end + end - context "when OS_CLIENT_CONFIG_FILE is set" do - let(:config) { { openstack_cloud: "mycloud" } } - let(:custom_path) { "/custom/path/clouds.yaml" } + describe "#load_yaml_file" do + it "returns an empty hash when no file is found" do + stub_env + allow(Dir).to receive_messages(pwd: File.join(tmpdir, "empty"), home: File.join(tmpdir, "empty")) - before do - allow(ENV).to receive(:[]).with("OS_CLIENT_CONFIG_FILE").and_return(custom_path) - allow(File).to receive(:exist?).and_return(false) - allow(File).to receive(:exist?).with(custom_path).and_return(true) - allow(File).to receive(:read) - .with(custom_path) - .and_return(YAML.dump(clouds_yaml_content)) - end + expect(driver.send(:load_yaml_file, "clouds.yaml", "OS_CLIENT_CONFIG_FILE")).to eq([{}, nil]) + end - it "uses the custom path" do - result = driver.send(:load_clouds_config) - expect(result[:openstack_auth_url]).to eq("https://keystone.example.com:5000/v3") - end + it "parses the first file that exists, and reports where it came from" do + path = use_clouds_file + + expect(driver.send(:load_yaml_file, "clouds.yaml", "OS_CLIENT_CONFIG_FILE")) + .to eq([clouds_yaml, path]) end - context "when clouds_yaml_path config is set" do - let(:config) { { openstack_cloud: "mycloud", clouds_yaml_path: "/my/clouds.yaml" } } + it "treats an empty file as an empty document" do + path = write_yaml("clouds.yaml", "") + stub_env("OS_CLIENT_CONFIG_FILE" => path) - before do - allow(File).to receive(:exist?).and_return(false) - allow(File).to receive(:exist?).with("/my/clouds.yaml").and_return(true) - allow(File).to receive(:read) - .with("/my/clouds.yaml") - .and_return(YAML.dump(clouds_yaml_content)) - end + expect(driver.send(:load_yaml_file, "clouds.yaml", "OS_CLIENT_CONFIG_FILE")).to eq([{}, path]) + end - it "uses the configured path" do - result = driver.send(:load_clouds_config) - expect(result[:openstack_auth_url]).to eq("https://keystone.example.com:5000/v3") - end + it "allows Date values, which appear in expiry fields" do + path = write_yaml("clouds.yaml", "clouds:\n mycloud:\n expires: 2030-01-01\n") + stub_env("OS_CLIENT_CONFIG_FILE" => path) + + expect(driver.send(:load_yaml_file, "clouds.yaml", "OS_CLIENT_CONFIG_FILE").first) + .to eq("clouds" => { "mycloud" => { "expires" => Date.new(2030, 1, 1) } }) end - context "when cloud entry does not exist in clouds.yaml" do - let(:config) { { openstack_cloud: "nonexistent" } } + it "names the file when the YAML is malformed" do + path = write_yaml("clouds.yaml", "clouds:\n mycloud:\n - broken: [\n") + stub_env("OS_CLIENT_CONFIG_FILE" => path) - before do - allow(File).to receive(:exist?).and_return(false) - allow(File).to receive(:exist?) - .with(File.join(Dir.pwd, "clouds.yaml")).and_return(true) - allow(File).to receive(:read) - .with(File.join(Dir.pwd, "clouds.yaml")) - .and_return(YAML.dump(clouds_yaml_content)) - end + expect { driver.send(:load_yaml_file, "clouds.yaml", "OS_CLIENT_CONFIG_FILE") } + .to raise_error(Kitchen::ActionFailed, /Could not parse #{Regexp.escape(path)}/) + end - it "returns an empty hash" do - expect(driver.send(:load_clouds_config)).to eq({}) - end + it "rejects a document that is not a mapping" do + path = write_yaml("clouds.yaml", "- one\n- two\n") + stub_env("OS_CLIENT_CONFIG_FILE" => path) + + expect { driver.send(:load_yaml_file, "clouds.yaml", "OS_CLIENT_CONFIG_FILE") } + .to raise_error(Kitchen::ActionFailed, /must contain a YAML mapping/) end - context "with application credential auth" do - let(:config) { { openstack_cloud: "appcred" } } + it "logs where it loaded the file from" do + path = write_yaml("clouds.yaml", clouds_yaml) + stub_env("OS_CLIENT_CONFIG_FILE" => path) - before do - allow(File).to receive(:exist?).and_return(false) - allow(File).to receive(:exist?) - .with(File.join(Dir.pwd, "clouds.yaml")).and_return(true) - allow(File).to receive(:read) - .with(File.join(Dir.pwd, "clouds.yaml")) - .and_return(YAML.dump(clouds_yaml_content)) - end + driver.send(:load_yaml_file, "clouds.yaml", "OS_CLIENT_CONFIG_FILE") - it "maps application credential fields" do - result = driver.send(:load_clouds_config) - expect(result[:openstack_application_credential_id]).to eq("abc123") - expect(result[:openstack_application_credential_secret]).to eq("secret456") - end + expect(logged_output.string).to include("Loading clouds.yaml from #{path}") end + end - context "with SSL settings" do - let(:config) { { openstack_cloud: "sslcloud" } } + describe "#extract_cloud" do + it "pulls out the named cloud" do + expect(driver.send(:extract_cloud, clouds_yaml, "minimal", "clouds.yaml")) + .to eq("auth" => { "auth_url" => "https://minimal.example.com:5000/v3", "username" => "minuser" }) + end - before do - allow(File).to receive(:exist?).and_return(false) - allow(File).to receive(:exist?) - .with(File.join(Dir.pwd, "clouds.yaml")).and_return(true) - allow(File).to receive(:read) - .with(File.join(Dir.pwd, "clouds.yaml")) - .and_return(YAML.dump(clouds_yaml_content)) - end + # An absent entry is normal: clouds.yaml and secure.yaml are merged, and a + # cloud is allowed to appear in only one of them. + it "returns an empty hash for an unknown cloud" do + expect(driver.send(:extract_cloud, clouds_yaml, "nope", "clouds.yaml")).to eq({}) + end - it "maps SSL settings" do - result = driver.send(:load_clouds_config) - expect(result[:ssl_verify_peer]).to eq(false) - expect(result[:ssl_ca_file]).to eq("/path/to/ca.crt") - end + it "returns an empty hash when there is no clouds key" do + expect(driver.send(:extract_cloud, { "other" => {} }, "mycloud", "clouds.yaml")).to eq({}) + end + + # A present-but-malformed entry is always a mistake. Returning {} here let + # the driver carry on with nil credentials, and the user saw an opaque + # Keystone auth failure that never mentioned their config file. + it "names the file when clouds is present but not a mapping" do + expect { driver.send(:extract_cloud, { "clouds" => "oops" }, "mycloud", "/etc/openstack/clouds.yaml") } + .to raise_error(Kitchen::ActionFailed, %r{clouds section of /etc/openstack/clouds.yaml must be a YAML mapping}) + end + + it "names the cloud when the entry is present but not a mapping" do + entry = { "clouds" => { "mycloud" => "https://example.com:5000/v3" } } + + expect { driver.send(:extract_cloud, entry, "mycloud", "/etc/openstack/clouds.yaml") } + .to raise_error(Kitchen::ActionFailed, %r{Cloud in /etc/openstack/clouds.yaml must be a YAML mapping}) end end - describe "#clouds_yaml_search_paths" do - before do - allow(ENV).to receive(:[]).and_call_original - allow(ENV).to receive(:[]).with("OS_CLIENT_CONFIG_FILE").and_return(nil) + describe "#deep_merge" do + it "merges nested hashes" do + base = { "auth" => { "username" => "a", "password" => "b" }, "region_name" => "r" } + override = { "auth" => { "password" => "c" } } + + expect(driver.send(:deep_merge, base, override)) + .to eq("auth" => { "username" => "a", "password" => "c" }, "region_name" => "r") end - context "with no env var or config path" do - it "returns standard search paths" do - paths = driver.send(:clouds_yaml_search_paths, "clouds.yaml", "OS_CLIENT_CONFIG_FILE") - expect(paths).to include(File.join(Dir.pwd, "clouds.yaml")) - expect(paths).to include(File.join(Dir.home, ".config", "openstack", "clouds.yaml")) - expect(paths).to include("/etc/openstack/clouds.yaml") - end + it "replaces scalars rather than merging them" do + expect(driver.send(:deep_merge, { "a" => 1 }, { "a" => 2 })).to eq("a" => 2) end - context "with env var set" do - before do - allow(ENV).to receive(:[]).with("OS_CLIENT_CONFIG_FILE").and_return("/custom/clouds.yaml") - end + it "replaces a hash with a scalar when the override says so" do + expect(driver.send(:deep_merge, { "a" => { "b" => 1 } }, { "a" => 2 })).to eq("a" => 2) + end - it "prepends the env var path" do - paths = driver.send(:clouds_yaml_search_paths, "clouds.yaml", "OS_CLIENT_CONFIG_FILE") - expect(paths.first).to eq("/custom/clouds.yaml") - end + it "adds keys only present in the override" do + expect(driver.send(:deep_merge, { "a" => 1 }, { "b" => 2 })).to eq("a" => 1, "b" => 2) end - context "with clouds_yaml_path config" do - let(:config) { { clouds_yaml_path: "/configured/clouds.yaml" } } + it "does not mutate either input" do + base = { "auth" => { "username" => "a" } } + override = { "auth" => { "password" => "b" } } - it "includes the configured path" do - paths = driver.send(:clouds_yaml_search_paths, "clouds.yaml", "OS_CLIENT_CONFIG_FILE") - expect(paths).to include("/configured/clouds.yaml") - end + driver.send(:deep_merge, base, override) - it "does not include config path for secure.yaml" do - paths = driver.send(:clouds_yaml_search_paths, "secure.yaml", "OS_CLIENT_SECURE_FILE") - expect(paths).not_to include("/configured/clouds.yaml") - end + expect(base).to eq("auth" => { "username" => "a" }) + expect(override).to eq("auth" => { "password" => "b" }) end end describe "#translate_cloud_config" do - it "maps all auth keys correctly" do + it "maps every auth key it knows about" do cloud = { "auth" => { - "auth_url" => "http://example.com:5000/v3", - "username" => "user", - "password" => "pass", - "project_name" => "proj", - "project_id" => "proj-id", - "user_domain_name" => "UDN", - "user_domain_id" => "udi", - "project_domain_name" => "PDN", - "project_domain_id" => "pdi", - "domain_id" => "did", - "domain_name" => "dname", - "application_credential_id" => "acid", - "application_credential_secret" => "acs", + "auth_url" => "https://keystone.example.com:5000/v3", + "username" => "testuser", + "password" => "testpass", + "project_name" => "testproject", + "project_id" => "pid", + "user_domain_name" => "Default", + "user_domain_id" => "udid", + "project_domain_name" => "Default", + "project_domain_id" => "pdid", + "domain_id" => "default", + "domain_name" => "Default", }, - "region_name" => "Region1", - "interface" => "internal", - "identity_api_version" => "3", - "verify" => true, - "cacert" => "/ca.pem", } - result = driver.send(:translate_cloud_config, cloud) - expect(result[:openstack_auth_url]).to eq("http://example.com:5000/v3") - expect(result[:openstack_username]).to eq("user") - expect(result[:openstack_api_key]).to eq("pass") - expect(result[:openstack_project_name]).to eq("proj") - expect(result[:openstack_project_id]).to eq("proj-id") - expect(result[:openstack_user_domain]).to eq("UDN") - expect(result[:openstack_user_domain_id]).to eq("udi") - expect(result[:openstack_project_domain]).to eq("PDN") - expect(result[:openstack_project_domain_id]).to eq("pdi") - expect(result[:openstack_domain_id]).to eq("did") - expect(result[:openstack_domain_name]).to eq("dname") - expect(result[:openstack_application_credential_id]).to eq("acid") - expect(result[:openstack_application_credential_secret]).to eq("acs") - expect(result[:openstack_region]).to eq("Region1") - expect(result[:openstack_endpoint_type]).to eq("internal") - expect(result[:openstack_identity_api_version]).to eq("3") - expect(result[:ssl_verify_peer]).to eq(true) - expect(result[:ssl_ca_file]).to eq("/ca.pem") - end - - it "handles empty auth section" do - result = driver.send(:translate_cloud_config, {}) - expect(result).to eq({}) - end - - it "coerces non-string scalar values for fog string config keys" do - cloud = { - "auth" => { - "project_id" => 12_345, - "domain_id" => 9, - }, - "identity_api_version" => 3, - } + expect(driver.send(:translate_cloud_config, cloud)).to eq( + openstack_auth_url: "https://keystone.example.com:5000/v3", + openstack_username: "testuser", + openstack_api_key: "testpass", + openstack_project_name: "testproject", + openstack_project_id: "pid", + openstack_user_domain: "Default", + openstack_user_domain_id: "udid", + openstack_project_domain: "Default", + openstack_project_domain_id: "pdid", + openstack_domain_id: "default", + openstack_domain_name: "Default" + ) + end + + it "maps the top-level keys" do + cloud = { "region_name" => "RegionOne", "interface" => "public", "identity_api_version" => "3" } - result = driver.send(:translate_cloud_config, cloud) - expect(result[:openstack_project_id]).to eq("12345") - expect(result[:openstack_domain_id]).to eq("9") - expect(result[:openstack_identity_api_version]).to eq("3") + expect(driver.send(:translate_cloud_config, cloud)).to eq( + openstack_region: "RegionOne", + openstack_endpoint_type: "public", + openstack_identity_api_version: "3" + ) end - end - describe "#deep_merge" do - it "deep merges nested hashes" do - base = { "auth" => { "username" => "user", "password" => "base_pass" }, "region" => "r1" } - override = { "auth" => { "password" => "override_pass" } } - result = driver.send(:deep_merge, base, override) - expect(result["auth"]["username"]).to eq("user") - expect(result["auth"]["password"]).to eq("override_pass") - expect(result["region"]).to eq("r1") - end - - it "does not mutate the original hashes" do - base = { "auth" => { "password" => "old" } } - override = { "auth" => { "password" => "new" } } - driver.send(:deep_merge, base, override) - expect(base["auth"]["password"]).to eq("old") + it "maps application credentials" do + cloud = { "auth" => { "application_credential_id" => "acid", "application_credential_secret" => "acsecret" } } + + expect(driver.send(:translate_cloud_config, cloud)).to eq( + openstack_application_credential_id: "acid", + openstack_application_credential_secret: "acsecret" + ) end - end - describe "#load_env_vars" do - before do - allow(ENV).to receive(:[]).and_call_original - Kitchen::Driver::Openstack::Clouds::ENV_VAR_MAP.each_key do |var| - allow(ENV).to receive(:[]).with(var).and_return(nil) - end + it "returns an empty hash for an empty cloud" do + expect(driver.send(:translate_cloud_config, {})).to eq({}) end - context "when no OS_* env vars are set" do - it "returns an empty hash" do - expect(driver.send(:load_env_vars)).to eq({}) - end + it "skips keys it does not recognize" do + expect(driver.send(:translate_cloud_config, { "auth" => { "nonsense" => "x" } })).to eq({}) end - context "when OS_AUTH_URL and OS_USERNAME are set" do - before do - allow(ENV).to receive(:[]).with("OS_AUTH_URL").and_return("https://env.example.com:5000/v3") - allow(ENV).to receive(:[]).with("OS_USERNAME").and_return("envuser") - end + # YAML parses `identity_api_version: 3` and numeric project ids as + # Integers; Fog then chokes on them. + it "stringifies values YAML parsed as numbers" do + cloud = { "identity_api_version" => 3, "auth" => { "project_id" => 12345 } } - it "maps them to fog config keys" do - result = driver.send(:load_env_vars) - expect(result[:openstack_auth_url]).to eq("https://env.example.com:5000/v3") - expect(result[:openstack_username]).to eq("envuser") - end + expect(driver.send(:translate_cloud_config, cloud)) + .to eq(openstack_identity_api_version: "3", openstack_project_id: "12345") end - context "when all standard OS_* env vars are set" do - before do - allow(ENV).to receive(:[]).with("OS_AUTH_URL").and_return("https://env.example.com:5000/v3") - allow(ENV).to receive(:[]).with("OS_USERNAME").and_return("envuser") - allow(ENV).to receive(:[]).with("OS_PASSWORD").and_return("envpass") - allow(ENV).to receive(:[]).with("OS_PROJECT_NAME").and_return("envproject") - allow(ENV).to receive(:[]).with("OS_USER_DOMAIN_NAME").and_return("EnvDomain") - allow(ENV).to receive(:[]).with("OS_PROJECT_DOMAIN_NAME").and_return("EnvProjDomain") - allow(ENV).to receive(:[]).with("OS_DOMAIN_ID").and_return("envdomid") - allow(ENV).to receive(:[]).with("OS_REGION_NAME").and_return("EnvRegion") - allow(ENV).to receive(:[]).with("OS_IDENTITY_API_VERSION").and_return("3") + describe "SSL settings" do + it "carries a cacert path through" do + expect(driver.send(:translate_cloud_config, { "cacert" => "/path/ca.crt" })) + .to eq(ssl_ca_file: "/path/ca.crt") end - it "maps all env vars to fog config keys" do - result = driver.send(:load_env_vars) - expect(result[:openstack_auth_url]).to eq("https://env.example.com:5000/v3") - expect(result[:openstack_username]).to eq("envuser") - expect(result[:openstack_api_key]).to eq("envpass") - expect(result[:openstack_project_name]).to eq("envproject") - expect(result[:openstack_user_domain]).to eq("EnvDomain") - expect(result[:openstack_project_domain]).to eq("EnvProjDomain") - expect(result[:openstack_domain_id]).to eq("envdomid") - expect(result[:openstack_region]).to eq("EnvRegion") - expect(result[:openstack_identity_api_version]).to eq("3") + it "carries verify: false through as a boolean" do + expect(driver.send(:translate_cloud_config, { "verify" => false })) + .to eq(ssl_verify_peer: false) end - end - context "when OS_CACERT is set" do - before do - allow(ENV).to receive(:[]).with("OS_CACERT").and_return("/env/ca.crt") + it "carries verify: true through" do + expect(driver.send(:translate_cloud_config, { "verify" => true })) + .to eq(ssl_verify_peer: true) end - it "maps to ssl_ca_file" do - result = driver.send(:load_env_vars) - expect(result[:ssl_ca_file]).to eq("/env/ca.crt") + it "omits ssl_verify_peer when verify is absent" do + expect(driver.send(:translate_cloud_config, {})).not_to have_key(:ssl_verify_peer) end end + end - context "when an OS_* var is empty string" do - before do - allow(ENV).to receive(:[]).with("OS_AUTH_URL").and_return("") - end + describe "#load_env_vars" do + it "is empty when nothing is set" do + stub_env + + expect(driver.send(:load_env_vars)).to eq({}) + end + + it "maps every variable it knows about" do + stub_env( + "OS_AUTH_URL" => "https://env.example.com:5000/v3", + "OS_USERNAME" => "envuser", + "OS_PASSWORD" => "envpass", + "OS_PROJECT_NAME" => "envproject", + "OS_PROJECT_ID" => "envpid", + "OS_USER_DOMAIN_NAME" => "EnvDomain", + "OS_USER_DOMAIN_ID" => "envudid", + "OS_PROJECT_DOMAIN_NAME" => "EnvProjDomain", + "OS_PROJECT_DOMAIN_ID" => "envpdid", + "OS_DOMAIN_ID" => "envdomid", + "OS_DOMAIN_NAME" => "EnvDomainName", + "OS_REGION_NAME" => "EnvRegion", + "OS_INTERFACE" => "internal", + "OS_IDENTITY_API_VERSION" => "3", + "OS_APPLICATION_CREDENTIAL_ID" => "envacid", + "OS_APPLICATION_CREDENTIAL_SECRET" => "envacsecret", + "OS_CACERT" => "/env/ca.crt" + ) + + expect(driver.send(:load_env_vars)).to eq( + openstack_auth_url: "https://env.example.com:5000/v3", + openstack_username: "envuser", + openstack_api_key: "envpass", + openstack_project_name: "envproject", + openstack_project_id: "envpid", + openstack_user_domain: "EnvDomain", + openstack_user_domain_id: "envudid", + openstack_project_domain: "EnvProjDomain", + openstack_project_domain_id: "envpdid", + openstack_domain_id: "envdomid", + openstack_domain_name: "EnvDomainName", + openstack_region: "EnvRegion", + openstack_endpoint_type: "internal", + openstack_identity_api_version: "3", + openstack_application_credential_id: "envacid", + openstack_application_credential_secret: "envacsecret", + ssl_ca_file: "/env/ca.crt" + ) + end + + it "covers the whole documented map" do + expect(described_class::ENV_VAR_MAP.keys).to all(start_with("OS_")) + end + + # An exported-but-empty variable is how shells leave a cleared setting; it + # must not shadow the same key from clouds.yaml. + it "ignores empty variables" do + stub_env("OS_AUTH_URL" => "", "OS_USERNAME" => "envuser") + + expect(driver.send(:load_env_vars)).to eq(openstack_username: "envuser") + end + end - it "ignores the empty value" do - result = driver.send(:load_env_vars) - expect(result).not_to have_key(:openstack_auth_url) - end + describe "#load_clouds_config" do + it "is empty when no cloud is named" do + use_clouds_file + + expect(driver.send(:load_clouds_config)).to eq({}) end - context "with application credential env vars" do - before do - allow(ENV).to receive(:[]).with("OS_APPLICATION_CREDENTIAL_ID").and_return("appcred-id") - allow(ENV).to receive(:[]).with("OS_APPLICATION_CREDENTIAL_SECRET").and_return("appcred-secret") - end + it "translates the named cloud" do + use_clouds_file(env: { "OS_CLOUD" => "mycloud" }) - it "maps application credential env vars" do - result = driver.send(:load_env_vars) - expect(result[:openstack_application_credential_id]).to eq("appcred-id") - expect(result[:openstack_application_credential_secret]).to eq("appcred-secret") - end + expect(driver.send(:load_clouds_config)).to include( + openstack_auth_url: "https://keystone.example.com:5000/v3", + openstack_username: "testuser", + openstack_api_key: "testpass", + openstack_region: "RegionOne", + openstack_identity_api_version: "3" + ) end - end - describe "#apply_clouds_config" do - before do - allow(ENV).to receive(:[]).and_call_original - allow(ENV).to receive(:[]).with("OS_CLIENT_CONFIG_FILE").and_return(nil) - allow(ENV).to receive(:[]).with("OS_CLIENT_SECURE_FILE").and_return(nil) - allow(ENV).to receive(:[]).with("OS_CLOUD").and_return(nil) - Kitchen::Driver::Openstack::Clouds::ENV_VAR_MAP.each_key do |var| - allow(ENV).to receive(:[]).with(var).and_return(nil) - end + it "is empty when the named cloud is absent" do + use_clouds_file(env: { "OS_CLOUD" => "nonexistent" }) + + expect(driver.send(:load_clouds_config)).to eq({}) end - context "when clouds.yaml provides settings" do - let(:config) { { openstack_cloud: "mycloud" } } + it "is empty when there is no clouds.yaml at all" do + stub_env("OS_CLOUD" => "mycloud") + allow(Dir).to receive_messages(pwd: File.join(tmpdir, "empty"), home: File.join(tmpdir, "empty")) - before do - allow(File).to receive(:exist?).and_return(false) - allow(File).to receive(:exist?) - .with(File.join(Dir.pwd, "clouds.yaml")).and_return(true) - allow(File).to receive(:read) - .with(File.join(Dir.pwd, "clouds.yaml")) - .and_return(YAML.dump(clouds_yaml_content)) - end + expect(driver.send(:load_clouds_config)).to eq({}) + end - it "merges clouds.yaml values into config" do - driver.send(:apply_clouds_config) - expect(driver[:openstack_auth_url]).to eq("https://keystone.example.com:5000/v3") - expect(driver[:openstack_username]).to eq("testuser") - expect(driver[:openstack_api_key]).to eq("testpass") - expect(driver[:openstack_domain_id]).to eq("default") - expect(driver[:openstack_region]).to eq("RegionOne") + it "reads clouds.yaml from clouds_yaml_path" do + stub_env + config[:openstack_cloud] = "mycloud" + config[:clouds_yaml_path] = write_yaml("clouds.yaml", clouds_yaml) + + expect(driver.send(:load_clouds_config)).to include(openstack_username: "testuser") + end + + describe "secure.yaml" do + let(:secure_yaml) do + { "clouds" => { "mycloud" => { "auth" => { "password" => "secretpass" } } } } end - it "does not override existing config values" do - config[:openstack_region] = "OverriddenRegion" - driver.send(:apply_clouds_config) - expect(driver[:openstack_region]).to eq("OverriddenRegion") - expect(driver[:openstack_auth_url]).to eq("https://keystone.example.com:5000/v3") + it "overrides the password from clouds.yaml" do + use_clouds_file(secure: secure_yaml, env: { "OS_CLOUD" => "mycloud" }) + + expect(driver.send(:load_clouds_config)).to include(openstack_api_key: "secretpass") end - end - context "when SSL verify is false in clouds.yaml" do - let(:config) { { openstack_cloud: "sslcloud" } } + it "leaves the rest of clouds.yaml intact" do + use_clouds_file(secure: secure_yaml, env: { "OS_CLOUD" => "mycloud" }) - before do - allow(File).to receive(:exist?).and_return(false) - allow(File).to receive(:exist?) - .with(File.join(Dir.pwd, "clouds.yaml")).and_return(true) - allow(File).to receive(:read) - .with(File.join(Dir.pwd, "clouds.yaml")) - .and_return(YAML.dump(clouds_yaml_content)) + expect(driver.send(:load_clouds_config)).to include(openstack_username: "testuser") end - it "sets disable_ssl_validation in config" do - driver.send(:apply_clouds_config) - expect(driver[:disable_ssl_validation]).to eq(true) + it "ignores entries for other clouds" do + other = { "clouds" => { "othercloud" => { "auth" => { "password" => "nope" } } } } + use_clouds_file(secure: other, env: { "OS_CLOUD" => "mycloud" }) + + expect(driver.send(:load_clouds_config)).to include(openstack_api_key: "testpass") end end + end - context "when no cloud is configured" do - it "does not modify config" do - original = driver[:openstack_username] - driver.send(:apply_clouds_config) - expect(driver[:openstack_username]).to eq(original) - end + describe "#apply_clouds_config" do + it "does nothing when there is nothing to apply" do + stub_env + allow(Dir).to receive_messages(pwd: File.join(tmpdir, "empty"), home: File.join(tmpdir, "empty")) + + driver.send(:apply_clouds_config) + + expect(driver[:openstack_username]).to be_nil end - context "using OS_CLOUD env var" do - before do - allow(ENV).to receive(:[]).with("OS_CLOUD").and_return("mycloud") - allow(File).to receive(:exist?).and_return(false) - allow(File).to receive(:exist?) - .with(File.join(Dir.pwd, "clouds.yaml")).and_return(true) - allow(File).to receive(:read) - .with(File.join(Dir.pwd, "clouds.yaml")) - .and_return(YAML.dump(clouds_yaml_content)) - end + it "fills in values from clouds.yaml" do + use_clouds_file(env: { "OS_CLOUD" => "mycloud" }) - it "merges values from clouds.yaml via env var" do - driver.send(:apply_clouds_config) - expect(driver[:openstack_auth_url]).to eq("https://keystone.example.com:5000/v3") - expect(driver[:openstack_username]).to eq("testuser") - end + driver.send(:apply_clouds_config) + + expect(driver[:openstack_username]).to eq("testuser") + expect(driver[:openstack_api_key]).to eq("testpass") + expect(driver[:openstack_region]).to eq("RegionOne") end - context "when only OS_* env vars are set (no clouds.yaml)" do - before do - allow(ENV).to receive(:[]).with("OS_AUTH_URL").and_return("https://env.example.com:5000/v3") - allow(ENV).to receive(:[]).with("OS_USERNAME").and_return("envuser") - allow(ENV).to receive(:[]).with("OS_PASSWORD").and_return("envpass") - allow(ENV).to receive(:[]).with("OS_DOMAIN_ID").and_return("envdomid") - allow(ENV).to receive(:[]).with("OS_REGION_NAME").and_return("EnvRegion") - end + it "fills in values from OS_* variables with no clouds.yaml" do + stub_env("OS_AUTH_URL" => "https://env.example.com:5000/v3", "OS_USERNAME" => "envuser") + allow(Dir).to receive_messages(pwd: File.join(tmpdir, "empty"), home: File.join(tmpdir, "empty")) + + driver.send(:apply_clouds_config) + + expect(driver[:openstack_username]).to eq("envuser") + end + + describe "precedence" do + it "lets OS_* variables beat clouds.yaml" do + use_clouds_file(env: { "OS_CLOUD" => "mycloud", "OS_USERNAME" => "envuser" }) - it "populates config from env vars" do driver.send(:apply_clouds_config) - expect(driver[:openstack_auth_url]).to eq("https://env.example.com:5000/v3") + expect(driver[:openstack_username]).to eq("envuser") - expect(driver[:openstack_api_key]).to eq("envpass") - expect(driver[:openstack_domain_id]).to eq("envdomid") - expect(driver[:openstack_region]).to eq("EnvRegion") end - end - context "when OS_* env vars override clouds.yaml values" do - let(:config) { { openstack_cloud: "mycloud" } } + it "lets kitchen.yml beat OS_* variables" do + use_clouds_file(env: { "OS_CLOUD" => "mycloud", "OS_USERNAME" => "envuser" }) + config[:openstack_username] = "kitchenuser" + + driver.send(:apply_clouds_config) - before do - allow(File).to receive(:exist?).and_return(false) - allow(File).to receive(:exist?) - .with(File.join(Dir.pwd, "clouds.yaml")).and_return(true) - allow(File).to receive(:read) - .with(File.join(Dir.pwd, "clouds.yaml")) - .and_return(YAML.dump(clouds_yaml_content)) - allow(ENV).to receive(:[]).with("OS_REGION_NAME").and_return("EnvRegionOverride") + expect(driver[:openstack_username]).to eq("kitchenuser") end - it "uses env var value over clouds.yaml" do + it "resolves the full three-way chain per key" do + use_clouds_file( + env: { + "OS_CLOUD" => "mycloud", + "OS_USERNAME" => "envuser", + "OS_REGION_NAME" => "EnvRegion", + } + ) + config[:openstack_username] = "kitchenuser" + driver.send(:apply_clouds_config) - expect(driver[:openstack_region]).to eq("EnvRegionOverride") - # clouds.yaml values still fill remaining keys - expect(driver[:openstack_auth_url]).to eq("https://keystone.example.com:5000/v3") - expect(driver[:openstack_username]).to eq("testuser") + + expect(driver[:openstack_username]).to eq("kitchenuser") # kitchen.yml wins + expect(driver[:openstack_region]).to eq("EnvRegion") # env beats clouds.yaml + expect(driver[:openstack_api_key]).to eq("testpass") # only clouds.yaml has it end end - context "when kitchen.yml overrides OS_* env vars" do - let(:config) { { openstack_region: "KitchenRegion" } } + describe "SSL handling" do + it "turns off validation when clouds.yaml says verify: false" do + cloud = clouds_yaml + cloud["clouds"]["mycloud"]["verify"] = false + use_clouds_file(cloud, env: { "OS_CLOUD" => "mycloud" }) + + driver.send(:apply_clouds_config) - before do - allow(ENV).to receive(:[]).with("OS_AUTH_URL").and_return("https://env.example.com:5000/v3") - allow(ENV).to receive(:[]).with("OS_USERNAME").and_return("envuser") - allow(ENV).to receive(:[]).with("OS_PASSWORD").and_return("envpass") - allow(ENV).to receive(:[]).with("OS_DOMAIN_ID").and_return("envdomid") - allow(ENV).to receive(:[]).with("OS_REGION_NAME").and_return("EnvRegion") + expect(driver[:disable_ssl_validation]).to be(true) end - it "uses kitchen.yml value over env var" do + it "leaves validation on when verify is true" do + cloud = clouds_yaml + cloud["clouds"]["mycloud"]["verify"] = true + use_clouds_file(cloud, env: { "OS_CLOUD" => "mycloud" }) + driver.send(:apply_clouds_config) - expect(driver[:openstack_region]).to eq("KitchenRegion") - # env var values still fill remaining keys - expect(driver[:openstack_auth_url]).to eq("https://env.example.com:5000/v3") - expect(driver[:openstack_username]).to eq("envuser") + + expect(driver[:disable_ssl_validation]).to be_nil end - end - context "full precedence: kitchen.yml > OS_* > clouds.yaml" do - let(:config) { { openstack_cloud: "mycloud", openstack_username: "kitchenuser" } } + it "leaves validation on when verify is absent" do + use_clouds_file(env: { "OS_CLOUD" => "mycloud" }) + + driver.send(:apply_clouds_config) - before do - allow(File).to receive(:exist?).and_return(false) - allow(File).to receive(:exist?) - .with(File.join(Dir.pwd, "clouds.yaml")).and_return(true) - allow(File).to receive(:read) - .with(File.join(Dir.pwd, "clouds.yaml")) - .and_return(YAML.dump(clouds_yaml_content)) - allow(ENV).to receive(:[]).with("OS_USERNAME").and_return("envuser") - allow(ENV).to receive(:[]).with("OS_REGION_NAME").and_return("EnvRegion") + expect(driver[:disable_ssl_validation]).to be_nil end - it "respects the full precedence chain" do + it "carries a cacert from clouds.yaml into ssl_ca_file" do + cloud = clouds_yaml + cloud["clouds"]["mycloud"]["cacert"] = "/path/ca.crt" + use_clouds_file(cloud, env: { "OS_CLOUD" => "mycloud" }) + driver.send(:apply_clouds_config) - # kitchen.yml wins over both env and clouds.yaml - expect(driver[:openstack_username]).to eq("kitchenuser") - # OS_* env var wins over clouds.yaml - expect(driver[:openstack_region]).to eq("EnvRegion") - # clouds.yaml fills remaining nils - expect(driver[:openstack_auth_url]).to eq("https://keystone.example.com:5000/v3") - expect(driver[:openstack_api_key]).to eq("testpass") + + expect(driver[:ssl_ca_file]).to eq("/path/ca.crt") end - end - end - describe "#openstack_server with clouds.yaml" do - let(:config) { { openstack_cloud: "mycloud" } } + it "carries OS_CACERT into ssl_ca_file" do + use_clouds_file(env: { "OS_CLOUD" => "mycloud", "OS_CACERT" => "/env/ca.crt" }) - before do - allow(ENV).to receive(:[]).and_call_original - allow(ENV).to receive(:[]).with("OS_CLIENT_CONFIG_FILE").and_return(nil) - allow(ENV).to receive(:[]).with("OS_CLIENT_SECURE_FILE").and_return(nil) - allow(ENV).to receive(:[]).with("OS_CLOUD").and_return(nil) - Kitchen::Driver::Openstack::Clouds::ENV_VAR_MAP.each_key do |var| - allow(ENV).to receive(:[]).with(var).and_return(nil) + driver.send(:apply_clouds_config) + + expect(driver[:ssl_ca_file]).to eq("/env/ca.crt") end - allow(File).to receive(:exist?).and_return(false) - allow(File).to receive(:exist?) - .with(File.join(Dir.pwd, "clouds.yaml")).and_return(true) - allow(File).to receive(:read) - .with(File.join(Dir.pwd, "clouds.yaml")) - .and_return(YAML.dump(clouds_yaml_content)) end + end + + describe "end-to-end through openstack_server" do + it "hands clouds.yaml credentials to Fog" do + use_clouds_file(env: { "OS_CLOUD" => "mycloud" }) - it "populates server settings after apply_clouds_config" do driver.send(:apply_clouds_config) - result = driver.send(:openstack_server) - expect(result[:openstack_auth_url]).to eq("https://keystone.example.com:5000/v3") - expect(result[:openstack_username]).to eq("testuser") - expect(result[:openstack_api_key]).to eq("testpass") - expect(result[:openstack_domain_id]).to eq("default") - expect(result[:openstack_region]).to eq("RegionOne") - end - - context "when kitchen.yml overrides clouds.yaml values" do - let(:config) do - { - openstack_cloud: "mycloud", - openstack_region: "OverriddenRegion", - } - end - it "uses the kitchen.yml value for the overridden key" do - driver.send(:apply_clouds_config) - result = driver.send(:openstack_server) - expect(result[:openstack_region]).to eq("OverriddenRegion") - expect(result[:openstack_auth_url]).to eq("https://keystone.example.com:5000/v3") - end + expect(driver.send(:openstack_server)).to include( + openstack_username: "testuser", + openstack_api_key: "testpass", + openstack_auth_url: "https://keystone.example.com:5000/v3", + openstack_domain_id: "default", + openstack_region: "RegionOne" + ) end - context "using OS_CLOUD env var" do - let(:config) { {} } + it "prefixes the identity API version so Fog does not re-coerce it" do + use_clouds_file(env: { "OS_CLOUD" => "mycloud" }) - before do - allow(ENV).to receive(:[]).with("OS_CLOUD").and_return("mycloud") - end + driver.send(:apply_clouds_config) - it "populates server settings from clouds.yaml via env var" do - driver.send(:apply_clouds_config) - result = driver.send(:openstack_server) - expect(result[:openstack_auth_url]).to eq("https://keystone.example.com:5000/v3") - expect(result[:openstack_username]).to eq("testuser") - end + expect(driver.send(:openstack_server)[:openstack_identity_api_version]).to eq("v3") end - context "using only OS_* env vars (no clouds.yaml)" do - let(:config) { {} } + it "routes a clouds.yaml cacert into the Excon connection options" do + cloud = clouds_yaml + cloud["clouds"]["mycloud"]["cacert"] = "/path/ca.crt" + use_clouds_file(cloud, env: { "OS_CLOUD" => "mycloud" }) - before do - allow(File).to receive(:exist?).and_return(false) - allow(ENV).to receive(:[]).with("OS_AUTH_URL").and_return("https://env.example.com:5000/v3") - allow(ENV).to receive(:[]).with("OS_USERNAME").and_return("envuser") - allow(ENV).to receive(:[]).with("OS_PASSWORD").and_return("envpass") - allow(ENV).to receive(:[]).with("OS_DOMAIN_ID").and_return("envdomid") - end + driver.send(:apply_clouds_config) - it "populates server settings from env vars" do - driver.send(:apply_clouds_config) - result = driver.send(:openstack_server) - expect(result[:openstack_auth_url]).to eq("https://env.example.com:5000/v3") - expect(result[:openstack_username]).to eq("envuser") - expect(result[:openstack_api_key]).to eq("envpass") - expect(result[:openstack_domain_id]).to eq("envdomid") - end + expect(driver.send(:openstack_server)[:connection_options]) + .to include(ssl_ca_file: "/path/ca.crt") + end + end + + describe "constants" do + # Pinned as a literal rather than recomputed from the two maps: the + # constant is *defined* as those maps' values, so asserting the same + # expression could never fail. Spelled out, adding a clouds.yaml mapping + # forces a deliberate decision about whether Fog wants it stringified. + it "treats every mapped auth and top-level key as string-valued" do + expect(described_class::STRING_CONFIG_KEYS).to match_array( + %i{ + openstack_auth_url + openstack_username + openstack_api_key + openstack_project_name + openstack_project_id + openstack_user_domain + openstack_user_domain_id + openstack_project_domain + openstack_project_domain_id + openstack_domain_id + openstack_domain_name + openstack_application_credential_id + openstack_application_credential_secret + openstack_region + openstack_endpoint_type + openstack_identity_api_version + } + ) + end + + it "does not stringify the SSL keys, which are a boolean and a path" do + expect(described_class::STRING_CONFIG_KEYS).not_to include(:ssl_verify_peer, :ssl_ca_file) end end end diff --git a/spec/kitchen/driver/openstack/config_spec.rb b/spec/kitchen/driver/openstack/config_spec.rb new file mode 100644 index 00000000..2c7cdae3 --- /dev/null +++ b/spec/kitchen/driver/openstack/config_spec.rb @@ -0,0 +1,211 @@ +# frozen_string_literal: true + +RSpec.describe Kitchen::Driver::Openstack::Config do + include_context "with a configured driver" + + let(:login) { "user" } + let(:hostname) { "host" } + + # Both lookups hit the host machine, so pin them or the assertions below + # depend on whoever is running the suite. + before do + allow(Etc).to receive(:getpwuid).and_return(Struct.new(:name).new(login)) + allow(Etc).to receive(:getlogin).and_return(login) + allow(Socket).to receive(:gethostname).and_return(hostname) + end + + describe "#config_server_name" do + context "when the user configured a server name" do + let(:config) { { server_name: "puppy" } } + + it "leaves it alone" do + driver.send(:config_server_name) + + expect(driver[:server_name]).to eq("puppy") + end + + it "does not consult the prefix" do + config[:server_name_prefix] = "parsnip" + + driver.send(:config_server_name) + + expect(driver[:server_name]).to eq("puppy") + end + end + + context "when only a prefix is configured" do + let(:config) { { server_name_prefix: "parsnip" } } + + it "builds the name from the prefix" do + driver.send(:config_server_name) + + expect(driver[:server_name]).to match(/\Aparsnip-[a-z]{8}\z/) + end + end + + context "when nothing is configured" do + it "generates a full default name" do + driver.send(:config_server_name) + + expect(driver[:server_name]).to match(/\Apotatoes-user-host-[a-z0-9]{7}\z/) + end + end + end + + describe "#default_name" do + it "joins instance, user, host and a random suffix" do + expect(driver.send(:default_name)).to match(/\Apotatoes-user-host-[a-z0-9]{7}\z/) + end + + it "returns a different name on each call" do + expect(driver.send(:default_name)).not_to eq(driver.send(:default_name)) + end + + context "with a long hostname" do + let(:hostname) { "ab.c" * 20 } + + it "stays within the OpenStack name limit" do + expect(driver.send(:default_name).length) + .to be <= Kitchen::Driver::Openstack::Config::MAX_SERVER_NAME_LENGTH + end + end + + context "with a long instance name, user and host" do + let(:instance_name) { "a" * 40 } + let(:login) { "b" * 40 } + let(:hostname) { "c" * 40 } + + it "stays within the OpenStack name limit" do + expect(driver.send(:default_name).length) + .to be <= Kitchen::Driver::Openstack::Config::MAX_SERVER_NAME_LENGTH + end + + # The longest name the generator can produce is exactly the limit, which + # is what makes MAX_SERVER_NAME_LENGTH a real constraint rather than a + # comment: the per-component budgets are derived from it. + it "uses the whole budget when every component is oversized" do + expect(driver.send(:default_name).length) + .to eq(Kitchen::Driver::Openstack::Config::MAX_SERVER_NAME_LENGTH) + end + + it "truncates each component to its documented budget" do + config_mod = Kitchen::Driver::Openstack::Config + instance_part, user_part, host_part, random_part = driver.send(:default_name).split("-") + + expect(instance_part.length).to eq(config_mod::NAME_INSTANCE_LENGTH) + expect(user_part.length).to eq(config_mod::NAME_USERNAME_LENGTH) + expect(host_part.length).to eq(config_mod::NAME_HOSTNAME_LENGTH) + expect(random_part.length).to eq(config_mod::NAME_RANDOM_LENGTH) + end + end + + context "with punctuation in the names" do + let(:login) { "some.u-se-r" } + let(:hostname) { "a.host-name" } + let(:instance_name) { "a.instance-name" } + + it "strips characters OpenStack rejects in server names" do + expect(driver.send(:default_name)).not_to include(".") + end + + it "leaves exactly the three separators" do + expect(driver.send(:default_name).count("-")).to eq(3) + end + end + + # Regression: the old code assumed Etc.getpwuid returns nil for an unknown + # uid. It raises, so this path used to blow up inside containers. + context "when the uid has no passwd entry" do + before { allow(Etc).to receive(:getpwuid).and_raise(ArgumentError, "can't find user for 501") } + + it "falls back to the login name" do + expect(driver.send(:default_name)).to match(/\Apotatoes-user-host-/) + end + + context "and there is no login name either" do + before { allow(Etc).to receive(:getlogin).and_return(nil) } + + it "substitutes a placeholder rather than raising" do + expect(driver.send(:default_name)).to match(/\Apotatoes-nologin-host-/) + end + end + end + + context "when getpwuid returns nil" do + before { allow(Etc).to receive_messages(getpwuid: nil, getlogin: nil) } + + it "substitutes a placeholder" do + expect(driver.send(:default_name)).to match(/\Apotatoes-nologin-host-/) + end + end + end + + describe "#server_name_prefix" do + it "appends a random lowercase suffix" do + expect(driver.send(:server_name_prefix, "parsnip")).to match(/\Aparsnip-[a-z]{8}\z/) + end + + it "returns a different name on each call" do + expect(driver.send(:server_name_prefix, "parsnip")) + .not_to eq(driver.send(:server_name_prefix, "parsnip")) + end + + it "strips characters OpenStack rejects" do + expect(driver.send(:server_name_prefix, "pars.nip-x")).to match(/\Aparsnipx-[a-z]{8}\z/) + end + + context "with a prefix over the length budget" do + let(:long_prefix) { "a" * 70 } + + it "stays within the OpenStack name limit" do + expect(driver.send(:server_name_prefix, long_prefix).length) + .to be <= Kitchen::Driver::Openstack::Config::MAX_SERVER_NAME_LENGTH + end + + it "warns the user it truncated" do + driver.send(:server_name_prefix, long_prefix) + + expect(logged_output.string).to match(/prefix too long/i) + end + end + + context "with a prefix that is empty once stripped" do + it "falls back to a fully generated name" do + expect(driver.send(:server_name_prefix, "...")).to match(/\Apotatoes-user-host-/) + end + + it "warns the user" do + driver.send(:server_name_prefix, "...") + + expect(logged_output.string).to match(/prefix empty or invalid/i) + end + + it "handles an entirely empty prefix" do + expect(driver.send(:server_name_prefix, "")).to match(/\Apotatoes-user-host-/) + end + end + + # Regression: the old implementation called gsub! on the argument, which + # rewrote config[:server_name_prefix] in place and raised FrozenError when + # the prefix came from a frozen string literal. + it "does not mutate the string it was given" do + prefix = +"pars.nip" + + driver.send(:server_name_prefix, prefix) + + expect(prefix).to eq("pars.nip") + end + + it "accepts a frozen prefix" do + expect { driver.send(:server_name_prefix, "pars.nip".freeze) }.not_to raise_error + end + + it "leaves the configured prefix intact after config_server_name" do + config[:server_name_prefix] = +"pars.nip" + + driver.send(:config_server_name) + + expect(driver[:server_name_prefix]).to eq("pars.nip") + end + end +end diff --git a/spec/kitchen/driver/openstack/helpers_spec.rb b/spec/kitchen/driver/openstack/helpers_spec.rb new file mode 100644 index 00000000..b7d7fc6a --- /dev/null +++ b/spec/kitchen/driver/openstack/helpers_spec.rb @@ -0,0 +1,181 @@ +# frozen_string_literal: true + +require "excon" unless defined?(Excon) +require "ohai" unless defined?(Ohai::System) + +RSpec.describe Kitchen::Driver::Openstack::Helpers do + include_context "with a configured driver" + + let(:state) { { hostname: "192.0.2.10", server_id: "test123" } } + + describe "#hints_path" do + it "returns ohai's first configured hints directory" do + allow(Ohai).to receive(:config).and_return(hints_path: %w{/etc/chef/ohai/hints /other}) + + expect(driver.send(:hints_path)).to eq("/etc/chef/ohai/hints") + end + end + + describe "#add_ohai_hint" do + let(:connection) { instance_double(Kitchen::Transport::Dummy::Connection, execute: true) } + + before do + allow(transport).to receive(:connection).with(state).and_return(connection) + allow(driver).to receive(:hints_path).and_return("/etc/chef/ohai/hints") + end + + context "on a Bourne-shell platform" do + before { allow(driver).to receive_messages(bourne_shell?: true, windows_os?: false) } + + it "creates the hints directory and the hint file in one command" do + driver.send(:add_ohai_hint, state) + + expect(connection).to have_received(:execute).with( + "sudo mkdir -p /etc/chef/ohai/hints && " \ + "sudo bash -c 'echo {} > /etc/chef/ohai/hints/openstack.json'" + ) + end + + it "tells the user what it is doing" do + driver.send(:add_ohai_hint, state) + + expect(logged_output.string).to include("Adding OpenStack hint for ohai") + end + end + + context "on Windows" do + before { allow(driver).to receive_messages(bourne_shell?: false, windows_os?: true) } + + it "creates the hint file with PowerShell" do + driver.send(:add_ohai_hint, state) + + expect(connection).to have_received(:execute).with( + "New-Item /etc/chef/ohai/hints\\openstack.json -Value '{}' -Force -Type file" + ) + end + end + + context "on an unrecognized platform" do + before { allow(driver).to receive_messages(bourne_shell?: false, windows_os?: false) } + + it "does not open a transport session" do + driver.send(:add_ohai_hint, state) + + expect(connection).not_to have_received(:execute) + end + + it "says why it skipped" do + driver.send(:add_ohai_hint, state) + + expect(logged_output.string).to include("skipping the OpenStack ohai hint") + end + end + end + + describe "#disable_ssl_validation" do + around do |example| + previous = Excon.defaults[:ssl_verify_peer] + example.run + Excon.defaults[:ssl_verify_peer] = previous + end + + it "turns off Excon peer verification" do + Excon.defaults[:ssl_verify_peer] = true + + driver.send(:disable_ssl_validation) + + expect(Excon.defaults[:ssl_verify_peer]).to be(false) + end + end + + describe "#countdown" do + before { allow(Kernel).to receive(:print) } + + it "prints a dot for each tick" do + # Two ticks' worth of wall clock: start, +10, then done. + now = Time.now + allow(Time).to receive(:now).and_return(now, now, now + 10, now + 20) + + driver.send(:countdown, 20) + + expect(Kernel).to have_received(:print).with(".").twice + end + + it "sleeps between ticks rather than spinning" do + now = Time.now + allow(Time).to receive(:now).and_return(now, now, now + 30) + + driver.send(:countdown, 20) + + expect(driver).to have_received(:sleep).with(described_class::COUNTDOWN_TICK) + end + + it "does nothing when the duration has already elapsed" do + driver.send(:countdown, 0) + + expect(Kernel).not_to have_received(:print) + end + end + + describe "#wait_for_server" do + let(:connection) { instance_double(Kitchen::Transport::Dummy::Connection, wait_until_ready: true) } + + before { allow(transport).to receive(:connection).with(state).and_return(connection) } + + it "waits for the transport to report ready" do + driver.send(:wait_for_server, state) + + expect(connection).to have_received(:wait_until_ready) + end + + it "does not sleep when no server_wait is configured" do + allow(driver).to receive(:countdown) + + driver.send(:wait_for_server, state) + + expect(driver).not_to have_received(:countdown) + end + + context "with server_wait configured" do + let(:config) { { server_wait: 30 } } + + before { allow(driver).to receive(:countdown) } + + it "counts down before checking readiness" do + driver.send(:wait_for_server, state) + + expect(driver).to have_received(:countdown).with(30) + end + + it "tells the user it is waiting" do + driver.send(:wait_for_server, state) + + expect(logged_output.string).to include("Sleeping for 30 seconds") + end + end + + context "when the server never becomes reachable" do + before do + allow(connection).to receive(:wait_until_ready).and_raise(Kitchen::Transport::TransportFailed, "timed out") + allow(driver).to receive(:destroy) + end + + it "destroys the unreachable server" do + expect { driver.send(:wait_for_server, state) }.to raise_error(Kitchen::Transport::TransportFailed) + + expect(driver).to have_received(:destroy).with(state) + end + + it "re-raises the original failure" do + expect { driver.send(:wait_for_server, state) } + .to raise_error(Kitchen::Transport::TransportFailed, /timed out/) + end + + it "logs the host, the id and the underlying reason" do + expect { driver.send(:wait_for_server, state) }.to raise_error(Kitchen::Transport::TransportFailed) + + expect(logged_output.string).to include("192.0.2.10", "test123", "timed out") + end + end + end +end diff --git a/spec/kitchen/driver/openstack/networking_spec.rb b/spec/kitchen/driver/openstack/networking_spec.rb new file mode 100644 index 00000000..30fb3735 --- /dev/null +++ b/spec/kitchen/driver/openstack/networking_spec.rb @@ -0,0 +1,482 @@ +# frozen_string_literal: true + +RSpec.describe Kitchen::Driver::Openstack::Networking do + include_context "with a configured driver" + + let(:server) { fog_server } + + describe "#attach_ip_from_pool" do + context "when reusing an already-allocated address" do + let(:config) { { floating_ip_pool: "swimmers" } } + + let(:addresses) do + [ + fog_address(ip: "1.1.1.1", pool: "swimmers", instance_id: "already-used"), + fog_address(ip: "1.1.1.2", pool: "some-other-pool"), + fog_address(ip: "1.1.1.3", pool: "swimmers", fixed_ip: "10.0.0.5"), + fog_address(ip: "1.1.1.4", pool: "swimmers"), + fog_address(ip: "1.1.1.5", pool: "swimmers"), + ] + end + + before { allow(driver).to receive(:compute).and_return(fog_compute(addresses: addresses)) } + + it "attaches the first genuinely free address in the pool" do + driver.send(:attach_ip_from_pool, server, "swimmers") + + expect(server).to have_received(:associate_address).with("1.1.1.4") + end + + it "records the chosen address in config" do + driver.send(:attach_ip_from_pool, server, "swimmers") + + expect(driver[:floating_ip]).to eq("1.1.1.4") + end + + it "does not allocate a new address" do + expect(driver).not_to receive(:network) + + driver.send(:attach_ip_from_pool, server, "swimmers") + end + + context "when every address in the pool is taken" do + let(:addresses) { [fog_address(ip: "1.1.1.1", pool: "swimmers", instance_id: "in-use")] } + + it "fails with the pool name" do + expect { driver.send(:attach_ip_from_pool, server, "swimmers") } + .to raise_error(Kitchen::ActionFailed, "No available IPs in pool ") + end + end + + context "when the pool holds no addresses at all" do + let(:addresses) { [] } + + it "fails rather than attaching nil" do + expect { driver.send(:attach_ip_from_pool, server, "swimmers") } + .to raise_error(Kitchen::ActionFailed, "No available IPs in pool ") + end + end + end + + context "when allocating a new address" do + let(:config) { { floating_ip_pool: "swimmers", allocate_floating_ip: true } } + + let(:networks_body) { { "networks" => [{ "id" => "net-uuid-1" }] } } + let(:net) do + fog_network( + list_networks: fog_response(networks_body), + create_floating_ip: fog_response( + "floatingip" => { "floating_ip_address" => "203.0.113.9" } + ) + ) + end + + before { allow(driver).to receive(:network).and_return(net) } + + it "looks the pool up by name" do + driver.send(:attach_ip_from_pool, server, "swimmers") + + expect(net).to have_received(:list_networks).with(name: "swimmers") + end + + it "creates a floating IP on the pool's network" do + driver.send(:attach_ip_from_pool, server, "swimmers") + + expect(net).to have_received(:create_floating_ip).with("net-uuid-1") + end + + it "attaches the newly allocated address" do + driver.send(:attach_ip_from_pool, server, "swimmers") + + expect(server).to have_received(:associate_address).with("203.0.113.9") + end + + # Only for the rest of this process: `get_ip` returns config[:floating_ip] + # verbatim when it is set, which saves re-deriving the address it just + # attached. `destroy` does not read it -- it rebuilds the address from the + # server's public addresses and :public_ip_order. + it "records the new address in config for get_ip to reuse" do + driver.send(:attach_ip_from_pool, server, "swimmers") + + expect(driver[:floating_ip]).to eq("203.0.113.9") + expect(driver.send(:get_ip, server)).to eq("203.0.113.9") + end + + it "builds only one network connection for the whole sequence" do + driver.send(:attach_ip_from_pool, server, "swimmers") + + expect(driver).to have_received(:network).once + end + + # Regression: the old code indexed straight into [0]["id"], so a typo in + # the pool name surfaced as NoMethodError on nil rather than a usable + # message. + context "when no network matches the pool name" do + let(:networks_body) { { "networks" => [] } } + + it "fails with the pool name" do + expect { driver.send(:attach_ip_from_pool, server, "typo") } + .to raise_error(Kitchen::ActionFailed, "Floating IP pool not found") + end + end + + context "when Neutron omits the networks key entirely" do + let(:networks_body) { {} } + + it "fails with the pool name" do + expect { driver.send(:attach_ip_from_pool, server, "swimmers") } + .to raise_error(Kitchen::ActionFailed, "Floating IP pool not found") + end + end + end + + it "serializes pool access across threads" do + allow(driver).to receive(:compute) + .and_return(fog_compute(addresses: [fog_address(ip: "1.1.1.4", pool: "swimmers")])) + + expect(Kitchen::Driver::Openstack::Networking::IP_POOL_LOCK).to receive(:synchronize).and_call_original + + driver.send(:attach_ip_from_pool, server, "swimmers") + end + end + + describe "#attach_ip" do + it "associates the address with the server" do + driver.send(:attach_ip, server, "1.2.3.4") + + expect(server).to have_received(:associate_address).with("1.2.3.4") + end + + it "reports what it attached" do + driver.send(:attach_ip, server, "1.2.3.4") + + expect(logged_output.string).to include("Attaching floating IP <1.2.3.4>") + end + end + + describe "#get_public_private_ips" do + it "reads the dedicated accessors when the extension is available" do + server = fog_server(public_ip_addresses: %w{1.2.3.4}, private_ip_addresses: %w{10.0.0.1}) + + expect(driver.send(:get_public_private_ips, server)).to eq([%w{1.2.3.4}, %w{10.0.0.1}]) + end + + # Deployments without the floating IP extension 404 or 403 on the + # dedicated accessors. See https://github.com/fog/fog/issues/2160 + [Fog::OpenStack::Compute::NotFound, Excon::Errors::Forbidden].each do |error| + context "when the accessors raise #{error}" do + let(:server) do + fog_server( + addresses: { + "public" => [{ "addr" => "1.2.3.4" }], + "private" => [{ "addr" => "10.0.0.1" }], + } + ) + end + + before do + allow(server).to receive(:public_ip_addresses).and_raise(error, "no floating IP extension") + allow(server).to receive(:private_ip_addresses).and_raise(error, "no floating IP extension") + end + + it "falls back to the addresses hash" do + expect(driver.send(:get_public_private_ips, server)).to eq([%w{1.2.3.4}, %w{10.0.0.1}]) + end + end + end + + context "when the fallback hash holds only public addresses" do + let(:server) { fog_server(addresses: { "public" => [{ "addr" => "1.2.3.4" }] }) } + + before { allow(server).to receive(:public_ip_addresses).and_raise(Excon::Errors::Forbidden, "nope") } + + it "returns nil for the missing side" do + expect(driver.send(:get_public_private_ips, server)).to eq([%w{1.2.3.4}, nil]) + end + end + + context "when the fallback hash is empty" do + let(:server) { fog_server(addresses: {}) } + + before { allow(server).to receive(:public_ip_addresses).and_raise(Excon::Errors::Forbidden, "nope") } + + it "returns nils rather than raising" do + expect(driver.send(:get_public_private_ips, server)).to eq([nil, nil]) + end + end + end + + describe "#get_ip" do + context "when a floating IP is configured" do + let(:config) { { floating_ip: "1.2.3.4" } } + + it "uses it verbatim" do + expect(driver.send(:get_ip, server)).to eq("1.2.3.4") + end + + it "does not wait on the server" do + driver.send(:get_ip, server) + + expect(server).not_to have_received(:wait_for) + end + end + + it "waits for network information before reading addresses" do + driver.send(:get_ip, server) + + expect(server).to have_received(:wait_for) + end + + # Driven through the real readiness predicate rather than by stubbing + # wait_for to raise: a server whose addresses hash stays empty is exactly + # the state the predicate exists to detect, and the fog_server factory's + # wait_for raises TimeoutError when the block never goes truthy. + context "when network information never arrives" do + let(:server) { fog_server(addresses: {}, public_ip_addresses: [], private_ip_addresses: []) } + + it "fails with a clear message" do + expect { driver.send(:get_ip, server) } + .to raise_error(Kitchen::ActionFailed, "Could not get network information (timed out)") + end + end + + it "logs how long it waited for network information" do + driver.send(:get_ip, server) + + expect(logged_output.string).to include("Waited 0 seconds for network information.") + end + + context "with both public and private addresses" do + let(:server) { fog_server(public_ip_addresses: %w{1.2.3.4}, private_ip_addresses: %w{10.0.0.1}) } + + it "prefers the public address" do + expect(driver.send(:get_ip, server)).to eq("1.2.3.4") + end + end + + context "with only public addresses" do + let(:server) { fog_server(public_ip_addresses: %w{1.2.3.4}, private_ip_addresses: []) } + + it "uses the public address" do + expect(driver.send(:get_ip, server)).to eq("1.2.3.4") + end + end + + context "with only private addresses" do + let(:server) { fog_server(public_ip_addresses: [], private_ip_addresses: %w{10.0.0.1}) } + + it "falls back to the private address" do + expect(driver.send(:get_ip, server)).to eq("10.0.0.1") + end + end + + context "with several addresses on each side" do + let(:server) do + fog_server( + public_ip_addresses: %w{1.2.3.4 5.6.7.8}, + private_ip_addresses: %w{10.0.0.1 10.0.0.2} + ) + end + + it "honours public_ip_order" do + config[:public_ip_order] = 1 + + expect(driver.send(:get_ip, server)).to eq("5.6.7.8") + end + + it "accepts a string public_ip_order from YAML" do + config[:public_ip_order] = "1" + + expect(driver.send(:get_ip, server)).to eq("5.6.7.8") + end + + it "falls through to private_ip_order when the public index is out of range" do + config[:public_ip_order] = 5 + config[:private_ip_order] = 1 + + expect(driver.send(:get_ip, server)).to eq("10.0.0.2") + end + end + + context "when the deployment has no floating IP extension" do + let(:server) do + fog_server( + addresses: { + "public" => [{ "addr" => "1.2.3.4" }], + "private" => [{ "addr" => "10.0.0.1" }], + } + ) + end + + before do + allow(server).to receive(:public_ip_addresses).and_raise(Fog::OpenStack::Compute::NotFound, "not found") + allow(server).to receive(:private_ip_addresses).and_raise(Fog::OpenStack::Compute::NotFound, "not found") + end + + it "reads the address out of the addresses hash" do + expect(driver.send(:get_ip, server)).to eq("1.2.3.4") + end + end + + context "when the server reports no public or private addresses" do + let(:server) do + fog_server(public_ip_addresses: [], private_ip_addresses: [], ip_addresses: %w{10.0.0.9}) + end + + it "falls back to the flat ip_addresses list" do + expect(driver.send(:get_ip, server)).to eq("10.0.0.9") + end + end + + context "when there is no address at all" do + let(:server) do + fog_server(public_ip_addresses: [], private_ip_addresses: [], ip_addresses: []) + end + + it "fails" do + expect { driver.send(:get_ip, server) } + .to raise_error(Kitchen::ActionFailed, "Could not find an IP") + end + end + + describe "openstack_network_name" do + let(:config) { { openstack_network_name: "mynet" } } + let(:server) do + fog_server( + addresses: { + "mynet" => [{ "addr" => "10.0.0.5" }, { "addr" => "fe80::1" }], + "public" => [{ "addr" => "1.2.3.4" }], + } + ) + end + + it "prefers the named network over the public address" do + expect(driver.send(:get_ip, server)).to eq("10.0.0.5") + end + + it "returns the IPv6 address when use_ipv6 is set" do + config[:use_ipv6] = true + + expect(driver.send(:get_ip, server)).to eq("fe80::1") + end + + # Regression: the old code was a single unguarded expression, + # `filter_ips(server.addresses[name]).first["addr"]`, which raised + # NoMethodError on nil in two different ways -- on `select` for a + # misspelled network name, and on `[]` when the network existed but had + # no address of the configured family. Both now name what went wrong. + context "when the server is not on that network" do + it "fails with the network name" do + config[:openstack_network_name] = "typo" + + expect { driver.send(:get_ip, server) } + .to raise_error(Kitchen::ActionFailed, "Server is not attached to network ") + end + end + + context "when the network has no address of the requested family" do + let(:server) { fog_server(addresses: { "mynet" => [{ "addr" => "10.0.0.5" }] }) } + + it "fails naming the family it wanted" do + config[:use_ipv6] = true + + expect { driver.send(:get_ip, server) } + .to raise_error(Kitchen::ActionFailed, "No IPv6 address found on network ") + end + end + + context "when the network entry is empty" do + let(:server) { fog_server(addresses: { "mynet" => [] }) } + + it "fails rather than returning nil" do + expect { driver.send(:get_ip, server) } + .to raise_error(Kitchen::ActionFailed, "No IPv4 address found on network ") + end + end + end + end + + describe "#filter_ips" do + let(:addresses) { [{ "addr" => "1.2.3.4" }, { "addr" => "fe80::1" }, { "addr" => "5.6.7.8" }] } + + it "keeps only IPv4 addresses by default" do + expect(driver.send(:filter_ips, addresses)) + .to eq([{ "addr" => "1.2.3.4" }, { "addr" => "5.6.7.8" }]) + end + + it "keeps only IPv6 addresses when use_ipv6 is set" do + config[:use_ipv6] = true + + expect(driver.send(:filter_ips, addresses)).to eq([{ "addr" => "fe80::1" }]) + end + + it "returns an empty list when nothing matches" do + config[:use_ipv6] = true + + expect(driver.send(:filter_ips, [{ "addr" => "1.2.3.4" }])).to eq([]) + end + end + + describe "#parse_ips" do + let(:pub) { %w{1.2.3.4 2001:db8::1} } + let(:priv) { %w{10.0.0.1 fe80::1} } + + context "with both public and private addresses" do + it "keeps IPv4 by default" do + expect(driver.send(:parse_ips, pub, priv)).to eq([%w{1.2.3.4}, %w{10.0.0.1}]) + end + + it "keeps IPv6 when use_ipv6 is set" do + config[:use_ipv6] = true + + expect(driver.send(:parse_ips, pub, priv)).to eq([%w{2001:db8::1}, %w{fe80::1}]) + end + end + + context "with only public addresses" do + it "returns an empty private list" do + expect(driver.send(:parse_ips, pub, nil)).to eq([%w{1.2.3.4}, []]) + end + end + + context "with only private addresses" do + it "returns an empty public list" do + expect(driver.send(:parse_ips, nil, priv)).to eq([[], %w{10.0.0.1}]) + end + end + + context "with nothing at all" do + it "returns two empty lists" do + expect(driver.send(:parse_ips, nil, nil)).to eq([[], []]) + end + end + + it "wraps a bare string into a list" do + expect(driver.send(:parse_ips, "1.2.3.4", "10.0.0.1")).to eq([%w{1.2.3.4}, %w{10.0.0.1}]) + end + + # Regression: this used `select!`, and `Array(x)` returns x itself when x is + # already an Array -- so the filter ran against the caller's list, which in + # `get_ip` is the Fog server model's own address data. The earlier version + # of this example passed `original.dup` and then asserted on `original`, + # so it could not have failed. + it "does not mutate the lists it was given" do + pub = %w{1.2.3.4 2001:db8::1} + priv = %w{10.0.0.1 fd00::1} + + driver.send(:parse_ips, pub, priv) + + expect(pub).to eq(%w{1.2.3.4 2001:db8::1}) + expect(priv).to eq(%w{10.0.0.1 fd00::1}) + end + + it "returns lists that are not the ones it was given" do + pub = %w{1.2.3.4} + + result_pub, = driver.send(:parse_ips, pub, []) + + expect(result_pub).to eq(%w{1.2.3.4}) + expect(result_pub).not_to be(pub) + end + end +end diff --git a/spec/kitchen/driver/openstack/server_helper_spec.rb b/spec/kitchen/driver/openstack/server_helper_spec.rb new file mode 100644 index 00000000..a0d6e553 --- /dev/null +++ b/spec/kitchen/driver/openstack/server_helper_spec.rb @@ -0,0 +1,422 @@ +# frozen_string_literal: true + +require "tempfile" + +RSpec.describe Kitchen::Driver::Openstack::ServerHelper do + include_context "with a configured driver" + + let(:images) { [fog_resource(id: "111", name: "ubuntu-24.04"), fog_resource(id: "222", name: "centos-9")] } + let(:flavors) { [fog_resource(id: "1", name: "m1.tiny"), fog_resource(id: "2", name: "m1.small")] } + let(:networks) do + [fog_resource(id: "net-1", name: "public"), fog_resource(id: "net-2", name: "private")] + end + + let(:created_server) { fog_server } + let(:servers) { double("Fog servers collection", create: created_server) } + let(:compute) { fog_compute(servers: servers, images: images, flavors: flavors) } + let(:net_service) { fog_network(networks: double("networks", all: networks)) } + + before do + allow(driver).to receive_messages(compute: compute, network: net_service) + end + + describe "#create_server" do + let(:config) do + { server_name: "hello", image_id: "111", flavor_id: "1" } + end + + it "returns whatever Nova created" do + expect(driver.send(:create_server)).to be(created_server) + end + + it "submits the mandatory attributes" do + driver.send(:create_server) + + expect(servers).to have_received(:create).with( + hash_including(name: "hello", image_ref: "111", flavor_ref: "1") + ) + end + + it "passes the availability zone through" do + config[:availability_zone] = "az1" + + driver.send(:create_server) + + expect(servers).to have_received(:create).with(hash_including(availability_zone: "az1")) + end + + it "omits optional keys that are not configured" do + driver.send(:create_server) + + expect(servers).to have_received(:create) do |server_def| + expect(server_def.keys).to contain_exactly(:name, :image_ref, :flavor_ref, :availability_zone) + end + end + + describe "image and flavor resolution" do + it "resolves image_ref by name" do + config.delete(:image_id) + config[:image_ref] = "centos-9" + + driver.send(:create_server) + + expect(servers).to have_received(:create).with(hash_including(image_ref: "222")) + end + + it "resolves flavor_ref by name" do + config.delete(:flavor_id) + config[:flavor_ref] = "m1.small" + + driver.send(:create_server) + + expect(servers).to have_received(:create).with(hash_including(flavor_ref: "2")) + end + + it "rejects both image_id and image_ref" do + config[:image_ref] = "ubuntu-24.04" + + expect { driver.send(:create_server) } + .to raise_error(Kitchen::ActionFailed, "Cannot specify both image_ref and image_id") + end + + it "rejects both flavor_id and flavor_ref" do + config[:flavor_ref] = "m1.tiny" + + expect { driver.send(:create_server) } + .to raise_error(Kitchen::ActionFailed, "Cannot specify both flavor_ref and flavor_id") + end + + it "fails when the image cannot be resolved" do + config.delete(:image_id) + config[:image_ref] = "no-such-image" + + expect { driver.send(:create_server) }.to raise_error(Kitchen::ActionFailed, "Image not found") + end + + it "fails when the flavor cannot be resolved" do + config.delete(:flavor_id) + config[:flavor_ref] = "no-such-flavor" + + expect { driver.send(:create_server) }.to raise_error(Kitchen::ActionFailed, "Flavor not found") + end + end + + describe "networking" do + it "builds nics from network_id" do + config[:network_id] = "net-abc" + + driver.send(:create_server) + + expect(servers).to have_received(:create) + .with(hash_including(nics: [{ "net_id" => "net-abc" }])) + end + + it "builds nics from a list of network ids" do + config[:network_id] = %w{net-abc net-def} + + driver.send(:create_server) + + expect(servers).to have_received(:create) + .with(hash_including(nics: [{ "net_id" => "net-abc" }, { "net_id" => "net-def" }])) + end + + it "resolves network_ref by name" do + config[:network_ref] = "private" + + driver.send(:create_server) + + expect(servers).to have_received(:create) + .with(hash_including(nics: [{ "net_id" => "net-2" }])) + end + + it "resolves a list of network refs" do + config[:network_ref] = %w{public private} + + driver.send(:create_server) + + expect(servers).to have_received(:create) + .with(hash_including(nics: [{ "net_id" => "net-1" }, { "net_id" => "net-2" }])) + end + + it "rejects both network_id and network_ref" do + config[:network_id] = "net-abc" + config[:network_ref] = "private" + + expect { driver.send(:create_server) } + .to raise_error(Kitchen::ActionFailed, "Cannot specify both network_ref and network_id") + end + + it "fails when a network ref cannot be resolved" do + config[:network_ref] = "no-such-net" + + expect { driver.send(:create_server) }.to raise_error(Kitchen::ActionFailed, "Network not found") + end + + it "sends no nics when neither is configured" do + driver.send(:create_server) + + expect(servers).to have_received(:create) { |sd| expect(sd).not_to have_key(:nics) } + end + end + + describe "block device mapping" do + it "delegates to the volume helper" do + config[:block_device_mapping] = { make_volume: true, volume_size: "5" } + allow(driver).to receive(:get_bdm).and_return(volume_id: "vol-1") + + driver.send(:create_server) + + expect(servers).to have_received(:create) + .with(hash_including(block_device_mapping: { volume_id: "vol-1" })) + end + + # get_bdm is the only step in create_server that creates a resource, and + # the id of a volume it creates lives only in the server_def that a later + # raise discards -- so `kitchen destroy` would never see it. Local config + # validation has to happen first. + it "validates the rest of the config before creating a volume" do + config[:block_device_mapping] = { make_volume: true, volume_size: "5" } + config[:security_groups] = "default" + allow(driver).to receive(:get_bdm).and_return(volume_id: "vol-1") + + expect { driver.send(:create_server) } + .to raise_error(Kitchen::ActionFailed, /security_groups config must be an array/) + + expect(driver).not_to have_received(:get_bdm) + end + + it "validates the user_data path before creating a volume" do + config[:block_device_mapping] = { make_volume: true, volume_size: "5" } + config[:user_data] = "/nonexistent/cloud-init.yml" + allow(driver).to receive(:get_bdm).and_return(volume_id: "vol-1") + + expect { driver.send(:create_server) } + .to raise_error(Kitchen::ActionFailed, /user_data file .* does not exist/) + + expect(driver).not_to have_received(:get_bdm) + end + + it "detects the cloud_config/user_data conflict before creating a volume" do + config[:block_device_mapping] = { make_volume: true, volume_size: "5" } + config[:cloud_config] = { packages: %w{git} } + # A readable file, so it is the conflict that raises and not the + # existence check above it. + config[:user_data] = "/tmp/whatever" + allow(File).to receive(:exist?).with("/tmp/whatever").and_return(true) + allow(File).to receive(:read).with("/tmp/whatever").and_return("#!/bin/sh") + allow(driver).to receive(:get_bdm).and_return(volume_id: "vol-1") + + expect { driver.send(:create_server) } + .to raise_error(Kitchen::ActionFailed, "Cannot specify both cloud_config and user_data") + + expect(driver).not_to have_received(:get_bdm) + end + end + + describe "optional settings" do + it "passes key_name through" do + config[:key_name] = "my-key" + + driver.send(:create_server) + + expect(servers).to have_received(:create).with(hash_including(key_name: "my-key")) + end + + it "passes metadata through" do + config[:metadata] = { "owner" => "me" } + + driver.send(:create_server) + + expect(servers).to have_received(:create).with(hash_including(metadata: { "owner" => "me" })) + end + + it "passes config_drive through" do + config[:config_drive] = true + + driver.send(:create_server) + + expect(servers).to have_received(:create).with(hash_including(config_drive: true)) + end + + it "passes a list of security groups through" do + config[:security_groups] = %w{default web} + + driver.send(:create_server) + + expect(servers).to have_received(:create).with(hash_including(security_groups: %w{default web})) + end + end + + describe "cloud_config" do + it "renders it as a #cloud-config user_data document" do + config[:cloud_config] = { packages: %w{htop} } + + driver.send(:create_server) + + expect(servers).to have_received(:create) do |server_def| + expect(server_def[:user_data]).to start_with("#cloud-config\n") + expect(server_def[:user_data]).to include("packages") + expect(server_def[:user_data]).to include("htop") + end + end + + it "rejects being combined with user_data" do + config[:cloud_config] = { packages: %w{htop} } + config[:user_data] = "/tmp/whatever" + allow(File).to receive(:exist?).with("/tmp/whatever").and_return(true) + allow(File).to receive(:read).with("/tmp/whatever").and_return("#!/bin/sh") + + expect { driver.send(:create_server) } + .to raise_error(Kitchen::ActionFailed, "Cannot specify both cloud_config and user_data") + end + end + end + + describe "#optional_config" do + let(:config) { {} } + + describe "user_data" do + it "reads the file contents" do + Tempfile.create("user_data") do |file| + file.write("#!/bin/sh\necho hi\n") + file.flush + config[:user_data] = file.path + + expect(driver.send(:optional_config, :user_data)).to eq("#!/bin/sh\necho hi\n") + end + end + + # Regression: the old code returned nil for a missing file, so the server + # booted without the user_data the operator asked for and nothing said so. + it "fails when the file does not exist" do + config[:user_data] = "/nonexistent/cloud-init.sh" + + expect { driver.send(:optional_config, :user_data) } + .to raise_error(Kitchen::ActionFailed, "The user_data file does not exist") + end + end + + describe "security_groups" do + it "passes a list through" do + config[:security_groups] = %w{default} + + expect(driver.send(:optional_config, :security_groups)).to eq(%w{default}) + end + + # Regression: a bare string used to be dropped silently, booting the + # server into the default security group instead of the requested one. + it "fails on a bare string rather than dropping it" do + config[:security_groups] = "default" + + expect { driver.send(:optional_config, :security_groups) } + .to raise_error(Kitchen::ActionFailed, /security_groups config must be an array/) + end + end + + it "returns anything else verbatim" do + config[:key_name] = "my-key" + + expect(driver.send(:optional_config, :key_name)).to eq("my-key") + end + end + + describe "#find_matching" do + let(:collection) do + [ + fog_resource(id: "111", name: "ubuntu-24.04"), + fog_resource(id: "222", name: "ubuntu-22.04"), + fog_resource(id: "333", name: "centos-9"), + ] + end + + it "matches an exact id" do + expect(driver.send(:find_matching, collection, "222").name).to eq("ubuntu-22.04") + end + + it "matches an exact name" do + expect(driver.send(:find_matching, collection, "centos-9").id).to eq("333") + end + + it "prefers an id match over a name match" do + ambiguous = [fog_resource(id: "aaa", name: "bbb"), fog_resource(id: "bbb", name: "ccc")] + + expect(driver.send(:find_matching, ambiguous, "bbb").id).to eq("bbb") + end + + it "matches a regex against the name" do + expect(driver.send(:find_matching, collection, "/^ubuntu-22/").id).to eq("222") + end + + it "returns the first regex match when several names match" do + expect(driver.send(:find_matching, collection, "/^ubuntu/").id).to eq("111") + end + + it "does not treat a regex ref as an id" do + expect(driver.send(:find_matching, collection, "/nope/")).to be_nil + end + + it "returns nil when nothing matches" do + expect(driver.send(:find_matching, collection, "debian")).to be_nil + end + + it "returns nil for an empty collection" do + expect(driver.send(:find_matching, [], "anything")).to be_nil + end + + # Nova hands back integer ids for flavors on some deployments, while + # kitchen.yml always carries strings. + it "matches an integer id against a string ref" do + numeric = [fog_resource(id: 1, name: "m1.tiny")] + + expect(driver.send(:find_matching, numeric, "1").name).to eq("m1.tiny") + end + + it "accepts a non-string ref" do + numeric = [fog_resource(id: "1", name: "m1.tiny")] + + expect(driver.send(:find_matching, numeric, 1).name).to eq("m1.tiny") + end + + # Unnamed networks are legal in Neutron, so a regex search has to walk past + # them to reach a named match rather than stopping at the first entry. + it "skips unnamed resources when matching a regex" do + unnamed = [fog_resource(id: "111", name: nil), fog_resource(id: "222", name: "ubuntu-24.04")] + + expect(driver.send(:find_matching, unnamed, "/ubuntu/").id).to eq("222") + end + + # The `single.name &&` guard is what makes the above true for *any* regex, + # not just one that happens not to match "". Coercing a nil name with + # `to_s` instead would hand an empty string to the regex, and a pattern + # like /.*/ matches that -- silently selecting the unnamed resource. + it "skips unnamed resources even for a regex that matches an empty string" do + unnamed = [fog_resource(id: "111", name: nil), fog_resource(id: "222", name: "ubuntu-24.04")] + + expect(driver.send(:find_matching, unnamed, "/.*/").id).to eq("222") + end + end + + describe "#find_image" do + it "logs which image it selected" do + driver.send(:find_image, "ubuntu-24.04") + + expect(logged_output.string).to include("Selected image: 111 ubuntu-24.04") + end + end + + describe "#find_flavor" do + it "logs which flavor it selected" do + driver.send(:find_flavor, "m1.tiny") + + expect(logged_output.string).to include("Selected flavor: 1 m1.tiny") + end + end + + describe "#find_network" do + it "logs which network it selected" do + driver.send(:find_network, "public") + + expect(logged_output.string).to include("Selected net: net-1 public") + end + end +end diff --git a/spec/kitchen/driver/openstack/volume_spec.rb b/spec/kitchen/driver/openstack/volume_spec.rb index 68e1223c..3046ddd4 100644 --- a/spec/kitchen/driver/openstack/volume_spec.rb +++ b/spec/kitchen/driver/openstack/volume_spec.rb @@ -1,38 +1,62 @@ # frozen_string_literal: true -require_relative "../../../spec_helper" -require_relative "../../../../lib/kitchen/driver/openstack/volume" - require "logger" require "stringio" unless defined?(StringIO) -require "rspec" -require "kitchen" -require "ohai" unless defined?(Ohai::System) -describe Kitchen::Driver::Openstack::Volume do +RSpec.describe Kitchen::Driver::Openstack::Volume do + subject(:vol_driver) { described_class.new(logger) } + + let(:logger_io) { StringIO.new } + let(:logger) { Kitchen::Logger.new(logdev: logger_io) } + + # No unit test waits on the wall clock. + before { allow(vol_driver).to receive(:sleep).and_return(0) } + let(:os) do { openstack_username: "twilight", openstack_domain_id: "default", openstack_api_key: "sparkle", - openstack_auth_url: "http:", + openstack_auth_url: "http://keystone.example.com:5000/v3", openstack_project_name: "trixie", openstack_region: "syd", - openstack_service_name: "the_service", } end - let(:logger_io) { StringIO.new } - let(:logger) { Kitchen::Logger.new(logdev: logger_io) } - describe "#volume" do - let(:vol_driver) do - described_class.new(logger) + + # A Cinder volume model. `wait_for` is what the driver actually blocks on, so + # the double follows Fog's contract rather than just running the block once: + # the block is evaluated in the model's own context, and a block that never + # goes truthy raises Fog::Errors::TimeoutError. Without that last part a + # never-ready volume would be indistinguishable from a ready one. + def volume_model(id: "555", status: "available", ready: true) + model = double("Fog volume #{id}", id: id, status: status, ready?: ready) + allow(model).to receive(:sleep).and_return(0) + allow(model).to receive(:wait_for) do |_timeout, &blk| + result = model.instance_exec(&blk) + raise Fog::Errors::TimeoutError, "The specified wait_for timeout was exceeded" unless result + + result end + model + end - it "creates a new block device connection" do - allow(Fog::OpenStack::Volume).to receive(:new) { |arg| arg } - expect(vol_driver.send(:volume, os)).to eq(os) + # The `volumes` collection only answers `get`, which is a direct GET by id. + # Nothing stubs iteration, so a driver that went back to scanning the listing + # would fail loudly here rather than quietly depending on page one. + def cinder(volumes: [volume_model], create_response: { body: { "volume" => { "id" => "555" } } }) + collection = double("Fog volumes collection") + allow(collection).to receive(:get) { |id| volumes.find { |v| v.id == id } } + double("Fog::OpenStack::Volume", volumes: collection, create_volume: create_response) + end + + describe "#volume" do + it "builds a Cinder connection from the given Fog settings" do + allow(Fog::OpenStack::Volume).to receive(:new).with(os).and_return(:cinder) + + expect(vol_driver.volume(os)).to eq(:cinder) end end + describe "#create_volume" do let(:config) do { @@ -40,61 +64,240 @@ block_device_mapping: { snapshot_id: "444", volume_size: "5", - creation_timeout: "30", - attach_timeout: 5, + creation_timeout: 30, }, } end - let(:create_volume) do - { - body: { "volume" => { "id" => "555" } }, - } + let(:cinder_service) { cinder } + + before { allow(vol_driver).to receive(:volume).and_return(cinder_service) } + + it "returns the id of the created volume" do + expect(vol_driver.create_volume(config, os)).to eq("555") end - let(:volume_model) do - { - id: "555", - status: "ACTIVE", - # wait_for: true - # ready?: true - } + it "names the volume after the server" do + vol_driver.create_volume(config, os) + + expect(cinder_service).to have_received(:create_volume) + .with("applejack-volume", "applejack volume", "5", hash_including(snapshot_id: "444")) end - let(:volume) do - double( - create_volume: create_volume, - volumes: [volume_model] + it "forwards only the recognized vanilla options to Cinder" do + config[:block_device_mapping].merge!( + volume_type: "ssd", + availability_zone: "az1", + delete_on_termination: true, + device_name: "vda" ) + + vol_driver.create_volume(config, os) + + expect(cinder_service).to have_received(:create_volume) + .with(anything, anything, anything, + { snapshot_id: "444", volume_type: "ssd", availability_zone: "az1" }) end - let(:wait_for) do - { - ready?: true, - status: "ACTIVE", - } + it "omits vanilla options that are not set" do + config[:block_device_mapping] = { volume_size: "5" } + + vol_driver.create_volume(config, os) + + expect(cinder_service).to have_received(:create_volume).with(anything, anything, anything, {}) + end + + it "authenticates to Cinder once and reuses the connection" do + vol_driver.create_volume(config, os) + + expect(vol_driver).to have_received(:volume).once + end + + # Regression: `Array#first` silently ignores a block, so the original + # implementation waited on whichever volume happened to be first in the + # account rather than the one it had just created. The id from the create + # response is now looked up directly. + context "when the account holds several volumes" do + let(:other) { volume_model(id: "111", status: "error") } + let(:mine) { volume_model(id: "555", status: "available") } + let(:cinder_service) { cinder(volumes: [other, mine]) } + + it "fetches the volume it just created by id" do + expect(vol_driver.create_volume(config, os)).to eq("555") + + expect(cinder_service.volumes).to have_received(:get).with("555") + expect(mine).to have_received(:wait_for) + expect(other).not_to have_received(:wait_for) + end end - let(:vol_driver) do - d = described_class.new(logger) - allow(d).to receive(:volume).and_return(volume) - allow(d).to receive(:volume_model).and_return(true) - d + context "when the created volume cannot be found afterwards" do + let(:cinder_service) { cinder(volumes: [volume_model(id: "999")]) } + + it "raises an ActionFailed rather than dereferencing nil" do + expect { vol_driver.create_volume(config, os) } + .to raise_error(Kitchen::ActionFailed, /Volume 555 disappeared/) + end + end + + context "when the volume goes to error state" do + let(:cinder_service) { cinder(volumes: [volume_model(status: "error")]) } + + it "raises an ActionFailed naming the volume" do + expect { vol_driver.create_volume(config, os) } + .to raise_error(Kitchen::ActionFailed, "Failed to make volume 555") + end + end + + it "treats the error status case-insensitively" do + service = cinder(volumes: [volume_model(status: "ERROR")]) + allow(vol_driver).to receive(:volume).and_return(service) + + expect { vol_driver.create_volume(config, os) } + .to raise_error(Kitchen::ActionFailed, "Failed to make volume 555") + end + + # `create` only rescues Fog and Excon errors, so anything else raised from + # here reaches the user as a raw backtrace instead of a Kitchen failure. + it "raises only errors that create/1 knows how to present" do + service = cinder(volumes: [volume_model(status: "error")]) + allow(vol_driver).to receive(:volume).and_return(service) + + expect { vol_driver.create_volume(config, os) } + .to raise_error(an_instance_of(Kitchen::ActionFailed)) end - it "creates a volume" do - # This seems like a hack - # how would we do this on the volume_model instead? - # This makes rspec work - # but the vol_driver doesnt have these methods properties? - allow(vol_driver).to receive(:status).and_return("ACTIVE") - allow(config).to receive(:attach_timeout).and_return(5) - allow(vol_driver).to receive(:ready?).and_return(true) - allow(volume_model).to receive(:wait_for) - .with(an_instance_of(String)).and_yield + context "when the volume never becomes ready" do + let(:cinder_service) { cinder(volumes: [volume_model(ready: false)]) } - # allow(vol_driver).a - expect(vol_driver.send(:create_volume, config, os)).to eq("555") + it "surfaces the Fog timeout" do + expect { vol_driver.create_volume(config, os) } + .to raise_error(Fog::Errors::TimeoutError) + end + end + + describe "timeouts" do + it "uses the default creation timeout when none is given" do + config[:block_device_mapping].delete(:creation_timeout) + model = volume_model + allow(vol_driver).to receive(:volume).and_return(cinder(volumes: [model])) + + vol_driver.create_volume(config, os) + + expect(model).to have_received(:wait_for).with(described_class::DEFAULT_CREATION_TIMEOUT) + end + + it "uses the configured creation timeout" do + model = volume_model + allow(vol_driver).to receive(:volume).and_return(cinder(volumes: [model])) + + vol_driver.create_volume(config, os) + + expect(model).to have_received(:wait_for).with(30) + end + + # Regression: YAML parses a quoted timeout as a String, and the old code + # then compared a String to an Integer. + it "accepts string timeouts from YAML" do + config[:block_device_mapping][:creation_timeout] = "30" + config[:block_device_mapping][:attach_timeout] = "5" + model = volume_model + allow(vol_driver).to receive(:volume).and_return(cinder(volumes: [model])) + + expect { vol_driver.create_volume(config, os) }.not_to raise_error + expect(model).to have_received(:wait_for).with(30) + end + + it "falls back to the default when the key is present but empty" do + config[:block_device_mapping][:creation_timeout] = nil + model = volume_model + allow(vol_driver).to receive(:volume).and_return(cinder(volumes: [model])) + + vol_driver.create_volume(config, os) + + expect(model).to have_received(:wait_for).with(described_class::DEFAULT_CREATION_TIMEOUT) + end + + it "sleeps for the attach timeout when one is set" do + config[:block_device_mapping][:attach_timeout] = 5 + + vol_driver.create_volume(config, os) + + expect(vol_driver).to have_received(:sleep).with(5) + end + + it "does not sleep when the attach timeout is zero" do + config[:block_device_mapping][:attach_timeout] = 0 + + vol_driver.create_volume(config, os) + + expect(vol_driver).not_to have_received(:sleep) + end + + it "does not sleep when no attach timeout is configured" do + vol_driver.create_volume(config, os) + + expect(vol_driver).not_to have_received(:sleep) + end + + # Regression: a bare `Integer("010")` reads the leading zero as octal and + # returns 8, so a user asking for 10 seconds silently got 8. `Integer("08")` + # is not even a legal octal literal and raised. + it "reads a zero-padded timeout as base 10" do + config[:block_device_mapping][:creation_timeout] = "010" + model = volume_model + allow(vol_driver).to receive(:volume).and_return(cinder(volumes: [model])) + + vol_driver.create_volume(config, os) + + expect(model).to have_received(:wait_for).with(10) + end + + it "accepts a zero-padded timeout that is not a legal octal literal" do + config[:block_device_mapping][:creation_timeout] = "08" + model = volume_model + allow(vol_driver).to receive(:volume).and_return(cinder(volumes: [model])) + + vol_driver.create_volume(config, os) + + expect(model).to have_received(:wait_for).with(8) + end + + it "names the offending key when a timeout is not a number" do + config[:block_device_mapping][:creation_timeout] = "soon" + + expect { vol_driver.create_volume(config, os) } + .to raise_error(Kitchen::ActionFailed, /creation_timeout must be a number, got "soon"/) + end + + # `attach_timeout: yes` parses as the Boolean true, which Integer() answers + # with a TypeError rather than an ArgumentError. + it "rejects a non-numeric YAML scalar without leaking a TypeError" do + config[:block_device_mapping][:attach_timeout] = true + + expect { vol_driver.create_volume(config, os) } + .to raise_error(Kitchen::ActionFailed, /attach_timeout must be a number, got true/) + end + + # Both timeouts are pure config parsing. Validating them after the Cinder + # call would strand a real volume whose id lives only in the server + # definition that is now being discarded, so `kitchen destroy` cannot + # reach it. + it "rejects a bad timeout before creating any volume" do + config[:block_device_mapping][:creation_timeout] = "soon" + + expect { vol_driver.create_volume(config, os) }.to raise_error(Kitchen::ActionFailed) + + expect(cinder_service).not_to have_received(:create_volume) + end + + it "rejects a bad attach timeout before creating any volume" do + config[:block_device_mapping][:attach_timeout] = "later" + + expect { vol_driver.create_volume(config, os) }.to raise_error(Kitchen::ActionFailed) + + expect(cinder_service).not_to have_received(:create_volume) + end end end @@ -102,28 +305,61 @@ let(:config) do { block_device_mapping: { - make_volue: true, + make_volume: true, snapshot_id: "333", volume_id: "555", volume_size: "5", - volume_device_name: "vda", + device_name: "vda", delete_on_termination: true, - attach_timeout: 5, }, } end - let(:vol_driver) do - d = described_class.new(logger) - allow(d).to receive(:create_volume).and_return("555") - d + before { allow(vol_driver).to receive(:create_volume).and_return("999") } + + it "strips the driver-only keys Nova does not understand" do + expect(vol_driver.get_bdm(config, os)).to eq( + volume_id: "999", + volume_size: "5", + device_name: "vda", + delete_on_termination: true + ) + end + + it "creates a volume and uses its id when make_volume is set" do + expect(vol_driver.get_bdm(config, os)[:volume_id]).to eq("999") + expect(vol_driver).to have_received(:create_volume).with(config, os) + end + + context "when make_volume is not set" do + before { config[:block_device_mapping][:make_volume] = false } + + it "keeps the volume id the user supplied" do + expect(vol_driver.get_bdm(config, os)[:volume_id]).to eq("555") + end + + it "does not create a volume" do + vol_driver.get_bdm(config, os) + + expect(vol_driver).not_to have_received(:create_volume) + end + end + + # Regression: the old implementation mutated config[:block_device_mapping] + # in place, so a second call (a retried create, or `kitchen diagnose` after + # a create) saw a hash that had already had its keys stripped. + it "does not mutate the caller's config" do + before_call = Marshal.load(Marshal.dump(config)) + + vol_driver.get_bdm(config, os) + + expect(config).to eq(before_call) end - it "returns the block device mapping config" do - expects = config[:block_device_mapping] - expects.delete_if { |k, _| k == :make_volume } - expects.delete_if { |k, _| k == :snapshot_id } - expect(vol_driver.send(:get_bdm, config, os)).to eq(expects) + it "is idempotent across repeated calls" do + first = vol_driver.get_bdm(config, os) + + expect(vol_driver.get_bdm(config, os)).to eq(first) end end end diff --git a/spec/kitchen/driver/openstack_spec.rb b/spec/kitchen/driver/openstack_spec.rb index 077ab82e..ae43ff72 100755 --- a/spec/kitchen/driver/openstack_spec.rb +++ b/spec/kitchen/driver/openstack_spec.rb @@ -1,1442 +1,596 @@ # frozen_string_literal: true -require_relative "../../spec_helper" -require_relative "../../../lib/kitchen/driver/openstack" - -require "logger" -require "stringio" unless defined?(StringIO) -require "rspec" -require "kitchen" -require "kitchen/driver/openstack" -require "kitchen/provisioner/dummy" -require "kitchen/transport/dummy" -require "kitchen/verifier/dummy" -require "ohai" unless defined?(Ohai::System) -require "excon" unless defined?(Excon) -require "fog/openstack" - -describe Kitchen::Driver::Openstack do - let(:logged_output) { StringIO.new } - let(:logger) { Logger.new(logged_output) } - let(:config) { {} } - let(:state) { {} } - let(:instance_name) { "potatoes" } - let(:transport) { Kitchen::Transport::Dummy.new } - let(:platform) { Kitchen::Platform.new(name: "fake_platform") } - let(:driver) { Kitchen::Driver::Openstack.new(config) } - - let(:instance) do - double( - name: instance_name, - transport: transport, - logger: logger, - platform: platform, - to_str: "instance" - ) - end - - let(:driver) { described_class.new(config) } - - before(:each) do - allow_any_instance_of(described_class).to receive(:instance) - .and_return(instance) - allow(File).to receive(:exist?).and_call_original - end - - describe "#finalize_config" do - before(:each) { allow(File).to receive(:exist?).and_return(false) } - end +RSpec.describe Kitchen::Driver::Openstack do + include_context "with a configured driver" - describe "#initialize" do - context "default options" do - it "uses the normal SSH status check" do - expect(driver[:no_ssh_tcp_check]).to eq(false) - end + let(:state) { {} } + let(:server) { fog_server } - it "sets a default TCP check wait time" do - expect(driver[:no_ssh_tcp_check_sleep]).to eq(120) - end - - it "sets a default Openstack API read timeout" do - expect(driver[:read_timeout]).to eq(60) - end - - it "sets a default Openstack API write timeout" do - expect(driver[:write_timeout]).to eq(60) - end - - it "sets a default ssh connection timeout" do - expect(driver[:connect_timeout]).to eq(60) - end - - nils = %i{ - server_name - openstack_cloud - clouds_yaml_path - openstack_project_name - openstack_region - openstack_service_name - floating_ip_pool - floating_ip - availability_zone - security_groups - network_ref - metadata - } - nils.each do |i| - it "defaults to no #{i}" do - expect(driver[i]).to eq(nil) - end - end + describe "plugin metadata" do + it "implements driver API version 2" do + expect(described_class.instance_variable_get(:@api_version)).to eq(2) end - context "overridden options" do - let(:config) do - { - image_ref: "22", - image_id: "4391b03e-f7fb-46fd-a356-fa5e42f6d728", - flavor_ref: "33", - flavor_id: "19a2281e-591e-4b47-be06-631c3c7704e8", - public_key_path: "/tmp", - username: "admin", - port: "2222", - server_name: "puppy", - server_name_prefix: "parsnip", - openstack_project_name: "that_one", - openstack_region: "atlantis", - openstack_service_name: "the_service", - floating_ip_pool: "swimmers", - floating_ip: "11111", - network_ref: "0xCAFFE", - network_id: "57d6e41a-f369-4c92-9ebe-1fbf198bc783", - use_ssh_agent: true, - connect_timeout: 123, - read_timeout: 234, - write_timeout: 345, - block_device_mapping: { - make_volume: true, - snapshot_id: "44", - volume_id: "55", - volume_size: "5", - device_name: "vda", - delete_on_termination: true, - }, - metadata: { - name: "test", - ohai: "chef", - }, - } - end - - it "uses all the overridden options" do - drv = driver - config.each do |k, v| - expect(drv[k]).to eq(v) - end - end - - it "overrides server name prefix with explicit server name, if given" do - expect(driver[:server_name]).to eq(config[:server_name]) - end + it "reports the gem version as its plugin version" do + expect(described_class.instance_variable_get(:@plugin_version)) + .to eq(Kitchen::Driver::OPENSTACK_VERSION) end end - describe "#create" do - let(:server) do - double(id: "test123", wait_for: true, public_ip_addresses: %w{1.2.3.4}) - end - let(:driver) do - d = super() - allow(d).to receive(:default_name).and_return("a_monkey!") - allow(d).to receive(:create_server).and_return(server) - allow(d).to receive(:wait_for_sshd).with("1.2.3.4", "root", port: "22") - .and_return(true) - allow(d).to receive(:get_ip).and_return("1.2.3.4") - allow(d).to receive(:add_ohai_hint).and_return(true) - allow(d).to receive(:do_ssh_setup).and_return(true) - allow(d).to receive(:sleep) - allow(d).to receive(:wait_for_ssh_key_access).and_return("SSH key authentication successful") - allow(d).to receive(:disable_ssl_validation).and_return(false) - d - end - - context "when a server is already created" do - it "does not create a new instance" do - state[:server_id] = "1" - expect(driver).not_to receive(:create_server) - driver.create(state) + describe "default config" do + { + port: "22", + use_ipv6: false, + private_ip_order: 0, + public_ip_order: 0, + no_ssh_tcp_check: false, + no_ssh_tcp_check_sleep: 120, + glance_cache_wait_timeout: 600, + allocate_floating_ip: false, + connect_timeout: 60, + read_timeout: 60, + write_timeout: 60, + }.each do |key, value| + it "defaults #{key} to #{value.inspect}" do + expect(driver[key]).to eq(value) end end - context "required options provided" do - let(:config) do - { - openstack_username: "hello", - openstack_domain_id: "default", - openstack_api_key: "world", - openstack_auth_url: "http:", - openstack_project_name: "www", - glance_cache_wait_timeout: 600, - disable_ssl_validation: false, - } - end - let(:server) do - double(id: "test123", wait_for: true, public_ip_addresses: %w{1.2.3.4}) - end - - let(:driver) do - d = described_class.new(config) - allow(d).to receive(:config_server_name).and_return("a_monkey!") - allow(d).to receive(:create_server).and_return(server) - allow(server).to receive(:id).and_return("test123") - - # Inside the yield block we are calling ready? So we fake it here - allow(d).to receive(:ready?).and_return(true) - allow(d).to receive(:failed?).and_return(false) - allow(server).to receive(:wait_for) - .with(an_instance_of(Integer)).and_yield - - allow(d).to receive(:get_ip).and_return("1.2.3.4") - allow(d).to receive(:bourne_shell?).and_return(false) - d - end - - it "returns nil, but modifies the state" do - expect(driver.send(:create, state)).to eq(nil) - expect(state[:server_id]).to eq("test123") + %i{ + openstack_cloud clouds_yaml_path server_name server_name_prefix key_name + openstack_project_name openstack_region openstack_service_name + openstack_network_name floating_ip_pool floating_ip availability_zone + security_groups network_ref network_id block_device_mapping metadata + }.each do |key| + it "leaves #{key} unset" do + expect(driver[key]).to be_nil end + end - it "throws an InstanceFailure error when server is in ERROR state" do - allow(driver).to receive(:failed?).and_return(true) - expect { driver.send(:create, state) }.to raise_error(Kitchen::InstanceFailure) - expect(driver).not_to receive(:failed?) - end + it "lets kitchen.yml override a default" do + config[:port] = "2222" - it "throws an ActionFailed error when trying to create_server" do - allow(driver).to receive(:create_server).and_raise(Fog::Errors::Error) - expect { driver.send(:create, state) }.to raise_error(Kitchen::ActionFailed) - end - - it "returns ready status" do - expect(driver.send(:ready?, state)).to be true - end + expect(driver[:port]).to eq("2222") end end - describe "#destroy" do - let(:server_id) { "12345" } - let(:hostname) { "example.com" } - let(:state) { { server_id: server_id, hostname: hostname } } - let(:server) { double(nil?: false, destroy: true) } - let(:servers) { double(get: server) } - let(:compute) { double(servers: servers) } - - let(:driver) do - d = super() - allow(d).to receive(:compute).and_return(compute) - d + describe "#create" do + let(:config) { { server_name: "hello", image_id: "111", flavor_id: "1" } } + + before do + allow(driver).to receive_messages( + create_server: server, + get_ip: "1.2.3.4", + wait_for_server: true, + add_ohai_hint: true + ) end - context "a live server that needs to be destroyed" do - it "destroys the server" do - expect(state).to receive(:delete).with(:server_id) - expect(state).to receive(:delete).with(:hostname) - driver.destroy(state) - end + it "records the server id in state" do + driver.create(state) - it "does not disable SSL cert validation" do - expect(driver).to_not receive(:disable_ssl_validation) - driver.destroy(state) - end + expect(state[:server_id]).to eq("test123") end - context "no server ID present" do - let(:state) { {} } + it "records the hostname in state" do + driver.create(state) - it "does nothing" do - allow(driver).to receive(:compute) - expect(driver).to_not receive(:compute) - expect(state).to_not receive(:delete) - driver.destroy(state) - end + expect(state[:hostname]).to eq("1.2.3.4") end - context "a server that was already destroyed" do - let(:servers) do - s = double("servers") - allow(s).to receive(:get).with("12345").and_return(nil) - s - end - let(:compute) { double(servers: servers) } - let(:driver) do - d = super() - allow(d).to receive(:compute).and_return(compute) - d - end + it "resolves the server name before creating" do + config.delete(:server_name) + allow(driver).to receive(:config_server_name).and_call_original - it "does not try to destroy the server again" do - allow_message_expectations_on_nil - driver.destroy(state) - end + driver.create(state) + + expect(driver[:server_name]).not_to be_nil end - context "SSL validation disabled" do - let(:config) { { disable_ssl_validation: true } } + it "waits for the server to become ready" do + driver.create(state) - it "disables SSL cert validation" do - expect(driver).to receive(:disable_ssl_validation) - driver.destroy(state) - end + expect(server).to have_received(:wait_for).with(600) end - context "Deallocate floating IP" do - let(:config) do - { - floating_ip_pool: "swimmers", - allocate_floating_ip: true, - } - end - let(:ip) { "1.1.1.1" } - let(:ip_id) { "123" } + it "honours a custom glance cache timeout" do + config[:glance_cache_wait_timeout] = 42 - let(:network_response) do - double(body: { "floatingips" => [{ "id" => ip_id }] }) - end + driver.create(state) - let(:network) do - s = double("network") - expect(s).to receive(:list_floating_ips).with(floating_ip_address: ip).and_return(network_response) - expect(s).to receive(:delete_floating_ip).with(ip_id) - s - end - - let(:driver) do - d = super() - allow(d).to receive(:get_public_private_ips).and_return([ip, nil]) - allow(d).to receive(:compute).and_return(compute) - allow(d).to receive(:network).and_return(network) - d - end - it "deallocates the ip" do - driver.destroy(state) - end + expect(server).to have_received(:wait_for).with(42) end - end - describe "#openstack_server" do - let(:config) do - { - openstack_username: "a", - openstack_domain_id: "default", - openstack_api_key: "b", - openstack_auth_url: "http://", - openstack_project_name: "me", - openstack_region: "ORD", - openstack_service_name: "stack", - connection_options: - { - read_timeout: 60, - write_timeout: 60, - connect_timeout: 60, - }, - } - end + it "waits for the transport before adding the ohai hint" do + driver.create(state) - it "returns a hash of server settings" do - expected = config.merge(config) - expect(driver.send(:openstack_server)).to eq(expected) + expect(driver).to have_received(:wait_for_server).with(state).ordered + expect(driver).to have_received(:add_ohai_hint).with(state).ordered end - context "when string-like fog settings are numeric" do - let(:config) do - { - openstack_username: "a", - openstack_domain_id: 12_345, - openstack_api_key: "b", - openstack_auth_url: "http://", - openstack_project_id: 99, - openstack_identity_api_version: 3, - } - end + context "when the server already exists" do + let(:state) { { server_id: "existing" } } - it "coerces them to strings before passing to fog" do - server = driver.send(:openstack_server) + it "does not create another one" do + driver.create(state) - expect(server[:openstack_domain_id]).to eq("12345") - expect(server[:openstack_project_id]).to eq("99") - # identity_api_version is prefixed with "v" so that - # Fog::Service#coerce_options does not turn it back into an - # Integer (which would later break Token.build's `=~`). - expect(server[:openstack_identity_api_version]).to eq("v3") + expect(driver).not_to have_received(:create_server) end - it "prefixes identity_api_version 2 with a v for fog v2 detection" do - config[:openstack_identity_api_version] = 2 - server = driver.send(:openstack_server) + it "says so" do + driver.create(state) - expect(server[:openstack_identity_api_version]).to eq("v2.0") + expect(logged_output.string).to include("hello (existing) already exists.") end - it "passes through identity_api_version values already prefixed" do - config[:openstack_identity_api_version] = "v3" - server = driver.send(:openstack_server) + it "leaves state untouched" do + driver.create(state) - expect(server[:openstack_identity_api_version]).to eq("v3") + expect(state).to eq(server_id: "existing") end end - end - describe "#required_server_settings" do - it "returns the required settings for an OpenStack server" do - expected = %i{ - openstack_username openstack_api_key openstack_auth_url openstack_domain_id - } - expect(driver.send(:required_server_settings)).to eq(expected) - end - end + context "while the server is building" do + # No re-stub of wait_for here: the shared fog_server factory already + # evaluates the readiness block in the server's own context. - describe "#optional_server_settings" do - it "returns the optional settings for an OpenStack server" do - excluded = %i{ - openstack_username openstack_api_key openstack_auth_url openstack_domain_id - } - expect(driver.send(:optional_server_settings)).not_to include(*excluded) - end - end - - describe "#compute" do - let(:config) do - { - openstack_username: "monkey", - openstack_domain_id: "default", - openstack_api_key: "potato", - openstack_auth_url: "http:", - openstack_project_name: "link", - openstack_region: "ord", - openstack_service_name: "the_service", - connection_options: - { - read_timeout: 60, - write_timeout: 60, - connect_timeout: 60, - }, - } - end + it "runs the readiness check" do + driver.create(state) - context "all requirements provided" do - it "creates a new compute connection" do - allow(Fog::OpenStack::Compute).to receive(:new) { |arg| arg } - res = config.merge(config) - expect(driver.send(:compute)).to eq(res) + expect(server).to have_received(:ready?) end - it "creates a new network connection" do - allow(Fog::OpenStack::Network).to receive(:new) { |arg| arg } - res = config.merge(config) - expect(driver.send(:network)).to eq(res) + it "checks for a failed build before reporting ready" do + driver.create(state) + + expect(server).to have_received(:failed?) end - end - context "only an API key provided" do - let(:config) { { openstack_api_key: "1234" } } + it "says the server was created" do + driver.create(state) - it "raises an error" do - expect { driver.send(:compute) }.to raise_error(ArgumentError) + expect(logged_output.string).to include("OpenStack server ID created") end end - context "only a username provided" do - let(:config) { { openstack_username: "monkey" } } + context "when the build fails" do + let(:server) { fog_server(failed?: true, ready?: false) } - it "raises an error" do - expect { driver.send(:compute) }.to raise_error(ArgumentError) + it "raises InstanceFailure naming the server" do + expect { driver.create(state) } + .to raise_error(Kitchen::InstanceFailure, /OpenStack server ID build failed/) end end - end - describe "#create_server" do - let(:config) do - { - server_name: "hello", - image_ref: "111", - flavor_ref: "1", - availability_zone: nil, - block_device_mapping: { - volume_size: "5", - volume_id: "333", - volume_device_name: "vda", - delete_on_termination: "true", - }, - } - end - let(:servers) do - s = double("servers") - allow(s).to receive(:create) { |arg| arg } - s - end - let(:vlan1_net) { double(id: "1", name: "vlan1") } - let(:vlan2_net) { double(id: "2", name: "vlan2") } - let(:ubuntu_image) { double(id: "111", name: "ubuntu") } - let(:fedora_image) { double(id: "222", name: "fedora") } - let(:tiny_flavor) { double(id: "1", name: "tiny") } - let(:small_flavor) { double(id: "2", name: "small") } - let(:compute) do - double( - servers: servers, - images: [ubuntu_image, fedora_image], - flavors: [tiny_flavor, small_flavor] - ) - end - let(:network) do - double(networks: double(all: [vlan1_net, vlan2_net])) - end - let(:block_device_mapping) do - { - volume_id: "333", - volume_size: "5", - volume_device_name: "vda", - delete_on_termination: "true", - } - end - let(:driver) do - d = super() - allow(d).to receive(:compute).and_return(compute) - allow(d).to receive(:network).and_return(network) - allow(d).to receive(:get_bdm).and_return(block_device_mapping) - d - end + describe "SSL validation" do + before { allow(driver).to receive(:disable_ssl_validation) } - context "a default config" do - before(:each) do - @expected = config.merge(name: config[:server_name]) - @expected.delete_if { |k, _| k == :server_name } - end + it "is left alone by default" do + driver.create(state) - it "creates the server using a compute connection" do - expect(driver.send(:create_server)).to eq(@expected) + expect(driver).not_to have_received(:disable_ssl_validation) end - end - context "a provided key name" do - let(:config) do - { - server_name: "hello", - image_ref: "111", - flavor_ref: "1", - availability_zone: nil, - key_name: "tarpals", - } - end + it "is disabled when configured" do + config[:disable_ssl_validation] = true - before(:each) do - @expected = config.merge(name: config[:server_name]) - @expected.delete_if { |k, _| k == :server_name } - end + driver.create(state) - it "passes that key name to Fog" do - expect(driver.send(:create_server)).to eq(@expected) + expect(driver).to have_received(:disable_ssl_validation) end end - context "a provided security group" do - let(:config) do - { - server_name: "hello", - image_ref: "111", - flavor_ref: "1", - availability_zone: nil, - key_name: "tarpals", - security_groups: ["ping-and-ssh"], - } + describe "floating IPs" do + before do + allow(driver).to receive(:attach_ip) + allow(driver).to receive(:attach_ip_from_pool) end - before(:each) do - @expected = config.merge(name: config[:server_name]) - @expected.delete_if { |k, _| k == :server_name } - end + it "attaches nothing by default" do + driver.create(state) - it "passes that security group to Fog" do - expect(driver.send(:create_server)).to eq(@expected) + expect(driver).not_to have_received(:attach_ip) + expect(driver).not_to have_received(:attach_ip_from_pool) end - end - context "a provided availability zone" do - let(:config) do - { - server_name: "hello", - image_ref: "111", - flavor_ref: "1", - availability_zone: "elsewhere", - key_name: "tarpals", - } - end + it "attaches an explicitly configured floating IP" do + config[:floating_ip] = "203.0.113.5" - before(:each) do - @expected = config.merge(name: config[:server_name]) - @expected.delete_if { |k, _| k == :server_name } - end + driver.create(state) - it "passes that availability zone to Fog" do - expect(driver.send(:create_server)).to eq(@expected) + expect(driver).to have_received(:attach_ip).with(server, "203.0.113.5") end - end - context "image_id specified" do - let(:config) do - { - server_name: "hello", - image_id: "1e1f4346-e3ea-48ba-9d1b-0002bfcb8981", - flavor_ref: "1", - } - end + it "attaches from the pool when only a pool is configured" do + config[:floating_ip_pool] = "swimmers" - it "exact id match" do - expect(servers).to receive(:create).with( - { - name: "hello", - image_ref: "1e1f4346-e3ea-48ba-9d1b-0002bfcb8981", - flavor_ref: "1", - availability_zone: nil, - } - ) - driver.send(:create_server) - end - end + driver.create(state) - context "image_id and image_ref specified" do - let(:config) do - { - server_name: "hello", - image_id: "1e1f4346-e3ea-48ba-9d1b-0002bfcb8981", - image_ref: "111", - flavor_ref: "1", - } + expect(driver).to have_received(:attach_ip_from_pool).with(server, "swimmers") end - it "raises an exception" do - expect { driver.send(:create_server) }.to \ - raise_error(Kitchen::ActionFailed) - end - end + it "prefers an explicit floating IP over the pool" do + config[:floating_ip] = "203.0.113.5" + config[:floating_ip_pool] = "swimmers" - context "flavor_id specified" do - let(:config) do - { - server_name: "hello", - flavor_id: "1e1f4346-e3ea-48ba-9d1b-0002bfcb8981", - image_ref: "111", - } - end + driver.create(state) - it "exact id match" do - expect(servers).to receive(:create).with( - { - name: "hello", - flavor_ref: "1e1f4346-e3ea-48ba-9d1b-0002bfcb8981", - image_ref: "111", - availability_zone: nil, - } - ) - driver.send(:create_server) + expect(driver).to have_received(:attach_ip).with(server, "203.0.113.5") + expect(driver).not_to have_received(:attach_ip_from_pool) end end - context "flavor_id and flavor_ref specified" do - let(:config) do - { - server_name: "hello", - image_id: "1e1f4346-e3ea-48ba-9d1b-0002bfcb8981", - flavor_id: "1e1f4346-e3ea-48ba-9d1b-0002bfcb8981", - flavor_ref: "1", - } - end + describe "error translation" do + it "wraps Fog errors in ActionFailed" do + allow(driver).to receive(:create_server).and_raise(Fog::Errors::Error, "boom") - it "raises an exception" do - expect { driver.send(:create_server) }.to \ - raise_error(Kitchen::ActionFailed) + expect { driver.create(state) }.to raise_error(Kitchen::ActionFailed, "boom") end - end - context "image/flavor specifies id" do - let(:config) do - { - server_name: "hello", - image_ref: "111", - flavor_ref: "1", - } - end + it "wraps Excon errors in ActionFailed" do + allow(driver).to receive(:create_server).and_raise(Excon::Errors::SocketError.new(StandardError.new("no route"))) - it "exact id match" do - expect(servers).to receive(:create).with( - { - name: "hello", - image_ref: "111", - flavor_ref: "1", - availability_zone: nil, - } - ) - driver.send(:create_server) + expect { driver.create(state) }.to raise_error(Kitchen::ActionFailed, /no route/) end - end - context "image/flavor specifies name" do - let(:config) do - { - server_name: "hello", - image_ref: "fedora", - flavor_ref: "small", - } - end + it "lets InstanceFailure through untouched" do + allow(driver).to receive(:create_server).and_raise(Kitchen::InstanceFailure, "nope") - it "exact name match" do - expect(servers).to receive(:create).with( - { - name: "hello", - image_ref: "222", - flavor_ref: "2", - availability_zone: nil, - } - ) - driver.send(:create_server) + expect { driver.create(state) }.to raise_error(Kitchen::InstanceFailure, "nope") end end + end - context "image/flavor specifies regex" do - let(:config) do - { - server_name: "hello", - # pass regex as string as yml returns string values - image_ref: "/edo/", - flavor_ref: "/in/", - } - end - - it "regex name match" do - expect(servers).to receive(:create).with( - { - name: "hello", - image_ref: "222", - flavor_ref: "1", - availability_zone: nil, - } - ) - driver.send(:create_server) - end + describe "#finalize_config!" do + it "returns itself for chaining" do + expect(driver.finalize_config!(instance)).to be(driver) end - context "network specifies network_id" do - let(:config) do - { - server_name: "hello", - image_ref: "111", - flavor_ref: "1", - network_id: "0922b7aa-0a2f-4e68-8ff7-2886c4fc472d", - } - end - - it "exact id match" do - networks = [ - { "net_id" => "0922b7aa-0a2f-4e68-8ff7-2886c4fc472d" }, - ] - expect(servers).to receive(:create).with( - { - name: "hello", - image_ref: "111", - flavor_ref: "1", - availability_zone: nil, - nics: networks, - } - ) - driver.send(:create_server) + it "merges clouds.yaml values into config" do + allow(driver).to receive(:apply_clouds_config) do + driver.send(:config)[:openstack_username] = "from-clouds-yaml" end - end - context "network_id and network_ref specified" do - let(:config) do - { - server_name: "hello", - image_id: "1e1f4346-e3ea-48ba-9d1b-0002bfcb8981", - flavor_id: "1e1f4346-e3ea-48ba-9d1b-0002bfcb8981", - network_id: "0922b7aa-0a2f-4e68-8ff7-2886c4fc472d", - network_ref: "1", - } - end + driver.finalize_config!(instance) - it "raises an exception" do - expect { driver.send(:create_server) }.to \ - raise_error(Kitchen::ActionFailed) - end + expect(driver[:openstack_username]).to eq("from-clouds-yaml") end - context "network specifies id" do - let(:config) do - { - server_name: "hello", - image_ref: "111", - flavor_ref: "1", - network_ref: "1", - } - end - - it "exact id match" do - networks = [ - { "net_id" => "1" }, - ] - expect(servers).to receive(:create).with( - { - name: "hello", - image_ref: "111", - flavor_ref: "1", - availability_zone: nil, - nics: networks, - } - ) - driver.send(:create_server) - end - end + it "applies the clouds config exactly once" do + allow(driver).to receive(:apply_clouds_config) - context "network specifies name" do - let(:config) do - { - server_name: "hello", - image_ref: "111", - flavor_ref: "1", - network_ref: "vlan1", - } - end + driver.finalize_config!(instance) - it "exact id match" do - networks = [ - { "net_id" => "1" }, - ] - expect(servers).to receive(:create).with( - { - name: "hello", - image_ref: "111", - flavor_ref: "1", - availability_zone: nil, - nics: networks, - } - ) - driver.send(:create_server) - end + expect(driver).to have_received(:apply_clouds_config).once end - context "multiple networks specifies id" do - let(:config) do - { - server_name: "hello", - image_ref: "111", - flavor_ref: "1", - network_ref: %w{1 2}, - } - end + it "still runs the base class finalization" do + driver.finalize_config!(instance) - it "exact id match" do - networks = [ - { "net_id" => "1" }, - { "net_id" => "2" }, - ] - expect(servers).to receive(:create).with( - { - name: "hello", - image_ref: "111", - flavor_ref: "1", - availability_zone: nil, - nics: networks, - } - ) - driver.send(:create_server) - end + expect(driver.instance).to be(instance) end + end - context "user_data specified" do - let(:config) do - { - server_name: "hello", - image_ref: "111", - flavor_ref: "1", - user_data: "cloud-init.txt", - } - end - let(:data) { "#cloud-config\n" } - - before(:each) do - allow(File).to receive(:exist?).and_return(true) - allow(File).to receive(:read).and_return(data) - end + describe "#destroy" do + let(:state) { { server_id: "test123", hostname: "1.2.3.4" } } + let(:servers) { double("Fog servers collection", get: server) } + let(:compute) { fog_compute(servers: servers) } - it "passes file contents" do - expect(servers).to receive(:create).with( - { - name: "hello", - image_ref: "111", - flavor_ref: "1", - availability_zone: nil, - user_data: data, - } - ) - driver.send(:create_server) - end - end + before { allow(driver).to receive(:compute).and_return(compute) } - context "config drive enabled" do - let(:config) do - { - server_name: "hello", - image_ref: "111", - flavor_ref: "1", - config_drive: true, - } - end + it "destroys the server" do + driver.destroy(state) - it "enables config drive" do - expect(servers).to receive(:create).with( - { - name: "hello", - image_ref: "111", - flavor_ref: "1", - availability_zone: nil, - config_drive: true, - } - ) - driver.send(:create_server) - end + expect(server).to have_received(:destroy) end - context "metadata specified" do - let(:config) do - { - server_name: "hello", - image_ref: "111", - flavor_ref: "1", - metadata: { - name: "hello", - ohai: "chef", - }, - } - end - let(:data) do - { - name: "hello", - ohai: "chef", - } - end + it "clears the server id from state" do + driver.destroy(state) - it "passes metadata contents" do - expect(servers).to receive(:create).with( - { - name: "hello", - image_ref: "111", - flavor_ref: "1", - availability_zone: nil, - metadata: data, - } - ) - driver.send(:create_server) - end + expect(state).not_to have_key(:server_id) end - end - describe "#default_name" do - let(:login) { "user" } - let(:hostname) { "host" } + it "clears the hostname from state" do + driver.destroy(state) - before(:each) do - allow(Etc).to receive(:getlogin).and_return(login) - allow(Socket).to receive(:gethostname).and_return(hostname) + expect(state).not_to have_key(:hostname) end - context "local node with a long hostname" do - let(:hostname) { "ab.c" * 20 } + it "says what it destroyed" do + driver.destroy(state) - it "limits the generated name to 63 characters" do - expect(driver.send(:default_name).length).to be <= 63 - end + expect(logged_output.string).to include("OpenStack instance destroyed.") end - context "a login and hostname with punctuation in them" do - let(:login) { "some.u-se-r" } - let(:hostname) { "a.host-name" } - let(:instance_name) { "a.instance-name" } + context "with no server id in state" do + let(:state) { {} } - it "strips out the dots to prevent bad server names" do - expect(driver.send(:default_name)).to_not include(".") - end + it "does nothing" do + driver.destroy(state) - it "strips out all but the three hyphen separators" do - expect(driver.send(:default_name).count("-")).to eq(3) + expect(driver).not_to have_received(:compute) end end - context "a non-login shell" do - let(:login) { nil } + context "when the server is already gone" do + let(:servers) { double("Fog servers collection", get: nil) } - it "subs in a placeholder login string" do - expect(driver.send(:default_name)).to match(/^potatoes-*-/) + it "does not raise" do + expect { driver.destroy(state) }.not_to raise_error end - end - end - - describe "#server_name_prefix" do - let(:login) { "user" } - let(:hostname) { "host" } - let(:prefix) { "parsnip" } - # These are still used in the "blank prefix" test - before(:each) do - allow(Etc).to receive(:getlogin).and_return(login) - allow(Socket).to receive(:gethostname).and_return(hostname) - end - - context "very long prefix provided" do - let(:long_prefix) { "a" * 70 } + it "still clears state" do + driver.destroy(state) - it "limits the generated name to 63 characters" do - expect(driver.send(:server_name_prefix, long_prefix).length) - .to be <= 63 + expect(state).to be_empty end end - end - - describe "#attach_ip_from_pool" do - let(:server) { nil } - let(:pool) { "swimmers" } - let(:ip) { "1.1.1.1" } - let(:address) do - double(ip: ip, fixed_ip: nil, instance_id: nil, pool: pool) - end - let(:compute) { double(addresses: [address]) } - before(:each) do - allow(driver).to receive(:attach_ip).with(server, ip).and_return("bing!") - allow(driver).to receive(:compute).and_return(compute) - end + describe "SSL validation" do + before { allow(driver).to receive(:disable_ssl_validation) } - it "determines an IP to attempt to attach" do - expect(driver.send(:attach_ip_from_pool, server, pool)).to eq("bing!") - end + it "is disabled when configured" do + config[:disable_ssl_validation] = true - context "no free addresses in the specified pool" do - let(:address) do - double(ip: ip, fixed_ip: nil, instance_id: nil, - pool: "some_other_pool") - end + driver.destroy(state) - it "raises an exception" do - expect { driver.send(:attach_ip_from_pool, server, pool) }.to \ - raise_error(Kitchen::ActionFailed) + expect(driver).to have_received(:disable_ssl_validation) end end - end - - describe "#allocate_ip_from_pool" do - let(:server) { nil } - let(:pool) { "swimmers" } - let(:config) { { allocate_floating_ip: true } } - let(:network_id) { 123 } - let(:ip) { "1.1.1.1" } - let(:address) do - double(ip: ip, fixed_ip: nil, instance_id: nil, pool: pool) - end - let(:list_networks_response) do - double(body: { "networks" => [{ "name" => pool, "id" => network_id }] }) - end - let(:create_ip_network_response) do - double(body: { "floatingip" => { "floating_ip_address" => ip } }) - end - let(:network) { double(list_networks: list_networks_response, create_floating_ip: create_ip_network_response) } - - before(:each) do - allow(driver).to receive(:attach_ip).with(server, ip).and_return("bing!") - allow(driver).to receive(:network).and_return(network) - end - - it "determines an IP to attempt to attach" do - expect(driver.send(:attach_ip_from_pool, server, pool)).to eq("bing!") - end - end - - describe "#attach_ip" do - let(:ip) { "1.1.1.1" } - let(:addresses) { {} } - let(:server) do - s = double("server") - expect(s).to receive(:associate_address).with(ip).and_return(true) - allow(s).to receive(:addresses).and_return(addresses) - s - end - - it "associates the IP address with the server" do - expect(driver.send(:attach_ip, server, ip)).to eq(true) - end - end - - describe "#get_ip" do - let(:addresses) { nil } - let(:public_ip_addresses) { nil } - let(:private_ip_addresses) { nil } - let(:ip_addresses) { nil } - let(:parsed_ips) { [[], []] } - let(:driver) do - d = super() - allow(d).to receive(:parse_ips).and_return(parsed_ips) - d - end - let(:server) do - double(addresses: addresses, - public_ip_addresses: public_ip_addresses, - private_ip_addresses: private_ip_addresses, - ip_addresses: ip_addresses, - wait_for: { duration: 0 }) - end - context "both public and private IPs" do - let(:public_ip_addresses) { %w{1::1 1.2.3.4} } - let(:private_ip_addresses) { %w{5.5.5.5} } - let(:parsed_ips) { [%w{1.2.3.4}, %w{5.5.5.5}] } + describe "releasing an allocated floating IP" do + let(:config) { { floating_ip_pool: "swimmers", allocate_floating_ip: true } } + let(:server) { fog_server(public_ip_addresses: %w{203.0.113.9}, private_ip_addresses: %w{10.0.0.1}) } - it "returns a public IPv4 address" do - expect(driver.send(:get_ip, server)).to eq("1.2.3.4") + let(:floating_ips_body) { { "floatingips" => [{ "id" => "fip-1" }] } } + let(:net) do + fog_network( + list_floating_ips: fog_response(floating_ips_body), + delete_floating_ip: true + ) end - end - context "only public IPs" do - let(:public_ip_addresses) { %w{4.3.2.1 2::1} } - let(:parsed_ips) { [%w{4.3.2.1}, []] } + before { allow(driver).to receive(:network).and_return(net) } - it "returns a public IPv4 address" do - expect(driver.send(:get_ip, server)).to eq("4.3.2.1") - end - end - - context "only private IPs" do - let(:private_ip_addresses) { %w{3::1 5.5.5.5} } - let(:parsed_ips) { [[], %w{5.5.5.5}] } + it "looks the floating IP up by address" do + driver.destroy(state) - it "returns a private IPv4 address" do - expect(driver.send(:get_ip, server)).to eq("5.5.5.5") + expect(net).to have_received(:list_floating_ips).with(floating_ip_address: "203.0.113.9") end - end - context "no predictable network name" do - let(:ip_addresses) { %w{3::1 5.5.5.5} } - let(:parsed_ips) { [[], %w{5.5.5.5}] } + it "deletes it" do + driver.destroy(state) - it "returns the first IP that matches the IP version" do - expect(driver.send(:get_ip, server)).to eq("5.5.5.5") + expect(net).to have_received(:delete_floating_ip).with("fip-1") end - end - context "IPs in user-defined network group" do - let(:config) { { openstack_network_name: "mynetwork" } } - let(:addresses) do - { - "mynetwork" => [ - { "addr" => "7.7.7.7" }, - { "addr" => "4::1" }, - ], - } - end + it "still destroys the server" do + driver.destroy(state) - it "returns a IPv4 address in user-defined network group" do - expect(driver.send(:get_ip, server)).to eq("7.7.7.7") + expect(server).to have_received(:destroy) end - end - context "when a floating ip is provided" do - let(:config) { { floating_ip: "1.2.3.4" } } + it "honours public_ip_order when picking the address to release" do + allow(server).to receive(:public_ip_addresses).and_return(%w{203.0.113.9 203.0.113.10}) + config[:public_ip_order] = 1 - it "returns the floating ip and skips reloading" do - allow(driver).to receive(:config).and_return(config) + driver.destroy(state) - expect(server).to_not receive(:wait_for) - expect(driver.send(:get_ip, server)).to eq("1.2.3.4") + expect(net).to have_received(:list_floating_ips).with(floating_ip_address: "203.0.113.10") end - end - context "an OpenStack deployment without the floating IP extension" do - before do - allow(server).to receive(:public_ip_addresses).and_raise( - Fog::OpenStack::Compute::NotFound - ) - allow(server).to receive(:private_ip_addresses).and_raise( - Fog::OpenStack::Compute::NotFound - ) - end + # Regression: the old code indexed straight into [0]["id"], so an IP + # Neutron had already reclaimed took the whole destroy down with a + # NoMethodError, stranding the server. + context "when Neutron no longer knows the address" do + let(:floating_ips_body) { { "floatingips" => [] } } - context "both public and private IPs in the addresses hash" do - let(:addresses) do - { - "public" => [{ "addr" => "6.6.6.6" }, { "addr" => "7.7.7.7" }], - "private" => [{ "addr" => "8.8.8.8" }, { "addr" => "9.9.9.9" }], - } + it "does not raise" do + expect { driver.destroy(state) }.not_to raise_error end - let(:parsed_ips) { [%w{6.6.6.6 7.7.7.7}, %w{8.8.8.8 9.9.9.9}] } - it "selects the first public IP" do - expect(driver.send(:get_ip, server)).to eq("6.6.6.6") - end - end + it "still destroys the server" do + driver.destroy(state) - context "when openstack_network_name is provided" do - let(:addresses) do - { - "public" => [{ "addr" => "6.6.6.6" }, { "addr" => "7.7.7.7" }], - "private" => [{ "addr" => "8.8.8.8" }, { "addr" => "9.9.9.9" }], - } + expect(server).to have_received(:destroy) end - let(:config) { { openstack_network_name: "public" } } - it "should respond with the first address from the addresses" do - allow(driver).to receive(:config).and_return(config) + it "warns that there was nothing to release" do + driver.destroy(state) - expect(driver.send(:get_ip, server)).to eq("6.6.6.6") + expect(logged_output.string).to include("No floating IP found matching <203.0.113.9>") end end - context "when openstack_network_name is provided and use_ipv6 is false" do - let(:addresses) do - { - "public" => [{ "addr" => "4::1" }, { "addr" => "7.7.7.7" }], - "private" => [{ "addr" => "5::1" }, { "addr" => "9.9.9.9" }], - } - end - let(:config) { { openstack_network_name: "public" } } + context "when the server has no public address" do + let(:server) { fog_server(public_ip_addresses: [], private_ip_addresses: %w{10.0.0.1}) } - it "should respond with the first IPv4 address from the addresses" do - allow(driver).to receive(:config).and_return(config) + it "does not try to release anything" do + driver.destroy(state) - expect(driver.send(:get_ip, server)).to eq("7.7.7.7") + expect(net).not_to have_received(:list_floating_ips) end end - context "when openstack_network_name is provided and use_ipv6 is true" do - let(:addresses) do - { - "public" => [{ "addr" => "4::1" }, { "addr" => "7.7.7.7" }], - "private" => [{ "addr" => "5::1" }, { "addr" => "9.9.9.9" }], - } - end - let(:config) { { openstack_network_name: "public", use_ipv6: true } } + context "when allocate_floating_ip is not set" do + let(:config) { { floating_ip_pool: "swimmers" } } - it "should respond with the first IPv6 address from the addresses" do - allow(driver).to receive(:config).and_return(config) + it "leaves the address alone, since the driver did not allocate it" do + driver.destroy(state) - expect(driver.send(:get_ip, server)).to eq("4::1") + expect(net).not_to have_received(:list_floating_ips) end end + end + end - context "only public IPs in the address hash" do - let(:addresses) do - { "public" => [{ "addr" => "6.6.6.6" }, { "addr" => "7.7.7.7" }] } - end - let(:parsed_ips) { [%w{6.6.6.6 7.7.7.7}, []] } + describe "#openstack_server" do + let(:config) do + { + openstack_username: "twilight", + openstack_api_key: "sparkle", + openstack_auth_url: "http://keystone.example.com:5000/v3", + openstack_domain_id: "default", + } + end - it "selects the first public IP" do - expect(driver.send(:get_ip, server)).to eq("6.6.6.6") - end - end + it "always sends the required settings" do + expect(driver.send(:openstack_server)).to include( + openstack_username: "twilight", + openstack_api_key: "sparkle", + openstack_auth_url: "http://keystone.example.com:5000/v3", + openstack_domain_id: "default" + ) + end - context "only private IPs in the address hash" do - let(:addresses) do - { "private" => [{ "addr" => "8.8.8.8" }, { "addr" => "9.9.9.9" }] } - end - let(:parsed_ips) { [[], %w{8.8.8.8 9.9.9.9}] } + it "sends required settings even when nil" do + config[:openstack_domain_id] = nil - it "selects the first private IP" do - expect(driver.send(:get_ip, server)).to eq("8.8.8.8") - end - end + expect(driver.send(:openstack_server)).to have_key(:openstack_domain_id) end - context "no IP addresses whatsoever" do - it "raises an exception" do - expected = Kitchen::ActionFailed - expect { driver.send(:get_ip, server) }.to raise_error(expected) - end - end + it "includes optional settings that are set" do + config[:openstack_region] = "atlantis" - context "when network information is not found" do - before do - allow(server).to receive(:wait_for).and_raise(Fog::Errors::TimeoutError) - end + expect(driver.send(:openstack_server)).to include(openstack_region: "atlantis") + end - it "raises an exception" do - expected = Kitchen::ActionFailed - expect { driver.send(:get_ip, server) }.to raise_error(expected) - end + it "omits optional settings that are not set" do + expect(driver.send(:openstack_server)).not_to have_key(:openstack_region) end - end - describe "#parse_ips" do - let(:pub_v4) { %w{1.1.1.1 2.2.2.2} } - let(:pub_v6) { %w{1::1 2::2} } - let(:priv_v4) { %w{3.3.3.3 4.4.4.4} } - let(:priv_v6) { %w{3::3 4::4} } - let(:pub) { pub_v4 + pub_v6 } - let(:priv) { priv_v4 + priv_v6 } - - context "both public and private IPs" do - context "IPv4 (default)" do - it "returns only the v4 IPs" do - expect(driver.send(:parse_ips, pub, priv)).to eq([pub_v4, priv_v4]) - end + describe "connection options" do + it "carries the timeouts" do + expect(driver.send(:openstack_server)[:connection_options]) + .to eq(read_timeout: 60, write_timeout: 60, connect_timeout: 60) end - context "IPv6" do - let(:config) { { use_ipv6: true } } + # Regression: ssl_ca_file is an Excon option, not a Fog one, so a CA + # bundle from OS_CACERT or clouds.yaml used to be parsed and dropped. + it "carries a custom CA bundle through to Excon" do + config[:ssl_ca_file] = "/etc/ssl/certs/private-ca.pem" - it "returns only the v6 IPs" do - expect(driver.send(:parse_ips, pub, priv)).to eq([pub_v6, priv_v6]) - end + expect(driver.send(:openstack_server)[:connection_options]) + .to include(ssl_ca_file: "/etc/ssl/certs/private-ca.pem") end - end - context "only public IPs" do - let(:priv) { nil } + it "does not leak the CA bundle into the Fog settings" do + config[:ssl_ca_file] = "/etc/ssl/certs/private-ca.pem" - context "IPv4 (default)" do - it "returns only the v4 IPs" do - expect(driver.send(:parse_ips, pub, priv)).to eq([pub_v4, []]) - end + expect(driver.send(:openstack_server)).not_to have_key(:ssl_ca_file) end + end - context "IPv6" do - let(:config) { { use_ipv6: true } } + describe "string coercion" do + # Fog::Service#coerce_options turns anything that looks numeric back into + # an Integer, which then breaks the auth token builder. + described_class::FOG_STRING_SETTINGS.each do |setting| + next if setting == :openstack_identity_api_version - it "returns only the v6 IPs" do - expect(driver.send(:parse_ips, pub, priv)).to eq([pub_v6, []]) + it "stringifies a numeric #{setting}" do + config[setting] = 12345 + + expect(driver.send(:openstack_server)[setting]).to eq("12345") end end - end - context "only private IPs" do - let(:pub) { nil } + it "leaves nil values alone" do + config[:openstack_username] = nil - context "IPv4 (default)" do - it "returns only the v4 IPs" do - expect(driver.send(:parse_ips, pub, priv)).to eq([[], priv_v4]) - end + expect(driver.send(:openstack_server)[:openstack_username]).to be_nil end - context "IPv6" do - let(:config) { { use_ipv6: true } } + it "leaves settings outside the list alone" do + config[:openstack_service_type] = %w{compute} - it "returns only the v6 IPs" do - expect(driver.send(:parse_ips, pub, priv)).to eq([[], priv_v6]) - end + expect(driver.send(:openstack_server)[:openstack_service_type]).to eq(%w{compute}) end end + end - context "no IPs whatsoever" do - let(:pub) { nil } - let(:priv) { nil } - - context "IPv4 (default)" do - it "returns empty lists" do - expect(driver.send(:parse_ips, pub, priv)).to eq([[], []]) - end + describe "#normalize_identity_api_version" do + { + 3 => "v3", + "3" => "v3", + 3.0 => "v3.0", + 2 => "v2.0", + "2" => "v2.0", + "2.0" => "v2.0", + "v2.0" => "v2.0", + "v3" => "v3", + "V3" => "V3", + " 3 " => "v3", + "" => "", + }.each do |input, expected| + it "normalizes #{input.inspect} to #{expected.inspect}" do + expect(driver.send(:normalize_identity_api_version, input)).to eq(expected) end + end - context "IPv6" do - let(:config) { { use_ipv6: true } } + it "is applied through openstack_server" do + config[:openstack_identity_api_version] = 3 - it "returns empty lists" do - expect(driver.send(:parse_ips, nil, nil)).to eq([[], []]) - end - end + expect(driver.send(:openstack_server)[:openstack_identity_api_version]).to eq("v3") end end - describe "#add_ohai_hint" do - let(:state) { { hostname: "host" } } - let(:ssh) do - s = double("ssh") - allow(s).to receive(:run) { |args| args } - s + describe "#required_server_settings" do + it "names the four settings Fog cannot authenticate without" do + expect(driver.send(:required_server_settings)) + .to eq(%i{openstack_username openstack_api_key openstack_auth_url openstack_domain_id}) end - it "opens an SSH session to the server" do - driver.send(:add_ohai_hint, state) + end + + describe "#optional_server_settings" do + subject(:optional) { driver.send(:optional_server_settings) } + + it "covers every openstack_* setting Fog recognizes" do + expect(optional).to include(:openstack_region, :openstack_project_name, :openstack_tenant) end - it "opens an Winrm session to the server" do - allow(driver).to receive(:bourne_shell?).and_return(false) - allow(driver).to receive(:windows_os?).and_return(true) - driver.send(:add_ohai_hint, state) + it "excludes the required settings" do + expect(optional).not_to include(*driver.send(:required_server_settings)) end - end - describe "#disable_ssl_validation" do - it "turns off Excon SSL cert validation" do - expect(driver.send(:disable_ssl_validation)).to eq(false) + it "excludes settings Fog does not recognize" do + expect(optional).not_to include(:openstack_network_name) end end - describe "#countdown" do - it "counts down to future time with 0 seconds with almost no time" do - current = Time.now - driver.send(:countdown, 0) - after = Time.now - expect(after - current).to be >= 0 - expect(after - current).to be < 10 + describe "Fog service construction" do + let(:config) do + { + openstack_username: "twilight", + openstack_api_key: "sparkle", + openstack_auth_url: "http://keystone.example.com:5000/v3", + openstack_domain_id: "default", + } end - it "counts down to future time with 1 seconds with at least 9 seconds" do - current = Time.now - driver.send(:countdown, 1) - after = Time.now - expect(after - current).to be >= 9 + it "builds compute from the resolved settings" do + allow(Fog::OpenStack::Compute).to receive(:new) { |args| args } + + expect(driver.send(:compute)).to include(openstack_username: "twilight") end - end - describe "#wait_for_server" do - let(:config) { { server_wait: 0 } } - let(:state) { { hostname: "host" } } + it "builds network from the resolved settings" do + allow(Fog::OpenStack::Network).to receive(:new) { |args| args } - it "waits for connection to be available" do - expect(driver.send(:wait_for_server, state)).to be(nil) + expect(driver.send(:network)).to include(openstack_username: "twilight") end - it "Fails when calling transport but still destroys the created system" do - allow(instance.transport).to receive(:connection).and_raise(ArgumentError) - expect(driver).to receive(:destroy) - - expect { driver.send(:wait_for_server, state) } - .to raise_error(ArgumentError) + it "builds the volume helper with the driver's logger" do + expect(driver.send(:volume)).to be_a(Kitchen::Driver::Openstack::Volume) end end describe "#get_bdm" do - let(:logger) { Logger.new(logged_output) } - let(:config) do - { - openstack_username: "a", - openstack_domain_id: "default", - openstack_api_key: "b", - openstack_auth_url: "http://", - openstack_project_name: "me", - openstack_region: "ORD", - openstack_service_name: "stack", - image_ref: "22", - flavor_ref: "33", - username: "admin", - port: "2222", - server_name: "puppy", - server_name_prefix: "parsnip", - floating_ip_pool: "swimmers", - floating_ip: "11111", - network_ref: "0xCAFFE", - block_device_mapping: { - volume_id: "55", - volume_size: "5", - device_name: "vda", - delete_on_termination: true, - }, - } - end - it "returns just the BDM config" do - expect(driver.send(:get_bdm, config)).to eq(config[:block_device_mapping]) + it "delegates to the volume helper with the Fog settings" do + helper = instance_double(Kitchen::Driver::Openstack::Volume, get_bdm: { volume_id: "vol-1" }) + allow(driver).to receive(:volume).and_return(helper) + + expect(driver.send(:get_bdm, config)).to eq(volume_id: "vol-1") + expect(helper).to have_received(:get_bdm).with(config, driver.send(:openstack_server)) end end end diff --git a/spec/kitchen/driver/openstack_version_spec.rb b/spec/kitchen/driver/openstack_version_spec.rb new file mode 100644 index 00000000..527bd737 --- /dev/null +++ b/spec/kitchen/driver/openstack_version_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require "json" + +# These examples deliberately read two files from the repository root, which is +# the one exemption to the "unit tests read nothing outside a Dir.mktmpdir" +# rule in AGENTS.md. The version number is duplicated across four places, and +# nothing but a test keeps them in step -- a release that ships a gemspec +# version disagreeing with the manifest is exactly the failure worth catching +# before it is tagged. +# +# Both reads degrade to a skip rather than a failure, so the suite still runs +# against a packaged gem or a filtered checkout where those files are absent. +RSpec.describe "Kitchen::Driver::OPENSTACK_VERSION" do + subject(:version) { Kitchen::Driver::OPENSTACK_VERSION } + + # @param name [String] repo-root-relative filename + # @return [String, nil] absolute path, or nil when it is not present + def repo_file(name) + path = File.expand_path("../../../#{name}", __dir__) + File.exist?(path) ? path : nil + end + + it "is a semantic version string" do + expect(version).to match(/\A\d+\.\d+\.\d+\z/) + end + + it "is what the gemspec publishes" do + path = repo_file("kitchen-openstack.gemspec") + skip "gemspec not present; not running from a source checkout" unless path + + expect(Gem::Specification.load(path).version.to_s).to eq(version) + end + + it "is what the driver reports as its plugin version" do + expect(Kitchen::Driver::Openstack.instance_variable_get(:@plugin_version)).to eq(version) + end + + it "matches the release-please manifest" do + path = repo_file(".release-please-manifest.json") + skip "release-please manifest not present; not running from a source checkout" unless path + + expect(JSON.parse(File.read(path))["."]).to eq(version) + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 397e6d62..ff65ded9 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,3 +1,66 @@ # frozen_string_literal: true +# +# Copyright:: (C) 2026, Oregon State University +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Coverage instrumentation costs more than the suite itself on a single-file +# run, so it is opt-in locally and always on in CI. Set COVERAGE=1 to get a +# report from a local run; `rake coverage` does that for you. +if ENV["CI"] || ENV["COVERAGE"] + require "simplecov" + + SimpleCov.start do + add_filter "/spec/" + add_group "Driver", "lib/kitchen/driver/openstack.rb" + add_group "Modules", "lib/kitchen/driver/openstack/" + enable_coverage :branch + end +end + require "rspec" +require "kitchen" +require "kitchen/driver/openstack" +require "kitchen/provisioner/dummy" +require "kitchen/transport/dummy" +require "kitchen/verifier/dummy" + +Dir[File.expand_path("support/**/*.rb", __dir__)].sort.each { |f| require f } + +RSpec.configure do |config| + config.expect_with :rspec do |expectations| + expectations.include_chain_clauses_in_custom_matcher_descriptions = true + expectations.syntax = :expect + end + + config.mock_with :rspec do |mocks| + # Fail the build when a double stubs a method the real object does not + # have. This is the single most valuable guard against specs that keep + # passing after the implementation they describe has been renamed. + mocks.verify_partial_doubles = true + mocks.syntax = :expect + end + + config.shared_context_metadata_behavior = :apply_to_host_groups + config.disable_monkey_patching! + config.raise_errors_for_deprecations! + config.define_derived_metadata { |meta| meta[:aggregate_failures] = true unless meta.key?(:aggregate_failures) } + + config.filter_run_when_matching :focus + config.example_status_persistence_file_path = ".rspec_status" + config.warnings = false + + config.order = :random + Kernel.srand config.seed +end diff --git a/spec/support/driver_context.rb b/spec/support/driver_context.rb new file mode 100644 index 00000000..0b6c59e0 --- /dev/null +++ b/spec/support/driver_context.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +# +# Copyright:: (C) 2026, Oregon State University +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +require "logger" +require "stringio" unless defined?(StringIO) + +# Shared setup for every example that needs a configured driver. +# +# Provides: +# config - the hash handed to Kitchen::Driver::Openstack.new (override with `let`) +# driver - the driver under test, with `instance` stubbed to `instance` +# instance - a stand-in Kitchen::Instance +# logged_output - a StringIO holding everything the driver logged +# +# The driver's `sleep` is neutered so no example ever waits on the wall clock. +RSpec.shared_context "with a configured driver" do + let(:logged_output) { StringIO.new } + let(:logger) { Logger.new(logged_output) } + let(:instance_name) { "potatoes" } + let(:transport) { Kitchen::Transport::Dummy.new } + let(:platform) { Kitchen::Platform.new(name: "fake_platform") } + let(:config) { {} } + + let(:instance) do + instance_double( + Kitchen::Instance, + name: instance_name, + transport: transport, + logger: logger, + platform: platform, + to_str: "instance" + ) + end + + let(:driver) do + Kitchen::Driver::Openstack.new(config).tap do |d| + allow(d).to receive_messages(instance: instance, sleep: 0) + end + end + + # Replaces ENV wholesale with a copy that has no OS_* variable in it, plus + # whatever the caller passes. + # + # Stubbing individual lookups was not enough: a developer with OS_CLOUD or + # OS_USERNAME exported got different results from CI. Examples that need a + # variable set opt in by passing it here. + # + # @param vars [Hash{String => String}] variables to add back + # @return [Hash] the stubbed ENV + def stub_env(vars = {}) + stub_const("ENV", ENV.to_h.reject { |k, _| k.start_with?("OS_") }.merge(vars)) + end + + before do + # Re-declare File.exist? as a partial double so that per-example `.with` + # stubs can be layered on it without breaking every unrelated caller. On + # its own this changes nothing -- real filesystem behavior is preserved. + allow(File).to receive(:exist?).and_call_original + + # This is what actually isolates the suite from the developer's machine: + # with no OS_* variables in scope, no clouds.yaml lookup can be triggered + # by ambient environment. Note that it does not, by itself, stop the + # clouds.yaml *search path* from reaching real files -- the specs that + # exercise that search pin Dir.pwd, Dir.home and /etc/openstack as well. + stub_env + end +end diff --git a/spec/support/fog_doubles.rb b/spec/support/fog_doubles.rb new file mode 100644 index 00000000..5c020b80 --- /dev/null +++ b/spec/support/fog_doubles.rb @@ -0,0 +1,143 @@ +# frozen_string_literal: true + +# +# Copyright:: (C) 2026, Oregon State University +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +require "fog/openstack" +require "fog/openstack/compute/models/server" + +# Factories for the Fog objects the driver talks to. +# +# Model objects (Server) get verifying doubles, so a Fog upgrade that renames +# `public_ip_addresses` fails the suite instead of silently passing. +# +# Service objects (Compute/Network/Volume) get plain doubles on purpose: Fog +# builds their `servers`/`images`/`list_networks` methods dynamically when the +# service is instantiated, so nothing is defined on the class for RSpec to +# verify against. There is no verifying double to be had here. +module FogDoubles + # The `addresses` hash Nova returns for a server, in the shape the driver + # actually parses. + # + # It is kept consistent with the default `public_ip_addresses` and + # `private_ip_addresses` below: in fog-openstack those readers are derived + # from this hash, so a server double with addresses populated one way and + # empty the other describes a state no real cloud can produce. + DEFAULT_ADDRESSES = { + "public" => [{ "addr" => "1.2.3.4", "version" => 4, "OS-EXT-IPS:type" => "floating" }], + "private" => [{ "addr" => "10.0.0.1", "version" => 4, "OS-EXT-IPS:type" => "fixed" }], + }.freeze + + # A Fog compute server model. + # + # @param overrides [Hash] attributes to override on the default server + # @return [InstanceDouble] + def fog_server(**overrides) + defaults = { + id: "test123", + name: "hello", + public_ip_addresses: %w{1.2.3.4}, + private_ip_addresses: %w{10.0.0.1}, + addresses: DEFAULT_ADDRESSES, + ip_addresses: [], + associate_address: nil, + destroy: true, + ready?: true, + failed?: false, + } + attrs = defaults.merge(overrides) + canned_wait = attrs.delete(:wait_for) + server = instance_double(Fog::OpenStack::Compute::Server, **attrs) + + if canned_wait + allow(server).to receive(:wait_for).and_return(canned_wait) + else + stub_wait_for(server) + end + server + end + + # Gives `server` a `wait_for` that follows Fog's contract. + # + # The block is evaluated in the server's own context and its result decides + # the outcome: truthy means ready, and a block that never goes truthy raises + # Fog::Errors::TimeoutError. Returning a canned `{ duration: 0 }` instead + # meant no example ever executed `get_ip`'s `!addresses.empty?` predicate -- + # while SimpleCov still counted the line as covered. + # + # @param server [InstanceDouble] the server double to stub + # @return [void] + def stub_wait_for(server) + # The block is instance_exec'd on the server, so a `sleep` inside it is a + # call on the server and not on the driver -- the driver's own stubbed + # sleep does not cover it, and the suite would wait on the wall clock. + allow(server).to receive(:sleep).and_return(0) + allow(server).to receive(:wait_for) do |_timeout, &blk| + unless blk.nil? || server.instance_exec(&blk) + raise Fog::Errors::TimeoutError, "The specified wait_for timeout was exceeded" + end + + { duration: 0 } + end + end + + # A named, identified resource as returned by an images/flavors/networks + # collection. `find_matching` only ever reads #id and #name. + # + # @param id [String] resource id + # @param name [String, nil] resource name + # @return [RSpec::Mocks::Double] + def fog_resource(id:, name: nil) + double("Fog resource #{id}", id: id, name: name) + end + + # A stand-in Fog compute service. See the note above on why this is not a + # verifying double. + # + # @param overrides [Hash] collections to expose + # @return [RSpec::Mocks::Double] + def fog_compute(**overrides) + double("Fog::OpenStack::Compute", **{ servers: [], images: [], flavors: [], addresses: [] }.merge(overrides)) + end + + # A stand-in Fog network service. + # + # @param overrides [Hash] requests/collections to expose + # @return [RSpec::Mocks::Double] + def fog_network(**overrides) + double("Fog::OpenStack::Network", **overrides) + end + + # Wraps a body hash the way Fog's request layer returns it. + # + # @param body [Hash] the parsed response body + # @return [RSpec::Mocks::Double] an object responding to #body + def fog_response(body) + double("Excon::Response", body: body) + end + + # A floating address entry from `compute.addresses`. + # + # @param ip [String] the floating IP + # @param pool [String] the pool it belongs to + # @param fixed_ip [String, nil] set when the address is already in use + # @param instance_id [String, nil] set when the address is already attached + # @return [RSpec::Mocks::Double] + def fog_address(ip:, pool:, fixed_ip: nil, instance_id: nil) + double("Fog address #{ip}", ip: ip, pool: pool, fixed_ip: fixed_ip, instance_id: instance_id) + end +end + +RSpec.configure { |config| config.include FogDoubles }