refactor(nodes): reuse remote node for domain (#1275)

When a domain has started in a remote node it already
has its volume there, make it reuse it.

issue #1274

lib/Ravada.pm

@@ -1688,7 +1688,8 @@ sub create_domain {
             or confess "Unknown base id: $id_base";
         $vm = $base->_vm;
     }
-    my $user = Ravada::Auth::SQL->search_by_id($id_owner);
+    my $user = Ravada::Auth::SQL->search_by_id($id_owner)
+        or confess "Error: Unkown user '$id_owner'";
 
     $request->status("creating machine")    if $request;
     if ( $base && $base->is_base && $base->volatile_clones || $user->is_temporary ) {

lib/Ravada/Domain.pm

@@ -326,9 +326,10 @@ sub _around_start($orig, $self, @arg) {
             $arg{listen_ip} = $display_ip;
         }
         eval { $self->$orig(%arg) };
-        last if !$@;
-        warn $@ if $@;
-        if ($@ && $self->id_base && !$self->_vm->is_local && $self->_vm->enabled) {
+        my $error = $@;
+        last if !$error;
+        warn "WARNING: $error ".$self->_vm->name." ".$self->_vm->enabled if $error;
+        if ($error && $self->id_base && !$self->is_local && $self->_vm->enabled) {
             $self->_request_set_base();
             next;
         }
@@ -345,7 +346,6 @@ sub _request_set_base($self) {
         uid => Ravada::Utils::user_daemon->id
         ,id_domain => $base->id
         ,id_vm => $self->_vm->id
-        ,at => time + int(rand(10))
     );
     my $vm_local = $self->_vm->new( host => 'localhost' );
     $self->_set_vm($vm_local, 1);
@@ -380,38 +380,52 @@ sub _start_preconditions{
 
 sub _start_checks($self, @args) {
     return if $self->_search_already_started('fast');
+    my $vm_local = $self->_vm->new( host => 'localhost' );
+    my $vm = $vm_local;
+
     my ($id_vm, $request);
     if (!scalar(@args) % 2) {
         my %args = @args;
+
+        # We may be asked to start the machine in a specific id_vmanager
         $id_vm = delete $args{id_vm};
         $request = delete $args{request} if exists $args{request};
     }
+    # If not specific id_manager we go to the last id_vmanager unless it was localhost
+    # If the last VManager was localhost it will try to balance here.
+    $id_vm = $self->_data('id_vm')
+    if !$id_vm && defined $self->_data('id_vm')
+    && $self->_data('id_vm') != $vm_local->id;
+
+    if ($id_vm) {
+        $vm = Ravada::VM->open($id_vm);
+        if ( !$vm->is_enabled || !$vm->ping ) {
+            $vm = $vm_local;
+            $id_vm = undef;
+        }
+    }
+    $self->_check_tmp_volumes();
 
     # if it is a clone ( it is not a base )
     if ($self->id_base) {
 #        $self->_set_last_vm(1)
-        if ( !$self->is_local && ( !$self->_vm->enabled || !$self->_vm->ping) ) {
-            my $vm_local = $self->_vm->new( host => 'localhost' );
+        if ( !$self->is_local
+            && ( !$self->_vm->enabled || !base_in_vm($self->id_base,$self->_vm->id)
+                || !$self->_vm->ping) ) {
             $self->_set_vm($vm_local, 1);
         }
-        $self->_check_tmp_volumes();
-        my $vm;
-        if ($id_vm) {
-            $vm = Ravada::VM->open($id_vm);
-            if ( !$vm->is_alive ) {
-                $vm->disconnect();
-                $vm->connect;
-            }
+        if ( !$vm->is_alive ) {
+            $vm->disconnect();
+            $vm->connect;
+            $vm = $vm_local if !$vm->is_local && !$vm->is_alive;
         };
-        warn $@ if $@;
-        if ($vm) {
+        if ($id_vm) {
             $self->_set_vm($vm);
         } else {
             $self->_balance_vm();
         }
         $self->rsync(request => $request)  if !$self->is_volatile && !$self->_vm->is_local();
     } elsif (!$self->is_local) {
-        my $vm_local = $self->_vm->new( host => 'localhost' );
         $self->_set_vm($vm_local, 1);
     }
     $self->_check_free_vm_memory();
@@ -875,19 +889,12 @@ sub _check_free_vm_memory {
 sub _check_tmp_volumes($self) {
     confess "Error: only clones temporary volumes can be checked."
         if !$self->id_base;
+    my $vm_local = $self->_vm->new( host => 'localhost' );
     for my $vol ( $self->list_volumes_info) {
         next unless $vol->file && $vol->file =~ /\.(TMP|SWAP)\./;
-        next if $self->_vm->file_exists($vol->file);
+        next if $vm_local->file_exists($vol->file);
 
         my $base = Ravada::Domain->open($self->id_base);
-        if (! $self->is_local) {
-            Ravada::Request->set_base_vm(
-                id_domain => $base->id
-                ,id_vm => $self->_vm->id
-                ,uid => Ravada::Utils::user_daemon->id
-            );
-            confess "Error: base file not found in node ".$self->_vm->name." ".$vol->file;
-        }
         my @volumes = $base->list_files_base_target;
         my ($file_base) = grep { $_->[1] eq $vol->info->{target} } @volumes;
         if (!$file_base) {
@@ -895,11 +902,9 @@ sub _check_tmp_volumes($self) {
                 .Dumper(\@volumes);
         }
         my $vol_base = Ravada::Volume->new( file => $file_base->[0]
-            , vm => $self->_vm
+            , vm => $vm_local
         );
         $vol_base->clone(file => $vol->file);
-        warn "cloned ".$self->name." ".$self->_vm->name." ".$vol_base->vm->name." "
-            .$vol->info->{target};
     }
 }
 
@@ -3338,10 +3343,12 @@ sub clean_swap_volumes {
     my $self = shift;
     for my $vol ( $self->list_volumes_info) {
         if ($vol->file && $vol->file =~ /\.SWAP\.\w+$/) {
+            next if !$self->_vm->file_exists($vol->file);
             my $backing_file;
             eval { $backing_file = $vol->backing_file };
             confess $@ if $@ && $@ !~ /No backing file/i;
             next if !$backing_file;
+            next if !$self->_vm->file_exists($backing_file);
             $vol->restore() if !$@;
         }
     }
@@ -3790,14 +3797,19 @@ sub _pre_migrate($self, $node, $request = undef) {
     confess "ERROR: Active domains can't be migrated"   if $self->is_active;
 
     my $base = Ravada::Domain->open($self->id_base);
-    confess "ERROR: base id ".$self->id_base." not found."  if !$base;
-
-    confess "ERROR: Base ".$base->name." files not migrated to ".$node->name
+    confess "ERROR: base ".$base->name." not prepared in node ".$node->name
         if !$base->base_in_vm($node->id);
+    confess "ERROR: base id ".$self->id_base." not found."  if !$base;
 
     for my $file ( $base->list_files_base ) {
         next if $node->file_exists($file);
-        confess "ERROR: file not found $file in ".$node->host;
+        warn "Warning: file not found $file in ".$node->name;
+        Ravada::Request->set_base_vm(
+            uid => Ravada::Utils::user_daemon->id
+            ,id_domain => $base->id
+            ,id_vm => $node->id
+        );
+        return;
     }
 
     $self->_set_base_vm_db($node->id,0);
@@ -3979,7 +3991,7 @@ sub list_vms($self) {
         my $vm;
         eval { $vm = Ravada::VM->open($id_vm) };
         warn "id_domain: ".$self->id."\n".$@ if $@;
-        push @vms,($vm) if $vm;
+        push @vms,($vm) if $vm && !$vm->is_locked();
     }
     my $vm_local = $self->_vm->new( host => 'localhost' );
     if ( !grep { $_->name eq $vm_local->name } @vms) {
@@ -3999,18 +4011,22 @@ Returns if this domain has a base prepared in this virtual manager
 
 sub base_in_vm($self,$id_vm) {
 
+    my $id = $self;
+    $id = $self->id if ref($self);
+
     confess "ERROR: id_vm must be a number, it is '$id_vm'"
         if $id_vm !~ /^\d+$/;
 
     confess "ERROR: Domain ".$self->name." is not a base"
-        if !$self->is_base;
+        if ref($self) && !$self->is_base;
 
     confess "Undefined id_vm " if !defined $id_vm;
+
     my $sth = $$CONNECTOR->dbh->prepare(
         "SELECT enabled FROM bases_vm "
         ." WHERE id_domain = ? AND id_vm = ?"
     );
-    $sth->execute($self->id, $id_vm);
+    $sth->execute($id, $id_vm);
     my ( $enabled ) = $sth->fetchrow;
     $sth->finish;
 #    return 1 if !defined $enabled

lib/Ravada/Domain/KVM.pm

@@ -662,14 +662,8 @@ sub start {
     }
     return if !$error || $error =~ /already running/i;
     if ($error =~ /libvirt error code: 38,/) {
-        warn "Error starting ".$self->name." on ".$self->_vm->name;
-        if (!$self->_vm->is_local) {
-            if ($error !~ /backing file/) {
-                warn "Disabling node ".$self->_vm->name();
-                $self->_vm->enabled(0);
-            }
-        }
-        die $error;
+        die "Error starting ".$self->name." on ".$self->_vm->name
+            ."\n$error";
     } elsif ( $error =~ /libvirt error code: 9, .*already defined with uuid/) {
         die "TODO";
     } elsif ( $error =~ /libvirt error code: 1,.*smbios/) {

lib/Ravada/VM.pm

@@ -1429,8 +1429,7 @@ sub balance_vm($self, $base=undef) {
     } else {
         @vms = $self->list_nodes();
     }
-#    warn Dumper([ map { $_->name } @vms]);
-    return $self if scalar(@vms)<2;
+    return $vms[0] if scalar(@vms)<1;
     for my $vm (_random_list( @vms )) {
         next if !$vm->enabled();
         my $active = 0;

t/kvm/n10_nodes.t

@@ -285,7 +285,7 @@ sub test_sync_base {
     );
 
     eval { $clone->migrate($node); };
-    like($@, qr'.');
+    like($@, qr'.'); # base not in node
 
     eval { $base->migrate_base(user => user_admin, vm => $node); };
     is(''.$@,'');

t/kvm/n20_nodes_hibernate.t

@@ -30,6 +30,7 @@ sub test_node_down($node, $action, $action_name) {
     my $domain = create_domain($node->type);
     $domain->prepare_base(user_admin);
     $domain->migrate_base(user => user_admin, node => $node);
+    is($domain->base_in_vm($node->id),1);
 
     my $clone = $domain->clone(
          user => user_admin
@@ -49,8 +50,12 @@ sub test_node_down($node, $action, $action_name) {
     is($@,'');
     is($clone->is_active, 1, "Expecting clone ".$clone->name." active");
     is($clone->is_local, 1,"Expecting clone ".$clone->name." local");
+    is($domain->base_in_vm($node->id),0);
 
+    $node->_clean_cache();
     start_node($node);
+    is($node->is_active,1);
+    wait_request(debug => 0);
 
     is($domain->_vm->is_active, 1);
 

t/nodes/10_basic.t

@@ -255,6 +255,7 @@ sub test_removed_local_swap($vm, $node) {
     $base->add_volume(size => 128*1024 , type => 'data');
     $base->prepare_base(user_admin);
     $base->set_base_vm(node => $node, user => user_admin);
+    ok(scalar($base->list_vms) >1) or die Dumper([map { $_->name } $base->list_vms]);
 
     my $found_clone;
     for my $try ( 1 .. 20 ) {
@@ -263,8 +264,9 @@ sub test_removed_local_swap($vm, $node) {
         $clone1->start(user_admin);
         $found_clone = $clone1;
         last if $clone1->_vm->id == $node->id;
+        ok(scalar($base->list_vms) >1) or die Dumper([map { $_->name } $base->list_vms]);
     }
-    is($found_clone->_vm->id, $node->id);
+    is($found_clone->_vm->id, $node->id) or exit;
     for my $clone_data ($base->clones) {
         my $clone = Ravada::Domain->open($clone_data->{id});
         $clone->remove(user_admin);
@@ -286,15 +288,18 @@ sub test_removed_remote_swap($vm, $node) {
 
     my $found_clone;
     for my $try ( 1 .. 20 ) {
-        diag("try $try");
         my $clone1 = $base->clone(name => new_domain_name, user => user_admin);
         $clone1->migrate($node);
         _remove_tmp($clone1,$node);
         $clone1->start(user_admin);
         $found_clone = $clone1;
-        last if $base->list_requests;
+        last if $clone1->_vm->id == $node->id;
     }
-    ok(grep { $_->command eq 'set_base_vm' } $base->list_requests );
+    is($found_clone->_vm->id,$node->id);
+    $found_clone->info(user_admin);
+    my $node_ip = $node->ip;
+    my $clone_v = Ravada::Front::Domain->open($found_clone->id);
+    like($clone_v->display(user_admin), qr($node_ip));
     for my $clone_data ($base->clones) {
         my $clone = Ravada::Domain->open($clone_data->{id});
         $clone->remove(user_admin);
@@ -310,6 +315,7 @@ sub test_removed_base_file($vm, $node) {
     my $base = create_domain($vm);
     $base->prepare_base(user_admin);
     $base->set_base_vm(node => $node, user => user_admin);
+    is($base->base_in_vm($node->id),1);
 
     for my $file ( $base->list_files_base ) {
         $node->remove_file($file);
@@ -318,7 +324,6 @@ sub test_removed_base_file($vm, $node) {
     my $found_req;
     my $found_clone;
     for my $try ( 1 .. 20 ) {
-        diag("try $try");
         my $clone1 = $base->clone(name => new_domain_name, user => user_admin);
         $clone1->start(user_admin);
         $found_clone = $clone1;
@@ -333,8 +338,9 @@ sub test_removed_base_file($vm, $node) {
         last if $found_req;
     }
     ok($found_req,"Expecting request to set base vm");
-    is($base->base_in_vm($node->id),0);
-    is(scalar($base->list_vms),1) or exit;
+    wait_request();
+    is($base->base_in_vm($node->id),1);
+    is(scalar($base->list_vms),2) or exit;
     my $node2 = Ravada::VM->open($node->id);
     is($node2->is_enabled,1);
     for my $clone_data ($base->clones) {
@@ -373,7 +379,7 @@ sub _remove_tmp($domain, $vm = $domain->_vm) {
 }
 
 sub test_removed_base_file_and_swap_remote($vm, $node) {
-    diag("Testing removed remote base in ".$vm->type);
+    diag("Testing removed remote base and swap in ".$vm->type);
     my $base = create_domain($vm);
     $base->add_volume(size => 128*1024 , type => 'tmp');
     $base->add_volume(size => 128*1024 , type => 'swap');
@@ -384,7 +390,6 @@ sub test_removed_base_file_and_swap_remote($vm, $node) {
     my $found_req;
     my $found_clone;
     for my $try ( 1 .. 20 ) {
-        diag("try $try");
         my $clone1 = $base->clone(name => new_domain_name, user => user_admin);
         $clone1->migrate($node);
         _remove_tmp($clone1,$node);
@@ -392,18 +397,14 @@ sub test_removed_base_file_and_swap_remote($vm, $node) {
         $clone1->start(user_admin);
         $found_clone = $clone1;
         my @req = $base->list_requests();
-        next if !scalar @req;
-        for my $req (@req) {
-            if($req->command eq 'set_base_vm') {
-                $found_req = $req;
-                last;
-            }
-        }
-        last if $found_req;
+        last if !$base->base_in_vm($node->id);
+        last if $base->list_requests();
     }
-    ok($found_req,"Expecting request to set base vm");
-    is($base->base_in_vm($node->id),0);
+    ok(grep { $_->command eq 'set_base_vm' } $base->list_requests)
+        or die $vm->type." ".Dumper([$base->list_requests]);
     is(scalar($base->list_vms),1) or exit;
+    wait_request(debug => 0);
+    is($base->base_in_vm($node->id),1);
     my $node2 = Ravada::VM->open($node->id);
     is($node2->is_enabled,1);
     for my $clone_data ($base->clones) {
@@ -685,7 +686,7 @@ sub test_remove_base($vm, $node, $volatile) {
     my @req = $base->list_requests();
     is(scalar @req,2);
     ok(grep {$_->command eq 'remove_base_vm' } @req) or die Dumper(\@req);
-    wait_request( debug => 1 );
+    wait_request( debug => 0 );
 
     for my $file ( @volumes ) {
         ok(!-e $file, "Expecting no file '$file' in local") or exit;
@@ -749,7 +750,7 @@ sub test_create_active($vm, $node) {
             ,remote_ip => '1.2.3.4'
             ,id_owner => user_admin->id
         );
-        wait_request(debug => 1);
+        wait_request(debug => 0);
         $clone = rvd_front->search_domain($name);
         ok($vm->search_domain($name),"Expecting clone $name in master node") or exit;
         last if $clone->display(user_admin) =~ /$remote_ip/;
@@ -760,6 +761,53 @@ sub test_create_active($vm, $node) {
     my $info = $clone2->info(user_admin);
     like($info->{display}->{display}, qr($remote_ip)) or die Dumper($info->{display});
 
+    test_keep_node($node, $clone);
+
+    _remove_domain($base);
+}
+
+sub test_keep_node($node, $clone) {
+    # check clone is always started in the same node
+    my $node_ip = $node->ip;
+    for ( 1 .. 3 ) {
+        Ravada::Request->shutdown_domain(id_domain => $clone->id, uid => user_admin->id
+            , timeout => 1);
+        wait_request();
+
+        {
+            my $clone2 = Ravada::Domain->open($clone->id);
+            is($clone2->is_active,0);
+            is($clone2->_vm->id, $node->id);
+            is($clone2->_data('id_vm'), $node->id);
+        }
+
+        Ravada::Request->start_domain(id_domain => $clone->id, uid => user_admin->id
+            ,remote_ip => '1.2.3.4'
+        );
+        wait_request();
+        {
+            my $clone3 = Ravada::Domain->open($clone->id);
+            is($clone3->is_active,1);
+            is($clone3->_vm->id, $node->id, $clone3->name) or exit;
+            like($clone3->display(user_admin),qr($node_ip));
+            $clone3->shutdown_now(user_admin);
+            $clone3->migrate($node);
+        }
+    }
+}
+
+sub test_base_unset($vm, $node) {
+    my $base = create_domain($vm);
+    $base->prepare_base(user_admin);
+    $base->set_base_vm(vm => $node, user => user_admin);
+
+    my $clone = $base->clone(name => new_domain_name, user => user_admin);
+    $clone->migrate($node);
+    $base->set_base_vm(id_vm => $node->id,value => 0, user => user_admin);
+    $clone->start(user_admin);
+
+    is($clone->_vm->id, $vm->id);
+
     _remove_domain($base);
 }
 
@@ -802,6 +850,7 @@ for my $vm_name ( 'Void', 'KVM') {
         is($node->is_local,0,"Expecting ".$node->name." ".$node->ip." is remote" ) or BAIL_OUT();
 
         test_create_active($vm, $node);
+        test_base_unset($vm,$node);
 
         test_removed_base_file_and_swap_remote($vm, $node);
         test_removed_remote_swap($vm, $node);