From d3a130310c7eb7cdfe8fc9f573491532e304a4b9 Mon Sep 17 00:00:00 2001 From: Tanvi Marballi Date: Tue, 6 Feb 2018 17:15:48 -0800 Subject: [PATCH 1/9] Use the compute instance type to autoscale instances --- AppController/djinn.rb | 12 ++++++++++-- AppController/lib/djinn_job_data.rb | 6 ++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/AppController/djinn.rb b/AppController/djinn.rb index 65507e9f36..d5d71aafd0 100644 --- a/AppController/djinn.rb +++ b/AppController/djinn.rb @@ -754,12 +754,13 @@ def check_layout(layout, keyname) raise AppScaleException.new(msg) end if !node['public_ip'] || !node['private_ip'] || !node['jobs'] || - !node['instance_id'] + !node['instance_id'] || !node['instance_type'] msg = "Error: node layout is missing information #{node}." Djinn.log_error(msg) raise AppScaleException.new(msg) elsif node['public_ip'].empty? || node['private_ip'].empty? || - node['jobs'].empty? || node['instance_id'].empty? + node['jobs'].empty? || node['instance_id'].empty? || + node['instance_type'].empty? msg = "Error: node layout is missing information #{node}." Djinn.log_error(msg) raise AppScaleException.new(msg) @@ -1010,6 +1011,13 @@ def set_parameters(layout, options, secret) @options['ec2_url'] = @options['EC2_URL'] end + @nodes.each { |node| + if node.jobs.include? 'compute' + @options['compute_instance_type'] = node.instance_type + break + end + } + 'OK' end diff --git a/AppController/lib/djinn_job_data.rb b/AppController/lib/djinn_job_data.rb index bfbb9ce63a..8ec20e7788 100644 --- a/AppController/lib/djinn_job_data.rb +++ b/AppController/lib/djinn_job_data.rb @@ -14,7 +14,7 @@ # costs, which may charge on an hourly basis). class DjinnJobData attr_accessor :public_ip, :private_ip, :jobs, :instance_id, :cloud, :ssh_key - attr_accessor :disk + attr_accessor :disk, :instance_type def initialize(json_data, keyname) if json_data.class != Hash @@ -39,6 +39,7 @@ def initialize(json_data, keyname) @instance_id = 'i-APPSCALE' @instance_id = json_data['instance_id'] if json_data['instance_id'] @disk = json_data['disk'] + @instance_type = json_data['instance_type'] @ssh_key = File.expand_path("/etc/appscale/keys/#{@cloud}/#{keyname}.key") end @@ -68,7 +69,8 @@ def to_hash 'instance_id' => @instance_id, 'cloud' => @cloud, 'ssh_key' => @ssh_key, - 'disk' => @disk + 'disk' => @disk, + 'instance_type' => @instance_type } end From 8e517bcacf6a06be657a051c09adbdcd24386f80 Mon Sep 17 00:00:00 2001 From: Tanvi Marballi Date: Tue, 6 Feb 2018 21:32:34 -0800 Subject: [PATCH 2/9] Pass in the instance type for autoscaling compute nodes --- AppController/lib/infrastructure_manager_client.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/AppController/lib/infrastructure_manager_client.rb b/AppController/lib/infrastructure_manager_client.rb index e70f2d69e3..f29a278bee 100644 --- a/AppController/lib/infrastructure_manager_client.rb +++ b/AppController/lib/infrastructure_manager_client.rb @@ -184,6 +184,7 @@ def spawn_vms(num_vms, options, jobs, disks) parameters['zone'] = options['zone'] parameters['region'] = options['region'] parameters['IS_VERBOSE'] = options['verbose'] + parameters['instance_type'] = options ['compute_instance_type'] run_result = run_instances(parameters) Djinn.log_debug("[IM] Run instances info says [#{run_result}]") From 3fc1270314c0b600732ca1e8b763d2086f2173e6 Mon Sep 17 00:00:00 2001 From: Tanvi Marballi Date: Mon, 12 Feb 2018 12:24:07 -0800 Subject: [PATCH 3/9] Fix broken unit tests for compute instance type --- AppController/test/tc_djinn.rb | 12 ++++++++---- .../test/tc_infrastructure_manager_client.rb | 12 ++++++++---- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/AppController/test/tc_djinn.rb b/AppController/test/tc_djinn.rb index 546efbb3cf..3779dfad3e 100644 --- a/AppController/test/tc_djinn.rb +++ b/AppController/test/tc_djinn.rb @@ -166,7 +166,8 @@ def test_set_params_w_bad_params 'private_ip' => 'private_ip', 'jobs' => ['compute', 'shadow', 'taskqueue_master', 'db_master', 'load_balancer', 'login', 'zookeeper', 'memcache'], - 'instance_id' => 'instance_id' + 'instance_id' => 'instance_id', + 'instance_type' => 'instance_type' }]) djinn = Djinn.new @@ -198,7 +199,8 @@ def test_set_params_w_good_params 'private_ip' => '1.2.3.4', 'jobs' => ['compute', 'shadow', 'taskqueue_master', 'db_master', 'load_balancer', 'login', 'zookeeper', 'memcache'], - 'instance_id' => 'instance_id' + 'instance_id' => 'instance_id', + 'instance_type' => 'instance_type' }]) flexmock(HelperFunctions).should_receive(:shell).with("ifconfig"). @@ -621,7 +623,8 @@ def test_get_property 'private_ip' => '1.2.3.4', 'jobs' => ['compute', 'shadow', 'taskqueue_master', 'db_master', 'load_balancer', 'login', 'zookeeper', 'memcache'], - 'instance_id' => 'instance_id' + 'instance_id' => 'instance_id', + 'instance_type' => 'instance_type' }]) flexmock(Djinn).should_receive(:log_run).with( "mkdir -p /opt/appscale/apps") @@ -659,7 +662,8 @@ def test_set_property 'private_ip' => '1.2.3.4', 'jobs' => ['compute', 'shadow', 'taskqueue_master', 'db_master', 'load_balancer', 'login', 'zookeeper', 'memcache'], - 'instance_id' => 'instance_id' + 'instance_id' => 'instance_id', + 'instance_type' => 'instance_type' }]) flexmock(Djinn).should_receive(:log_run).with( "mkdir -p /opt/appscale/apps") diff --git a/AppController/test/tc_infrastructure_manager_client.rb b/AppController/test/tc_infrastructure_manager_client.rb index e2dcfd9a20..66fd4fdd3e 100644 --- a/AppController/test/tc_infrastructure_manager_client.rb +++ b/AppController/test/tc_infrastructure_manager_client.rb @@ -33,7 +33,7 @@ def test_spawn_one_vm_in_ec2 'group' => 'boogroup', 'image_id' => 'booid', 'infrastructure' => 'booinfrastructure', - 'instance_type' => 'booinstancetype', + 'instance_type' => 'boocomputeinstancetype', 'keyname' => 'bookeyname', 'num_vms' => '1', 'cloud' => 'cloud1', @@ -90,6 +90,7 @@ def test_spawn_one_vm_in_ec2 'machine' => 'booid', 'infrastructure' => 'booinfrastructure', 'instance_type' => 'booinstancetype', + 'compute_instance_type' => 'boocomputeinstancetype', 'keyname' => 'bookeyname', 'ec2_access_key' => 'booaccess', 'ec2_secret_key' => 'boosecret', @@ -126,7 +127,7 @@ def test_spawn_three_vms_in_ec2 'group' => 'boogroup', 'image_id' => 'booid', 'infrastructure' => 'booinfrastructure', - 'instance_type' => 'booinstancetype', + 'instance_type' => 'boocomputeinstancetype', 'keyname' => 'bookeyname', 'num_vms' => '3', 'cloud' => 'cloud1', @@ -183,6 +184,7 @@ def test_spawn_three_vms_in_ec2 'machine' => 'booid', 'infrastructure' => 'booinfrastructure', 'instance_type' => 'booinstancetype', + 'compute_instance_type' => 'boocomputeinstancetype', 'keyname' => 'bookeyname', 'ec2_access_key' => 'booaccess', 'ec2_secret_key' => 'boosecret', @@ -233,7 +235,7 @@ def test_spawn_three_vms_in_gce 'group' => 'boogroup', 'image_id' => 'booid', 'infrastructure' => 'booinfrastructure', - 'instance_type' => 'booinstancetype', + 'instance_type' => 'boocomputeinstancetype', 'keyname' => 'bookeyname', 'num_vms' => '3', 'cloud' => 'cloud1', @@ -290,6 +292,7 @@ def test_spawn_three_vms_in_gce 'machine' => 'booid', 'infrastructure' => 'booinfrastructure', 'instance_type' => 'booinstancetype', + 'compute_instance_type' => 'boocomputeinstancetype', 'keyname' => 'bookeyname', 'ec2_access_key' => nil, 'ec2_secret_key' => nil, @@ -337,7 +340,7 @@ def test_spawn_one_vm_in_azure 'group' => nil, 'image_id' => 'booid', 'infrastructure' => 'booinfrastructure', - 'instance_type' => 'booinstancetype', + 'instance_type' => 'boocomputeinstancetype', 'keyname' => 'bookeyname', 'num_vms' => '1', 'cloud' => 'cloud1', @@ -393,6 +396,7 @@ def test_spawn_one_vm_in_azure 'infrastructure' => 'booinfrastructure', 'machine' => 'booid', 'instance_type' => 'booinstancetype', + 'compute_instance_type' => 'boocomputeinstancetype', 'keyname' => 'bookeyname', 'zone' => 'my-zone-1b', 'azure_subscription_id' => 'boosubscriptionid', From ab443b4125ffe8755e46f32861f94cb333b4219d Mon Sep 17 00:00:00 2001 From: Tanvi Marballi Date: Fri, 16 Feb 2018 16:41:54 -0800 Subject: [PATCH 4/9] Pass in instance type in the autoscaled node information. --- AppController/lib/infrastructure_manager_client.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/AppController/lib/infrastructure_manager_client.rb b/AppController/lib/infrastructure_manager_client.rb index f29a278bee..886168bbf5 100644 --- a/AppController/lib/infrastructure_manager_client.rb +++ b/AppController/lib/infrastructure_manager_client.rb @@ -215,7 +215,8 @@ def spawn_vms(num_vms, options, jobs, disks) 'private_ip' => vm_info['private_ips'][index], 'jobs' => tmp_jobs, 'instance_id' => vm_info['instance_ids'][index], - 'disk' => disks[index] + 'disk' => disks[index], + 'instance_type' => parameters['instance_type'] } } From 01571ce8f1d56679428dda918c09e4ca5ff8c89f Mon Sep 17 00:00:00 2001 From: Tanvi Marballi Date: Mon, 19 Feb 2018 20:23:11 -0800 Subject: [PATCH 5/9] Fix some broken unit tests --- .../test/tc_infrastructure_manager_client.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/AppController/test/tc_infrastructure_manager_client.rb b/AppController/test/tc_infrastructure_manager_client.rb index 66fd4fdd3e..aff3c82724 100644 --- a/AppController/test/tc_infrastructure_manager_client.rb +++ b/AppController/test/tc_infrastructure_manager_client.rb @@ -72,7 +72,8 @@ def test_spawn_one_vm_in_ec2 'vm_info' => { 'public_ips' => ['public-ip'], 'private_ips' => ['private-ip'], - 'instance_ids' => ['i-id'] + 'instance_ids' => ['i-id'], + 'instance_type' => ['boocomputeinstancetype'] } } @@ -166,7 +167,8 @@ def test_spawn_three_vms_in_ec2 'vm_info' => { 'public_ips' => ['public-ip1', 'public-ip2', 'public-ip3'], 'private_ips' => ['private-ip1', 'private-ip2', 'private-ip3'], - 'instance_ids' => ['i-id1', 'i-id2', 'i-id3'] + 'instance_ids' => ['i-id1', 'i-id2', 'i-id3'], + 'instance_type' => ['boocomputeinstancetype', 'boocomputeinstancetype', 'boocomputeinstancetype'] } } @@ -274,7 +276,8 @@ def test_spawn_three_vms_in_gce 'vm_info' => { 'public_ips' => ['public-ip1', 'public-ip2', 'public-ip3'], 'private_ips' => ['private-ip1', 'private-ip2', 'private-ip3'], - 'instance_ids' => ['i-id1', 'i-id2', 'i-id3'] + 'instance_ids' => ['i-id1', 'i-id2', 'i-id3'], + 'instance_type' => ['boocomputeinstancetype', 'boocomputeinstancetype', 'boocomputeinstancetype'] } } @@ -379,7 +382,8 @@ def test_spawn_one_vm_in_azure 'vm_info' => { 'public_ips' => ['public_ip'], 'private_ips' => ['private_ip'], - 'instance_ids' => ['i-id'] + 'instance_ids' => ['i-id'], + 'instance_type' => ['boocomputeinstancetype'] } } From 9a3d3e6ca63b0b28709c96c89d68701b886b3d4e Mon Sep 17 00:00:00 2001 From: Tanvi Marballi Date: Mon, 19 Feb 2018 22:23:05 -0800 Subject: [PATCH 6/9] Fix broken unit tests expected results --- .../test/tc_infrastructure_manager_client.rb | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/AppController/test/tc_infrastructure_manager_client.rb b/AppController/test/tc_infrastructure_manager_client.rb index aff3c82724..736ed19b15 100644 --- a/AppController/test/tc_infrastructure_manager_client.rb +++ b/AppController/test/tc_infrastructure_manager_client.rb @@ -107,7 +107,8 @@ def test_spawn_one_vm_in_ec2 "private_ip" => "private-ip", "jobs" => "open", "instance_id" => "i-id", - "disk" => nil + "disk" => nil, + "instance_type" => "boocomputeinstancetype" }] actual = imc.spawn_vms(1, options, ["open"], [nil]) assert_equal(expected, actual) @@ -203,20 +204,23 @@ def test_spawn_three_vms_in_ec2 'private_ip' => 'private-ip1', 'jobs' => 'a', 'instance_id' => 'i-id1', - 'disk' => nil + 'disk' => nil, + 'instance_type' => 'boocomputeinstancetype' }, { 'public_ip' => 'public-ip2', 'private_ip' => 'private-ip2', 'jobs' => 'b', 'instance_id' => 'i-id2', - 'disk' => nil + 'disk' => nil, + 'instance_type' => 'boocomputeinstancetype' }, { 'public_ip' => 'public-ip3', 'private_ip' => 'private-ip3', 'jobs' => 'c', 'instance_id' => 'i-id3', - 'disk' => nil + 'disk' => nil, + 'instance_type' => 'boocomputeinstancetype' }] actual = imc.spawn_vms(3, options, ["a", "b", "c"], [nil, nil, nil]) assert_equal(expected, actual) @@ -310,20 +314,23 @@ def test_spawn_three_vms_in_gce 'private_ip' => 'private-ip1', 'jobs' => 'a', 'instance_id' => 'i-id1', - 'disk' => nil + 'disk' => nil, + 'instance_type' => 'boocomputeinstancetype' }, { 'public_ip' => 'public-ip2', 'private_ip' => 'private-ip2', 'jobs' => 'b', 'instance_id' => 'i-id2', - 'disk' => nil + 'disk' => nil, + 'instance_type' => 'boocomputeinstancetype' }, { 'public_ip' => 'public-ip3', 'private_ip' => 'private-ip3', 'jobs' => 'c', 'instance_id' => 'i-id3', - 'disk' => nil + 'disk' => nil, + 'instance_type' => 'boocomputeinstancetype' }] actual = imc.spawn_vms(3, options, ["a", "b", "c"], [nil, nil, nil]) assert_equal(expected, actual) @@ -417,7 +424,8 @@ def test_spawn_one_vm_in_azure 'private_ip' => 'private_ip', 'jobs' => 'open', 'instance_id' => 'i-id', - 'disk' => nil + 'disk' => nil, + 'instance_type' => 'boocomputeinstancetype' }] actual = imc.spawn_vms(1, options, ['open'], [nil]) assert_equal(expected, actual) From 96dba436a5584f4ea7e161ad3e03b2be6612e1b2 Mon Sep 17 00:00:00 2001 From: Tanvi Marballi Date: Wed, 21 Feb 2018 13:46:14 -0800 Subject: [PATCH 7/9] Allow compatibility with older AppScale tools which do not pass instance type under roles/nodes. --- AppController/djinn.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/AppController/djinn.rb b/AppController/djinn.rb index 696772e1b7..3fea8bbef5 100644 --- a/AppController/djinn.rb +++ b/AppController/djinn.rb @@ -755,13 +755,12 @@ def check_layout(layout, keyname) raise AppScaleException.new(msg) end if !node['public_ip'] || !node['private_ip'] || !node['jobs'] || - !node['instance_id'] || !node['instance_type'] + !node['instance_id'] msg = "Error: node layout is missing information #{node}." Djinn.log_error(msg) raise AppScaleException.new(msg) elsif node['public_ip'].empty? || node['private_ip'].empty? || - node['jobs'].empty? || node['instance_id'].empty? || - node['instance_type'].empty? + node['jobs'].empty? || node['instance_id'].empty? msg = "Error: node layout is missing information #{node}." Djinn.log_error(msg) raise AppScaleException.new(msg) @@ -1012,9 +1011,14 @@ def set_parameters(layout, options, secret) @options['ec2_url'] = @options['EC2_URL'] end + Djinn.log_warn("OPTIONS #{@options}") @nodes.each { |node| if node.jobs.include? 'compute' - @options['compute_instance_type'] = node.instance_type + if not node.instance_type.nil? + @options['compute_instance_type'] = node.instance_type + break + end + @options['compute_instance_type'] = @options['instance_type'] break end } From d4b1f67435f8537cbfab74f8c4ce28d913764a8d Mon Sep 17 00:00:00 2001 From: Tanvi Marballi Date: Wed, 21 Feb 2018 13:46:59 -0800 Subject: [PATCH 8/9] Remove debugging log line. --- AppController/djinn.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/AppController/djinn.rb b/AppController/djinn.rb index 3fea8bbef5..b6ba779e97 100644 --- a/AppController/djinn.rb +++ b/AppController/djinn.rb @@ -1011,7 +1011,6 @@ def set_parameters(layout, options, secret) @options['ec2_url'] = @options['EC2_URL'] end - Djinn.log_warn("OPTIONS #{@options}") @nodes.each { |node| if node.jobs.include? 'compute' if not node.instance_type.nil? From 74f4a756c99377029f8ba479a48d65f6268b61f3 Mon Sep 17 00:00:00 2001 From: Tanvi Marballi Date: Thu, 22 Feb 2018 11:04:53 -0800 Subject: [PATCH 9/9] Address code review comments --- AppController/djinn.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/AppController/djinn.rb b/AppController/djinn.rb index b6ba779e97..0d86dda10f 100644 --- a/AppController/djinn.rb +++ b/AppController/djinn.rb @@ -1013,11 +1013,11 @@ def set_parameters(layout, options, secret) @nodes.each { |node| if node.jobs.include? 'compute' - if not node.instance_type.nil? - @options['compute_instance_type'] = node.instance_type - break + if node.instance_type.nil? + @options['compute_instance_type'] = @options['instance_type'] + else + @options['compute_instance_type'] = node.instance_type end - @options['compute_instance_type'] = @options['instance_type'] break end }