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

Saltcloud virtualbox provider #31058

Merged
merged 245 commits into from Feb 18, 2016
Merged

Conversation

LoveIsGrief
Copy link
Contributor

Related to #27089 Saltcloud virtualbox provider

A virtualbox salt-cloud provider was added with the following features:

  • create: clone virtual machines (salt-cloud -p profile_name instance_name)
  • destroy: salt-cloud -d instance_name
  • list_nodes(_select or _full)
  • actions:
    • start: attempts to start a VM
    • stop: attempts to stop a VM
  • functions:
    • show_image: describe a VM dict

It should be noted that for the moment all this cannot create a VM from scratch and needs a base VM.

The PR includes integration tests for which the virtualbox-sdk has to be installed (easiest if virtualbox is installed). There are tests that will need a bootable machine called SaltMiniBuntuTest (name up for debate of course) to execute the tests e.g deployment, booting and network address retrieval tests (I used a vagrant box ).

I attempted to comment as much and as sensibly as possible and am looking forward to constructive feedback and am of course open to discussions about the code organization, logic and of course naming (bad with that) :)

Cheers

No need to go online and search for it
Also get rid of keys we don't need in the provider config
Test VM creation, destruction and cloning
Related to saltstack#27089 Saltcloud virtualbox provider
Related to saltstack#27089 Saltcloud virtualbox provider
Related to saltstack#27089 Saltcloud virtualbox provider
Simplifies the code and tries to wait for the correct session state when starting machines

Related to saltstack#27089 Saltcloud virtualbox provider
Related to saltstack#27089 Saltcloud virtualbox provider
Related to saltstack#27089 Saltcloud virtualbox provider
Related to saltstack#27089 Saltcloud virtualbox provider
Outdated values may be retrieved from machines that aren't running

Related to saltstack#27089 Saltcloud virtualbox provider
Related to saltstack#27089 Saltcloud virtualbox provider
Related to saltstack#27089 Saltcloud virtualbox provider
Related to saltstack#27089 Saltcloud virtualbox provider
Related to saltstack#27089 Saltcloud virtualbox provider
It might leave the test env in a dirty state, maybe further tests might fail, but we have the log to trace it back to that

Related to saltstack#27089 Saltcloud virtualbox provider
Conflicts:
	.gitignore
	salt/config/__init__.py
	tests/integration/cloud/providers/virtualbox.py
Related to saltstack#27089 Saltcloud virtualbox provider
Related to saltstack#27089 Saltcloud virtualbox provider
@LoveIsGrief
Copy link
Contributor Author

@cachedout , I can't seem to get rid of the W1699: Incompatible Python3 code found. One is a line comment.

    8   
    9   Followed
    10  https://docs.saltstack.com/en/latest/topics/cloud/cloud.html#non-libcloud-based-modules
    11  to create this.
    12  
    13  Dicts provided by salt:
    14      __opts__ : contains the options used to run Salt Cloud,
    15          as well as a set of configuration and environment variables
    16  """
    17  # Import python libs   <- W1699 here
    18  import logging
    19  
    20  # Import salt libs
    21  
    22  from salt.exceptions import SaltCloudSystemExit
    23  import salt.config as config
    24  import salt.utils.cloud as cloud
    25  from salt.utils.virtualbox import vb_list_machines, vb_clone_vm, HAS_LIBS, vb_machine_exists, vb_destroy_machine, \
    26      vb_get_machine, vb_stop_vm, treat_machine_dict, vb_start_vm, vb_wait_for_network_address
    27  

I tried running tests/utils/test_timeout.py with python3 and it doesn't throw any exception. There is no proposed fix and I'm at loss. Any suggestions?

@cachedout
Copy link
Contributor

Hi @LoveIsGrief

I submitted a PR against your fork. It's here: LoveIsGrief#1

If you can review and merge that, it will automatically update this one and then the tests should automatically re-run. Then we'll see if lint is passing and we'll get this in. Thanks!

Related to saltstack#27089 Saltcloud virtualbox provider
Related to saltstack#27089 Saltcloud virtualbox provider
Related to saltstack#27089 Saltcloud virtualbox provider
Related to saltstack#27089 Saltcloud virtualbox provider
@LoveIsGrief
Copy link
Contributor Author

Hi @cachedout

Thanks for the PR. It resolved most of the pylint issues. Now everything's green :)

Cheers

@@ -1,3 +1,4 @@
user: ubuntu
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is my username and I couldn't run tests as salt-cloud always wanted to use root and I don't have passwordless sudo. Maybe I should remove this, maybe this needs discussion, I dunno.

@cachedout
Copy link
Contributor

@LoveIsGrief This looks just about ready to go but it looks like you've somehow pulled in some unrelated commits. You clearly didn't change the zsh completion as a part of this, for example. ;]

Could you rebase this PR against develop so that we just have your changes? Then we can go ahead and get this in. Thanks!

@LoveIsGrief
Copy link
Contributor Author

Hi @cachedout

There seems to be a problem with this pull request. I don't know why it's showing files that haven't been changed by me. When I do a new compare it only does show my changes and no one else's. I'll create a support request for this and create a new pull-request in the meantime.

@cachedout cachedout merged commit 04795b1 into saltstack:develop Feb 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending-changes The pull request needs additional changes before it can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants