Skip to content

Commit

Permalink
fix: Avoid mutating config to fix concurrent execution (#279)
Browse files Browse the repository at this point in the history
* Avoid mutating config to fix concurrent execution

It seems that Kitchen does not provide separate objects in all cases when doing concurrent execution. This means that mutating config[:binds] and config[:volumes] caused inconsistent state in concurrent actions.

We now copy these before mutating to avoid this.
  • Loading branch information
PJB3005 authored Nov 25, 2023
1 parent 02e6f23 commit 116ed4a
Showing 1 changed file with 26 additions and 22 deletions.
48 changes: 26 additions & 22 deletions lib/kitchen/driver/dokken.rb
Original file line number Diff line number Diff line change
Expand Up @@ -236,22 +236,10 @@ def work_image
[image_prefix, instance_name].compact.join("/").downcase
end

def dokken_binds
ret = []
ret << "#{dokken_kitchen_sandbox}:/opt/kitchen" unless dokken_kitchen_sandbox.nil? || remote_docker_host? || running_inside_docker?
ret << "#{dokken_verifier_sandbox}:/opt/verifier" unless dokken_verifier_sandbox.nil? || remote_docker_host? || running_inside_docker?
ret << Array(config[:binds]) unless config[:binds].nil?
ret.flatten
end

def dokken_tmpfs
coerce_tmpfs(config[:tmpfs])
end

def dokken_volumes
coerce_volumes(config[:volumes])
end

def coerce_tmpfs(v)
case v
when Hash, nil
Expand All @@ -264,37 +252,53 @@ def coerce_tmpfs(v)
end
end

def coerce_volumes(v)
def dokken_volumes_from
ret = []
ret << chef_container_name
ret << data_container_name if remote_docker_host? || running_inside_docker?
ret
end

def coerce_volumes(v, binds)
case v
when PartialHash, nil
v
when Hash
PartialHash[v]
else
b = []
v = Array(v).to_a # in case v.is_A?(Chef::Node::ImmutableArray)
v.delete_if do |x|
parts = x.split(":")
b << x if parts.length > 1
end
b = nil if b.empty?
config[:binds].push(b) unless config[:binds].include?(b) || b.nil?
binds.push(b) unless binds.include?(b) || b.nil?
return PartialHash.new if v.empty?

v.each_with_object(PartialHash.new) { |volume, h| h[volume] = {} }
end
end

def dokken_volumes_from
ret = []
ret << chef_container_name
ret << data_container_name if remote_docker_host? || running_inside_docker?
ret
def calc_volumes_binds
volumes = Array.new(Array(config[:volumes]))
binds = Array.new(Array(config[:binds]))

# Binds is mutated in-place, volumes *may* be.
volumes = coerce_volumes(volumes, binds)

binds_ret = []
binds_ret << "#{dokken_kitchen_sandbox}:/opt/kitchen" unless dokken_kitchen_sandbox.nil? || remote_docker_host? || running_inside_docker?
binds_ret << "#{dokken_verifier_sandbox}:/opt/verifier" unless dokken_verifier_sandbox.nil? || remote_docker_host? || running_inside_docker?
binds_ret << binds unless binds.nil?

[volumes, binds_ret.flatten]
end

def start_runner_container(state)
debug "driver - starting #{runner_container_name}"

volumes, binds = calc_volumes_binds

config = {
"name" => runner_container_name,
"Cmd" => Shellwords.shellwords(self[:pid_one_command]),
Expand All @@ -303,11 +307,11 @@ def start_runner_container(state)
"Hostname" => self[:hostname],
"Env" => self[:env],
"ExposedPorts" => exposed_ports,
"Volumes" => dokken_volumes,
"Volumes" => volumes,
"HostConfig" => {
"Privileged" => self[:privileged],
"VolumesFrom" => dokken_volumes_from,
"Binds" => dokken_binds,
"Binds" => binds,
"Dns" => self[:dns],
"DnsSearch" => self[:dns_search],
"Links" => Array(self[:links]),
Expand Down

0 comments on commit 116ed4a

Please sign in to comment.