-
Notifications
You must be signed in to change notification settings - Fork 241
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.
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
added functionality for add/delete node(s) #166
base: master
Are you sure you want to change the base?
Changes from 7 commits
06551b1
73f3df3
be75cec
3295855
ff8c65c
b9aecc9
8e3aada
bca38e9
eb045a5
f81078e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
{ | ||
"node_label": "label", | ||
"executors": "", | ||
"template_detail": { | ||
"guest_username": "root", | ||
"guest_password": "passwd", | ||
"guest_user_credentials_id": "xxxxxxxx" | ||
}, | ||
"host_list":[ | ||
"xxxx" | ||
] | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we indent this file so it is easier to read? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,7 @@ | |
# | ||
|
||
require 'thor' | ||
require 'json' | ||
require 'thor/group' | ||
require 'terminal-table' | ||
|
||
|
@@ -58,6 +59,79 @@ def print_general_attributes | |
puts table | ||
end | ||
|
||
desc "create", "creates new node(s)" | ||
# CLI command that creates new node | ||
option :json | ||
option :name | ||
option :slave_host | ||
option :credentials_id | ||
option :private_key_file | ||
option :executors | ||
option :labels | ||
def create | ||
@client = Helper.setup(parent_options) | ||
if options[:json] | ||
json_file = "#{options[:json]}" | ||
file = File.read(json_file) | ||
node_details = JSON.parse(file) | ||
node_label = node_details['node_label'] | ||
executors = node_details['executors'] | ||
template_detail_hash = node_details['template_detail'] | ||
host_list_array = node_details['host_list'] | ||
host_list_array.each_with_index {|val, index| | ||
host_name = "#{val}" | ||
if template_detail_hash['guest_user_credentials_id'] != "" | ||
credentials_id = template_detail_hash['guest_user_credentials_id'] | ||
else | ||
puts "failed to find a credentials_id for "+template_detail_hash['guest_username']+"\nPlease add this user in jenkins and #{options[:json]}" | ||
end | ||
@client.node.create_dumb_slave( | ||
:name => host_name, | ||
:slave_host => host_name, | ||
:credentials_id => credentials_id, | ||
:private_key_file => "", | ||
:executors => executors ? (executors) : (12), | ||
:labels => node_label) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The indentation above seems to be off. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed. also fixed line 93. checking if executors is empty and then assigning 2 executors instead of 12 |
||
elsif options[:name] && options[:credentials_id] && options[:labels] | ||
@client.node.create_dumb_slave( | ||
:name => options[:name], | ||
:slave_host => options[:name], | ||
:credentials_id => options[:credentials_id], | ||
:private_key_file => "", | ||
:executors => options[:executors] ? (options[:executors]) : (12), | ||
:labels => options[:labels]) | ||
else | ||
puts "incorrect usage" | ||
|
||
end | ||
end | ||
|
||
desc "delete","deletes a given node or a list of nodes in json file" | ||
# CLI command that deletes node | ||
option :json | ||
option :node_name | ||
|
||
def delete | ||
@client = Helper.setup(parent_options) | ||
if options[:json] | ||
json_file = "#{options[:json]}" | ||
file = File.read(json_file) | ||
node_details = JSON.parse(file) | ||
template_detail_hash = node_details['template_detail'] | ||
host_list_array = node_details['host_list'] | ||
host_list_array.each_with_index {|val, index| | ||
host_name = "#{val}" | ||
@client.node.delete(host_name) | ||
} | ||
elsif options[:node_name] | ||
@client.node.delete(options[:node_name]) | ||
else | ||
puts "incorrect usage" | ||
end | ||
end | ||
|
||
|
||
desc "print_node_attributes NODE", "Prints attributes specific to a node" | ||
# CLI command to print the attributes specific to a node | ||
# | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -128,7 +128,7 @@ def to_s | |
# ) | ||
# | ||
def create_dumb_slave(params) | ||
unless params[:name] && params[:slave_host] && params[:private_key_file] | ||
unless params[:name] && params[:slave_host] && (params[:private_key_file] || params[:credentials_id]) | ||
raise ArgumentError, "Name, slave host, and private key file are" + | ||
" required for creating a slave." | ||
end | ||
|
@@ -137,7 +137,7 @@ def create_dumb_slave(params) | |
@logger.debug "Creating a dumb slave with params: #{params.inspect}" | ||
default_params = { | ||
:description => "Automatically created through jenkins_api_client", | ||
:executors => 2, | ||
:executors => 6, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's not change this default value since it may be breaking anybody relying on this default behavior. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agreed and reverted... |
||
:remote_fs => "/var/jenkins", | ||
:labels => params[:name], | ||
:slave_port => 22, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,10 @@ def prompt_for_password | |
get_from_stdin("Password: ", true) | ||
end | ||
|
||
def prompt_for_jenkins_api_token | ||
get_from_stdin("Jenkins api token for "+#{client_opts[:username]}+": ", false) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't seem to be quoted properly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sorry. fixed. |
||
end | ||
|
||
def get_from_stdin(prompt, mask = false) | ||
$stdout.write(prompt) | ||
|
||
|
@@ -45,7 +49,9 @@ def get_from_stdin(prompt, mask = false) | |
unless client_opts.has_key?(:password) or client_opts.has_key?(:password_base64) | ||
client_opts[:password] = prompt_for_password() | ||
end | ||
|
||
unless client_opts.has_key?(:jenkins_api_token) | ||
client_opts[:jenkins_api_token] = prompt_for_jenkins_api_token() | ||
end | ||
@client = JenkinsApi::Client.new(client_opts) | ||
puts "logged-in to the Jenkins API, use the '@client' variable to use the client" | ||
end | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are these settings specific to the guest user? Can't it be done by any regular user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i had a VM in mind - Host/Guest concept. when adding a guest(VM) as a node, users can relate to guest_. I've changed it to node_. also i've removed guest_password field. not needed