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