From 5ff5719a00306fd3c913ac71663b85fb7e28d809 Mon Sep 17 00:00:00 2001
From: Jeffrey Clark <h0tw1r3@users.noreply.github.com>
Date: Mon, 12 Feb 2024 17:30:27 -0600
Subject: [PATCH 1/8] (maint) reconcile docker provisioner facts and name

* os_release facts added to docker_exp
* container_id fact added to docker
* consistent naming convention, remove generated name from docker (use id)
---
 lib/docker_helper.rb  | 30 ++++++++++++++++++++++++++++
 tasks/docker.json     |  3 ++-
 tasks/docker.rb       | 46 ++++++++++---------------------------------
 tasks/docker_exp.json |  3 ++-
 tasks/docker_exp.rb   |  4 +++-
 5 files changed, 47 insertions(+), 39 deletions(-)
 create mode 100644 lib/docker_helper.rb

diff --git a/lib/docker_helper.rb b/lib/docker_helper.rb
new file mode 100644
index 00000000..a4cf6dd7
--- /dev/null
+++ b/lib/docker_helper.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+require 'json'
+
+def docker_image_os_release_facts(image)
+  os_release_facts = {}
+  begin
+    os_release = run_local_command("docker run --rm #{image} cat /etc/os-release")
+    # The or-release file is a newline-separated list of environment-like
+    # shell-compatible variable assignments.
+    re = '^(.+)=(.+)'
+    os_release.each_line do |line|
+      line = line.strip || line
+      next if line.nil? || line.empty?
+
+      _, key, value = line.match(re).to_a
+      # The values seems to be quoted most of the time, however debian only quotes
+      # some of the values :/.  Parse it, as if it was a JSON string.
+      value = JSON.parse(value) unless value[0] != '"'
+      os_release_facts[key] = value
+    end
+  rescue StandardError
+    # fall through to parsing the id and version from the image if it doesn't have `/etc/os-release`
+    id, version_id = image.split(':')
+    id = id.sub(%r{/}, '_')
+    os_release_facts['ID'] = id
+    os_release_facts['VERSION_ID'] = version_id
+  end
+  os_release_facts
+end
diff --git a/tasks/docker.json b/tasks/docker.json
index 23b481c9..d14fe2c7 100644
--- a/tasks/docker.json
+++ b/tasks/docker.json
@@ -26,6 +26,7 @@
     }
   },
   "files": [
-    "provision/lib/task_helper.rb"
+    "provision/lib/task_helper.rb",
+    "provision/lib/docker_helper.rb"
   ]
 }
diff --git a/tasks/docker.rb b/tasks/docker.rb
index 88799a80..aa651509 100755
--- a/tasks/docker.rb
+++ b/tasks/docker.rb
@@ -6,6 +6,7 @@
 require 'yaml'
 require 'puppet_litmus'
 require_relative '../lib/task_helper'
+require_relative '../lib/docker_helper'
 
 def install_ssh_components(distro, version, container)
   case distro
@@ -86,33 +87,6 @@ def fix_ssh(distro, version, container)
   end
 end
 
-def get_image_os_release_facts(image)
-  os_release_facts = {}
-  begin
-    os_release = run_local_command("docker run --rm #{image} cat /etc/os-release")
-    # The or-release file is a newline-separated list of environment-like
-    # shell-compatible variable assignments.
-    re = '^(.+)=(.+)'
-    os_release.each_line do |line|
-      line = line.strip || line
-      next unless !line.nil? && !line.empty?
-
-      _, key, value = line.match(re).to_a
-      # The values seems to be quoted most of the time, however debian only quotes
-      # some of the values :/.  Parse it, as if it was a JSON string.
-      value = JSON.parse(value) unless value[0] != '"'
-      os_release_facts[key] = value
-    end
-  rescue StandardError
-    # fall through to parsing the id and version from the image if it doesn't have `/etc/os-release`
-    id, version_id = image.split(':')
-    id = id.sub(%r{/}, '_')
-    os_release_facts['ID'] = id
-    os_release_facts['VERSION_ID'] = version_id
-  end
-  os_release_facts
-end
-
 # We check for a local port open by binding a raw socket to it
 # If the socket can successfully bind, then the port is open
 def local_port_open?(port)
@@ -157,7 +131,7 @@ def provision(image, inventory_location, vars)
   include PuppetLitmus::InventoryManipulation
   inventory_full_path = File.join(inventory_location, '/spec/fixtures/litmus_inventory.yaml')
   inventory_hash = get_inventory_hash(inventory_full_path)
-  os_release_facts = get_image_os_release_facts(image)
+  os_release_facts = docker_image_os_release_facts(image)
   distro = os_release_facts['ID']
   version = os_release_facts['VERSION_ID']
 
@@ -174,7 +148,6 @@ def provision(image, inventory_location, vars)
   group_name = 'ssh_nodes'
   warn '!!! Using private port forwarding!!!'
   front_facing_port = random_ssh_forwarding_port
-  full_container_name = "#{image.gsub(%r{[/:.]}, '_')}-#{front_facing_port}"
 
   node = {
     'uri' => "#{hostname}:#{front_facing_port}",
@@ -184,7 +157,6 @@ def provision(image, inventory_location, vars)
     },
     'facts' => {
       'provisioner' => 'docker',
-      'container_name' => full_container_name,
       'platform' => image,
       'os-release' => os_release_facts
     }
@@ -201,15 +173,17 @@ def provision(image, inventory_location, vars)
   docker_run_opts += ' --cgroupns=host' if (image =~ %r{debian|ubuntu}) \
   && !docker_run_opts.include?('--cgroupns')
 
-  creation_command = "docker run -d -it --privileged --tmpfs /tmp:exec -p #{front_facing_port}:22 --name #{full_container_name} "
+  creation_command = "docker run -d -it --privileged --tmpfs /tmp:exec -p #{front_facing_port}:22 "
   creation_command += "#{docker_run_opts} " unless docker_run_opts.nil?
   creation_command += image
-  run_local_command(creation_command).strip
-  install_ssh_components(distro, version, full_container_name)
-  fix_ssh(distro, version, full_container_name)
+  container_id = run_local_command(creation_command).strip[0..11]
+  node['name'] = container_id
+  node['facts']['container_id'] = container_id
+  install_ssh_components(distro, version, container_id)
+  fix_ssh(distro, version, container_id)
   add_node_to_group(inventory_hash, node, group_name)
   File.open(inventory_full_path, 'w') { |f| f.write inventory_hash.to_yaml }
-  { status: 'ok', node_name: "#{hostname}:#{front_facing_port}", node: node }
+  { status: 'ok', node_name: container_id, node: node }
 end
 
 def tear_down(node_name, inventory_location)
@@ -219,7 +193,7 @@ def tear_down(node_name, inventory_location)
 
   inventory_hash = inventory_hash_from_inventory_file(inventory_full_path)
   node_facts = facts_from_node(inventory_hash, node_name)
-  remove_docker = "docker rm -f #{node_facts['container_name']}"
+  remove_docker = "docker rm -f #{node_facts['container_id']}"
   run_local_command(remove_docker)
   remove_node(inventory_hash, node_name)
   puts "Removed #{node_name}"
diff --git a/tasks/docker_exp.json b/tasks/docker_exp.json
index 23b481c9..d14fe2c7 100644
--- a/tasks/docker_exp.json
+++ b/tasks/docker_exp.json
@@ -26,6 +26,7 @@
     }
   },
   "files": [
-    "provision/lib/task_helper.rb"
+    "provision/lib/task_helper.rb",
+    "provision/lib/docker_helper.rb"
   ]
 }
diff --git a/tasks/docker_exp.rb b/tasks/docker_exp.rb
index 31ce40d7..9b2047c8 100755
--- a/tasks/docker_exp.rb
+++ b/tasks/docker_exp.rb
@@ -5,6 +5,7 @@
 require 'yaml'
 require 'puppet_litmus'
 require_relative '../lib/task_helper'
+require_relative '../lib/docker_helper'
 
 # TODO: detect what shell to use
 @shell_command = 'bash -lc'
@@ -13,6 +14,7 @@ def provision(docker_platform, inventory_location, vars)
   include PuppetLitmus::InventoryManipulation
   inventory_full_path = File.join(inventory_location, '/spec/fixtures/litmus_inventory.yaml')
   inventory_hash = get_inventory_hash(inventory_full_path)
+  os_release_facts = docker_image_os_release_facts(docker_platform)
 
   docker_run_opts = ''
   unless vars.nil?
@@ -30,7 +32,7 @@ def provision(docker_platform, inventory_location, vars)
   fix_missing_tty_error_message(container_id) unless platform_is_windows?(docker_platform)
   node = { 'uri' => container_id,
            'config' => { 'transport' => 'docker', 'docker' => { 'shell-command' => @shell_command, 'connect-timeout' => 120 } },
-           'facts' => { 'provisioner' => 'docker_exp', 'container_id' => container_id, 'platform' => docker_platform } }
+           'facts' => { 'provisioner' => 'docker_exp', 'container_id' => container_id, 'platform' => docker_platform, 'os-release' => os_release_facts } }
   unless vars.nil?
     var_hash = YAML.safe_load(vars)
     node['vars'] = var_hash

From 0eb0774fcccba960e4623067897a2935790188d3 Mon Sep 17 00:00:00 2001
From: Jeffrey Clark <h0tw1r3@users.noreply.github.com>
Date: Mon, 12 Feb 2024 18:58:29 -0600
Subject: [PATCH 2/8] (maint) add docker_exec helper method

should make writing tests easier
---
 lib/docker_helper.rb |  4 +++
 tasks/docker.rb      | 67 ++++++++++++++++++++++----------------------
 2 files changed, 37 insertions(+), 34 deletions(-)

diff --git a/lib/docker_helper.rb b/lib/docker_helper.rb
index a4cf6dd7..fce964fb 100644
--- a/lib/docker_helper.rb
+++ b/lib/docker_helper.rb
@@ -2,6 +2,10 @@
 
 require 'json'
 
+def docker_exec(container, command)
+  run_local_command("docker exec #{container} #{command}")
+end
+
 def docker_image_os_release_facts(image)
   os_release_facts = {}
   begin
diff --git a/tasks/docker.rb b/tasks/docker.rb
index aa651509..410742ca 100755
--- a/tasks/docker.rb
+++ b/tasks/docker.rb
@@ -12,76 +12,75 @@ def install_ssh_components(distro, version, container)
   case distro
   when %r{debian}, %r{ubuntu}, %r{cumulus}
     warn '!!! Disabling ESM security updates for ubuntu - no access without privilege !!!'
-    run_local_command("docker exec #{container} rm -f /etc/apt/sources.list.d/ubuntu-esm-infra-trusty.list")
-    run_local_command("docker exec #{container} apt-get update")
-    run_local_command("docker exec #{container} apt-get install -y openssh-server openssh-client")
+    docker_exec(container, 'rm -f /etc/apt/sources.list.d/ubuntu-esm-infra-trusty.list')
+    docker_exec(container, 'apt-get update')
+    docker_exec(container, 'apt-get install -y openssh-server openssh-client')
   when %r{fedora}
-    run_local_command("docker exec #{container} dnf clean all")
-    run_local_command("docker exec #{container} dnf install -y sudo openssh-server openssh-clients")
-    run_local_command("docker exec #{container} ssh-keygen -A")
+    docker_exec(container, 'dnf clean all')
+    docker_exec(container, 'dnf install -y sudo openssh-server openssh-clients')
+    docker_exec(container, 'ssh-keygen -A')
   when %r{centos}, %r{^el-}, %r{eos}, %r{oracle}, %r{ol}, %r{rhel|redhat}, %r{scientific}, %r{amzn}, %r{rocky}, %r{almalinux}
     if version == '6'
       # sometimes the redhat 6 variant containers like to eat their rpmdb, leading to
       # issues with "rpmdb: unable to join the environment" errors
       # This "fix" is from https://www.srv24x7.com/criticalyum-main-error-rpmdb-open-failed/
-      run_local_command("docker exec #{container} bash -exc \"rm -f /var/lib/rpm/__db*; " \
+      docker_exec(container, 'bash -exc "rm -f /var/lib/rpm/__db*; ' \
                         'db_verify /var/lib/rpm/Packages; ' \
                         'rpm --rebuilddb; ' \
-                        'yum clean all; ' \
-                        'yum install -y sudo openssh-server openssh-clients"')
+                        'yum clean all"')
     else
       # If systemd is running for init, ensure systemd has finished starting up before proceeding:
       check_init_cmd = 'if [[ "$(readlink /proc/1/exe)" == "/usr/lib/systemd/systemd" ]]; then ' \
                        'count=0 ; while ! [[ "$(systemctl is-system-running)" =~ ^running|degraded$ && $count > 20 ]]; ' \
                        'do sleep 0.1 ; count=$((count+1)) ; done ; fi'
-      run_local_command("docker exec #{container} bash -c '#{check_init_cmd}'")
-      run_local_command("docker exec #{container} yum install -y sudo openssh-server openssh-clients")
+      docker_exec(container, "bash -c '#{check_init_cmd}'")
     end
-    ssh_folder = run_local_command("docker exec #{container} ls /etc/ssh/")
-    run_local_command("docker exec #{container} ssh-keygen -t rsa -f /etc/ssh/ssh_host_rsa_key -N \"\"") unless ssh_folder.include?('ssh_host_rsa_key')
-    run_local_command("docker exec #{container} ssh-keygen -t dsa -f /etc/ssh/ssh_host_dsa_key -N \"\"") unless ssh_folder.include?('ssh_host_dsa_key')
+    docker_exec(container, 'yum install -y sudo openssh-server openssh-clients')
+    ssh_folder = docker_exec(container, 'ls /etc/ssh/')
+    docker_exec(container, 'ssh-keygen -t rsa -f /etc/ssh/ssh_host_rsa_key -N ""') unless ssh_folder.include?('ssh_host_rsa_key')
+    docker_exec(container, 'ssh-keygen -t dsa -f /etc/ssh/ssh_host_dsa_key -N ""') unless ssh_folder.include?('ssh_host_dsa_key')
   when %r{opensuse}, %r{sles}
-    run_local_command("docker exec #{container} zypper -n in openssh")
-    run_local_command("docker exec #{container} ssh-keygen -A")
-    run_local_command("docker exec #{container} sed -ri \"s/^#?UsePAM .*/UsePAM no/\" /etc/ssh/sshd_config")
+    docker_exec(container, 'zypper -n in openssh')
+    docker_exec(container, 'ssh-keygen -A')
+    docker_exec(container, 'sed -ri "s/^#?UsePAM .*/UsePAM no/" /etc/ssh/sshd_config')
   when %r{archlinux}
-    run_local_command("docker exec #{container} pacman --noconfirm -Sy archlinux-keyring")
-    run_local_command("docker exec #{container} pacman --noconfirm -Syu")
-    run_local_command("docker exec #{container} pacman -S --noconfirm openssh")
-    run_local_command("docker exec #{container} ssh-keygen -A")
-    run_local_command("docker exec #{container} sed -ri \"s/^#?UsePAM .*/UsePAM no/\" /etc/ssh/sshd_config")
-    run_local_command("docker exec #{container} systemctl enable sshd")
+    docker_exec(container, 'pacman --noconfirm -Sy archlinux-keyring')
+    docker_exec(container, 'pacman --noconfirm -Syu')
+    docker_exec(container, 'pacman -S --noconfirm openssh')
+    docker_exec(container, 'ssh-keygen -A')
+    docker_exec(container, 'sed -ri "s/^#?UsePAM .*/UsePAM no/" /etc/ssh/sshd_config')
+    docker_exec(container, 'systemctl enable sshd')
   else
     raise "distribution #{distro} not yet supported on docker"
   end
 
   # Make sshd directory, set root password
-  run_local_command("docker exec #{container} mkdir -p /var/run/sshd")
-  run_local_command("docker exec #{container} bash -c \"echo root:root | /usr/sbin/chpasswd\"")
+  docker_exec(container, 'mkdir -p /var/run/sshd')
+  docker_exec(container, 'bash -c "echo root:root | /usr/sbin/chpasswd"')
 end
 
 def fix_ssh(distro, version, container)
-  run_local_command("docker exec #{container} sed -ri \"s/^#?PermitRootLogin .*/PermitRootLogin yes/\" /etc/ssh/sshd_config")
-  run_local_command("docker exec #{container} sed -ri \"s/^#?PasswordAuthentication .*/PasswordAuthentication yes/\" /etc/ssh/sshd_config")
-  run_local_command("docker exec #{container} sed -ri \"s/^#?UseDNS .*/UseDNS no/\" /etc/ssh/sshd_config")
-  run_local_command("docker exec #{container} sed -e \"/HostKey.*ssh_host_e.*_key/ s/^#*/#/\" -ri /etc/ssh/sshd_config")
+  docker_exec(container, 'sed -ri "s/^#?PermitRootLogin .*/PermitRootLogin yes/" /etc/ssh/sshd_config')
+  docker_exec(container, 'sed -ri "s/^#?PasswordAuthentication .*/PasswordAuthentication yes/" /etc/ssh/sshd_config')
+  docker_exec(container, 'sed -ri "s/^#?UseDNS .*/UseDNS no/" /etc/ssh/sshd_config')
+  docker_exec(container, 'sed -e "/HostKey.*ssh_host_e.*_key/ s/^#*/#/" -ri /etc/ssh/sshd_config')
   case distro
   when %r{debian}, %r{ubuntu}
-    run_local_command("docker exec #{container} service ssh restart")
+    docker_exec(container, 'service ssh restart')
   when %r{centos}, %r{^el-}, %r{eos}, %r{fedora}, %r{ol}, %r{rhel|redhat}, %r{scientific}, %r{amzn}, %r{rocky}, %r{almalinux}
     # Current RedHat/CentOs 7 packs an old version of pam, which are missing a
     # crucial patch when running unprivileged containers.  See:
     # https://bugzilla.redhat.com/show_bug.cgi?id=1728777
-    run_local_command("docker exec #{container} sed 's@session\s*required\s*pam_loginuid.so@session optional pam_loginuid.so@g' -i /etc/pam.d/sshd") \
+    docker_exec(container, 'sed "s@session\s*required\s*pam_loginuid.so@session optional pam_loginuid.so@g" -i /etc/pam.d/sshd') \
       if distro =~ %r{rhel|redhat|centos} && version =~ %r{^7}
 
     if %r{^(7|8|9|2)}.match?(version)
-      run_local_command("docker exec #{container} /usr/sbin/sshd")
+      docker_exec(container, '/usr/sbin/sshd')
     else
-      run_local_command("docker exec #{container} service sshd restart")
+      docker_exec(container, 'service sshd restart')
     end
   when %r{sles}
-    run_local_command("docker exec #{container} /usr/sbin/sshd")
+    docker_exec(container, '/usr/sbin/sshd')
   else
     raise "distribution #{distro} not yet supported on docker"
   end

From cedf8cfd8d26698cd6a0f7bfeb7f56e627c82355 Mon Sep 17 00:00:00 2001
From: Jeffrey Clark <h0tw1r3@users.noreply.github.com>
Date: Mon, 12 Feb 2024 18:10:47 -0600
Subject: [PATCH 3/8] (maint) consolidate install and fix ssh

---
 tasks/docker.rb | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/tasks/docker.rb b/tasks/docker.rb
index 410742ca..47cde8f5 100755
--- a/tasks/docker.rb
+++ b/tasks/docker.rb
@@ -57,23 +57,23 @@ def install_ssh_components(distro, version, container)
   # Make sshd directory, set root password
   docker_exec(container, 'mkdir -p /var/run/sshd')
   docker_exec(container, 'bash -c "echo root:root | /usr/sbin/chpasswd"')
-end
 
-def fix_ssh(distro, version, container)
+  # fix ssh
   docker_exec(container, 'sed -ri "s/^#?PermitRootLogin .*/PermitRootLogin yes/" /etc/ssh/sshd_config')
   docker_exec(container, 'sed -ri "s/^#?PasswordAuthentication .*/PasswordAuthentication yes/" /etc/ssh/sshd_config')
   docker_exec(container, 'sed -ri "s/^#?UseDNS .*/UseDNS no/" /etc/ssh/sshd_config')
   docker_exec(container, 'sed -e "/HostKey.*ssh_host_e.*_key/ s/^#*/#/" -ri /etc/ssh/sshd_config')
+  # Current RedHat/CentOs 7 packs an old version of pam, which are missing a
+  # crucial patch when running unprivileged containers.  See:
+  # https://bugzilla.redhat.com/show_bug.cgi?id=1728777
+  docker_exec(container, 'sed "s@session\s*required\s*pam_loginuid.so@session optional pam_loginuid.so@g" -i /etc/pam.d/sshd') \
+    if distro =~ %r{rhel|redhat|centos} && version =~ %r{^7}
+
+  # install ssh
   case distro
   when %r{debian}, %r{ubuntu}
     docker_exec(container, 'service ssh restart')
   when %r{centos}, %r{^el-}, %r{eos}, %r{fedora}, %r{ol}, %r{rhel|redhat}, %r{scientific}, %r{amzn}, %r{rocky}, %r{almalinux}
-    # Current RedHat/CentOs 7 packs an old version of pam, which are missing a
-    # crucial patch when running unprivileged containers.  See:
-    # https://bugzilla.redhat.com/show_bug.cgi?id=1728777
-    docker_exec(container, 'sed "s@session\s*required\s*pam_loginuid.so@session optional pam_loginuid.so@g" -i /etc/pam.d/sshd') \
-      if distro =~ %r{rhel|redhat|centos} && version =~ %r{^7}
-
     if %r{^(7|8|9|2)}.match?(version)
       docker_exec(container, '/usr/sbin/sshd')
     else
@@ -179,7 +179,6 @@ def provision(image, inventory_location, vars)
   node['name'] = container_id
   node['facts']['container_id'] = container_id
   install_ssh_components(distro, version, container_id)
-  fix_ssh(distro, version, container_id)
   add_node_to_group(inventory_hash, node, group_name)
   File.open(inventory_full_path, 'w') { |f| f.write inventory_hash.to_yaml }
   { status: 'ok', node_name: container_id, node: node }

From 0e8087f5c5422ec77b1caccda55903395d368321 Mon Sep 17 00:00:00 2001
From: Jeffrey Clark <h0tw1r3@users.noreply.github.com>
Date: Mon, 12 Feb 2024 19:02:40 -0600
Subject: [PATCH 4/8] (maint) move docker tear_down to helper

duplicate method
---
 lib/docker_helper.rb | 16 ++++++++++++++++
 tasks/docker.rb      | 17 +----------------
 tasks/docker_exp.rb  | 17 +----------------
 3 files changed, 18 insertions(+), 32 deletions(-)

diff --git a/lib/docker_helper.rb b/lib/docker_helper.rb
index fce964fb..d4038a48 100644
--- a/lib/docker_helper.rb
+++ b/lib/docker_helper.rb
@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require 'json'
+require 'puppet_litmus'
 
 def docker_exec(container, command)
   run_local_command("docker exec #{container} #{command}")
@@ -32,3 +33,18 @@ def docker_image_os_release_facts(image)
   end
   os_release_facts
 end
+
+def docker_tear_down(node_name, inventory_location)
+  extend PuppetLitmus::InventoryManipulation
+  inventory_full_path = File.join(inventory_location, '/spec/fixtures/litmus_inventory.yaml')
+  raise "Unable to find '#{inventory_full_path}'" unless File.file?(inventory_full_path)
+
+  inventory_hash = inventory_hash_from_inventory_file(inventory_full_path)
+  node_facts = facts_from_node(inventory_hash, node_name)
+  remove_docker = "docker rm -f #{node_facts['container_id']}"
+  run_local_command(remove_docker)
+  remove_node(inventory_hash, node_name)
+  puts "Removed #{node_name}"
+  File.open(inventory_full_path, 'w') { |f| f.write inventory_hash.to_yaml }
+  { status: 'ok' }
+end
diff --git a/tasks/docker.rb b/tasks/docker.rb
index 47cde8f5..3b7819c1 100755
--- a/tasks/docker.rb
+++ b/tasks/docker.rb
@@ -184,21 +184,6 @@ def provision(image, inventory_location, vars)
   { status: 'ok', node_name: container_id, node: node }
 end
 
-def tear_down(node_name, inventory_location)
-  include PuppetLitmus::InventoryManipulation
-  inventory_full_path = File.join(inventory_location, '/spec/fixtures/litmus_inventory.yaml')
-  raise "Unable to find '#{inventory_full_path}'" unless File.file?(inventory_full_path)
-
-  inventory_hash = inventory_hash_from_inventory_file(inventory_full_path)
-  node_facts = facts_from_node(inventory_hash, node_name)
-  remove_docker = "docker rm -f #{node_facts['container_id']}"
-  run_local_command(remove_docker)
-  remove_node(inventory_hash, node_name)
-  puts "Removed #{node_name}"
-  File.open(inventory_full_path, 'w') { |f| f.write inventory_hash.to_yaml }
-  { status: 'ok' }
-end
-
 params = JSON.parse($stdin.read)
 platform = params['platform']
 action = params['action']
@@ -221,7 +206,7 @@ def tear_down(node_name, inventory_location)
 
 begin
   result = provision(platform, inventory_location, vars) if action == 'provision'
-  result = tear_down(node_name, inventory_location) if action == 'tear_down'
+  result = docker_tear_down(node_name, inventory_location) if action == 'tear_down'
   puts result.to_json
   exit 0
 rescue StandardError => e
diff --git a/tasks/docker_exp.rb b/tasks/docker_exp.rb
index 9b2047c8..1b5afe3d 100755
--- a/tasks/docker_exp.rb
+++ b/tasks/docker_exp.rb
@@ -44,21 +44,6 @@ def provision(docker_platform, inventory_location, vars)
   { status: 'ok', node_name: container_id, node: node }
 end
 
-def tear_down(node_name, inventory_location)
-  include PuppetLitmus::InventoryManipulation
-  inventory_full_path = File.join(inventory_location, '/spec/fixtures/litmus_inventory.yaml')
-  raise "Unable to find '#{inventory_full_path}'" unless File.file?(inventory_full_path)
-
-  inventory_hash = inventory_hash_from_inventory_file(inventory_full_path)
-  node_facts = facts_from_node(inventory_hash, node_name)
-  remove_docker = "docker rm -f #{node_facts['container_id']}"
-  run_local_command(remove_docker)
-  remove_node(inventory_hash, node_name)
-  puts "Removed #{node_name}"
-  File.open(inventory_full_path, 'w') { |f| f.write inventory_hash.to_yaml }
-  { status: 'ok' }
-end
-
 params = JSON.parse($stdin.read)
 action = params['action']
 inventory_location = sanitise_inventory_location(params['inventory'])
@@ -81,7 +66,7 @@ def tear_down(node_name, inventory_location)
 
 begin
   result = provision(platform, inventory_location, vars) if action == 'provision'
-  result = tear_down(node_name, inventory_location) if action == 'tear_down'
+  result = docker_tear_down(node_name, inventory_location) if action == 'tear_down'
   puts result.to_json
   exit 0
 rescue StandardError => e

From 72a47df72129481c612e91d45c3d9a65468c350e Mon Sep 17 00:00:00 2001
From: Jeffrey Clark <h0tw1r3@users.noreply.github.com>
Date: Mon, 12 Feb 2024 20:03:33 -0600
Subject: [PATCH 5/8] (maint) reconcile docker provision methods

* tmpdir missing from docker_exp run
* docker_exp docker_run_opts optional
* use container_id as node name for both implementations
---
 tasks/docker.rb     | 48 +++++++++++++++++++++++++++++----------------
 tasks/docker_exp.rb | 47 ++++++++++++++++++++++++++++++--------------
 2 files changed, 63 insertions(+), 32 deletions(-)

diff --git a/tasks/docker.rb b/tasks/docker.rb
index 3b7819c1..bbcf5bb3 100755
--- a/tasks/docker.rb
+++ b/tasks/docker.rb
@@ -126,14 +126,15 @@ def random_ssh_forwarding_port(start_port = 52_222, end_port = 52_999)
   random_ssh_forwarding_port(new_start_port, new_end_port)
 end
 
-def provision(image, inventory_location, vars)
+def provision(docker_platform, inventory_location, vars)
   include PuppetLitmus::InventoryManipulation
   inventory_full_path = File.join(inventory_location, '/spec/fixtures/litmus_inventory.yaml')
   inventory_hash = get_inventory_hash(inventory_full_path)
-  os_release_facts = docker_image_os_release_facts(image)
+  os_release_facts = docker_image_os_release_facts(docker_platform)
   distro = os_release_facts['ID']
   version = os_release_facts['VERSION_ID']
 
+  # support for ssh to remote docker
   hostname = (ENV['DOCKER_HOST'].nil? || ENV['DOCKER_HOST'].empty?) ? 'localhost' : URI.parse(ENV.fetch('DOCKER_HOST', nil)).host || ENV.fetch('DOCKER_HOST', nil)
   begin
     # Use the current docker context to determine the docker hostname
@@ -144,44 +145,57 @@ def provision(image, inventory_location, vars)
     # old clients may not support docker context
   end
 
-  group_name = 'ssh_nodes'
   warn '!!! Using private port forwarding!!!'
   front_facing_port = random_ssh_forwarding_port
 
-  node = {
+  inventory_node = {
     'uri' => "#{hostname}:#{front_facing_port}",
+    'alias' => "#{hostname}:#{front_facing_port}",
     'config' => {
       'transport' => 'ssh',
-      'ssh' => { 'user' => 'root', 'password' => 'root', 'port' => front_facing_port, 'host-key-check' => false, 'connect-timeout' => 120 }
+      'ssh' => {
+        'user' => 'root',
+        'password' => 'root',
+        'port' => front_facing_port,
+        'host-key-check' => false,
+        'connect-timeout' => 120
+      }
     },
     'facts' => {
       'provisioner' => 'docker',
-      'platform' => image,
+      'platform' => docker_platform,
       'os-release' => os_release_facts
     }
   }
+
   docker_run_opts = ''
   unless vars.nil?
     var_hash = YAML.safe_load(vars)
-    node['vars'] = var_hash
+    inventory_node['vars'] = var_hash
     docker_run_opts = var_hash['docker_run_opts'].flatten.join(' ') unless var_hash['docker_run_opts'].nil?
   end
 
-  docker_run_opts += ' --volume /sys/fs/cgroup:/sys/fs/cgroup:rw' if (image =~ %r{debian|ubuntu}) \
-  && !docker_run_opts.include?('--volume /sys/fs/cgroup:/sys/fs/cgroup')
-  docker_run_opts += ' --cgroupns=host' if (image =~ %r{debian|ubuntu}) \
-  && !docker_run_opts.include?('--cgroupns')
+  if docker_platform.match?(%r{debian|ubuntu})
+    docker_run_opts += ' --volume /sys/fs/cgroup:/sys/fs/cgroup:rw' unless docker_run_opts.include?('--volume /sys/fs/cgroup:/sys/fs/cgroup')
+    docker_run_opts += ' --cgroupns=host' unless docker_run_opts.include?('--cgroupns')
+  end
 
-  creation_command = "docker run -d -it --privileged --tmpfs /tmp:exec -p #{front_facing_port}:22 "
+  creation_command = 'docker run -d -it --privileged --tmpfs /tmp:exec '
+  creation_command += "-p #{front_facing_port}:22 "
   creation_command += "#{docker_run_opts} " unless docker_run_opts.nil?
-  creation_command += image
+  creation_command += docker_platform
+
   container_id = run_local_command(creation_command).strip[0..11]
-  node['name'] = container_id
-  node['facts']['container_id'] = container_id
+
   install_ssh_components(distro, version, container_id)
-  add_node_to_group(inventory_hash, node, group_name)
+
+  inventory_node['name'] = container_id
+  inventory_node['facts']['container_id'] = container_id
+
+  add_node_to_group(inventory_hash, inventory_node, 'ssh_nodes')
   File.open(inventory_full_path, 'w') { |f| f.write inventory_hash.to_yaml }
-  { status: 'ok', node_name: container_id, node: node }
+
+  { status: 'ok', node_name: inventory_node['name'], node: inventory_node }
 end
 
 params = JSON.parse($stdin.read)
diff --git a/tasks/docker_exp.rb b/tasks/docker_exp.rb
index 1b5afe3d..e5cac0c1 100755
--- a/tasks/docker_exp.rb
+++ b/tasks/docker_exp.rb
@@ -16,32 +16,49 @@ def provision(docker_platform, inventory_location, vars)
   inventory_hash = get_inventory_hash(inventory_full_path)
   os_release_facts = docker_image_os_release_facts(docker_platform)
 
+  inventory_node = {
+    'config' => {
+      'transport' => 'docker',
+      'docker' => {
+        'shell-command' => @shell_command,
+        'connect-timeout' => 120
+      }
+    },
+    'facts' => {
+      'provisioner' => 'docker_exp',
+      'platform' => docker_platform,
+      'os-release' => os_release_facts,
+    }
+  }
+
   docker_run_opts = ''
   unless vars.nil?
     var_hash = YAML.safe_load(vars)
+    inventory_node['vars'] = var_hash
     docker_run_opts = var_hash['docker_run_opts'].flatten.join(' ') unless var_hash['docker_run_opts'].nil?
   end
 
-  docker_run_opts += ' --volume /sys/fs/cgroup:/sys/fs/cgroup:rw' if (docker_platform =~ %r{debian|ubuntu}) \
-  && !docker_run_opts.include?('--volume /sys/fs/cgroup:/sys/fs/cgroup')
-  docker_run_opts += ' --cgroupns=host' if (docker_platform =~ %r{debian|ubuntu}) \
-  && !docker_run_opts.include?('--cgroupns')
+  if docker_platform.match?(%r{debian|ubuntu})
+    docker_run_opts += ' --volume /sys/fs/cgroup:/sys/fs/cgroup:rw' unless docker_run_opts.include?('--volume /sys/fs/cgroup:/sys/fs/cgroup')
+    docker_run_opts += ' --cgroupns=host' unless docker_run_opts.include?('--cgroupns')
+  end
+
+  creation_command = 'docker run -d -it --privileged --tmpfs /tmp:exec '
+  creation_command += "#{docker_run_opts} " unless docker_run_opts.nil?
+  creation_command += docker_platform
 
-  creation_command = "docker run -d -it --privileged #{docker_run_opts} #{docker_platform}"
   container_id = run_local_command(creation_command).strip[0..11]
+
   fix_missing_tty_error_message(container_id) unless platform_is_windows?(docker_platform)
-  node = { 'uri' => container_id,
-           'config' => { 'transport' => 'docker', 'docker' => { 'shell-command' => @shell_command, 'connect-timeout' => 120 } },
-           'facts' => { 'provisioner' => 'docker_exp', 'container_id' => container_id, 'platform' => docker_platform, 'os-release' => os_release_facts } }
-  unless vars.nil?
-    var_hash = YAML.safe_load(vars)
-    node['vars'] = var_hash
-  end
 
-  group_name = 'docker_nodes'
-  add_node_to_group(inventory_hash, node, group_name)
+  inventory_node['name'] = container_id
+  inventory_node['uri'] = container_id
+  inventory_node['facts']['container_id'] = container_id
+
+  add_node_to_group(inventory_hash, inventory_node, 'docker_nodes')
   File.open(inventory_full_path, 'w') { |f| f.write inventory_hash.to_yaml }
-  { status: 'ok', node_name: container_id, node: node }
+
+  { status: 'ok', node_name: inventory_node['name'], node: inventory_node }
 end
 
 params = JSON.parse($stdin.read)

From 19c972f5f860aad2f6e8f84d4d33d14eed8aecbe Mon Sep 17 00:00:00 2001
From: Jeffrey Clark <h0tw1r3@users.noreply.github.com>
Date: Mon, 12 Feb 2024 20:27:33 -0600
Subject: [PATCH 6/8] (maint) cleanup docker ssh opensuse and archlinux

* dropped archlinux as it never worked
* support opensuse ssh service start
---
 tasks/docker.rb | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/tasks/docker.rb b/tasks/docker.rb
index bbcf5bb3..385243ff 100755
--- a/tasks/docker.rb
+++ b/tasks/docker.rb
@@ -43,13 +43,6 @@ def install_ssh_components(distro, version, container)
     docker_exec(container, 'zypper -n in openssh')
     docker_exec(container, 'ssh-keygen -A')
     docker_exec(container, 'sed -ri "s/^#?UsePAM .*/UsePAM no/" /etc/ssh/sshd_config')
-  when %r{archlinux}
-    docker_exec(container, 'pacman --noconfirm -Sy archlinux-keyring')
-    docker_exec(container, 'pacman --noconfirm -Syu')
-    docker_exec(container, 'pacman -S --noconfirm openssh')
-    docker_exec(container, 'ssh-keygen -A')
-    docker_exec(container, 'sed -ri "s/^#?UsePAM .*/UsePAM no/" /etc/ssh/sshd_config')
-    docker_exec(container, 'systemctl enable sshd')
   else
     raise "distribution #{distro} not yet supported on docker"
   end
@@ -79,7 +72,7 @@ def install_ssh_components(distro, version, container)
     else
       docker_exec(container, 'service sshd restart')
     end
-  when %r{sles}
+  when %r{opensuse}, %r{sles}
     docker_exec(container, '/usr/sbin/sshd')
   else
     raise "distribution #{distro} not yet supported on docker"

From 69d7968a4254a4eafab427d8af46dfec28767195 Mon Sep 17 00:00:00 2001
From: Jeffrey Clark <h0tw1r3@users.noreply.github.com>
Date: Wed, 14 Feb 2024 12:38:17 -0600
Subject: [PATCH 7/8] (maint) move docker_exp tty fix to helper

---
 lib/docker_helper.rb | 5 +++++
 lib/task_helper.rb   | 5 -----
 tasks/docker_exp.rb  | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/docker_helper.rb b/lib/docker_helper.rb
index d4038a48..d0c020ab 100644
--- a/lib/docker_helper.rb
+++ b/lib/docker_helper.rb
@@ -48,3 +48,8 @@ def docker_tear_down(node_name, inventory_location)
   File.open(inventory_full_path, 'w') { |f| f.write inventory_hash.to_yaml }
   { status: 'ok' }
 end
+
+# Workaround for fixing the bash message in stderr when tty is missing
+def docker_fix_missing_tty_error_message(container_id)
+  system("docker exec #{container_id} sed -i 's/^mesg n/tty -s \\&\\& mesg n/g' /root/.profile")
+end
diff --git a/lib/task_helper.rb b/lib/task_helper.rb
index 5d7adb44..7cc21d01 100644
--- a/lib/task_helper.rb
+++ b/lib/task_helper.rb
@@ -69,8 +69,3 @@ def token_from_fogfile(provider = 'abs')
 rescue StandardError
   puts 'Failed to get token from .fog file'
 end
-
-# Workaround for fixing the bash message in stderr when tty is missing
-def fix_missing_tty_error_message(container_id)
-  system("docker exec #{container_id} sed -i 's/^mesg n/tty -s \\&\\& mesg n/g' /root/.profile")
-end
diff --git a/tasks/docker_exp.rb b/tasks/docker_exp.rb
index e5cac0c1..4c4b0849 100755
--- a/tasks/docker_exp.rb
+++ b/tasks/docker_exp.rb
@@ -49,7 +49,7 @@ def provision(docker_platform, inventory_location, vars)
 
   container_id = run_local_command(creation_command).strip[0..11]
 
-  fix_missing_tty_error_message(container_id) unless platform_is_windows?(docker_platform)
+  docker_fix_missing_tty_error_message(container_id) unless platform_is_windows?(docker_platform)
 
   inventory_node['name'] = container_id
   inventory_node['uri'] = container_id

From 2c0ad5828b2de46dbeee16c9bd9bdb05af2c9360 Mon Sep 17 00:00:00 2001
From: Jeffrey Clark <h0tw1r3@users.noreply.github.com>
Date: Wed, 14 Feb 2024 21:20:24 -0600
Subject: [PATCH 8/8] (maint) add spec tests for docker_helper methods

---
 spec/unit/docker_helper_spec.rb | 107 ++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)
 create mode 100644 spec/unit/docker_helper_spec.rb

diff --git a/spec/unit/docker_helper_spec.rb b/spec/unit/docker_helper_spec.rb
new file mode 100644
index 00000000..51a006e2
--- /dev/null
+++ b/spec/unit/docker_helper_spec.rb
@@ -0,0 +1,107 @@
+# frozen_string_literal: true
+
+require 'docker_helper'
+require 'stringio'
+
+describe 'Docker Helper Functions' do
+  let(:container_id) { 'abc12345' }
+  let(:inventory_location) { '.' }
+  let(:full_inventory_location) { "#{inventory_location}/spec/fixtures/litmus_inventory.yaml" }
+  let(:inventory_yaml) do
+    <<-YAML
+    version: 2
+    groups:
+    - name: docker_nodes
+      targets:
+      - name: #{container_id}
+        uri: #{container_id}
+        config:
+          transport: docker
+          docker:
+            shell-command: bash -lc
+            connect-timeout: 120
+        facts:
+          provisioner: docker_exp
+          container_id: #{container_id}
+          platform: litmusimage/debian:12
+          os-release:
+            PRETTY_NAME: Debian GNU/Linux 12 (bookworm)
+            NAME: Debian GNU/Linux
+            VERSION_ID: '12'
+            VERSION: 12 (bookworm)
+            VERSION_CODENAME: bookworm
+            ID: debian
+            HOME_URL: https://www.debian.org/
+            SUPPORT_URL: https://www.debian.org/support
+            BUG_REPORT_URL: https://bugs.debian.org/
+    - name: ssh_nodes
+      targets: []
+    - name: winrm_nodes
+      targets: []
+    - name: lxd_nodes
+      targets: []
+    YAML
+  end
+
+  let(:os_release_facts) do
+    <<-FILE
+    PRETTY_NAME="Debian GNU/Linux 12 (bookworm)"
+    NAME="Debian GNU/Linux"
+    VERSION_ID="12"
+    VERSION="12 (bookworm)"
+    VERSION_CODENAME=bookworm
+    ID=debian
+    HOME_URL="https://www.debian.org/"
+    SUPPORT_URL="https://www.debian.org/support"
+    BUG_REPORT_URL="https://bugs.debian.org/"
+    FILE
+  end
+
+  describe '.docker_exec' do
+    it 'calls run_local_command' do
+      allow(self).to receive(:run_local_command).with("docker exec #{container_id} a command").and_return('some output')
+      expect(docker_exec(container_id, 'a command')).to eq('some output')
+    end
+  end
+
+  describe '.docker_image_os_release_facts' do
+    it 'returns parsed hash of /etc/os-release from container' do
+      allow(self).to receive(:run_local_command)
+        .with('docker run --rm litmusimage/debian:12 cat /etc/os-release')
+        .and_return(os_release_facts)
+      expect(docker_image_os_release_facts('litmusimage/debian:12')).to match(hash_including('PRETTY_NAME' => 'Debian GNU/Linux 12 (bookworm)'))
+    end
+
+    it 'returns minimal facts if parse fails for any reason' do
+      allow(self).to receive(:run_local_command)
+        .with('docker run --rm litmusimage/debian:12 cat /etc/os-release')
+        .and_return(StandardError)
+      expect(docker_image_os_release_facts('litmusimage/debian:12')).to match(hash_including('ID' => 'litmusimage_debian'))
+    end
+  end
+
+  describe '.docker_tear_down' do
+    it 'expect to raise error if inventory file is not found' do
+      allow(File).to receive(:file?).and_return(false)
+      expect { docker_tear_down(container_id, inventory_location) }.to raise_error(RuntimeError, "Unable to find '#{inventory_location}/spec/fixtures/litmus_inventory.yaml'")
+    end
+
+    it 'expect to return status ok' do
+      allow(File).to receive(:file?).with(full_inventory_location).and_return(true)
+      allow(File).to receive(:exist?).with(full_inventory_location).and_return(true)
+      allow(File).to receive(:open).with(full_inventory_location, anything).and_yield(StringIO.new(inventory_yaml.dup))
+      allow(self).to receive(:run_local_command).with("docker rm -f #{container_id}")
+      allow(self).to receive(:remove_node).and_return(nil)
+      expect {
+        expect(docker_tear_down(container_id, inventory_location)).to eql({ status: 'ok' })
+      }.to output("Removed #{container_id}\n").to_stdout
+    end
+  end
+
+  describe '.docker_fix_missing_tty_error_message' do
+    it 'execute command on container to disable mesg' do
+      allow(self).to receive(:system).with("docker exec #{container_id} sed -i 's/^mesg n/tty -s \\&\\& mesg n/g' /root/.profile")
+      expect(docker_fix_missing_tty_error_message(container_id)).to be_nil
+    end
+  end
+end