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

Conversation

djz88
Copy link

@djz88 djz88 commented May 24, 2017

Add migration tests for KVM and XEN into CCT. Plus add 2 openstack commands as well.

  • new openstack commands hypervisor and server
  • allow use options in show method
  • KVM migration test in features:base
  • Xen migration test in newly created features:base_xen
  • features:base_xen is clone of base (including tests)
  • default action for :migration is :migration:kvm

Kvm migration can be called directly with mkcloud environment variable cct_tests="feature:migration:kvm"
Xen migration can be called directly with mkcloud environment variable cct_tests="feature:migration:xen"


@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

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

And I turn off the instance(so it will not use resources)

@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

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

@@ -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

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

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

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}")
@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

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

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 .

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: obsolete parameter RunRailsCops (for AllCops) found in /tmp/d20170531-...
Error: obsolete parameter RunRailsCops (for AllCops) found in /tmp/d20170531-24157-xmbv2w/.rubocop.yml
Use the following configuration instead:
Rails:
Enabled: true
Error: obsolete parameter RunRailsCops (for AllCops) found in /tmp/d20170531-...
Error: obsolete parameter RunRailsCops (for AllCops) found in /tmp/d20170531-12973-pr7ymh/.rubocop.yml
Use the following configuration instead:
Rails:
Enabled: true
Error: obsolete parameter RunRailsCops (for AllCops) found in /tmp/d20170531-...
Error: obsolete parameter RunRailsCops (for AllCops) found in /tmp/d20170531-24164-10axlp2/.rubocop.yml
Use the following configuration instead:
Rails:
Enabled: true
Error: obsolete parameter RunRailsCops (for AllCops) found in /tmp/d20170531-...
Error: obsolete parameter RunRailsCops (for AllCops) found in /tmp/d20170531-31178-192cjtk/.rubocop.yml
Use the following configuration instead:
Rails:
Enabled: true
Error: obsolete parameter RunRailsCops (for AllCops) found in /tmp/d20170531-...
Error: obsolete parameter RunRailsCops (for AllCops) found in /tmp/d20170531-12503-1f6gc0n/.rubocop.yml
Use the following configuration instead:
Rails:
Enabled: true
Error: obsolete parameter RunRailsCops (for AllCops) found in /tmp/d20170531-...
Error: obsolete parameter RunRailsCops (for AllCops) found in /tmp/d20170531-31164-7bho99/.rubocop.yml
Use the following configuration instead:
Rails:
Enabled: true
Error: obsolete parameter RunRailsCops (for AllCops) found in /tmp/d20170531-...
Error: obsolete parameter RunRailsCops (for AllCops) found in /tmp/d20170531-31150-17894s0/.rubocop.yml
Use the following configuration instead:
Rails:
Enabled: true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants