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.

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

Migration tests for KVM/Xen #79

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion features/controller_node.feature
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,3 @@ Feature: Controller node

@system
Scenario: Essential system requirements

28 changes: 28 additions & 0 deletions features/migration.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
@migration
Feature: Instance migration
As administrator
I want to make sure that Instance migration is working correctly

Background:
Given At least 2 Compute nodes must be enabled
And Image is available

@kvm
Scenario: Test KVM migration
Given At least 2 KVM Hypervisors are available

Choose a reason for hiding this comment

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

Let's make it more explicit we expect having 2 nodes, e.g. At least 2 nodes with KVM hypervisors

Copy link
Author

Choose a reason for hiding this comment

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

ok

When I create an KVM instance
And Instance is running
And I migrate KVM instance
Then I expect the instance will run on different host
And will be in state "ACTIVE"
And I turn off the instance(so it will not use resources)

Choose a reason for hiding this comment

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

Parenthesis should be avoided in feature files, usually it works by adding short sentence e.g. to save resources.

Copy link
Author

Choose a reason for hiding this comment

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

ok


@xen
Scenario: Test Xen migrationnd I turn off the instance(so it will not use resources)

Choose a reason for hiding this comment

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

All following after word migration doesn't seem to belong there

Copy link
Author

Choose a reason for hiding this comment

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

ok

Given At least 2 Xen Hypervisors are available
When I create an Xen instance
And Instance is running
And I migrate Xen instance
Then I expect the instance will run on different host
And will be in state "ACTIVE"
And I turn off the instance(so it will not use resources)

Choose a reason for hiding this comment

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

Parenthesis

Copy link
Author

Choose a reason for hiding this comment

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

ok

15 changes: 15 additions & 0 deletions features/step_definitions/migration/background_steps.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
Given(/^At least (\d+) Compute nodes must be enabled$/) do |arg1|

Choose a reason for hiding this comment

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

Usually you want to give some real name to parameters in blocks to recognize later what kind of object you are expecting, in this case it would be e.g. number_of_nodes instead of arg1

Copy link
Author

Choose a reason for hiding this comment

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

ok

# get hypervisors and count Xen's ones - for migration we need 2
enabled_host_list = control_node.exec!("openstack host list -f value --zone nova").output

Choose a reason for hiding this comment

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

You could handle the output directly here by output.split("\n").size and get the number of nodes right there...

expect(enabled_host_list.each_line(separator=$/).to_a.count).to be >= arg1.to_i

Choose a reason for hiding this comment

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

... and bring the variable in it's final form into the expectation line.

end

Given(/^Image is available$/) do
# check image
image_name = "jeos"
images = control_node.openstack.image.list
images.each do |i|
@image_id = i.id if i.name == image_name

Choose a reason for hiding this comment

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

The @image_id is not used outside this step, so the instance variable is not needed here. Also, querying the image id could be done by find:

image = images.find {|img| img.name == image_name}

And to test existence of the image, you don't need id value here or later.

Copy link
Author

Choose a reason for hiding this comment

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

ok

end
expect(@image_id).not_to be_empty
end
128 changes: 128 additions & 0 deletions features/step_definitions/migration/migration_steps.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
Given(/^At least (\d+) Xen Hypervisors are available$/) do |arg1|

Choose a reason for hiding this comment

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

Use of default param name

Copy link
Author

Choose a reason for hiding this comment

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

ok

# get hypervisors and count Xen's ones - for migration at least 2
hashypervisor_xen = 0
@hypervisors = control_node.openstack.hypervisor.list
expect(@hypervisors).not_to be_empty

@hypervisors.each do |h|
hypervisor = control_node.openstack.hypervisor.show(h.id)
hashypervisor_xen = (hashypervisor_xen + 1) if hypervisor.hypervisor_type == "Xen"
end

Choose a reason for hiding this comment

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

In Ruby you have nice methods for handling collections, you could use select here:

xen_nodes = @hypervisors.select do |hv|  
  control_node.openstack.hypervisor.show(hv.id).hypervisor_type == "Xen"
end
expect(xen_node.size).to be >= arg1.to_i

Copy link
Author

Choose a reason for hiding this comment

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

cool...ok

expect(hashypervisor_xen).to be >= arg1.to_i
end

Given(/^At least (\d+) KVM Hypervisors are available$/) do |arg1|
# get hypervisors and count KVM's ones - for migration at least 2
hashypervisor_kvm = 0
@hypervisors = control_node.openstack.hypervisor.list
expect(@hypervisors).not_to be_empty

@hypervisors.each do |h|
hypervisor = control_node.openstack.hypervisor.show(h.id)
hashypervisor_kvm = (hashypervisor_kvm + 1) if hypervisor.hypervisor_type == "QEMU"
end

Choose a reason for hiding this comment

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

Similar as for Xen nodes.

Copy link
Author

Choose a reason for hiding this comment

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

ok

expect(hashypervisor_kvm).to be >= arg1.to_i
end

When(/^I create an KVM instance$/) do
def delete_old_instances(server_id)
puts("old instance #{server_id } found - deleting")
control_node.exec!("openstack server delete #{server_id}")
end

Choose a reason for hiding this comment

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

Would be better to move this method into step helpers https://github.com/SUSE-Cloud/cct/blob/master/features/support/step_helpers.rb , then you don't need to define it again in the context of the next step.

Copy link
Author

Choose a reason for hiding this comment

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

yes, makes sense... ok

## TODO add --key-name zkubala

Choose a reason for hiding this comment

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

Redundant comment

Copy link
Author

Choose a reason for hiding this comment

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

ok

# clean old instances
@servers = control_node.openstack.server.list
@instance_name = "kvm_mig_instance"
@servers.each do |s|
s.name == @instance_name and delete_old_instances(s.id)
end
# create a new instance
new_instance = control_node.exec!("openstack server create -f shell --flavor m1.smaller --image jeos --security-group default -c id #{@instance_name}")

Choose a reason for hiding this comment

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

This line is a bit too long, splitting it into several lines usually works:

command = "openstack server create -f shell --flavor m1.smaller --image jeos " +
         "--security-group default -c id #{@instance_name}"

Copy link
Author

Choose a reason for hiding this comment

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

ok

@new_instance_id = new_instance.output[4, 36]

Choose a reason for hiding this comment

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

Is this needed? The output only should contain the server id (looking at -c id from the command above).

Copy link
Author

Choose a reason for hiding this comment

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

it shows id="6499c426-1c57-4516-934e-c5e7460f675a"
I didn't dig deeper in exec! but if there is a option to get it directly it shouldn't be needed.
Maybe sth like 'new_instance.id'

Copy link
Author

Choose a reason for hiding this comment

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

ok so -f value returns only ID.. that could simplify it.

Copy link
Author

Choose a reason for hiding this comment

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

At the end .split used since -f value will return \n which broke other commands. ok

puts ("New instance id: #{@new_instance_id}")
end

When(/^I create an Xen instance$/) do
def delete_old_instances(server_id)
puts("old instance #{server_id } found - deleting")
control_node.exec!("openstack server delete #{server_id}")
end
## TODO add --key-name zkubala
# clean old instances
@servers = control_node.openstack.server.list
@instance_name = "xen_mig_instance"
@servers.each do |s|
s.name == @instance_name and delete_old_instances(s.id)
end
# create a new instance
new_instance = control_node.exec!("openstack server create -f shell --flavor m1.smaller --image jeos --security-group default -c id #{@instance_name}")

Choose a reason for hiding this comment

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

Also a line too long.

Copy link
Author

Choose a reason for hiding this comment

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

ok

@new_instance_id = new_instance.output[4, 36]
puts ("New instance id: #{@new_instance_id}")
end

When(/^Instance is running$/) do
## TODO Raise an expection if in ERROR state
# it may take some time before the instance turns active
wait_for "Checking that instance status is active", max: "120 seconds", sleep: "4 seconds" do
@instance_show = control_node.openstack.server.show(@new_instance_id)
break if @instance_show.status == "ACTIVE"
end
end

When(/^I migrate Xen instance$/) do
# get actual host of the instance and migrate it
@instance_host = @instance_show.send("os-ext-srv-attr:host")
puts "Instance running on: #{@instance_host}"
instance_migrate = control_node.exec!("openstack server migrate --shared-migration #{@new_instance_id}")

# it may take some time before the instance turns verify_resize
wait_for "Checking that instance status is verify_resize", max: "300 seconds", sleep: "10 seconds" do
instance_show = control_node.openstack.server.show(@new_instance_id)
if instance_show.status == "VERIFY_RESIZE"
puts "Migration finished, confirming..."
control_node.exec!("openstack server resize --confirm #{@new_instance_id}")
break
end
end
end

When(/^I migrate KVM instance$/) do
# get actual host of the instance and migrate it
@instance_host = @instance_show.send("os-ext-srv-attr:host")
puts "Instance running on: #{@instance_host}"
instance_migrate = control_node.exec!("openstack server migrate #{@new_instance_id}")

# it may take some time before the instance turns verify_resize
wait_for "Checking that instance status is verify_resize", max: "300 seconds", sleep: "10 seconds" do
instance_show = control_node.openstack.server.show(@new_instance_id)
if instance_show.status == "VERIFY_RESIZE"
puts "Migration finished, confirming..."
control_node.exec!("openstack server resize --confirm #{@new_instance_id}")
break
end
end
end

Choose a reason for hiding this comment

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

This step has exact same implementation as the one for Xen, it should work for both to parametrize the hypervisor name in the step line in the feature file, e.g. When(/^I migrate "KVM" instance$/) do .


Then(/^I expect the instance will run on different host$/) do
instance_show = control_node.openstack.server.show(@new_instance_id)
instance_host = instance_show.send("os-ext-srv-attr:host")
puts "Instance running on: #{instance_host}"
expect(@instance_host).not_to eq(instance_host)

end

Then(/^will be in state "([^"]*)"$/) do |arg1|
wait_for "Checking that instance status is ACTIVE", max: "40 seconds", sleep: "10 seconds" do
instance_show = control_node.openstack.server.show(@new_instance_id)
break if instance_show.status == "ACTIVE"
end
end

Then(/^I turn off the instance\(so it will not use resources\)$/) do
control_node.exec!("openstack server stop #{@new_instance_id}")
wait_for "Checking that instance status is SHUTOFF", max: "120 seconds", sleep: "10 seconds" do
instance_show = control_node.openstack.server.show(@new_instance_id)
break if instance_show.status == "SHUTOFF"
end
end

10 changes: 8 additions & 2 deletions lib/cct/commands/openstack.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ class Client
attr_reader :project
attr_reader :network
attr_reader :role
attr_reader :hypervisor
attr_reader :server

# @param [Cct::Node] as the receiver for the openstack client
def initialize node
Expand All @@ -30,6 +32,8 @@ def initialize node
@project = Openstack::Project.new(node)
@network = Openstack::Network.new(node)
@role = Openstack::Role.new(node)
@hypervisor = Openstack::Hypervisor.new(node)
@server= Openstack::Server.new(node)
end

def actions
Expand Down Expand Up @@ -112,9 +116,9 @@ def list *options
end
end

def show id_or_name
def show id_or_name, *options
params.clear
OpenStruct.new(shell_parse(exec!("show", id_or_name, "--format=shell").output))
OpenStruct.new(shell_parse(exec!("show", id_or_name, options, "--format=shell").output))
end

def exist? id_or_name
Expand Down Expand Up @@ -222,4 +226,6 @@ def filter_params type
require 'cct/commands/openstack/project'
require 'cct/commands/openstack/network'
require 'cct/commands/openstack/role'
require 'cct/commands/openstack/hypervisor'
require 'cct/commands/openstack/server'

12 changes: 12 additions & 0 deletions lib/cct/commands/openstack/hypervisor.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
module Cct
module Commands
module Openstack
class Hypervisor < Command
self.command = ["hypervisor"]
def list *options
super(*(options << {row: Struct.new(:id, :"Hypervisor Hostname")}))
end
end
end
end
end
9 changes: 9 additions & 0 deletions lib/cct/commands/openstack/server.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module Cct
module Commands
module Openstack
class Server < Command
self.command = ["server"]
end
end
end
end
10 changes: 10 additions & 0 deletions tasks/features.rake
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,18 @@ namespace :features do
invoke_task "feature:users"
invoke_task "feature:images"
invoke_task "features:barclamps"
invoke_task "feature:migration:kvm"
end

desc "Run basic tests for cloud with xen computes"
task :base_xen do
invoke_task "feature:admin"
invoke_task "feature:controller"
invoke_task "feature:users"
invoke_task "feature:images"
invoke_task "features:barclamps"
invoke_task "feature:migration:xen"
end
desc "Run functional client tests"
task :functional do
invoke_task "test:func:all"
Expand Down
14 changes: 14 additions & 0 deletions tasks/features/migration.rake
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
namespace :feature do
feature_name "Instance migration"

namespace :migration do
desc "Test KVM migration"
feature_task :kvm, tags: :@kvm

desc "Test Xen migration"
feature_task :xen, tags: :@xen
end

desc "Complete verification of 'Instance migration' feature"
task :migration => "migration:kvm"
end