[WIP] Add basic libvirt instance plugin #477
Conversation
|
Test failure is:
which I guess is unrelated. edit: it seems I'm able to kick retry myself, so I did. |
|
@ijc25 Good Contribution! Please add some tests for libvirt instance! |
|
@YujiOshima is there a good example of an existing plugin I can work from? I looked at |
|
https://github.com/docker/infrakit.gcp has a bunch of tests with mocks and the like. Maybe an inspiration? |
|
I think that terraform or maas is good in the sense that they have many tests.
I agree. |
|
Ah @rneugeba looks referred same. |
|
The build inside Docker container failed... compiling libvirt: |
|
@chungers I'll take a look, thanks (LOL @ @rneugeba / @YujiOshima it's not clear to me that mocking is all that useful here: the interesting thing to test is whether the XML given to the libvirt APIs (as a bare string) is well formed and semantically meaningful etc, which I don't think mock is capable of (I'd have to manually write the expected XML in the mock, but whose to say that matches what libvirt wants). I'm going to try and use the |
Codecov Report
@@ Coverage Diff @@
## master #477 +/- ##
==========================================
+ Coverage 56.65% 56.79% +0.13%
==========================================
Files 53 54 +1
Lines 3401 3622 +221
==========================================
+ Hits 1927 2057 +130
- Misses 1241 1296 +55
- Partials 233 269 +36
Continue to review full report at Codecov.
|
|
Fixed the container build and CI is nopw passing. I still want to add some tests. NB Friday and Monday are public holidays here and I'm on PTO on Thursday, so it might not be until next week. |
chungers
changed the title
Supports basic VM creation but does not yet include storage or networking. Hardcodes a bunch of things which should likely be exposed as properties, such as the use of KVM and a serial console. By default it targets `qemu:///session` which is a per-user session. To drive the system instance run with `--uri qemu:///system`, this may require authentication which is not yet supported (dependening on your desktop environment it might result in a auth popup for each action). The `Label` method is untested because I couldn't figure out how to cause it to be called. Signed-off-by: Ian Campbell <ian.campbell@docker.com>
|
Forced update:
@chungers my |
|
@ijc25 Great! |
|
@ijc25 I am sorry, I was misunderstanding, I edited my comment. |
|
|
||
| require.False(t, instanceExists(t, plugin, instance.ID(domname)), "domain was found in plugin instances before label") | ||
|
|
||
| labels := map[string]string{} |
There was a problem hiding this comment.
Set not only emply but also some labels and check it.
Set some label when provisioning and add or edit labels with Label function.
There was a problem hiding this comment.
Set some label when provisioning and add or edit labels with Label function.
I'll do that when someone points me to the documentation regarding the correct/expected semantics of the Label method. Otherwise I'm just guessing how it supposed to work and writing the answers to my guesses in both the implementation and the test.
@chungers asked me to do it this way. |
|
Please sign your commits following these rules: $ git clone -b "instance-libvirt" git@github.com:ijc25/infrakit.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354339792
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -fAmending updates the existing PR. You DO NOT need to open a new one. |
|
@ijc25 can this work with virtualbox on MacOSX? I think we can merge this to get it in and iterate. |
Does libvirt run on MacOSX and support VB there? If so then it ought to be possible to make it work. Note that libvirt doesn't abstract away all of the hypervisor specific bits and I've written/tested this with Qemu/KVM under Linux plus the test driver only so it is likely there will be a need to parameterise some more things to support other hypervisors and depending on the functionality available under those other hypervisors perhaps adding support for things like booting from ISO (e.g. if VB doesn't support direct kernel boot). Should all be pretty minor updates compared with the scaffolding added here though. Looks like you have a policy of back merging |
|
@ijc25 - let's merge this and iterate. Thanks for starting this plugin! |
Updated Azure logs to match AWS format
Updated Azure logs to match AWS format
Supports basic VM creation but does not yet include storage or networking.
Hardcodes a bunch of things which should likely be exposed as properties, such
as the use of KVM and a serial console.
By default it targets
qemu:///sessionwhich is a per-user session. To drivethe system instance run with
--uri qemu:///system, this may requireauthentication which is not yet supported (dependening on your desktop
environment it might result in a auth popup for each action).
The
Labelmethod is untested because I couldn't figure out how to cause it tobe called.
Signed-off-by: Ian Campbell ian.campbell@docker.com