Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Allow different instance type per role #2672

Merged
merged 17 commits into from
Mar 1, 2018
Merged

Allow different instance type per role #2672

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
d3a1303
Use the compute instance type to autoscale instances
tmarballi Feb 7, 2018
8e517bc
Pass in the instance type for autoscaling compute nodes
tmarballi Feb 7, 2018
22bebce
Merge branch 'master' into allow-type-per-role
tmarballi Feb 8, 2018
6cedc8d
Merge branch 'master' into allow-type-per-role
tmarballi Feb 12, 2018
3fc1270
Fix broken unit tests for compute instance type
tmarballi Feb 12, 2018
273a4eb
Merge branch 'master' into allow-type-per-role
tmarballi Feb 15, 2018
0c5738e
Merge branch 'master' into allow-type-per-role
tmarballi Feb 16, 2018
ab443b4
Pass in instance type in the autoscaled node information.
tmarballi Feb 17, 2018
01571ce
Fix some broken unit tests
tmarballi Feb 20, 2018
a217689
Merge branch 'master' into allow-type-per-role
tmarballi Feb 20, 2018
9a3d3e6
Fix broken unit tests expected results
tmarballi Feb 20, 2018
96dba43
Allow compatibility with older AppScale tools which do not pass insta…
tmarballi Feb 21, 2018
d4b1f67
Remove debugging log line.
tmarballi Feb 21, 2018
74f4a75
Address code review comments
tmarballi Feb 22, 2018
686b120
Merge branch 'master' into allow-type-per-role
tmarballi Feb 23, 2018
c87aacf
Merge branch 'master' into allow-type-per-role
tmarballi Feb 26, 2018
9284db5
Merge branch 'master' into allow-type-per-role
tmarballi Feb 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions AppController/djinn.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1011,6 +1011,17 @@ def set_parameters(layout, options, secret)
@options['ec2_url'] = @options['EC2_URL']
end

@nodes.each { |node|
if node.jobs.include? 'compute'
if node.instance_type.nil?
@options['compute_instance_type'] = @options['instance_type']
else
@options['compute_instance_type'] = node.instance_type
end
break
end
}

'OK'
end

Expand Down
6 changes: 4 additions & 2 deletions AppController/lib/djinn_job_data.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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

Expand Down
4 changes: 3 additions & 1 deletion AppController/lib/infrastructure_manager_client.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be associated with the jobs type? that is, if job == compute (or job.include?(compute) or whatever trickery ruby uses) then do the assignment of the type?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this spawn vms method is used only for autoscaling compute nodes right? So it just needs an instance type to be specified which it can use for autoscaling. At this point, I don't believe it would need another check to determine that.

But I might need a check to see if it already has a default instance type specified outside of the ips layout, in which case it might override it. Will test that and update here.


run_result = run_instances(parameters)
Djinn.log_debug("[IM] Run instances info says [#{run_result}]")
Expand Down Expand Up @@ -214,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']
}
}

Expand Down
12 changes: 8 additions & 4 deletions AppController/test/tc_djinn.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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").
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down
48 changes: 32 additions & 16 deletions AppController/test/tc_infrastructure_manager_client.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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']
}
}

Expand All @@ -90,6 +91,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',
Expand All @@ -105,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)
Expand All @@ -126,7 +129,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',
Expand Down Expand Up @@ -165,7 +168,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']
}
}

Expand All @@ -183,6 +187,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',
Expand All @@ -199,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)
Expand All @@ -233,7 +241,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',
Expand Down Expand Up @@ -272,7 +280,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']
}
}

Expand All @@ -290,6 +299,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,
Expand All @@ -304,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)
Expand All @@ -337,7 +350,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',
Expand Down Expand Up @@ -376,7 +389,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']
}
}

Expand All @@ -393,6 +407,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',
Expand All @@ -409,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)
Expand Down