Unverified Commit bcb7a80f authored by Francesc Guasch's avatar Francesc Guasch Committed by GitHub
Browse files

Fix #1131 pool (#1133)

fix(backend): fixed create clone when pooling

* fix(backend): reviewed pool clones management
* refactor(frontend): improved user feedback
* test(backend): pick a machine from pool

issue #1131
parent 7ead6d59
......@@ -1551,7 +1551,7 @@ sub create_domain {
my $start = $args{start};
my $id_base = $args{id_base};
my $id_owner = $args{id_owner} or confess "Error: missing id_owner ".Dumper(\%args);
_check_args(\%args,qw(iso_file id_base id_iso id_owner name active swap memory disk id_template start remote_ip request vm));
_check_args(\%args,qw(iso_file id_base id_iso id_owner name active swap memory disk id_template start remote_ip request vm add_to_pool));
confess "ERROR: Argument vm required" if !$id_base && !$vm_name;
......@@ -2418,7 +2418,7 @@ sub _cmd_manage_pools($self, $request) {
}
sub _pool_create_clones($self, $domain, $number, $request) {
my @arg_clone = ( no_pool => 1 );
my @arg_clone = ( );
$request->status("cloning $number");
if (!$domain->is_base) {
my @requests = $domain->list_requests();
......@@ -2434,7 +2434,7 @@ sub _pool_create_clones($self, $domain, $number, $request) {
uid => $request->args('uid')
,id_domain => $domain->id
,number => $number
,is_pool => 1
,add_to_pool => 1
,start => 1
,@arg_clone
);
......@@ -2489,12 +2489,16 @@ sub _cmd_create{
if ( $request->defined_arg('id_base') ) {
my $base = Ravada::Domain->open($request->args('id_base'));
if ( $base->pools && !$request->defined_arg('no_pools') ) {
$request->{args}->{id_domain} = delete $request->{args}->{id_base};
$request->{args}->{uid} = delete $request->{args}->{id_owner};
my $clone = $self->_cmd_clone($request);
$request->id_domain($clone->id);
return $clone;
if ( $request->defined_arg('pool') ) {
if ( $base->pools ) {
$request->{args}->{id_domain} = delete $request->{args}->{id_base};
$request->{args}->{uid} = delete $request->{args}->{id_owner};
my $clone = $self->_cmd_clone($request);
$request->id_domain($clone->id);
return $clone;
} else {
confess "Error: this base has no pools";
}
}
}
......@@ -2646,10 +2650,8 @@ sub _cmd_open_iptables {
}
sub _cmd_clone($self, $request) {
my $number = $request->defined_arg('number');
my $no_pool = $request->defined_arg('no_pool');
return _req_clone_many($self, $request) if $number;
return _req_clone_many($self, $request) if $request->defined_arg('number');
my $domain = Ravada::Domain->open($request->args('id_domain'))
or confess "Error: Domain ".$request->args('id_domain')." not found";
......@@ -2665,18 +2667,6 @@ sub _cmd_clone($self, $request) {
}
my $name = ( $request->defined_arg('name') or $domain->name."-".$user->name );
if ( $domain->pools && !$no_pool ) {
my $clone = $domain->_search_pool_clone($user);
my $remote_ip = $request->defined_arg('remote_ip');
my $start = $request->defined_arg('start');
if ($start || $clone->is_active) {
$clone->start(user => $user, remote_ip => $remote_ip);
$clone->_data('client_status', 'connecting ...');
$clone->_data('client_status_time_checked',time);
Ravada::Request->manage_pools( uid => Ravada::Utils::user_daemon->id);
}
return $clone;
}
my $clone = $domain->clone(
name => $name
......
......@@ -1843,18 +1843,31 @@ sub clone {
confess "ERROR: Clones can't be created in readonly mode"
if $self->_vm->readonly();
return $self->_copy_clone(@_) if $self->id_base();
my $add_to_pool = delete $args{add_to_pool};
my $from_pool = delete $args{from_pool};
my $remote_ip = delete $args{remote_ip};
my $request = delete $args{request};
my $memory = delete $args{memory};
my $start = delete $args{start};
my $is_pool = delete $args{is_pool};
my $no_pool = delete $args{no_pool};
confess "ERROR: Unknown args ".join(",",sort keys %args)
if keys %args;
confess "Error: This base has no pools"
if $add_to_pool && !$self->pools;
$from_pool = 1 if !defined $from_pool && !$add_to_pool && $self->pools;
confess "Error: you can't add to pool if you pick from pool"
if $from_pool && $add_to_pool;
return $self->_clone_from_pool(@_) if $from_pool;
my %args2 = @_;
delete $args2{from_pool};
return $self->_copy_clone(%args2) if $self->id_base();
my $uid = $user->id;
if ( !$self->is_base() ) {
......@@ -1867,6 +1880,8 @@ sub clone {
push @args_copy, ( memory => $memory ) if $memory;
push @args_copy, ( request => $request ) if $request;
push @args_copy, ( remote_ip => $remote_ip) if $remote_ip;
push @args_copy, ( from_pool => $from_pool) if defined $from_pool;
push @args_copy, ( add_to_pool => $add_to_pool) if defined $add_to_pool;
my $vm = $self->_vm;
if ($self->volatile_clones ) {
......@@ -1882,7 +1897,23 @@ sub clone {
,id_owner => $uid
,@args_copy
);
$clone->is_pool(1) if $is_pool;
$clone->is_pool(1) if $add_to_pool;
return $clone;
}
sub _clone_from_pool($self, %args) {
my $user = delete $args{user};
my $remote_ip = delete $args{remote_ip};
my $start = delete $args{start};
my $clone = $self->_search_pool_clone($user);
if ($start || $clone->is_active) {
$clone->start(user => $user, remote_ip => $remote_ip);
$clone->_data('client_status', 'connecting ...');
$clone->_data('client_status_time_checked',time);
Ravada::Request->manage_pools( uid => Ravada::Utils::user_daemon->id);
}
return $clone;
}
......@@ -1891,6 +1922,7 @@ sub _copy_clone($self, %args) {
my $user = delete $args{user} or confess "ERROR: Missing user";
my $memory = delete $args{memory};
my $request = delete $args{request};
my $add_to_pool = delete $args{add_to_pool};
confess "ERROR: Unknown arguments ".join(",",sort keys %args)
if keys %args;
......@@ -1907,6 +1939,7 @@ sub _copy_clone($self, %args) {
name => $name
,id_base => $base->id
,id_owner => $user->id
,from_pool => 0
,@copy_arg
);
my @volumes = $self->list_volumes_info(device => 'disk');
......@@ -1918,6 +1951,7 @@ sub _copy_clone($self, %args) {
copy($volumes{$target}, $copy_volumes{$target})
or die "$! $volumes{$target}, $copy_volumes{$target}"
}
$copy->is_pool(1) if $add_to_pool;
return $copy;
}
......@@ -3556,7 +3590,7 @@ sub _pre_clone($self,%args) {
confess "ERROR: Missing user owner of new domain" if !$user;
for (qw(is_pool start no_pool)) {
for (qw(is_pool start add_to_pool from_pool)) {
delete $args{$_};
}
confess "ERROR: Unknown arguments ".join(",",sort keys %args) if keys %args;
......@@ -3706,6 +3740,7 @@ sub _search_pool_clone($self, $user) {
if !$self->pools;
my ($clone_down, $clone_free_up, $clone_free_down);
my ($clones_in_pool, $clones_used) = (0,0);
for my $current ( $self->clones) {
if ( $current->{id_owner} == $user->id
&& $current->{status} =~ /^(active|hibernated)$/) {
......@@ -3716,6 +3751,7 @@ sub _search_pool_clone($self, $user) {
if ( $current->{id_owner} == $user->id ) {
$clone_down = $current;
} elsif ($current->{is_pool}) {
$clones_in_pool++;
my $clone = Ravada::Domain->open($current->{id});
if(!$clone->client_status || $clone->client_status eq 'disconnected') {
if ( $clone->status =~ /^(active|hibernated)$/ ) {
......@@ -3723,6 +3759,8 @@ sub _search_pool_clone($self, $user) {
} else {
$clone_free_down = $current;
}
} else {
$clones_used++;
}
}
}
......@@ -3730,6 +3768,7 @@ sub _search_pool_clone($self, $user) {
my $clone_data = ($clone_down or $clone_free_up or $clone_free_down);
die "Error: no free clones in pool for ".$self->name
.". Usage: $clones_used used from $clones_in_pool virtual machines created.\n"
if !$clone_data;
my $clone = Ravada::Domain->open($clone_data->{id});
......
......@@ -76,8 +76,14 @@ our %VALID_ARG = (
,refresh_storage => { id_vm => 2 }
,set_base_vm=> {uid => 1, id_vm=> 1, id_domain => 1, value => 2 }
,cleanup => { }
,clone => { uid => 1, id_domain => 1, name => 2, memory => 2, number => 2, is_pool => 2
,start => 2, no_pool => 2
,clone => { uid => 1, id_domain => 1, name => 2, memory => 2, number => 2
# If base has pools, from_pool = 1 if undefined
# when from_pool is true the clone is picked from the pool
# when from_pool is false the clone is created
,from_pool => 2
# If base has pools, create anew and add to the pool
,add_to_pool => 2
,start => 2,
,remote_ip => 2
}
,change_owner => {uid => 1, id_domain => 1}
......
......@@ -362,6 +362,7 @@ sub _around_create_domain {
my $self = shift;
my %args = @_;
my $remote_ip = delete $args{remote_ip};
my $add_to_pool = delete $args{add_to_pool};
my %args_create = %args;
my $id_owner = delete $args{id_owner} or confess "ERROR: Missing id_owner";
......@@ -374,6 +375,7 @@ sub _around_create_domain {
my $active = delete $args{active};
my $name = delete $args{name};
my $swap = delete $args{swap};
my $from_pool = delete $args{from_pool};
# args get deleted but kept on %args_create so when we call $self->$orig below are passed
delete $args{disk};
......@@ -390,6 +392,11 @@ sub _around_create_domain {
$base = $self->search_domain_by_id($id_base)
or confess "Error: I can't find domain $id_base on ".$self->name;
$volatile = 1 if $base->volatile_clones;
if ($add_to_pool) {
confess "Error: you can't add to pool and also pick from pool" if $from_pool;
$from_pool = 0;
}
$from_pool = 1 if !defined $from_pool && $base->pools();
}
confess "ERROR: User ".$owner->name." is not allowed to create machines"
......@@ -400,10 +407,18 @@ sub _around_create_domain {
confess "ERROR: Base ".$base->name." is private"
if !$owner->is_admin && $base && !$base->is_public();
if ($add_to_pool) {
confess "Error: This machine can only be added to a pool if it is a clone"
if !$base;
confess("Error: Requested to add a clone for the pool but this base has no pools")
if !$base->pools;
}
$args_create{listen_ip} = $self->listen_ip($remote_ip);
$args_create{spice_password} = $self->_define_spice_password($remote_ip);
$self->_pre_create_domain(%args_create);
return $base->_search_pool_clone($owner) if $from_pool;
my $domain = $self->$orig(%args_create, volatile => $volatile);
$domain->add_volume_swap( size => $swap ) if $swap;
......@@ -432,6 +447,7 @@ sub _around_create_domain {
$domain->info($owner);
$domain->display($owner) if $domain->is_active;
$domain->is_pool(1) if $add_to_pool;
return $domain;
}
......@@ -665,7 +681,7 @@ sub _check_require_base {
delete $args{start};
delete $args{remote_ip};
delete @args{'_vm','name','vm', 'memory','description','id_iso','listen_ip','spice_password'};
delete @args{'_vm','name','vm', 'memory','description','id_iso','listen_ip','spice_password','from_pool'};
confess "ERROR: Unknown arguments ".join(",",keys %args)
if keys %args;
......
......@@ -293,17 +293,31 @@
$scope.new_name_duplicated=false;
}
};
$scope.set_bool = function(field, value) {
if (value ) value=1;
else value=0;
console.log(field+" "+value);
$scope.showmachine[field]=value;
value_show = true;
if (! value) {
value_show = false;
}
$scope.add_message("Setting "+$scope.showmachine.name+" "+field+" to "+value_show);
$http.get("/machine/set/"+$scope.showmachine.id+"/"+field+"/"+value);
};
$scope.set = function(field) {
console.log(field+" = "+$scope.showmachine[field]);
$scope.add_message("Setting "+$scope.showmachine.name+" "+field+" to "
+$scope.showmachine[field]);
$http.get("/machine/set/"+$scope.showmachine.id+"/"+field+"/"+$scope.showmachine[field]);
};
$scope.add_message = function(text) {
$scope.message.push(text);
setTimeout(function () {
$scope.message = [];
}, 5000);
};
$scope.set_public = function(machineId, value) {
if (value) value=1;
else value=0;
......@@ -555,6 +569,7 @@
capacity: '1G',
allocation: '0.1G'
};
$scope.message = [];
$scope.disk_remove = [];
$scope.pending_before = 10;
// $scope.getSingleMachine();
......
......@@ -2100,7 +2100,7 @@ sub copy_machine {
my $base = $RAVADA->search_domain_by_id($id_base) or confess "I can't find domain $id_base";
my $name = ( $arg->{new_name} or $base->name."-".$USER->name );
my @create_args = ( no_pool => 1 );
my @create_args = ( from_pool => 0 );
push @create_args,( memory => $ram ) if $ram;
my @reqs;
if ($number == 1 ) {
......
......@@ -29,6 +29,7 @@ sub test_new_domain {
,vm => $vm->type
,id_owner => $USER->id
,memory => 1.5*1024*1024
,disk => 1 * 1024 * 1024
)
};
if ($freemem < 1 ) {
......@@ -64,6 +65,7 @@ sub test_new_domain_req {
,vm => $vm->type
,id_owner => $USER->id
,memory => (_check_free_memory() * 2) * 1024 * 1024
,disk => 1 * 1024 * 1024
)
};
is(''.$@,'') or return;
......
......@@ -637,7 +637,7 @@ sub wait_request {
$done_all = 0;
} elsif (!$done{$req->id}) {
$done{$req->{id}}++;
is($req->error,'') if $check_error;
is($req->error,'') or confess if $check_error;
}
}
my $post = join(".",_list_requests);
......
......@@ -42,14 +42,14 @@ sub test_clones($domain, $n_clones) {
wait_request();
$domain->pool_clones($n_clones);
is($domain->pool_clones, $n_clones);
wait_request();
wait_request(debug => 1);
is($domain->is_base,1);
is($domain->clones(), $n_clones) or exit;
is($domain->clones(is_pool => 1), $n_clones);
my $clone = $domain->clone(name => new_domain_name, user => user_admin);
my $clone = $domain->clone(name => new_domain_name, user => user_admin, from_pool => 0);
ok($clone);
is($domain->clones(), $n_clones+1);
is($domain->clones(), $n_clones+1) or exit;
is($domain->clones(is_pool => 1), $n_clones);
my $clone_f = Ravada::Front::Domain->open($clone->id);
......@@ -92,6 +92,38 @@ sub _set_clones_client_status($base) {
$sth->finish;
}
sub test_user_create($base, $n_start) {
$base->is_public(1);
my $user = create_user('kevin','carter');
my @clones = $base->clones();
wait_request();
_remove_enforce_limits();
_set_clones_client_status($base);
my $req = Ravada::Request->create_domain(
id_owner => $user->id
,start => 1
,name => new_domain_name()
,id_base => $base->id
,remote_ip => '1.2.3.4'
);
ok($req);
wait_request(debug => 0,skip => 'enforce_limits');
_remove_enforce_limits();
is($req->status,'done');
is($req->error,'');
my @clones2 = $base->clones();
is(scalar(@clones2), scalar(@clones));
my ($clone) = grep { $_->{id_owner} == $user->id } @clones2;
ok($clone,"Expecting clone that belongs to ".$user->name);
like($clone->{client_status},qr'^1.2.3.4$', $clone->{name}) or exit;
is($clone->{is_pool},1) or exit;
$user->remove();
}
sub test_user($base, $n_start) {
# TODO : test $domain->_data('comment');
$base->is_public(1);
......@@ -164,23 +196,89 @@ sub test_user($base, $n_start) {
$user_r->remove();
}
sub test_clone_regular($base) {
sub test_clone_regular($base, $add_to_pool) {
is($base->pools,1) or confess "Base ".$base->name." has no pools";
my $name = new_domain_name();
my $req = Ravada::Request->clone(
name => $name
,id_domain => $base->id
,uid => user_admin->id
,no_pool => 1
,add_to_pool => $add_to_pool
,from_pool => 0
);
ok($req) or exit;
wait_request(debug => 1);
wait_request(debug => 0);
is($req->status,'done');
is($req->error,'');
my $domain = rvd_back->search_domain($name);
ok($domain,"Expecting clone from ".$base->name);
ok($domain,"Expecting clone from ".$base->name) or exit;
is($domain->is_pool, $add_to_pool);
$domain->pools(1);
# copy the clone
my $name_clone = new_domain_name();
$req = Ravada::Request->clone(
name => $name_clone
,id_domain => $domain->id
,uid => user_admin->id
,add_to_pool => $add_to_pool
,from_pool => 0
);
ok($req) or exit;
wait_request(debug => 0);
is($req->status,'done');
is($req->error,'') or exit;
my $clone = rvd_back->search_domain($name_clone);
ok($clone,"Expecting clone from ".$name) or exit;
is($clone->is_pool, $add_to_pool) or exit;
$domain->remove(user_admin);
}
sub test_no_pool($vm) {
my $base = create_domain($vm);
$base->prepare_base(user_admin);
my $clone_name = new_domain_name();
my $clone;
eval {
$clone = rvd_back->create_domain(
id_base => $base->id
,name => $clone_name
,add_to_pool => 1
,id_owner => user_admin->id
);
};
like($@,qr(this base has no pools)i);
ok(!$clone);
my $clone2 = rvd_back->search_domain($clone_name);
ok(!$clone2);
$clone->remove(user_admin) if $clone;
$clone_name = new_domain_name();
eval {
$clone = $base->clone(
name => $clone_name
,user => user_admin
,add_to_pool => 1
);
};
like($@,qr(this base has no pools)i);
$clone2 = rvd_back->search_domain($clone_name);
ok(!$clone);
ok(!$clone2);
$clone->remove(user_admin) if $clone;
$base->remove(user_admin);
wait_request( debug => 0);
}
init();
......@@ -205,6 +303,8 @@ for my $vm_name (reverse vm_names() ) {
test_duplicate_req();
test_no_pool($vm);
my $domain = create_domain($vm);
my $n_clones = 6;
......@@ -216,8 +316,13 @@ for my $vm_name (reverse vm_names() ) {
test_active($domain, $n_start);
test_user($domain, $n_start);
test_user_create($domain, $n_start);
# add the clone to the pool => 1 <=
test_clone_regular($domain , 1);
# do not add the clone to the pool
test_clone_regular($domain, 0);
test_clone_regular($domain);
_remove_enforce_limits();
}
}
......
......@@ -275,9 +275,10 @@
</tr>
<tr ng-hide="hide_clones" ng-repeat="child in machine.childs | orderBy : orderParam">
<td class="lgMachName">
&nbsp;<i title="[cloned]" class="fa fa-fw fa-long-arrow-right"
&nbsp;<i title="[cloned]" class="fa fa-fw fa-long-arrow-right"
style="color:white"></i>
<i class="far fa-clone" title="Cloned"></i>
<i class="far fa-clone" title="Cloned" ng-hide="child.is_pool"></i>
<i class="far fa-file-alt" title="Pool" ng-show="child.is_pool"></i>
<a align="right" href="/machine/manage/{{child.id}}.html"
ng-class="{disabled: !child.can_manage}"
title ="Manage machine"><i
......
......@@ -22,9 +22,7 @@
<%=l 'Settings' %>
</h2>
% }
</div> <!-- del panel heading-->
%= include 'main/show_requests'
<div>
</div>
<div class="card-body">
<div class="row">
<div class="col-2">
......
% if (scalar @$errors) {
<div class="card text-white bg-danger">
<div class="card-header">
......@@ -10,6 +11,7 @@
</div>
% }
<div class="card_body">
% if ($USER->can_change_settings($domain->id)) {
<div class="tab-content" id="v-pills-tabContent">
<div class="tab-pane fade show active" id="v-pills-description" role="tabpanel" aria-labelledby="v-pills-description-tab">
......@@ -93,4 +95,15 @@
%= include 'main/vm_pool'
</div>
% }
<div ng-show="message.length" class="alert alert-light border-primary text-primary mt-4">
<div ng-repeat="text in message">
{{text}}
</div>
</div>
<div class="mt-4"> <!-- del panel heading-->
%= include 'main/show_requests'
</div>
</div>
<div class="card-body">
<div class="row">
<div class="col-md-1"></div>
<div class="col-lg-2 mt-2">Action</div>
<div class="col-lg-2 mt-2">
Port
</div>
......@@ -16,7 +16,9 @@
</div>
<div class="row">