[Ovirt-devel] [PATCH server] BUG#460147 - Deleting a VM based on a Cobbler system does not delete

[Darryl L. Pierce]

This patch addresses this issue, first calling Cobbler and deleting the
system prior to deleting the VM. It also changes ovirt-server's dependence
to since this is new functionality in the rubygem-cobbler package.

Signed-off-by: Darryl L. Pierce <dpierce at redhat.com>
---
 ovirt-server.spec.in                 |    2 +-
 src/app/controllers/vm_controller.rb |   12 ++++++++++++
 src/app/models/vm.rb                 |    6 ++++++
 src/test/unit/vm_test.rb             |   12 ++++++++++++
 4 files changed, 31 insertions(+), 1 deletions(-)

diff --git a/ovirt-server.spec.in b/ovirt-server.spec.in
index 123e280..e2620d1 100644
--- a/ovirt-server.spec.in
+++ b/ovirt-server.spec.in
@@ -17,7 +17,7 @@ Requires: rubygem(activeldap) >= 0.10.0
 Requires: rubygem(rails) >= 2.1.1
 Requires: rubygem(mongrel) >= 1.0.1
 Requires: rubygem(krb5-auth) >= 0.6
-Requires: rubygem(cobbler) >= 0.0.2
+Requires: rubygem(cobbler) >= 0.1.2
 Requires: rubygem(gettext)
 Requires: ruby-flexmock
 Requires: postgresql-server
diff --git a/src/app/controllers/vm_controller.rb b/src/app/controllers/vm_controller.rb
index a0fcfbc..fc992b3 100644
--- a/src/app/controllers/vm_controller.rb
+++ b/src/app/controllers/vm_controller.rb
@@ -144,6 +144,7 @@ class VmController < ApplicationController
         vms = Vm.find(:all, :conditions => "id in (#{vm_ids.join(', ')})")
         vms.each do |vm|
           if vm.is_destroyable?
+            destroy_cobbler_system(vm)
             vm.destroy
           else
             failure_list << vm.description
@@ -166,6 +167,7 @@ class VmController < ApplicationController
   def destroy
     vm_resource_pool = @vm.vm_resource_pool_id
     if (@vm.is_destroyable?)
+      destroy_cobbler_system(@vm)
       @vm.destroy
       render :json => { :object => "vm", :success => true,
         :alert => "Virtual Machine was successfully deleted." }
@@ -349,4 +351,14 @@ class VmController < ApplicationController
     pre_edit
     authorize_user
   end
+
+  private
+
+  def destroy_cobbler_system(vm)
+    # Destroy the Cobbler system first if it's defined
+    if vm.uses_cobbler?
+      system = Cobbler::System.find_one(vm.cobbler_system_name)
+      system.remove if system
+    end
+  end
 end
diff --git a/src/app/models/vm.rb b/src/app/models/vm.rb
index 05073f0..ae157f7 100644
--- a/src/app/models/vm.rb
+++ b/src/app/models/vm.rb
@@ -277,6 +277,12 @@ class Vm < ActiveRecord::Base
     end
   end
 
+  # Returns the system name in Cobbler for this VM.
+  #
+  def cobbler_system_name
+    self.uuid
+  end
+
   # whether this VM may be validly deleted. running VMs should not be
   # allowed to be deleted. Currently we restrict deletion to VMs that
   # are currently stopped, pending (new without any create_vm tasks having
diff --git a/src/test/unit/vm_test.rb b/src/test/unit/vm_test.rb
index 2291e9d..a9ed9dc 100644
--- a/src/test/unit/vm_test.rb
+++ b/src/test/unit/vm_test.rb
@@ -76,4 +76,16 @@ class VmTest < Test::Unit::TestCase
       vm.cobbler_name,
       "Wrong name reported."
   end
+
+  # Ensures that the right value is used when requesting the cobbler system
+  # name for a VM backed by Cobbler.
+  #
+  def test_cobbler_system_name
+    @vm = Vm.new
+    @vm.provisioning_and_boot_settings = @cobbler_profile_provisioning
+    @vm.uuid = "oicu812"
+
+    assert_equal @vm.cobbler_system_name, @vm.uuid,
+      "VMs should be using the UUID as their Cobbler system name."
+  end
 end
-- 
1.5.6.5