Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added timeout and docker file directory #22

Merged
merged 5 commits into from
Nov 7, 2013
Merged

Added timeout and docker file directory #22

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fb22d9f
Added timeout argument to container and image providers that gets pas…
Oct 2, 2013
828a725
Changed timeout resource attribute to be cmd_timeout to prevent colli…
Oct 2, 2013
c850a96
Added option to perform a docker build by passing the directory that …
Oct 8, 2013
568ffcc
Added missing close of if block
Oct 8, 2013
21e7a2d
Updated timeout value to have a default instead of using ||. Updated …
Oct 21, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions providers/container.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

def load_current_resource
@current_resource = Chef::Resource::DockerContainer.new(new_resource)
dps = shell_out("docker ps -a -notrunc")
dps = shell_out("docker ps -a -notrunc", :timeout => new_resource.cmd_timeout || 60)
dps.stdout.each_line do |dps_line|
next unless dps_line.include?(new_resource.image) && dps_line.include?(new_resource.command)
container_ps = dps_line.split(%r{\s\s+})
Expand Down Expand Up @@ -62,7 +62,7 @@ def load_current_resource
run_args += " -privileged" if new_resource.privileged
run_args += " -u #{new_resource.user}" if new_resource.user
run_args += " -v #{new_resource.volume}" if new_resource.volume
dr = shell_out("docker run #{run_args} #{new_resource.image} #{new_resource.command}")
dr = shell_out("docker run #{run_args} #{new_resource.image} #{new_resource.command}", :timeout => new_resource.cmd_timeout || 60)
new_resource.id(dr.stdout.chomp)
new_resource.updated_by_last_action(true)
end
Expand All @@ -79,21 +79,21 @@ def load_current_resource
end

def remove
shell_out("docker rm #{current_resource.id}")
shell_out("docker rm #{current_resource.id}", :timeout => new_resource.cmd_timeout || 60)
end

def restart
shell_out("docker restart #{current_resource.id}")
shell_out("docker restart #{current_resource.id}", :timeout => new_resource.cmd_timeout || 60)
end

def running?
@current_resource.running
end

def start
shell_out("docker start #{current_resource.id}")
shell_out("docker start #{current_resource.id}", :timeout => new_resource.cmd_timeout || 60)
end

def stop
shell_out("docker stop #{current_resource.id}")
shell_out("docker stop #{current_resource.id}", :timeout => new_resource.cmd_timeout || 60)
end
13 changes: 9 additions & 4 deletions providers/image.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

def load_current_resource
@current_resource = Chef::Resource::DockerImage.new(new_resource)
di = shell_out("docker images -a")
di = shell_out("docker images -a", :timeout => new_resource.cmd_timeout || 60)
if di.stdout.include?(new_resource.image_name)
di.stdout.each_line do |di_line|
next unless di_line.include?(new_resource.image_name)
Expand All @@ -42,7 +42,7 @@ def load_current_resource
pull_args = ""
pull_args += " -registry #{new_resource.registry}" if new_resource.registry
pull_args += " -t #{new_resource.tag}" if new_resource.tag
shell_out("docker pull #{new_resource.image_name} #{pull_args}")
shell_out("docker pull #{new_resource.image_name} #{pull_args}", :timeout => new_resource.cmd_timeout || 60)
new_resource.updated_by_last_action(true)
end
end
Expand All @@ -51,7 +51,12 @@ def load_current_resource
unless installed?
full_image_name = new_resource.image_name
full_image_name += ":#{new_resource.tag}" if new_resource.tag
shell_out("docker build -t #{full_image_name} - < #{new_resource.dockerfile}")
if new_resource.dockerfile
command = "- < #{new_resource.dockerfile}"
elsif new_resource.dockerfile_directory
command = "#{new_resource.dockerfile_directory}"
end
shell_out("docker build -t #{full_image_name} #{command}", :timeout => new_resource.cmd_timeout || 60)
new_resource.updated_by_last_action(true)
end
end
Expand All @@ -62,7 +67,7 @@ def load_current_resource
end

def remove
shell_out("docker rmi #{new_resource.image_name}")
shell_out("docker rmi #{new_resource.image_name}", :timeout => new_resource.cmd_timeout || 60)
end

def installed?
Expand Down
1 change: 1 addition & 0 deletions resources/container.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,4 @@
attribute :privileged, :kind_of => [TrueClass, FalseClass]
attribute :user, :kind_of => [String]
attribute :volume, :kind_of => [String]
attribute :cmd_timeout, :kind_of => [Integer]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we not just set the :default => 60 here rather than all the || 60 for each shell_out? Also is there a specific reason for 60 seconds versus any other arbitrary value?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as image: Should this just be called timeout instead? While the current implementation is shell_out, I think the feature should be abstracted away.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm adding in the :default => 60. The reason for choosing 60 is because it is the default value for the shell_out command, so I figured it would be reasonable to maintain that. As far as the timeout vs cmd_timeout, I had tried that at first but it was causing issues because it was overridden by a built in chef attribute.

2 changes: 2 additions & 0 deletions resources/image.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,5 @@
attribute :repository, :kind_of => [String]
attribute :tag, :kind_of => [String]
attribute :dockerfile, :kind_of => [String]
attribute :cmd_timeout, :kind_of => [Integer]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above: Can we not just set the :default => 60 here rather than all the || 60 for each shell_out? Also is there a specific reason for 60 seconds versus any other arbitrary value?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this just be called timeout instead? While the current implementation is shell_out, I think the feature should be abstracted away.

attribute :dockerfile_directory, :kind_of => [String]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docker documentation refers to this type of build as PATH.
http://docs.docker.io/en/latest/commandline/command/build/

Can we change the attribute to path instead?