From e2f62ecc067a3c7ba456e236c60855915dcb82c9 Mon Sep 17 00:00:00 2001 From: Lance Albertson Date: Mon, 19 Oct 2026 12:49:53 -0700 Subject: [PATCH] feat!: record server origin and lead the name with the project Cleaning up a shared OpenStack project means working out who created each server and from which cookbook or repo, so the right person can run kitchen destroy. The default name was ---, which names the person but not the project, and a suite like "default" exists in almost every cookbook. Servers now carry metadata saying where they came from, and the generated name starts with the project. Instances created by earlier releases are unaffected: once a server exists, every action finds it through state[:server_id], never by name. - Add default_metadata (on by default) to set kitchen_instance, kitchen_suite, kitchen_platform, kitchen_project, kitchen_user and kitchen_host - Merge the user's metadata over those defaults with stringified keys, since Kitchen symbolizes config keys and both spellings would otherwise be sent - Send user metadata values as strings, the only type Nova accepts, and reject a metadata setting that is not a hash with ActionFailed - Truncate default values to Nova's 255-byte limit without splitting a multibyte character - Generate ---, where the project is the basename of kitchen_root; the hostname moves to kitchen_host - Leave out name components that strip to nothing instead of leaving a stray separator - Rename NAME_HOSTNAME_LENGTH to NAME_PROJECT_LENGTH, keeping its 23 characters - Log the server id when create finds an existing server, instead of a newly generated name that server never had - Document both in the README - Cover the metadata rules in server_helper_spec, the name layout in config_spec and the create message in openstack_spec BREAKING CHANGE: generated server names, and the guest hostnames Nova derives from them, now start with the project instead of ending with the workstation hostname. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Lance Albertson --- README.md | 21 +++- lib/kitchen/driver/openstack.rb | 8 +- lib/kitchen/driver/openstack/config.rb | 31 ++++-- lib/kitchen/driver/openstack/server_helper.rb | 48 +++++++- spec/kitchen/driver/openstack/config_spec.rb | 65 +++++++---- .../driver/openstack/server_helper_spec.rb | 105 +++++++++++++++++- spec/kitchen/driver/openstack_spec.rb | 12 +- spec/support/driver_context.rb | 2 + 8 files changed, 257 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index 9bb78e2e..deabc4c1 100644 --- a/README.md +++ b/README.md @@ -290,11 +290,25 @@ Everything below goes under `driver:` in `kitchen.yml`. | `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. | +| `metadata` | `nil` | Hash of Nova instance metadata, added to the defaults below. Your keys win, and values are sent as strings. | +| `default_metadata` | `true` | Record where the instance came from in its metadata (see below). | 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. +Unless `default_metadata` is `false`, every instance is tagged with where it +came from, so whoever cleans up a shared project can tell who created an +instance and what for: + +| Key | Value | +| --- | --- | +| `kitchen_instance` | Test Kitchen instance name, e.g. `default-ubuntu-2404` | +| `kitchen_suite` | Suite name | +| `kitchen_platform` | Platform name | +| `kitchen_project` | Basename of the Kitchen root, usually the cookbook | +| `kitchen_user` | Local user running Test Kitchen | +| `kitchen_host` | Hostname of the machine running Test Kitchen, where its state lives | + The `_ref` options accept three forms: ```yaml @@ -310,8 +324,11 @@ image_ref: /^ubuntu-24\.04/ # regex, first match wins | `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 `---`, +With neither set, the name is `---`, +where the project is the basename of the Kitchen root (usually the cookbook), truncated to OpenStack's 63-character limit. Non-word characters are stripped. +The workstation hostname, which earlier releases put in the name, is in the +`kitchen_host` metadata instead. ### Networking diff --git a/lib/kitchen/driver/openstack.rb b/lib/kitchen/driver/openstack.rb index 14814f2f..bc02f693 100755 --- a/lib/kitchen/driver/openstack.rb +++ b/lib/kitchen/driver/openstack.rb @@ -110,6 +110,7 @@ class Openstack < Kitchen::Driver::Base default_config :read_timeout, 60 default_config :write_timeout, 60 default_config :metadata, nil + default_config :default_metadata, true # Merges clouds.yaml and `OS_*` values into the config hash. # @@ -134,11 +135,14 @@ def finalize_config!(instance) # @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 + # The name is generated with a random suffix, so it only means + # something for a server about to be created. if state[:server_id] - info "#{config[:server_name]} (#{state[:server_id]}) already exists." + info "OpenStack server <#{state[:server_id]}> already exists." return end + + config_server_name disable_ssl_validation if config[:disable_ssl_validation] server = create_server state[:server_id] = server.id diff --git a/lib/kitchen/driver/openstack/config.rb b/lib/kitchen/driver/openstack/config.rb index 9e83ec1d..8442907c 100644 --- a/lib/kitchen/driver/openstack/config.rb +++ b/lib/kitchen/driver/openstack/config.rb @@ -22,7 +22,6 @@ # limitations under the License. require "etc" unless defined?(Etc) -require "socket" unless defined?(Socket) module Kitchen module Driver @@ -63,13 +62,14 @@ module Config # @return [Integer] NAME_RANDOM_LENGTH = 7 - # Character budget for the hostname in a fully generated name. + # Character budget for the project (the basename of the Kitchen root, + # usually the cookbook) 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 = + NAME_PROJECT_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. @@ -93,17 +93,34 @@ def config_server_name # Generates a unique server name of at most 63 characters. # - # --- - # 15 15 23 7 + 3 separators = 63 + # --- + # 23 15 15 7 + 3 separators = 63 + # + # The project leads so that servers from one cookbook sort together and + # whoever is cleaning up a shared project can tell where a server came + # from. The workstation hostname is recorded in the server metadata + # instead (see {ServerHelper#default_metadata}). + # + # A component that is empty once stripped (a Kitchen root of `/`, a + # username with no ASCII word characters) is left out rather than + # leaving a stray separator. # # @return [String] the generated server name def default_name [ + kitchen_project.gsub(/\W/, "")[0, NAME_PROJECT_LENGTH], 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("-") + ].reject(&:empty?).join("-") + end + + # Name of the project Test Kitchen is running in: the basename of the + # Kitchen root, which for a cookbook is the cookbook's directory. + # + # @return [String] + def kitchen_project + File.basename(config[:kitchen_root] || Dir.pwd) end # Best-effort lookup of the name of the user running Test Kitchen. diff --git a/lib/kitchen/driver/openstack/server_helper.rb b/lib/kitchen/driver/openstack/server_helper.rb index 2b9a2716..f69a4fef 100644 --- a/lib/kitchen/driver/openstack/server_helper.rb +++ b/lib/kitchen/driver/openstack/server_helper.rb @@ -21,6 +21,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +require "socket" unless defined?(Socket) + module Kitchen module Driver class Openstack < Kitchen::Driver::Base @@ -35,9 +37,13 @@ module ServerHelper key_name user_data config_drive - metadata }.freeze + # Longest metadata value Nova accepts, in bytes. + # + # @return [Integer] + MAX_METADATA_VALUE_BYTES = 255 + private # Builds the Nova server definition and submits it. @@ -69,6 +75,9 @@ def create_server server_def[c] = optional_config(c) if config[c] end + metadata = server_metadata + server_def[:metadata] = metadata unless metadata.empty? + if config[:cloud_config] raise(ActionFailed, "Cannot specify both cloud_config and user_data") if config[:user_data] @@ -106,6 +115,43 @@ def init_configuration } end + # Builds the server metadata: {#default_metadata} unless + # `default_metadata` is false, overlaid with the user's `metadata`. + # + # Keys are stringified because Kitchen symbolizes config keys, so + # without this a user's `kitchen_user:` and the default + # `"kitchen_user"` would both be sent. Values are stringified because + # Nova only accepts strings, and YAML turns `keep: true` into a boolean. + # + # @return [Hash{String => String}] possibly empty + # @raise [Kitchen::ActionFailed] if `metadata` is not a hash + def server_metadata + user = config[:metadata] || {} + raise ActionFailed, "The metadata config must be a hash, got #{user.class}" unless user.is_a?(Hash) + + defaults = config[:default_metadata] ? default_metadata : {} + defaults.merge(user.to_h { |k, v| [k.to_s, v.to_s] }) + end + + # Metadata recording where a server came from, so that whoever cleans + # up a shared project can tell who created a server, from which + # project and suite, and on which workstation the Kitchen state lives. + # + # Values are cut to Nova's limit in bytes, not characters, dropping any + # multibyte character the cut splits. + # + # @return [Hash{String => String}] + def default_metadata + { + "kitchen_instance" => instance.name, + "kitchen_suite" => instance.suite.name, + "kitchen_platform" => instance.platform.name, + "kitchen_project" => kitchen_project, + "kitchen_user" => current_username, + "kitchen_host" => Socket.gethostname, + }.transform_values { |v| v.to_s.byteslice(0, MAX_METADATA_VALUE_BYTES).scrub("") } + end + # Resolves one optional server setting to the value Nova expects. # # @param c [Symbol] the config key diff --git a/spec/kitchen/driver/openstack/config_spec.rb b/spec/kitchen/driver/openstack/config_spec.rb index 2c7cdae3..9630bcf8 100644 --- a/spec/kitchen/driver/openstack/config_spec.rb +++ b/spec/kitchen/driver/openstack/config_spec.rb @@ -3,15 +3,16 @@ RSpec.describe Kitchen::Driver::Openstack::Config do include_context "with a configured driver" - let(:login) { "user" } - let(:hostname) { "host" } + let(:login) { "user" } + let(:kitchen_root) { "/src/carrots" } - # Both lookups hit the host machine, so pin them or the assertions below - # depend on whoever is running the suite. + # The user lookup hits the host machine and the project comes from the + # Kitchen root, so pin both or the assertions below depend on whoever is + # running the suite and from where. 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) + config[:kitchen_root] = kitchen_root unless config.key?(:kitchen_root) end describe "#config_server_name" do @@ -47,22 +48,46 @@ 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/) + expect(driver[:server_name]).to match(/\Acarrots-potatoes-user-[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/) + it "joins project, instance, user and a random suffix" do + expect(driver.send(:default_name)).to match(/\Acarrots-potatoes-user-[a-z0-9]{7}\z/) + end + + it "does not include the workstation hostname" do + allow(Socket).to receive(:gethostname).and_return("workstation") + + expect(driver.send(:default_name)).not_to include("workstation") + end + + context "when kitchen_root is not set" do + let(:config) { { kitchen_root: nil } } + + it "uses the current directory as the project" do + allow(Dir).to receive(:pwd).and_return("/work/parsnips") + + expect(driver.send(:default_name)).to match(/\Aparsnips-potatoes-user-/) + end 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 } + context "when a component is empty once stripped" do + let(:kitchen_root) { "/" } + + it "leaves it out instead of leaving a stray separator" do + expect(driver.send(:default_name)).to match(/\Apotatoes-user-[a-z0-9]{7}\z/) + end + end + + context "with a long project directory" do + let(:kitchen_root) { "/src/#{"ab.c" * 20}" } it "stays within the OpenStack name limit" do expect(driver.send(:default_name).length) @@ -70,10 +95,10 @@ end end - context "with a long instance name, user and host" do + context "with a long project, instance name and user" do + let(:kitchen_root) { "/src/#{"c" * 40}" } 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) @@ -90,18 +115,18 @@ 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("-") + project_part, instance_part, user_part, random_part = driver.send(:default_name).split("-") + expect(project_part.length).to eq(config_mod::NAME_PROJECT_LENGTH) 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(:kitchen_root) { "/src/a.cook-book_name" } let(:instance_name) { "a.instance-name" } it "strips characters OpenStack rejects in server names" do @@ -119,14 +144,14 @@ 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-/) + expect(driver.send(:default_name)).to match(/\Acarrots-potatoes-user-/) 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-/) + expect(driver.send(:default_name)).to match(/\Acarrots-potatoes-nologin-/) end end end @@ -135,7 +160,7 @@ 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-/) + expect(driver.send(:default_name)).to match(/\Acarrots-potatoes-nologin-/) end end end @@ -171,7 +196,7 @@ 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-/) + expect(driver.send(:server_name_prefix, "...")).to match(/\Acarrots-potatoes-user-/) end it "warns the user" do @@ -181,7 +206,7 @@ end it "handles an entirely empty prefix" do - expect(driver.send(:server_name_prefix, "")).to match(/\Apotatoes-user-host-/) + expect(driver.send(:server_name_prefix, "")).to match(/\Acarrots-potatoes-user-/) end end diff --git a/spec/kitchen/driver/openstack/server_helper_spec.rb b/spec/kitchen/driver/openstack/server_helper_spec.rb index 86c9153f..490ca45a 100644 --- a/spec/kitchen/driver/openstack/server_helper_spec.rb +++ b/spec/kitchen/driver/openstack/server_helper_spec.rb @@ -18,11 +18,14 @@ before do allow(driver).to receive_messages(compute: compute, network: net_service) + # Pinned so the default metadata does not depend on who runs the suite. + allow(Etc).to receive(:getpwuid).and_return(Struct.new(:name).new("user")) + allow(Socket).to receive(:gethostname).and_return("workstation") end describe "#create_server" do let(:config) do - { server_name: "hello", image_id: "111", flavor_id: "1" } + { server_name: "hello", image_id: "111", flavor_id: "1", kitchen_root: "/src/carrots" } end it "returns whatever Nova created" do @@ -46,6 +49,8 @@ end it "omits optional keys that are not configured" do + config[:default_metadata] = false + driver.send(:create_server) expect(servers).to have_received(:create) do |server_def| @@ -213,6 +218,101 @@ end end + describe "metadata" do + let(:defaults) do + { + "kitchen_instance" => "potatoes", + "kitchen_suite" => "fake_suite", + "kitchen_platform" => "fake_platform", + "kitchen_project" => "carrots", + "kitchen_user" => "user", + "kitchen_host" => "workstation", + } + end + + def sent_metadata + metadata = nil + expect(servers).to have_received(:create) { |server_def| metadata = server_def[:metadata] } + metadata + end + + it "records where the server came from by default" do + driver.send(:create_server) + + expect(sent_metadata).to eq(defaults) + end + + it "adds the user's metadata to the defaults" do + config[:metadata] = { owner: "me" } + + driver.send(:create_server) + + expect(sent_metadata).to eq(defaults.merge("owner" => "me")) + end + + # Kitchen symbolizes config keys, so a user's kitchen_user: arrives as a + # Symbol and has to replace the String default rather than sit beside it. + it "lets the user override a default key" do + config[:metadata] = { kitchen_user: "ci" } + + driver.send(:create_server) + + expect(sent_metadata).to eq(defaults.merge("kitchen_user" => "ci")) + end + + it "sends the user's values as strings, which is all Nova accepts" do + config[:metadata] = { keep: true, build: 42 } + + driver.send(:create_server) + + expect(sent_metadata).to include("keep" => "true", "build" => "42") + end + + it "rejects metadata that is not a hash" do + config[:metadata] = "owner=me" + + expect { driver.send(:create_server) } + .to raise_error(Kitchen::ActionFailed, "The metadata config must be a hash, got String") + end + + it "truncates default values to what Nova accepts" do + config[:kitchen_root] = "/src/#{"x" * 300}" + + driver.send(:create_server) + + expect(sent_metadata["kitchen_project"].bytesize) + .to eq(Kitchen::Driver::Openstack::ServerHelper::MAX_METADATA_VALUE_BYTES) + end + + # Nova's limit is 255 bytes, so 255 characters of a multibyte name would + # still be rejected. + it "truncates by bytes without splitting a character" do + config[:kitchen_root] = "/src/#{"é" * 200}" + + driver.send(:create_server) + + expect(sent_metadata["kitchen_project"]).to eq("é" * 127) + end + + context "when default_metadata is false" do + before { config[:default_metadata] = false } + + it "sends only the user's metadata" do + config[:metadata] = { "owner" => "me" } + + driver.send(:create_server) + + expect(sent_metadata).to eq("owner" => "me") + end + + it "sends no metadata when the user configured none" do + driver.send(:create_server) + + expect(servers).to have_received(:create) { |sd| expect(sd).not_to have_key(:metadata) } + end + end + end + describe "optional settings" do it "passes key_name through" do config[:key_name] = "my-key" @@ -227,7 +327,8 @@ driver.send(:create_server) - expect(servers).to have_received(:create).with(hash_including(metadata: { "owner" => "me" })) + expect(servers).to have_received(:create) + .with(hash_including(metadata: hash_including("owner" => "me"))) end it "passes config_drive through" do diff --git a/spec/kitchen/driver/openstack_spec.rb b/spec/kitchen/driver/openstack_spec.rb index 0b3b0ec3..397e504e 100755 --- a/spec/kitchen/driver/openstack_spec.rb +++ b/spec/kitchen/driver/openstack_spec.rb @@ -120,7 +120,17 @@ it "says so" do driver.create(state) - expect(logged_output.string).to include("hello (existing) already exists.") + expect(logged_output.string).to include("OpenStack server already exists.") + end + + # The name carries a random suffix, so generating one here would only + # log a name the existing server does not have. + it "does not generate a server name" do + allow(driver).to receive(:config_server_name) + + driver.create(state) + + expect(driver).not_to have_received(:config_server_name) end it "leaves state untouched" do diff --git a/spec/support/driver_context.rb b/spec/support/driver_context.rb index 0b6c59e0..f7fd7dce 100644 --- a/spec/support/driver_context.rb +++ b/spec/support/driver_context.rb @@ -33,6 +33,7 @@ let(:instance_name) { "potatoes" } let(:transport) { Kitchen::Transport::Dummy.new } let(:platform) { Kitchen::Platform.new(name: "fake_platform") } + let(:suite) { Kitchen::Suite.new(name: "fake_suite") } let(:config) { {} } let(:instance) do @@ -42,6 +43,7 @@ transport: transport, logger: logger, platform: platform, + suite: suite, to_str: "instance" ) end