refactore stack_servers_persister

devops-service/spec/workers/stack_bootstrap/stack_servers_persister_spec.rb

@@ -1,15 +1,9 @@
 require 'workers/stack_bootstrap/stack_servers_persister'
 
 RSpec.describe StackServersPersister, stubbed_connector: true do
-  let(:out) { double(:out, puts: nil, flush: nil) }
+  def info_hash_for_id(id)
-  let(:run_list) { ['role[asd]'] }
-  let(:stack) { build(:stack_ec2, deploy_env: 'foo', run_list: run_list) }
-  let(:project) { build(:project, id: 'name') }
-  let(:persister) { described_class.new(stack, out) }
-  let(:provider) { instance_double(Provider::Ec2, name: 'ec2') }
-  let(:server_info_hash) do
     {
-      'id' => 'i-abcdef',
+      'id' => id,
       'name' => 'server_name',
       'key_name' => 'key',
       'private_ip' => '127.0.0.1',
@@ -21,41 +15,61 @@ RSpec.describe StackServersPersister, stubbed_connector: true do
     }
   end
 
+  let(:out) { double(:out, puts: nil, flush: nil) }
+  let(:run_list) { ['role[asd]'] }
+  let(:stack) { build(:stack_ec2, deploy_env: 'foo', run_list: run_list) }
+  let(:project) { build(:project, id: 'name') }
+  let(:persister) { described_class.new(stack, out) }
+  let(:provider) { instance_double(Provider::Ec2, name: 'ec2') }
+  let(:new_server_info) { info_hash_for_id('i-new') }
+  let(:deleted_server_info) { info_hash_for_id('i-deleted') }
+  let(:persisted_server_info) { info_hash_for_id('i-persisted') }
+
   before do
     allow(stubbed_connector).to receive(:project) { project }
     allow(stubbed_connector).to receive(:image) {
       instance_double(Devops::Model::Image, remote_user: 'user')
     }
     allow(stubbed_connector).to receive(:server_insert)
+    allow(stubbed_connector).to receive(:stack_servers) { [build(:server, id: 'i-deleted'), build(:server, id: 'i-persisted')] }
     allow(stack).to receive(:provider_instance) { provider }
-    allow(provider).to receive(:stack_servers) {[server_info_hash]}
+    allow(provider).to receive(:stack_servers) {[new_server_info, persisted_server_info]}
   end
 
-  describe '#persist' do
+  describe '#initialize' do
     it 'fetches stack servers info' do
       expect(provider).to receive(:stack_servers).with(stack)
-      persister.persist
+      described_class.new(stack, out)
     end
 
     it "doesn't raise error if cid:priority tag is absent" do
-      server_info_hash['tags'].delete('cid:priority')
+      new_server_info['tags'].delete('cid:priority')
-      expect {persister.persist}.not_to raise_error
+      expect { described_class.new(stack, out) }.not_to raise_error
     end
 
-    it 'returns hash {priority_as_integer => array of Devops::Model::Server}' do
+    it 'sets proper statuses' do
-      result = persister.persist
+      infos = described_class.new(stack, out).servers_info
-      expect(result).to be_a(Hash)
+      expect(infos.length).to eq 2
-      expect(result[3]).to be_an_array_of(Devops::Model::Server).and have_size(1)
+      expect(infos.find{|t| t[:id] == 'i-persisted'}[:state]).to eq 'persisted'
+      expect(infos.find{|t| t[:id] == 'i-new'}[:state]).to eq 'new'
     end
 
+    it 'sets deleted servers' do
+      deleted = described_class.new(stack, out).deleted
+      expect(deleted.length).to eq 1
+      expect(deleted.first.id).to eq 'i-deleted'
+    end
+  end
+
+  describe '#persist_new_servers' do
     it 'takes id, key_name, private_ip and public_ip attrs from info hash' do
       expect(stubbed_connector).to receive(:server_insert) do |server|
-        expect(server.id).to eq 'i-abcdef'
+        expect(server.id).to eq 'i-new'
         expect(server.key).to eq 'key'
         expect(server.private_ip).to eq '127.0.0.1'
         expect(server.public_ip).to eq '127.0.0.2'
       end
-      persister.persist
+      persister.persist_new_servers
     end
 
     it 'takes created_by, run_list and stack attrs from stack' do
@@ -64,30 +78,30 @@ RSpec.describe StackServersPersister, stubbed_connector: true do
         expect(server.run_list).to eq run_list
         expect(server.stack).to eq 'iamstack'
       end
-      persister.persist
+      persister.persist_new_servers
     end
 
     it 'takes remote_user from image user' do
       expect(stubbed_connector).to receive(:server_insert) do |server|
         expect(server.remote_user).to eq 'user'
       end
-      persister.persist
+      persister.persist_new_servers
     end
 
     it "takes deploy_env from project's deploy_env identifier" do
       expect(stubbed_connector).to receive(:server_insert) do |server|
         expect(server.deploy_env).to eq 'foo'
       end
-      persister.persist
+      persister.persist_new_servers
     end
 
     it "takes default provider's ssh key if info doesn't contain it" do
       allow(provider).to receive(:ssh_key) { 'default_key' }
-      server_info_hash.delete('key_name')
+      new_server_info.delete('key_name')
       expect(stubbed_connector).to receive(:server_insert) do |server|
         expect(server.key).to eq 'default_key'
       end
-      persister.persist
+      persister.persist_new_servers
     end
 
     it "sets server's run list to empty array if stack's run_list is nil" do
@@ -95,22 +109,22 @@ RSpec.describe StackServersPersister, stubbed_connector: true do
       expect(stubbed_connector).to receive(:server_insert) do |server|
         expect(server.run_list).to eq []
       end
-      persister.persist
+      persister.persist_new_servers
     end
 
     it 'build chef_node_name with default mask ":project-:env-:instanceid"' do
       expect(stubbed_connector).to receive(:server_insert) do |server|
-        expect(server.chef_node_name).to eq 'name-foo-i-abcdef'
+        expect(server.chef_node_name).to eq 'name-foo-i-new'
       end
-      persister.persist
+      persister.persist_new_servers
     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-:instancename-123'
+      new_server_info['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
+      persister.persist_new_servers
     end
 
     it "sets provider and provider account from stack" do
@@ -119,7 +133,7 @@ RSpec.describe StackServersPersister, stubbed_connector: true do
         expect(server.provider).to eq 'ec2'
         expect(server.provider_account).to eq 'foo'
       end
-      persister.persist
+      persister.persist_new_servers
     end
 
     describe 'incremented variables' do
@@ -134,9 +148,19 @@ RSpec.describe StackServersPersister, stubbed_connector: true do
         expect(stubbed_connector).to receive(:server_insert) do |server|
           expect(server.chef_node_name).to eq 'node-02-dev'
         end
-        persister.persist
+        persister.persist_new_servers
       end
     end
   end
 
+  describe '#just_persisted_by_priority' do
+    it 'returns hash {priority_as_integer => array of Devops::Model::Server}' do
+      persister.persist_new_servers
+      result = persister.just_persisted_by_priority
+      expect(result).to be_a(Hash)
+      expect(result.size).to eq 1
+      expect(result[3]).to be_an_array_of(Devops::Model::Server).and have_size(1)
+    end
+  end
+
 end

devops-service/spec/workers/stack_bootstrap_worker_spec.rb

@@ -94,7 +94,10 @@ RSpec.describe StackBootstrapWorker, type: :worker, stubbed_connector: true, ini
       allow(worker).to receive(:persist_stack_servers).and_call_original
       allow(worker).to receive(:bootstrap_servers_by_priority).and_call_original
       allow(StackServersPersister).to receive(:new) {
-        instance_double(StackServersPersister, persist: {1 => build_list(:server, 2)})
+        instance_double(StackServersPersister,
+          persist_new_servers: nil,
+          just_persisted_by_priority: {1 => build_list(:server, 2)}
+        )
       }
       allow(PrioritizedGroupsBootstrapper).to receive(:new) {
         instance_double(PrioritizedGroupsBootstrapper, bootstrap_servers_by_priority: ServersBootstrapper::Result.new(0))

devops-service/workers/stack_bootstrap/stack_servers_persister.rb

@@ -3,37 +3,72 @@ require 'workers/stack_bootstrap/chef_node_name_builder'
 # Fetches info about stack servers from provider and then persist them in mongo.
 class StackServersPersister
   include PutsAndFlush
-  attr_reader :stack, :out
+  attr_reader :stack, :out, :deleted, :servers_info
+
+  NEW = 'new'
+  DELETED = 'deleted'
+  PERSISTED = 'persisted'
+  JUST_PERSISTED = 'just_persisted'
 
   def initialize(stack, out)
     @stack, @out = stack, out
     @project = mongo.project(stack.project)
     @deploy_env = @project.deploy_env(stack.deploy_env)
-    @provider = stack.provider_instance
+    fetch_provider_servers_info
+    set_servers_states
   end
 
-  # returns: {  priority_as_integer => [Servers] }
+  def persist_new_servers
-  def persist
+    with_state(NEW).each do |info|
-    puts_and_flush 'Start saving stack servers into CID database.'
+      info[:server] = persist_stack_server(info[:provider_info])
+      info[:state] = JUST_PERSISTED
+    end
+  end
+
+  # returns: { priority_as_integer => [Servers] }
+  def just_persisted_by_priority
     stack_servers_with_priority = {}
-
+    with_state(JUST_PERSISTED).each do |info|
-    @provider.stack_servers(stack).each do |provider_info|
+      stack_servers_with_priority[info[:priority]] ||= []
-      server = persist_stack_server(provider_info)
+      stack_servers_with_priority[info[:priority]] << info[:server]
-      priority = provider_info['tags']['cid:priority'].to_i
-      stack_servers_with_priority[priority] ||= []
-      stack_servers_with_priority[priority] << server
     end
-
-    puts_and_flush "Stack servers have been saved."
-    stack_servers_with_priority.each do |priority, servers|
-      out.puts "Servers with priority '#{priority}': #{servers.map(&:id).join(", ")}"
-    end
-    out.flush
     stack_servers_with_priority
   end
 
   private
 
+  def fetch_provider_servers_info
+    @servers_info = stack.provider_instance.stack_servers(stack).map do |provider_info|
+      {
+        id: provider_info['id'],
+        provider_info: provider_info,
+        priority: provider_info['tags']['cid:priority'].to_i
+      }
+    end
+  end
+
+  def set_servers_states
+    persisted = Devops::Db.connector.stack_servers(stack.id)
+    persisted_ids = persisted.map(&:id)
+    in_cloud_ids = @servers_info.map {|info| info[:id]}
+    new_ids = in_cloud_ids - persisted_ids
+    deleted_ids = persisted_ids - in_cloud_ids
+
+    @servers_info.each do |info|
+      if new_ids.include?(info[:id])
+        info[:state] = NEW
+      else
+        info[:state] = PERSISTED
+      end
+    end
+
+    @deleted = persisted.select { |server| deleted_ids.include?(server.id) }
+  end
+
+  def with_state(state)
+    @servers_info.select { |info| info[:state] == state }
+  end
+
   # takes a hash, returns Server model
   def persist_stack_server(server_info)
     server_attrs = {
@@ -41,7 +76,7 @@ class StackServersPersister
       'chef_node_name' => get_name_builder(server_info).build_node_name!(incrementers_values),
       'created_by' => stack.owner,
       'deploy_env' => @deploy_env.identifier,
-      'key' => server_info['key_name'] || @provider.ssh_key,
+      'key' => server_info['key_name'] || stack.provider_instance.ssh_key,
       'project' => @project.id,
       'provider' => @stack.provider,
       'provider_account' => @stack.provider_account,

devops-service/workers/stack_bootstrap_worker.rb

@@ -23,7 +23,7 @@ class StackBootstrapWorker < Worker
       end
 
       begin
-        @servers_by_priorities = persist_stack_servers
+        persist_stack_servers
         return 0 if without_bootstrap
 
         bootstrap_result = bootstrap_servers_by_priority
@@ -41,10 +41,19 @@ class StackBootstrapWorker < Worker
   private
 
   def persist_stack_servers
-    StackServersPersister.new(@stack, out).persist
+    puts_and_flush 'Start saving stack servers into CID database.'
+    persister = StackServersPersister.new(@stack, out)
+    persister.persist_new_servers
+    @servers_by_priorities = persister.just_persisted_by_priority
+    puts_and_flush "Stack servers have been saved."
   end
 
   def bootstrap_servers_by_priority
+    out.puts "Bootstrapping just persisted servers"
+    @servers_by_priorities.each do |priority, servers|
+      out.puts "Servers with priority '#{priority}': #{servers.map(&:id).join(", ")}"
+    end
+    out.flush
     PrioritizedGroupsBootstrapper.new(out, jid, @servers_by_priorities).bootstrap_servers_by_priority
   end
 
@@ -73,7 +82,7 @@ class StackBootstrapWorker < Worker
       Devops::Db.connector.stack_servers_delete(stack.name)
       Devops::Db.connector.stack_delete(stack.id)
       puts_and_flush "Stack rollback has been completed"
-    rescue StandardError, Sinatra::NotFound # Sinatra::NotFound often raised in tests
+    rescue StandardError, Sinatra::NotFound # Sinatra::NotFound is often raised in tests
       puts_and_flush "Stack rollback failed"
     end
   end