From 03532e68e8ce8299148e15085aecef902fe80cfa Mon Sep 17 00:00:00 2001 From: Anton Chuchkalov Date: Tue, 1 Mar 2016 17:41:51 +0300 Subject: [PATCH 1/6] CID-418: add ability to set chef_client_options for single deploy --- devops-client/lib/devops-client/handler/deploy.rb | 3 +-- .../lib/devops-client/options/deploy_options.rb | 4 +++- devops-client/locales/en.yml | 1 + devops-service/app/api2/handlers/deploy.rb | 13 +++++++++++++ devops-service/app/api2/routes/deploy.rb | 1 + devops-service/lib/executors/server_executor.rb | 5 +++-- .../spec/executors/server_executor_spec.rb | 8 ++++---- 7 files changed, 26 insertions(+), 9 deletions(-) diff --git a/devops-client/lib/devops-client/handler/deploy.rb b/devops-client/lib/devops-client/handler/deploy.rb index 7ab707c..9cfb34d 100644 --- a/devops-client/lib/devops-client/handler/deploy.rb +++ b/devops-client/lib/devops-client/handler/deploy.rb @@ -20,13 +20,12 @@ class Deploy < Handler end def deploy_handler args - tags = options[:tags] names = args[1..-1] if names.empty? @options_parser.invalid_deploy_command abort() end - job_ids = post("/deploy", :names => names, :tags => tags) + job_ids = post("/deploy", names: names, tags: options[:tags], chef_client_options: options[:chef_client_options]) reports_urls(job_ids) end diff --git a/devops-client/lib/devops-client/options/deploy_options.rb b/devops-client/lib/devops-client/options/deploy_options.rb index abb657d..a0f45c9 100644 --- a/devops-client/lib/devops-client/options/deploy_options.rb +++ b/devops-client/lib/devops-client/options/deploy_options.rb @@ -13,10 +13,12 @@ class DeployOptions < CommonOptions def deploy_options options do |parser, options| parser.banner << self.banner + parser.resource_name = :deploy - parser.recognize_option_value(:tag, resource_name: :deploy, variable_name: 'TAG1,TAG2...') do |tags| + parser.recognize_option_value(:tag, variable: 'TAG1,TAG2...') do |tags| options[:tags] = tags.split(",") end + parser.recognize_option_value(:chef_client_options) end end diff --git a/devops-client/locales/en.yml b/devops-client/locales/en.yml index a4a968b..51341fb 100644 --- a/devops-client/locales/en.yml +++ b/devops-client/locales/en.yml @@ -314,6 +314,7 @@ en: descriptions: deploy: tag: 'Tag names, comma separated list' + chef_client_options: 'String like "-o role[foo]"' image: provider: Image provider image_id: Image identifier diff --git a/devops-service/app/api2/handlers/deploy.rb b/devops-service/app/api2/handlers/deploy.rb index b0e3045..0af3ef3 100644 --- a/devops-service/app/api2/handlers/deploy.rb +++ b/devops-service/app/api2/handlers/deploy.rb @@ -19,6 +19,7 @@ module Devops names = body["names"] tags = body["tags"] || [] run_list = body["run_list"] + single_run_chef_client_options = body['chef_client_options'] files = [] jid = nil owner = parser.current_user @@ -33,6 +34,7 @@ module Devops begin deploy_info = create_deploy_info(s, project, body["build_number"]) deploy_info["run_list"] = run_list if run_list + set_chef_client_options(deploy_info, s, project, single_run_chef_client_options) jid = Worker.start_async(DeployWorker, server_attrs: s.to_hash, @@ -117,6 +119,17 @@ module Devops @deploy_info_buf[buf_key] = project.deploy_info(deploy_env_model, build_number) end end + + private + + # env's chef client options may be nil or empty string; it's OK. + def set_chef_client_options(deploy_info, server, project, single_run_options) + if single_run_options + deploy_info['chef_client_options'] = single_run_options + else + deploy_info['chef_client_options'] = project.deploy_env(server.deploy_env).chef_client_options + end + end end end end diff --git a/devops-service/app/api2/routes/deploy.rb b/devops-service/app/api2/routes/deploy.rb index 7a2a208..c42a69b 100644 --- a/devops-service/app/api2/routes/deploy.rb +++ b/devops-service/app/api2/routes/deploy.rb @@ -18,6 +18,7 @@ module Devops # "tags": [], -> array of tags to apply on each server before running chef-client # "build_number": "", -> string, build number to deploy # "run_list": [], -> array of strings to set run_list for chef-client + # "chef_client_options": "", String, optional. May be used to redefine run_list # } # # * *Returns* : text stream diff --git a/devops-service/lib/executors/server_executor.rb b/devops-service/lib/executors/server_executor.rb index 08e4dc3..0a6e657 100644 --- a/devops-service/lib/executors/server_executor.rb +++ b/devops-service/lib/executors/server_executor.rb @@ -394,8 +394,9 @@ module Devops @out.flush cmd << " -j http://#{DevopsConfig.config[:address]}:#{DevopsConfig.config[:port]}/#{DevopsConfig.config[:url_prefix]}/v2.0/deploy/data/#{file}" else - if @deploy_env.chef_client_options && !@deploy_env.chef_client_options.empty? - cmd << " #{@deploy_env.chef_client_options}" + chef_client_options = deploy_info['chef_client_options'] + if chef_client_options && !chef_client_options.empty? + cmd << " #{chef_client_options}" else cmd << " -r #{deploy_info["run_list"].join(",")}" unless @server.stack.nil? end diff --git a/devops-service/spec/executors/server_executor_spec.rb b/devops-service/spec/executors/server_executor_spec.rb index ce8135f..d9fd37f 100644 --- a/devops-service/spec/executors/server_executor_spec.rb +++ b/devops-service/spec/executors/server_executor_spec.rb @@ -576,14 +576,14 @@ RSpec.describe Devops::Executor::ServerExecutor, type: :executor, stubbed_connec deploy_server end - it "uses deploy_env's chef_client_options if they are set" do - deploy_env.chef_client_options = '-r role' + it "uses chef_client_options from deploy_info if it is set" do + deploy_info['chef_client_options'] = '-r role' expect(stubbed_knife).to receive(:ssh_stream).with(anything, 'chef-client --no-color -r role', any_args) deploy_server end - it "doesn't use deploy_env's chef_client_options if it's blank string" do - deploy_env.chef_client_options = '' + it "doesn't use chef_client_options from deploy_info if it's blank string" do + deploy_info['chef_client_options'] = '' expect(stubbed_knife).to receive(:ssh_stream).with(anything, 'chef-client --no-color', any_args) deploy_server end From 6761c811dd506074ce69894ba4644ad3c69d7c87 Mon Sep 17 00:00:00 2001 From: Anton Chuchkalov Date: Tue, 1 Mar 2016 22:05:25 +0300 Subject: [PATCH 2/6] CID-410: implement named tasks --- .../lib/devops-client/handler/deploy.rb | 7 +- .../devops-client/options/deploy_options.rb | 2 +- devops-client/locales/en.yml | 1 + devops-service/app/api2/handlers/deploy.rb | 4 +- devops-service/app/api2/handlers/project.rb | 1 - devops-service/app/api2/parsers/deploy.rb | 1 + devops-service/app/api2/routes/project.rb | 28 ++------ devops-service/db/mongo/connectors/project.rb | 7 +- devops-service/db/mongo/models/project.rb | 15 ++++- .../field_validators/named_tasks.rb | 65 +++++++++++++++++++ .../lib/executors/server_executor.rb | 18 ++--- .../spec/executors/server_executor_spec.rb | 7 ++ devops-service/spec/models/project_spec.rb | 31 ++++++--- 13 files changed, 140 insertions(+), 47 deletions(-) create mode 100644 devops-service/db/validators/field_validators/named_tasks.rb diff --git a/devops-client/lib/devops-client/handler/deploy.rb b/devops-client/lib/devops-client/handler/deploy.rb index 9cfb34d..d8860ae 100644 --- a/devops-client/lib/devops-client/handler/deploy.rb +++ b/devops-client/lib/devops-client/handler/deploy.rb @@ -25,7 +25,12 @@ class Deploy < Handler @options_parser.invalid_deploy_command abort() end - job_ids = post("/deploy", names: names, tags: options[:tags], chef_client_options: options[:chef_client_options]) + job_ids = post("/deploy", + names: names, + tags: options[:tags], + chef_client_options: options[:chef_client_options], + named_task: options[:named_task] + ) reports_urls(job_ids) end diff --git a/devops-client/lib/devops-client/options/deploy_options.rb b/devops-client/lib/devops-client/options/deploy_options.rb index a0f45c9..aee6340 100644 --- a/devops-client/lib/devops-client/options/deploy_options.rb +++ b/devops-client/lib/devops-client/options/deploy_options.rb @@ -19,7 +19,7 @@ class DeployOptions < CommonOptions options[:tags] = tags.split(",") end parser.recognize_option_value(:chef_client_options) - + parser.recognize_option_value(:named_task) end end diff --git a/devops-client/locales/en.yml b/devops-client/locales/en.yml index 51341fb..20f390b 100644 --- a/devops-client/locales/en.yml +++ b/devops-client/locales/en.yml @@ -315,6 +315,7 @@ en: deploy: tag: 'Tag names, comma separated list' chef_client_options: 'String like "-o role[foo]"' + named_task: Name of task to run image: provider: Image provider image_id: Image identifier diff --git a/devops-service/app/api2/handlers/deploy.rb b/devops-service/app/api2/handlers/deploy.rb index 0af3ef3..71b6059 100644 --- a/devops-service/app/api2/handlers/deploy.rb +++ b/devops-service/app/api2/handlers/deploy.rb @@ -19,7 +19,6 @@ module Devops names = body["names"] tags = body["tags"] || [] run_list = body["run_list"] - single_run_chef_client_options = body['chef_client_options'] files = [] jid = nil owner = parser.current_user @@ -34,7 +33,8 @@ module Devops begin deploy_info = create_deploy_info(s, project, body["build_number"]) deploy_info["run_list"] = run_list if run_list - set_chef_client_options(deploy_info, s, project, single_run_chef_client_options) + set_chef_client_options(deploy_info, s, project, body['chef_client_options']) + deploy_info["named_task"] = body["named_task"] jid = Worker.start_async(DeployWorker, server_attrs: s.to_hash, diff --git a/devops-service/app/api2/handlers/project.rb b/devops-service/app/api2/handlers/project.rb index 5f8f918..0ccd928 100644 --- a/devops-service/app/api2/handlers/project.rb +++ b/devops-service/app/api2/handlers/project.rb @@ -5,7 +5,6 @@ require "app/api2/parsers/project" require "lib/project/type/types_factory" require "lib/executors/server_executor" require "workers/delete_server_worker" - require_relative "../helpers/version_2.rb" require_relative "request_handler" diff --git a/devops-service/app/api2/parsers/deploy.rb b/devops-service/app/api2/parsers/deploy.rb index 74a92db..084070a 100644 --- a/devops-service/app/api2/parsers/deploy.rb +++ b/devops-service/app/api2/parsers/deploy.rb @@ -12,6 +12,7 @@ module Devops build_number = check_string(r["build_number"], "Parameter 'build_number' should be a not empty string", true) rl = check_array(r["run_list"], "Parameter 'run_list' should be an array of string", String, true) Validators::Helpers::RunList.new(rl).validate! unless rl.nil? + check_string(r["named_task"], "Parameter 'named_task' should be a not empty string", true) r end diff --git a/devops-service/app/api2/routes/project.rb b/devops-service/app/api2/routes/project.rb index 62e4b12..df9faa0 100644 --- a/devops-service/app/api2/routes/project.rb +++ b/devops-service/app/api2/routes/project.rb @@ -87,28 +87,12 @@ module Devops # - Content-Type: application/json # - body : # { - # "deploy_envs": [ - # { - # "identifier": "dev", - # "provider": "openstack", - # "flavor": "m1.small", - # "image": "image id", - # "subnets": [ - # "private" - # ], - # "groups": [ - # "default" - # ], - # "users": [ - # "user" - # ], - # "run_list": [ - # - # ], - # "expires": null - # } - # ], - # "name": "project_1" + # "run_list": [], + # "description": "Description", + # "named_tasks": [{ + # "name": "restart", + # "run_list": ["role[restart_service]"] + # }] # } # # * *Returns* : diff --git a/devops-service/db/mongo/connectors/project.rb b/devops-service/db/mongo/connectors/project.rb index abe74f9..d42792f 100644 --- a/devops-service/db/mongo/connectors/project.rb +++ b/devops-service/db/mongo/connectors/project.rb @@ -135,13 +135,16 @@ module Connectors end def project_update id, params - keys = %w(run_list description) - params.delete_if{|k,v| !keys.include?(k)} + params.keep_if {|k,v| permitted_project_fields.include?(k)} @collection.update({"_id" => id}, {'$set' => params }) end private + def permitted_project_fields + %w(run_list description named_tasks) + end + def list(query={}, query_options={}) @collection.find(query, query_options).to_a.map {|bson| model_from_bson(bson)} end diff --git a/devops-service/db/mongo/models/project.rb b/devops-service/db/mongo/models/project.rb index 0bb61ac..32b71c1 100644 --- a/devops-service/db/mongo/models/project.rb +++ b/devops-service/db/mongo/models/project.rb @@ -41,12 +41,23 @@ module Devops ::Validators::FieldValidator::FieldType::Array, ::Validators::FieldValidator::RunList] + # should be an array of hashes like: + # { + # 'name' => 'restart', + # 'run_list' => [ + # 'role[restart_service]' + # ] + # } + set_field_validators :named_tasks, [::Validators::FieldValidator::Nil, + ::Validators::FieldValidator::FieldType::Array, + ::Validators::FieldValidator::NamedTasks] + set_validators ::Validators::DeployEnv::RunList, ::Validators::DeployEnv::DeployEnvs def self.fields - ["deploy_envs", "type", "description"] + ["deploy_envs", "type", "description", "named_tasks"] end def initialize p={} @@ -56,6 +67,7 @@ module Devops self.description = p["description"] self.archived = p["archived"] || false self.run_list = p["run_list"] || [] + self.named_tasks = p["named_tasks"] || [] @handler = Devops::TypesFactory.type self.type @handler.prepare(self) @@ -174,6 +186,7 @@ module Devops h["archived"] = self.archived if self.archived h["description"] = self.description h["run_list"] = self.run_list + h["named_tasks"] = self.named_tasks if self.multi? h["type"] = MULTI_TYPE end diff --git a/devops-service/db/validators/field_validators/named_tasks.rb b/devops-service/db/validators/field_validators/named_tasks.rb new file mode 100644 index 0000000..d7934bf --- /dev/null +++ b/devops-service/db/validators/field_validators/named_tasks.rb @@ -0,0 +1,65 @@ +require_relative "base" + +module Validators + module FieldValidator + class NamedTasks < Base + + def valid? + @value.each do |named_task| + break unless check_name!(named_task) + break unless check_run_list!(named_task) + break unless check_additional_keys!(named_task) + end + @message.nil? + end + + def message + @message + end + + private + + def check_name!(task) + if task.key?('name') + true + else + @message = "One of tasks doesn't have a name" + false + end + end + + def check_run_list!(task) + if !task.key?('run_list') + @message = "Task #{task['name']} doesn't have run_list" + return false + end + + if !task['run_list'].is_a?(Array) + @message = "Run list of #{task['name']} isn't an array" + return false + end + + wrong_elements = task['run_list'].select do |element| + Validators::Helpers::RunList::RUN_LIST_REGEX.match(element).nil? + end + + if wrong_elements.empty? + true + else + @message = "Invalid run list elements: '#{wrong_elements.join(', ')}' for task #{task['name']}." + false + end + end + + def check_additional_keys!(task) + if task.keys.length > 2 + @message = "Task hash should contain only name and run_list" + false + else + true + end + end + + end + end +end diff --git a/devops-service/lib/executors/server_executor.rb b/devops-service/lib/executors/server_executor.rb index 0a6e657..297e0d4 100644 --- a/devops-service/lib/executors/server_executor.rb +++ b/devops-service/lib/executors/server_executor.rb @@ -1,5 +1,6 @@ require "lib/knife/knife_factory" require "lib/executors/expiration_scheduler" +require "lib/puts_and_flush" require "hooks" require 'net/ssh' @@ -7,6 +8,7 @@ module Devops module Executor class ServerExecutor include Hooks + include PutsAndFlush ERROR_CODES = { server_bootstrap_fail: 2, @@ -41,6 +43,7 @@ module Devops attr_accessor :server, :deploy_env, :report, :project + attr_reader :out def initialize server, out, options={} if server @@ -394,9 +397,13 @@ module Devops @out.flush cmd << " -j http://#{DevopsConfig.config[:address]}:#{DevopsConfig.config[:port]}/#{DevopsConfig.config[:url_prefix]}/v2.0/deploy/data/#{file}" else - chef_client_options = deploy_info['chef_client_options'] - if chef_client_options && !chef_client_options.empty? - cmd << " #{chef_client_options}" + if deploy_info['chef_client_options'].present? + cmd << " #{deploy_info['chef_client_options']}" + elsif deploy_info['named_task'].present? + named_task = @project.named_tasks.detect {|task| task['name'] == deploy_info['named_task']} + raise "Named task #{deploy_info['named_task']} doesn't exist." unless named_task + puts_and_flush "Using named task #{deploy_info['named_task']}." + cmd << " -o #{named_task['run_list'].join(',')}" else cmd << " -r #{deploy_info["run_list"].join(",")}" unless @server.stack.nil? end @@ -504,11 +511,6 @@ module Devops private - def puts_and_flush(message) - @out.puts message - @out.flush - end - def schedule_expiration if @deploy_env.expires @out << "Planning expiration in #{@deploy_env.expires}" diff --git a/devops-service/spec/executors/server_executor_spec.rb b/devops-service/spec/executors/server_executor_spec.rb index d9fd37f..1372bd7 100644 --- a/devops-service/spec/executors/server_executor_spec.rb +++ b/devops-service/spec/executors/server_executor_spec.rb @@ -587,6 +587,13 @@ RSpec.describe Devops::Executor::ServerExecutor, type: :executor, stubbed_connec expect(stubbed_knife).to receive(:ssh_stream).with(anything, 'chef-client --no-color', any_args) deploy_server end + + it "uses run list from named_task if it's set" do + project.named_tasks = [{'name' => 'foo', 'run_list' => ['role[backend]', 'role[frontend]']}] + deploy_info['named_task'] = 'foo' + expect(stubbed_knife).to receive(:ssh_stream).with(anything, 'chef-client --no-color -o role[backend],role[frontend]', any_args) + deploy_server + end end it "uses server's key" do diff --git a/devops-service/spec/models/project_spec.rb b/devops-service/spec/models/project_spec.rb index 5fc0521..4d8a2fa 100644 --- a/devops-service/spec/models/project_spec.rb +++ b/devops-service/spec/models/project_spec.rb @@ -22,20 +22,33 @@ RSpec.describe Devops::Model::Project, type: :model do include_examples 'field type validation', :deploy_envs, :not_nil, :non_empty_array include_examples 'field type validation', :description, :maybe_nil, :maybe_empty_string include_examples 'field type validation', :run_list, :not_nil, :maybe_empty_array, :run_list + include_examples 'field type validation', :named_tasks, :maybe_nil, :maybe_empty_array it "isn't valid when has envs with same identifier" do - project = build(:project, with_deploy_env_identifiers: %w(foo foo)) - expect(project).not_to be_valid + expect(build(:project, with_deploy_env_identifiers: %w(foo foo))).not_to be_valid end it "is valid when all envs have uniq identifiers" do - project = build(:project, with_deploy_env_identifiers: %w(foo bar)) - expect(project).to be_valid + expect(build(:project, with_deploy_env_identifiers: %w(foo bar))).to be_valid end it "isn't valid when at least one of envs isn't valid" do - project = build(:project, with_deploy_env_identifiers: ['foo', nil]) - expect(project).not_to be_valid + expect(build(:project, with_deploy_env_identifiers: ['foo', nil])).not_to be_valid + end + + describe 'named_tasks validation' do + it 'is valid with array of hashes like {"name" => "foo", "run_list" => ["role[bar]"]}' do + expect(build(:project, named_tasks: [{"name" => "foo", "run_list" => ["role[bar]"]}] )).to be_valid + end + + it "isn't valid with array of hashes with invalid run_list elements" do + expect(build(:project, named_tasks: [{"name" => "foo", "run_list" => ["bar"]}] )).not_to be_valid + end + + it "isn't valid with array of hashes without name or run_list" do + expect(build(:project, named_tasks: [{run_list: ["role[bar]"]}] )).not_to be_valid + expect(build(:project, named_tasks: [{"name" => "foo"}] )).not_to be_valid + end end describe 'components validation' do @@ -58,7 +71,7 @@ RSpec.describe Devops::Model::Project, type: :model do describe '.fields' do subject { described_class.fields } - it { should eq %w(deploy_envs type description) } + it { should eq %w(deploy_envs type description named_tasks) } end describe '#initialize' do @@ -286,8 +299,8 @@ RSpec.describe Devops::Model::Project, type: :model do expect(subject['deploy_envs']).to be_an_array_of(Hash) end - it 'also contains descriptions and run_list' do - expect(subject).to include('description', 'run_list') + it 'also contains descriptions, run_list and named_tasks' do + expect(subject).to include('description', 'run_list', 'named_tasks') end it 'contains archived key if project is archived' do From 26969cb94cda9bdf7e0e45e98be1d6f9a4d26b38 Mon Sep 17 00:00:00 2001 From: Anton Chuchkalov Date: Fri, 4 Mar 2016 13:00:17 +0300 Subject: [PATCH 3/6] remove chef client options --- devops-client/lib/devops-client/handler/deploy.rb | 1 - .../lib/devops-client/options/deploy_options.rb | 1 - devops-client/locales/en.yml | 1 - devops-service/app/api2/handlers/deploy.rb | 11 ----------- devops-service/app/api2/routes/deploy.rb | 1 - .../db/mongo/models/deploy_env/deploy_env_base.rb | 7 +------ devops-service/lib/executors/server_executor.rb | 4 +--- .../spec/executors/server_executor_spec.rb | 12 ------------ .../models/deploy_env/shared_deploy_env_specs.rb | 1 - 9 files changed, 2 insertions(+), 37 deletions(-) diff --git a/devops-client/lib/devops-client/handler/deploy.rb b/devops-client/lib/devops-client/handler/deploy.rb index d8860ae..4a9c46f 100644 --- a/devops-client/lib/devops-client/handler/deploy.rb +++ b/devops-client/lib/devops-client/handler/deploy.rb @@ -28,7 +28,6 @@ class Deploy < Handler job_ids = post("/deploy", names: names, tags: options[:tags], - chef_client_options: options[:chef_client_options], named_task: options[:named_task] ) reports_urls(job_ids) diff --git a/devops-client/lib/devops-client/options/deploy_options.rb b/devops-client/lib/devops-client/options/deploy_options.rb index aee6340..27faff1 100644 --- a/devops-client/lib/devops-client/options/deploy_options.rb +++ b/devops-client/lib/devops-client/options/deploy_options.rb @@ -18,7 +18,6 @@ class DeployOptions < CommonOptions parser.recognize_option_value(:tag, variable: 'TAG1,TAG2...') do |tags| options[:tags] = tags.split(",") end - parser.recognize_option_value(:chef_client_options) parser.recognize_option_value(:named_task) end end diff --git a/devops-client/locales/en.yml b/devops-client/locales/en.yml index 20f390b..2c216c3 100644 --- a/devops-client/locales/en.yml +++ b/devops-client/locales/en.yml @@ -314,7 +314,6 @@ en: descriptions: deploy: tag: 'Tag names, comma separated list' - chef_client_options: 'String like "-o role[foo]"' named_task: Name of task to run image: provider: Image provider diff --git a/devops-service/app/api2/handlers/deploy.rb b/devops-service/app/api2/handlers/deploy.rb index 71b6059..204d35d 100644 --- a/devops-service/app/api2/handlers/deploy.rb +++ b/devops-service/app/api2/handlers/deploy.rb @@ -33,7 +33,6 @@ module Devops begin deploy_info = create_deploy_info(s, project, body["build_number"]) deploy_info["run_list"] = run_list if run_list - set_chef_client_options(deploy_info, s, project, body['chef_client_options']) deploy_info["named_task"] = body["named_task"] jid = Worker.start_async(DeployWorker, @@ -120,16 +119,6 @@ module Devops end end - private - - # env's chef client options may be nil or empty string; it's OK. - def set_chef_client_options(deploy_info, server, project, single_run_options) - if single_run_options - deploy_info['chef_client_options'] = single_run_options - else - deploy_info['chef_client_options'] = project.deploy_env(server.deploy_env).chef_client_options - end - end end end end diff --git a/devops-service/app/api2/routes/deploy.rb b/devops-service/app/api2/routes/deploy.rb index c42a69b..7a2a208 100644 --- a/devops-service/app/api2/routes/deploy.rb +++ b/devops-service/app/api2/routes/deploy.rb @@ -18,7 +18,6 @@ module Devops # "tags": [], -> array of tags to apply on each server before running chef-client # "build_number": "", -> string, build number to deploy # "run_list": [], -> array of strings to set run_list for chef-client - # "chef_client_options": "", String, optional. May be used to redefine run_list # } # # * *Returns* : text stream diff --git a/devops-service/db/mongo/models/deploy_env/deploy_env_base.rb b/devops-service/db/mongo/models/deploy_env/deploy_env_base.rb index f2885da..d95d71c 100644 --- a/devops-service/db/mongo/models/deploy_env/deploy_env_base.rb +++ b/devops-service/db/mongo/models/deploy_env/deploy_env_base.rb @@ -30,9 +30,6 @@ module Devops ::Validators::FieldValidator::FieldType::String, ::Validators::FieldValidator::Expires] - set_field_validators :chef_client_options, [::Validators::FieldValidator::Nil, - ::Validators::FieldValidator::FieldType::String] - def initialize d={} self.identifier = d["identifier"] set_provider(d) @@ -41,7 +38,6 @@ module Devops self.expires = d["expires"] b = d["users"] || [] self.users = b.uniq - self.chef_client_options = d["chef_client_options"] end def to_hash @@ -49,8 +45,7 @@ module Devops "identifier" => self.identifier, "run_list" => self.run_list, "expires" => self.expires, - "users" => self.users, - "chef_client_options" => self.chef_client_options + "users" => self.users }.merge(provider_hash) end diff --git a/devops-service/lib/executors/server_executor.rb b/devops-service/lib/executors/server_executor.rb index 297e0d4..01b2afe 100644 --- a/devops-service/lib/executors/server_executor.rb +++ b/devops-service/lib/executors/server_executor.rb @@ -397,9 +397,7 @@ module Devops @out.flush cmd << " -j http://#{DevopsConfig.config[:address]}:#{DevopsConfig.config[:port]}/#{DevopsConfig.config[:url_prefix]}/v2.0/deploy/data/#{file}" else - if deploy_info['chef_client_options'].present? - cmd << " #{deploy_info['chef_client_options']}" - elsif deploy_info['named_task'].present? + if deploy_info['named_task'].present? named_task = @project.named_tasks.detect {|task| task['name'] == deploy_info['named_task']} raise "Named task #{deploy_info['named_task']} doesn't exist." unless named_task puts_and_flush "Using named task #{deploy_info['named_task']}." diff --git a/devops-service/spec/executors/server_executor_spec.rb b/devops-service/spec/executors/server_executor_spec.rb index 1372bd7..36bac6f 100644 --- a/devops-service/spec/executors/server_executor_spec.rb +++ b/devops-service/spec/executors/server_executor_spec.rb @@ -576,18 +576,6 @@ RSpec.describe Devops::Executor::ServerExecutor, type: :executor, stubbed_connec deploy_server end - it "uses chef_client_options from deploy_info if it is set" do - deploy_info['chef_client_options'] = '-r role' - expect(stubbed_knife).to receive(:ssh_stream).with(anything, 'chef-client --no-color -r role', any_args) - deploy_server - end - - it "doesn't use chef_client_options from deploy_info if it's blank string" do - deploy_info['chef_client_options'] = '' - expect(stubbed_knife).to receive(:ssh_stream).with(anything, 'chef-client --no-color', any_args) - deploy_server - end - it "uses run list from named_task if it's set" do project.named_tasks = [{'name' => 'foo', 'run_list' => ['role[backend]', 'role[frontend]']}] deploy_info['named_task'] = 'foo' diff --git a/devops-service/spec/models/deploy_env/shared_deploy_env_specs.rb b/devops-service/spec/models/deploy_env/shared_deploy_env_specs.rb index ffdd708..538f838 100644 --- a/devops-service/spec/models/deploy_env/shared_deploy_env_specs.rb +++ b/devops-service/spec/models/deploy_env/shared_deploy_env_specs.rb @@ -10,7 +10,6 @@ RSpec.shared_examples 'deploy env' do include_examples 'field type validation', :run_list, :not_nil, :maybe_empty_array, :run_list, :field_validator include_examples 'field type validation', :users, :not_nil, :maybe_empty_array, :field_validator include_examples 'field type validation', :expires, :maybe_nil, :non_empty_string, :field_validator - include_examples 'field type validation', :chef_client_options, :maybe_nil, :maybe_empty_string, :field_validator it 'should be valid only with all users available' do expect(build(validated_model_name, users: ['root'])).to be_valid From 6ae3042456fbfb117f10d7e2b568f44c8b99f05d Mon Sep 17 00:00:00 2001 From: Anton Chuchkalov Date: Wed, 2 Mar 2016 17:00:20 +0400 Subject: [PATCH 4/6] implement increment variables implement increment variables --- .../workers/chef_node_name_builder_spec.rb | 53 +++++++------- .../workers/stack_servers_persister_spec.rb | 20 +++++- .../stack_bootstrap/chef_node_name_builder.rb | 71 +++++++++++++------ .../stack_servers_persister.rb | 25 +++++-- .../stack_bootstrap/stack_synchronizer.rb | 8 ++- 5 files changed, 121 insertions(+), 56 deletions(-) diff --git a/devops-service/spec/workers/chef_node_name_builder_spec.rb b/devops-service/spec/workers/chef_node_name_builder_spec.rb index ef14d0c..b78cec3 100644 --- a/devops-service/spec/workers/chef_node_name_builder_spec.rb +++ b/devops-service/spec/workers/chef_node_name_builder_spec.rb @@ -1,6 +1,6 @@ require 'workers/stack_bootstrap/chef_node_name_builder' RSpec.describe ChefNodeNameBuilder do - # real response + # test with real response to ensure it is processed correctly let(:server_info) do { "name"=>"stack-achuchkalov-aws-test-1455976199-master01", @@ -21,33 +21,26 @@ RSpec.describe ChefNodeNameBuilder do } } end - let(:project) { build(:project, id: 'proj', with_deploy_env_identifiers: %w(dev)) } - let(:env) { project.deploy_env('dev') } - let(:build_node_name) { - described_class.new(server_info, project, env).build_node_name + let(:node_name_builder) { + ChefNodeNameBuilder.new( + provider_server_info: server_info, + project_id: 'proj', + env_id: 'dev' + ) } + let(:build_node_name) { node_name_builder.build_node_name!({}) } def set_mask(mask) server_info['tags']['cid:node-name-mask'] = mask end describe '#build_node_name' do - it 'uses default mask ("$project-$cfname-$env")' do + it 'uses default mask (":project-:instancename-:env")' do expect(build_node_name).to eq 'proj-master01-dev' end - it 'substitutes $project, $env, $instanceid and $cfname' do - set_mask('$project/$env/$instanceid/$cfname') - expect(build_node_name).to eq 'proj/dev/i-fac32c7e/master01' - end - - it 'substitutes $time' do - set_mask('$project-$time') - expect(build_node_name).to match /proj-\d+/ - end - - it 'substitutes :project, :env, :instanceid and :cfname' do - set_mask(':project/:env/:instanceid/:cfname') + it 'substitutes :project, :env, :instanceid and :instancename' do + set_mask(':project/:env/:instanceid/:instancename') expect(build_node_name).to eq 'proj/dev/i-fac32c7e/master01' end @@ -56,14 +49,24 @@ RSpec.describe ChefNodeNameBuilder do expect(build_node_name).to match /proj-\d+/ end - it 'works with both colon and dollar variables' do - set_mask('$project/$env/:instanceid/:cfname') - expect(build_node_name).to eq 'proj/dev/i-fac32c7e/master01' + describe 'substitutes incrementers variables :increment-groupname: with numbers depending on @incrementers_values param' do + it 'starts with 01 for empty hash' do + set_mask('node-:increment-slave:') + expect(node_name_builder.build_node_name!({})).to eq 'node-01' + end + + it "continues with next values if hash isn't empty" do + set_mask('node-:increment-slave:') + expect(node_name_builder.build_node_name!({'slave' => nil})).to eq 'node-01' + expect(node_name_builder.build_node_name!({'slave' => 1})).to eq 'node-02' + expect(node_name_builder.build_node_name!({'slave' => 50})).to eq 'node-51' + end + + it 'could substitute different incrementers at once' do + set_mask('node-:increment-slave:-:increment-master:') + expect(node_name_builder.build_node_name!({'slave' => 1, 'master' => 3})).to eq 'node-02-04' + end end - it 'substitutes underscores to dashes' do - server_info['tags']['Name'] = 'server_1' - expect(build_node_name).to match 'proj-server-1-dev' - end end end \ No newline at end of file diff --git a/devops-service/spec/workers/stack_servers_persister_spec.rb b/devops-service/spec/workers/stack_servers_persister_spec.rb index 00a844e..93a7cb0 100644 --- a/devops-service/spec/workers/stack_servers_persister_spec.rb +++ b/devops-service/spec/workers/stack_servers_persister_spec.rb @@ -98,7 +98,7 @@ RSpec.describe StackServersPersister, stubbed_connector: true do persister.persist end - it 'build chef_node_name with default mask "$project-$cfname-$env"' do + it 'build chef_node_name with default mask ":project-:instancename-:env"' do expect(stubbed_connector).to receive(:server_insert) do |server| expect(server.chef_node_name).to eq 'name-server1-foo' end @@ -106,12 +106,28 @@ RSpec.describe StackServersPersister, stubbed_connector: true do end it "builds chef_node_name with custom mask if info['tags']['cid:node-name-mask'] exists" do - server_info_hash['tags']['cid:node-name-mask'] = '$project-$cfname-123' + server_info_hash['tags']['cid:node-name-mask'] = ':project-:instancename-123' expect(stubbed_connector).to receive(:server_insert) do |server| expect(server.chef_node_name).to eq 'name-server1-123' end persister.persist end + + describe 'incremented variables' do + it 'substitutes :increment-groupid: with incrementing numbers' do + allow(provider).to receive(:stack_servers) {[ + {'id' => 'server1', 'tags' => {'cid:node-name-mask' => 'node-:increment-group1:-dev'}, 'key_name' => 'key'}, + {'id' => 'server1', 'tags' => {'cid:node-name-mask' => 'node-:increment-group1:-dev'}, 'key_name' => 'key'} + ]} + expect(stubbed_connector).to receive(:server_insert) do |server| + expect(server.chef_node_name).to eq 'node-01-dev' + end.ordered + expect(stubbed_connector).to receive(:server_insert) do |server| + expect(server.chef_node_name).to eq 'node-02-dev' + end + persister.persist + end + end end end \ No newline at end of file diff --git a/devops-service/workers/stack_bootstrap/chef_node_name_builder.rb b/devops-service/workers/stack_bootstrap/chef_node_name_builder.rb index 6315696..0be1073 100644 --- a/devops-service/workers/stack_bootstrap/chef_node_name_builder.rb +++ b/devops-service/workers/stack_bootstrap/chef_node_name_builder.rb @@ -1,34 +1,63 @@ -class ChefNodeNameBuilder - DEFAULT_MASK = '$project-$cfname-$env' +# Builds node name from mask. Mask could be passed in server's +cid:node-name-mask+ tag, +# default mask is used otherwise. Mask is a string with possible several variables inserted, e.g. +# +':project/:env/:instanceid/:instancename/:increment-group1:'+ +# Variables meanings: +# - +:project+ is replaced with project name (given in constructor) +# - +:env+ is replaced with env name (given in constructor) +# - +:instanceid+ is replaced with provider instance id (fetched from server info) +# - +:instancename+ is replaced with value of Name tag (fetched from server info) +# - +:increment-groupname:+ is replaced with incremented number tied to group name. There could be several groups in one stack. +# P.S. Colons are used instead of dollar signs, because it's convinient to set nodename mask tag to AWS stack and not in template +# (you set tag once and it propagates to all instances), but stacks don't support dollar signs in tags (unlike EC2 instances). - def initialize(server_info, project, env) - @server_info, @project, @env = server_info, project, env - @mask = server_info['tags']['cid:node-name-mask'] || DEFAULT_MASK +class ChefNodeNameBuilder + DEFAULT_MASK = ':project-:instancename-:env' + + # @param attrs [Hash] should contain + # +:provider_server_info+ + # +:project_id+ + # +:env_id+ + def initialize(attrs) + @server_info = attrs[:provider_server_info] + @project, @env = attrs[:project_id], attrs[:env_id] + @mask = @server_info['tags']['cid:node-name-mask'] || DEFAULT_MASK end - def build_node_name + + # @param incrementers_values [Hash] is a hash in which key is name of a variable and value is last substituted number for that var. + # This method modifies +incrementers_values+, updating values for substituted variables. + # + # Examples (assume mask is set to +':project-master-:increment-group1:'+): + # incremeters_values = {} + # builder.build_node_name!(incremeters_values) # returns 'mpda-master-01' + # puts incremeters_values # {'group1' => 1} + # builder.build_node_name!(incremeters_values) # returns 'mpda-master-02' + # puts incremeters_values # {'group1' => 2} + def build_node_name!(incrementers_values) result = @mask.dup - replace_dollar_variables!(result) - replace_colon_variables!(result) - result.gsub!('_', '-') + replace_variables!(result) + replace_incrementers!(result, incrementers_values) result end private - def replace_dollar_variables!(result) - result.gsub!('$project', @project.id) - result.gsub!('$env', @env.identifier) - result.gsub!('$instanceid', @server_info['id']) - result.gsub!('$cfname', @server_info['tags']['Name'] || '') - result.gsub!('$time', Time.now.to_i.to_s) - end - - def replace_colon_variables!(result) - result.gsub!(':project', @project.id) - result.gsub!(':env', @env.identifier) + def replace_variables!(result) + result.gsub!(':project', @project) + result.gsub!(':env', @env) result.gsub!(':instanceid', @server_info['id']) - result.gsub!(':cfname', @server_info['tags']['Name'] || '') + result.gsub!(':instancename', @server_info['tags']['Name'] || '') result.gsub!(':time', Time.now.to_i.to_s) end + + def replace_incrementers!(result, incrementers_values) + groupname_regexp = /(?<=:increment-)\w+(?=:)/ + result.gsub!(/:increment-\w+:/) do |incrementer| + group_name = groupname_regexp.match(incrementer)[0] + prev_value = incrementers_values[group_name] || 0 + current_value = prev_value + 1 + incrementers_values[group_name] = current_value + current_value.to_s.rjust(2, '0') + end + end end \ No newline at end of file diff --git a/devops-service/workers/stack_bootstrap/stack_servers_persister.rb b/devops-service/workers/stack_bootstrap/stack_servers_persister.rb index 3a30eab..cb022f3 100644 --- a/devops-service/workers/stack_bootstrap/stack_servers_persister.rb +++ b/devops-service/workers/stack_bootstrap/stack_servers_persister.rb @@ -42,18 +42,18 @@ class StackServersPersister end # takes a hash, returns Server model - def persist_stack_server(info_hash) + def persist_stack_server(server_info) server_attrs = { - '_id' => info_hash['id'], - 'chef_node_name' => ChefNodeNameBuilder.new(info_hash, @project, @deploy_env).build_node_name, + '_id' => server_info['id'], + 'chef_node_name' => get_name_builder(server_info).build_node_name!(incrementers_values), 'created_by' => stack.owner, 'deploy_env' => @deploy_env.identifier, - 'key' => info_hash['key_name'] || @provider.ssh_key, + 'key' => server_info['key_name'] || @provider.ssh_key, 'project' => @project.id, 'provider' => @provider.name, 'remote_user' => mongo.image(@deploy_env.image).remote_user, - 'private_ip' => info_hash['private_ip'], - 'public_ip' => info_hash['public_ip'], + 'private_ip' => server_info['private_ip'], + 'public_ip' => server_info['public_ip'], 'run_list' => stack.run_list || [], 'stack' => stack.name } @@ -63,6 +63,19 @@ class StackServersPersister server end + def get_name_builder(server_info) + ChefNodeNameBuilder.new( + provider_server_info: server_info, + project_id: @project.id, + env_id: @deploy_env.identifier, + owner: stack.owner + ) + end + + def incrementers_values + @incrementers_values ||= {} + end + def mongo Devops::Db.connector end diff --git a/devops-service/workers/stack_bootstrap/stack_synchronizer.rb b/devops-service/workers/stack_bootstrap/stack_synchronizer.rb index c889f47..d420954 100644 --- a/devops-service/workers/stack_bootstrap/stack_synchronizer.rb +++ b/devops-service/workers/stack_bootstrap/stack_synchronizer.rb @@ -18,7 +18,7 @@ class StackSynchronizer stack.sync! print_new_events case stack.stack_status - when 'CREATE_IN_PROGRESS', 'ROLLBACK_IN_PROGRESS' + when 'CREATE_IN_PROGRESS', 'ROLLBACK_IN_PROGRESS', 'DELETE_IN_PROGRESS' when 'CREATE_COMPLETE' ::Devops::Db.connector.stack_update(stack) puts_and_flush "Stack '#{stack.id}' status is now #{stack.stack_status}" @@ -26,6 +26,9 @@ class StackSynchronizer when 'ROLLBACK_COMPLETE' puts_and_flush "Stack '#{stack.id}' status is rolled back" return error_code(:stack_rolled_back) + when 'DELETE_COMPLETE' + puts_and_flush "Stack '#{stack.id}' status is deleted" + return error_code(:stack_deleted) else puts_and_flush "Unknown stack status: '#{stack.stack_status}'" return error_code(:unkown_status) @@ -54,7 +57,8 @@ class StackSynchronizer stack_rolled_back: 1, unkown_status: 2, timeout: 3, - error: 5 + error: 5, + stack_deleted: 6 } end From 74d167a2e73258a700dbfdd0b0154d9a9fb9a696 Mon Sep 17 00:00:00 2001 From: Anton Chuchkalov Date: Fri, 4 Mar 2016 17:04:06 +0300 Subject: [PATCH 5/6] CID-428: add missing :time variable to docs --- .../workers/stack_bootstrap/chef_node_name_builder.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/devops-service/workers/stack_bootstrap/chef_node_name_builder.rb b/devops-service/workers/stack_bootstrap/chef_node_name_builder.rb index 0be1073..4b59781 100644 --- a/devops-service/workers/stack_bootstrap/chef_node_name_builder.rb +++ b/devops-service/workers/stack_bootstrap/chef_node_name_builder.rb @@ -4,11 +4,12 @@ # Variables meanings: # - +:project+ is replaced with project name (given in constructor) # - +:env+ is replaced with env name (given in constructor) +# - +:time+ is replaced with current timestamp # - +:instanceid+ is replaced with provider instance id (fetched from server info) # - +:instancename+ is replaced with value of Name tag (fetched from server info) # - +:increment-groupname:+ is replaced with incremented number tied to group name. There could be several groups in one stack. -# P.S. Colons are used instead of dollar signs, because it's convinient to set nodename mask tag to AWS stack and not in template -# (you set tag once and it propagates to all instances), but stacks don't support dollar signs in tags (unlike EC2 instances). +# P.S. Colons are used instead of dollar signs, because stacks don't support dollar signs in tags (unlike EC2 instances), +# but it's convenient to set mask tag directly to a stack (not in template): you set tag once and it propagates to all instances. class ChefNodeNameBuilder DEFAULT_MASK = ':project-:instancename-:env' From 02e92be1cd31744d93fa0c5143424e15b5b3e519 Mon Sep 17 00:00:00 2001 From: Anton Chuchkalov Date: Wed, 9 Mar 2016 15:44:57 +0300 Subject: [PATCH 6/6] CID-449: fix expiration worker --- .../lib/executors/expiration_scheduler.rb | 2 +- .../lib/executors/server_executor.rb | 4 ++-- .../executors/expiration_scheduler_spec.rb | 2 +- .../spec/executors/server_executor_spec.rb | 2 +- .../workers/delete_expired_server_worker.rb | 19 +++++++------------ 5 files changed, 12 insertions(+), 17 deletions(-) diff --git a/devops-service/lib/executors/expiration_scheduler.rb b/devops-service/lib/executors/expiration_scheduler.rb index d84702d..33cc2e7 100644 --- a/devops-service/lib/executors/expiration_scheduler.rb +++ b/devops-service/lib/executors/expiration_scheduler.rb @@ -9,7 +9,7 @@ module Devops def schedule_expiration! return unless @expires - DeleteExpiredServerWorker.perform_in(interval_in_seconds, server_chef_node_name: @server.chef_node_name) + DeleteExpiredServerWorker.perform_in(interval_in_seconds, server_id: @server.id) end def interval_in_seconds diff --git a/devops-service/lib/executors/server_executor.rb b/devops-service/lib/executors/server_executor.rb index 54090d4..a31d471 100644 --- a/devops-service/lib/executors/server_executor.rb +++ b/devops-service/lib/executors/server_executor.rb @@ -511,8 +511,8 @@ module Devops def schedule_expiration if @deploy_env.expires - puts_and_flush "Planning expiration in #{@deploy_env.expires}" - ExpirationScheduler.new(@deploy_env.expires, @server).schedule_expiration! + job_id = ExpirationScheduler.new(@deploy_env.expires, @server).schedule_expiration! + puts_and_flush "Planning expiration in #{@deploy_env.expires}, job_id: #{job_id}" end end diff --git a/devops-service/spec/executors/expiration_scheduler_spec.rb b/devops-service/spec/executors/expiration_scheduler_spec.rb index f8009f8..c3882aa 100644 --- a/devops-service/spec/executors/expiration_scheduler_spec.rb +++ b/devops-service/spec/executors/expiration_scheduler_spec.rb @@ -5,7 +5,7 @@ RSpec.describe Devops::Executor::ExpirationScheduler do describe '#schedule_expiration!' do it 'schedules server deleting at given time' do - expect(DeleteExpiredServerWorker).to receive(:perform_in).with(120, server_chef_node_name: 'chef_node_name') + expect(DeleteExpiredServerWorker).to receive(:perform_in).with(120, server_id: server.id) described_class.new('2m', server).schedule_expiration! end diff --git a/devops-service/spec/executors/server_executor_spec.rb b/devops-service/spec/executors/server_executor_spec.rb index 7c57f75..d3ff57a 100644 --- a/devops-service/spec/executors/server_executor_spec.rb +++ b/devops-service/spec/executors/server_executor_spec.rb @@ -140,7 +140,7 @@ RSpec.describe Devops::Executor::ServerExecutor, type: :executor, stubbed_connec it 'schedules expiration for server' do deploy_env.expires = '2m' allow(DeleteExpiredServerWorker).to receive(:perform_in) - expect(DeleteExpiredServerWorker).to receive(:perform_in).with(120, {server_chef_node_name: 'node_name'}) + expect(DeleteExpiredServerWorker).to receive(:perform_in).with(120, hash_including(:server_id)) create_server end diff --git a/devops-service/workers/delete_expired_server_worker.rb b/devops-service/workers/delete_expired_server_worker.rb index a749e73..0d2ee7f 100644 --- a/devops-service/workers/delete_expired_server_worker.rb +++ b/devops-service/workers/delete_expired_server_worker.rb @@ -6,12 +6,11 @@ require "workers/worker" class DeleteExpiredServerWorker < Worker def perform(options) - chef_node_name = options.fetch('server_chef_node_name') - - call() do |out, file| - out.puts "Expire server '#{chef_node_name}'." - server = mongo.server_by_chef_node_name(chef_node_name) - report = save_report(file, server) + call do + server_id = options.fetch('server_id') + puts_and_flush "Expire server '#{server_id}'." + server = mongo.server_by_instance_id(server_id) + report = save_report(server) e = Devops::Executor::ServerExecutor.new(server, out) e.report = report @@ -21,17 +20,13 @@ class DeleteExpiredServerWorker < Worker private - def save_report(file, server) - report = Devops::Model::Report.new( - "file" => file, - "_id" => jid, + def save_report(server) + update_report( "created_by" => 'SYSTEM', "project" => server.project, "deploy_env" => server.deploy_env, "type" => Devops::Model::Report::EXPIRE_SERVER_TYPE ) - mongo.save_report(report) - report end end