From 89ffd1b647d92bb434683d9d5054e8629c173976 Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Sat, 22 Aug 2026 21:28:26 -0700 Subject: [PATCH 1/2] fix!: rebuild the unit test suite, document lib/, fix the bugs it found Rebuild the test suite around one spec file per lib file, document every method in lib/ with YARD, and fix the bugs the new coverage exposed. Tests - Replace the 1442-line monolith with per-module specs mirroring lib/. - Enable verify_partial_doubles, random ordering, aggregate_failures and disable_monkey_patching. Fog *model* objects now use instance_double; Fog *service* objects cannot, since Fog defines their methods dynamically at instantiation, and spec/support/fog_doubles.rb says so. - Replace the File.exist? stubs in the clouds.yaml specs with real files in a Dir.mktmpdir, so the search-path logic is actually exercised, and pin Dir.pwd, Dir.home and /etc/openstack so a real clouds.yaml or secure.yaml on the developer's machine can never be read -- which also keeps a real password out of any RSpec diff. - Replace ENV wholesale with an OS_*-free hash, so a developer's own OpenStack environment cannot leak into a run. The helper that does it now lives in the shared context rather than being duplicated. - Model Fog's wait_for contract in the doubles instead of returning a canned value: the readiness block is evaluated in the model's own context and a block that never goes truthy raises TimeoutError. The canned version meant get_ip's readiness predicate was never executed by any example while SimpleCov still counted it covered. - Stub every sleep. 141 examples in 20.19s becomes 360 in 0.31s. - Add SimpleCov reporting to coverage/, with no enforced threshold. It is on in CI and opt-in locally via `rake coverage` or COVERAGE=1, so a single-file run does not pay for instrumentation. Line coverage is 100%; the five uncovered branches are all `require x unless defined?(X)` guards. Bug fixes - volume: `volumes.first { |x| x.id == vol_id }` silently ignored its block, so creation waited on whichever volume happened to be listed first rather than the one just created. Fetch it by id with volumes.get, which is a direct GET and so is correct at any volume count -- a listing returns only one page. - volume: get_bdm mutated config[:block_device_mapping] in place, so a retried create saw an already-stripped hash. Work on a copy. - volume: coerce creation_timeout/attach_timeout, which YAML parses as Strings when quoted; "5" > 0 raised ArgumentError. Parse in base 10 so "010" is ten seconds rather than eight, validate both before the Cinder volume is created rather than after, and report a bad value as ActionFailed naming the key instead of a raw backtrace plus a leaked volume. - volume: raise Kitchen::ActionFailed rather than RuntimeError, which create's `rescue Fog::Errors::Error, Excon::Errors::Error` does not catch and so surfaced as a raw backtrace. - config: server_name_prefix called gsub! on its argument, rewriting config[:server_name_prefix] in place and raising FrozenError on a frozen literal. - config: Etc.getpwuid raises for a uid with no passwd entry rather than returning nil, so the "nologin" fallback was dead code and the container case crashed. Rescue it. - config: require etc and socket instead of relying on transitive loads. - config: derive the per-component name budgets from MAX_SERVER_NAME_LENGTH so the 63-character limit is maintained by the code rather than only described by a constant nothing read. - networking: guard the pool lookup in attach_ip_from_pool and the named network in get_ip. Both indexed straight into [0], turning a typo into a NoMethodError on nil instead of a usable message. - networking: parse_ips used select!, and Array(x) returns x itself when x is already an Array, so it filtered the Fog server model's own address lists in place. Use select. - networking: free_ip_from_pool built and compacted a whole throwaway array while holding IP_POOL_LOCK just to take the first element. Stop at the first free address. - openstack: guard the floating IP release in destroy the same way. An address Neutron had already reclaimed took the whole destroy down and stranded the server. - openstack: ssl_ca_file is an Excon connection option, not a Fog one, so a CA bundle from OS_CACERT or a clouds.yaml cacert entry was parsed and then dropped. Pass it through connection_options. - server_helper: a missing user_data file silently became nil, booting a server without the cloud-init the operator asked for. Raise instead. - server_helper: a security_groups value that was not an Array was silently dropped, booting into the default group. Raise instead. - server_helper: validate the whole config before get_bdm runs. get_bdm is the only step that creates a resource, and a volume it created was orphaned when a later purely-local check raised, since the new volume's id lived only in the discarded server definition. - server_helper: compare resource ids as strings, since Nova returns integer flavor ids on some deployments. - clouds: the SSL ternary read env[:ssl_verify_peer], which ENV_VAR_MAP can never produce. Remove the dead branch; report the file by name on a YAML syntax error or a non-mapping document, and report a named cloud entry that is present but is not a mapping rather than carrying on with nil credentials and failing later inside Keystone. - clouds: Dir.home raises when HOME is unset and the uid has no passwd entry, so with OS_CLOUD set the search crashed before /etc/openstack -- the one location such a container is likely to have -- was tried. - clouds: use YAML.safe_load_file now that Ruby 3.1 is the floor. Docs - YARD tags on all 57 methods and 18 constants (100% documented). - Add .yardopts plus `rake yard` and `rake yard_stats`. Neither is wired into rake default or CI, per the no-gating requirement. - Fix the README's stale `rake spec` / `rake rubocop` instructions. BREAKING CHANGE: three behaviours change in ways that can fail a converge that previously succeeded. * A `user_data` file that does not exist now raises ActionFailed. It was previously read as nil, booting the server without the cloud-init the operator asked for and saying nothing about it. * A `security_groups` value that is not an Array now raises ActionFailed. It was previously dropped, booting the server into the default security group rather than the requested one. * `ssl_ca_file` -- from OS_CACERT or a clouds.yaml `cacert` entry -- is now actually handed to Excon. It was previously parsed and then discarded, so a stale or wrong CA path had no effect. Deployments carrying one will now fail TLS until it is corrected or removed. Two smaller changes worth noting, neither of which should break a working converge: * image_ref/flavor_ref: a purely numeric ref now matches an id before a name. On clouds where Nova returns Integer ids the id branch could not match a String ref, so a resource *named* "2" won. A flavor named "2" alongside a different flavor with id 2 now resolves to the latter, which is what the documented id-first contract says should happen. * A block_device_mapping timeout that is not a number now raises ActionFailed naming the key, and is checked before the Cinder volume is created rather than after it. Co-Authored-By: Claude Opus 5 (1M context) --- .gitignore | 2 + .rspec | 3 + .yardopts | 10 + AGENTS.md | 17 +- CONTRIBUTING.md | 85 +- Gemfile | 5 + Rakefile | 18 + lib/kitchen/driver/openstack.rb | 114 +- lib/kitchen/driver/openstack/clouds.rb | 147 +- lib/kitchen/driver/openstack/config.rb | 127 +- lib/kitchen/driver/openstack/helpers.rb | 49 +- lib/kitchen/driver/openstack/networking.rb | 159 +- lib/kitchen/driver/openstack/server_helper.rb | 112 +- lib/kitchen/driver/openstack/volume.rb | 146 +- lib/kitchen/driver/openstack_version.rb | 6 + spec/kitchen/driver/openstack/clouds_spec.rb | 1094 ++++++------ spec/kitchen/driver/openstack/config_spec.rb | 211 +++ spec/kitchen/driver/openstack/helpers_spec.rb | 181 ++ .../driver/openstack/networking_spec.rb | 482 +++++ .../driver/openstack/server_helper_spec.rb | 422 +++++ spec/kitchen/driver/openstack/volume_spec.rb | 372 +++- spec/kitchen/driver/openstack_spec.rb | 1558 ++++------------- spec/kitchen/driver/openstack_version_spec.rb | 45 + spec/spec_helper.rb | 63 + spec/support/driver_context.rb | 81 + spec/support/fog_doubles.rb | 143 ++ 26 files changed, 3617 insertions(+), 2035 deletions(-) create mode 100644 .rspec create mode 100644 .yardopts create mode 100644 spec/kitchen/driver/openstack/config_spec.rb create mode 100644 spec/kitchen/driver/openstack/helpers_spec.rb create mode 100644 spec/kitchen/driver/openstack/networking_spec.rb create mode 100644 spec/kitchen/driver/openstack/server_helper_spec.rb create mode 100644 spec/kitchen/driver/openstack_version_spec.rb create mode 100644 spec/support/driver_context.rb create mode 100644 spec/support/fog_doubles.rb 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/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 } From 0daa30c3880707b8a5cbf67384c179600450bb69 Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Sat, 22 Aug 2026 21:28:26 -0700 Subject: [PATCH 2/2] docs: rewrite the README for someone arriving new The README pointed at kitchen.ci for all configuration and documented only clouds.yaml, which read as a changelog entry for that feature rather than as documentation. There was no path from "I have an OpenStack account" to "I have a running instance". - Add a quick start that goes end to end: get credentials into your shell, verify them with the openstack CLI first, find an image and flavor, write a minimal kitchen.yml, run kitchen test. - Add a full configuration reference, grouped by task rather than alphabetical. Every option was derived from what the code actually reads -- `diagnose.keys` plus `Fog::OpenStack::Compute.recognized` plus `required_server_settings` -- and the tables are diffed against that set, so there are no invented options and no real ones missing. - Call out the settings people reach for that this driver does not own: `username`, `ssh_key` and `port` belong to the transport, and `no_ssh_tcp_check`, `no_ssh_tcp_check_sleep` and `pre_create_command` are accepted but have no effect. - Add worked examples for floating IPs, multiple networks, cloud-init, booting from a volume, and a cloud behind a private CA. - Add a troubleshooting table keyed on the actual error strings the driver raises, each verified to exist in lib/. - Name OS_CLIENT_SECURE_FILE, which was described but never spelled out. - Add a contents list, and expand the development section with the real rake tasks. - Use `device_name` in the boot-from-volume example. fog-openstack reads exactly :delete_on_termination, :device_name, :volume_id and :volume_size out of a block device mapping, so `volume_device_name` would have been sent as device_name => nil and the volume attached wherever the hypervisor chose. Verified: markdownlint clean, all 10 internal anchors resolve, every external link returns 200. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 577 +++++++++++++++++++++++++++++++++++------------------- 1 file changed, 375 insertions(+), 202 deletions(-) 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).