CID-472: add ability to use missed incrementer values

devops-service/db/mongo/connectors/server.rb

@@ -33,8 +33,8 @@ module Connectors
       servers_find(q, f)
     end
 
-    def stack_servers(stack_id, reserved=nil, options={})
+    def stack_servers(stack_name, reserved=nil, options={})
-      q = {'stack' => stack_id}
+      q = {'stack' => stack_name}
       q['reserved_by'] = {'$ne' => nil} unless reserved.nil?
       list(q, options)
     end

devops-service/db/mongo/models/server.rb

@@ -26,6 +26,7 @@ module Devops
 
       attr_accessor :chef_node_name, :id, :remote_user, :project, :deploy_env, :private_ip, :public_ip, :created_at, :without_bootstrap, :created_by, :reserved_by, :stack, :run_list
       attr_accessor :key, :last_operation
+      attr_accessor :stack_info # hash like {mask: '', incrementers_values: {'master' => 3}}
 
       types :id => {:type => String, :empty => false},
             :provider => {:type => String, :empty => false},
@@ -64,6 +65,7 @@ module Devops
         self.stack = s["stack"]
         self.run_list = s["run_list"] || []
         self.last_operation = s["last_operation"]
+        self.stack_info = s["stack_info"] || {}
         self
       end
 
@@ -88,7 +90,8 @@ module Devops
           "reserved_by" => self.reserved_by,
           "stack" => stack,
           "run_list" => self.run_list,
-          "last_operation" => self.last_operation
+          "last_operation" => self.last_operation,
+          "stack_info" => self.stack_info
         }.merge(provider_hash).delete_if { |k,v| v.nil? }
       end
 

devops-service/lib/executors/stack_executor/chef_node_name_builder.rb

@@ -10,6 +10,8 @@
 # - :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 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.
+#
+# You could see several examples in specs for this class.
 
 class Devops::Executor::StackExecutor
   class ChefNodeNameBuilder
@@ -17,33 +19,28 @@ class Devops::Executor::StackExecutor
     # several servers are persisting at once on stack creating at it's likely that Time.now.to_i will give similar values
     # to different servers. So (by default) we should use :instanceid.
     DEFAULT_MASK = ':project-:env-:instanceid'
+    attr_reader :mask, :incrementers_values
 
     # @param attrs [Hash] should contain
     #   :provider_server_info
     #   :project_id
     #   :env_id
+    #   :already_used_incrementers_values (hash like {'master' => [1,2,4]})
     def initialize(attrs)
-      @server_info = attrs[:provider_server_info]
+      @server_info = attrs.fetch(:provider_info)
-      @project, @env = attrs[:project_id], attrs[:env_id]
+      @project = attrs.fetch(:project_id)
+      @env = attrs.fetch(:env_id)
+      @already_used_incrementers_values = attrs.fetch(:already_used_incrementers_values)
+
       @mask = @server_info['tags']['cid:node-name-mask'] if @server_info['tags']
       @mask ||= DEFAULT_MASK
+      @incrementers_values = {}
     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_variables!(result)
-      replace_incrementers!(result, incrementers_values)
+      replace_incrementers!(result)
       result
     end
 
@@ -57,15 +54,18 @@ class Devops::Executor::StackExecutor
       result.gsub!(':time', Time.now.to_i.to_s)
     end
 
-    def replace_incrementers!(result, incrementers_values)
+    def replace_incrementers!(result)
       groupname_regexp = /(?<=:increment-)\w+(?=:)/
       result.gsub!(/:increment-\w+:/) do |incrementer|
-        group_name = groupname_regexp.match(incrementer)[0]
+        variable_name = groupname_regexp.match(incrementer)[0]
-        prev_value = incrementers_values[group_name] || 0
+        increment_variable_value(variable_name).to_s.rjust(2, '0')
-        current_value = prev_value + 1
+      end
-        incrementers_values[group_name] = current_value
+    end
-        current_value.to_s.rjust(2, '0')
+
-      end
+    def increment_variable_value(variable_name)
+      used_values = @already_used_incrementers_values[variable_name] || []
+      prev_value = used_values.sort.detect {|t| !used_values.include?(t+1)}
+      @incrementers_values[variable_name] = (prev_value || 0) + 1
     end
   end
 end

devops-service/lib/executors/stack_executor/stack_servers_persister.rb

@@ -8,12 +8,19 @@ class Devops::Executor::StackExecutor
       @stack = stack
       @project = mongo.project(stack.project)
       @deploy_env = @project.deploy_env(stack.deploy_env)
+      @already_used_incrementers_values = mongo.stack_servers(stack.name).inject({}) do |hash, server|
+        next hash unless server.stack_info['incrementers_values']
+        server.stack_info['incrementers_values'].each do |name, value|
+          hash[name] ||= []
+          hash[name] << value
+        end
+        hash
+      end
     end
 
     def persist(provider_info)
       server_attrs = {
         '_id' => provider_info['id'],
-        'chef_node_name' => get_name_builder(provider_info).build_node_name!(incrementers_values),
         'created_by' => stack.owner,
         'deploy_env' => @deploy_env.identifier,
         'key' => provider_info['key_name'] || stack.provider_instance.ssh_key,
@@ -27,6 +34,8 @@ class Devops::Executor::StackExecutor
         'stack' => stack.name
       }
 
+      apply_name_builder(server_attrs, provider_info)
+
       server = ::Devops::Model::Server.new(server_attrs)
       mongo.server_insert(server)
       # here custom insert method is used and it doesn't return server model
@@ -35,17 +44,31 @@ class Devops::Executor::StackExecutor
 
     private
 
-    def get_name_builder(provider_info)
+    def apply_name_builder(server_attrs, provider_info)
-      ChefNodeNameBuilder.new(
+      name_builder = get_name_builder(provider_info)
-        provider_server_info: provider_info,
+      server_attrs['chef_node_name'] = name_builder.build_node_name
-        project_id: @project.id,
+      server_attrs['stack_info'] = {
-        env_id: @deploy_env.identifier,
+        'mask' => name_builder.mask,
-        owner: stack.owner
+        'incrementers_values' => name_builder.incrementers_values
-      )
+      }
+      update_increment_variables(name_builder.incrementers_values)
     end
 
-    def incrementers_values
+    def update_increment_variables(just_added_incrementer_values)
-      @incrementers_values ||= {}
+      just_added_incrementer_values.each do |incrementer_name, value|
+        @already_used_incrementers_values[incrementer_name] ||= []
+        @already_used_incrementers_values[incrementer_name] << value
+      end
+    end
+
+    def get_name_builder(provider_info)
+      ChefNodeNameBuilder.new(
+        provider_info: provider_info,
+        project_id: @project.id,
+        env_id: @deploy_env.identifier,
+        owner: stack.owner,
+        already_used_incrementers_values: @already_used_incrementers_values
+      )
     end
 
     def mongo

devops-service/spec/executors/stack_executor/chef_node_name_builder_spec.rb

@@ -22,19 +22,23 @@ class Devops::Executor::StackExecutor
         }
       }
     end
-    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
 
+    def builder_with_incrementers(already_used_incrementers_values)
+      ChefNodeNameBuilder.new(
+        provider_info: server_info,
+        project_id: 'proj',
+        env_id: 'dev',
+        already_used_incrementers_values: already_used_incrementers_values
+      )
+    end
+
+    let(:build_node_name) { builder_with_incrementers({}).build_node_name }
+
+
     describe '#build_node_name' do
       it 'uses default mask (":project-:env-:instanceid")' do
         expect(build_node_name).to eq 'proj-dev-i-fac32c7e'
@@ -53,19 +57,24 @@ class Devops::Executor::StackExecutor
       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'
+          expect(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( builder_with_incrementers('slave' => []).build_node_name ).to eq 'node-01'
-          expect(node_name_builder.build_node_name!({'slave' => 1})).to eq 'node-02'
+          expect( builder_with_incrementers('slave' => [1]).build_node_name ).to eq 'node-02'
-          expect(node_name_builder.build_node_name!({'slave' => 50})).to eq 'node-51'
+          expect( builder_with_incrementers('slave' => [50]).build_node_name ).to eq 'node-51'
+        end
+
+        it 'uses missed value' do
+          set_mask('node-:increment-slave:')
+          expect( builder_with_incrementers('slave' => [1,2,4]).build_node_name ).to eq 'node-03'
         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'
+          expect( builder_with_incrementers('slave' => [1], 'master' => [3]).build_node_name ).to eq 'node-02-04'
         end
       end
 

devops-service/spec/executors/stack_executor/stack_servers_persister_spec.rb

@@ -28,6 +28,7 @@ class Devops::Executor::StackExecutor
         instance_double(Devops::Model::Image, remote_user: 'user')
       }
       allow(stubbed_connector).to receive(:server_insert) {|server| server}
+      allow(stubbed_connector).to receive(:stack_servers) { [] }
       allow(stack).to receive(:provider_instance) { provider }
     end
 
@@ -95,6 +96,14 @@ class Devops::Executor::StackExecutor
           expect(persister.persist(provider_info).chef_node_name).to eq 'node-01-dev'
           expect(persister.persist(provider_info).chef_node_name).to eq 'node-02-dev'
         end
+
+        it 'considers already persisted servers' do
+          server1 = build(:server, 'stack_info' => {'incrementers_values' => {'master' => 1}})
+          server3 = build(:server, 'stack_info' => {'incrementers_values' => {'master' => 3}})
+          allow(stubbed_connector).to receive(:stack_servers) { [server1, server3] }
+          provider_info['tags']['cid:node-name-mask'] = 'node-:increment-master:-dev'
+          expect(persister.persist(provider_info).chef_node_name).to eq 'node-02-dev'
+        end
       end
     end
   end

devops-service/spec/models/server_spec.rb

@@ -54,9 +54,9 @@ RSpec.describe Devops::Model::Server, type: :model do
 
   it '#to_hash_without_id returns not nil fields' do
     server = described_class.new('run_list' => [], 'project' =>  'asd')
-    expect(server.to_hash_without_id.keys).to match_array(%w(run_list project))
+    expect(server.to_hash_without_id.keys).to match_array(%w(run_list project stack_info))
     server.stack = 'stack_id'
-    expect(server.to_hash_without_id.keys).to match_array(%w(run_list project stack))
+    expect(server.to_hash_without_id.keys).to match_array(%w(run_list project stack stack_info))
   end
 
   describe '#info' do