Skip to content

Commit

Permalink
Merge pull request #5917 from Martchus/check-ovs-service
Browse files Browse the repository at this point in the history
Avoid incomplete jobs if Open vSwitch related service is not running
  • Loading branch information
mergify[bot] authored Sep 11, 2024
2 parents 54de77b + eeb2e67 commit c2016a2
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 1 deletion.
16 changes: 15 additions & 1 deletion lib/OpenQA/Worker.pm
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ sub new ($class, $cli_options) {
$self->{_pool_directory_lock_fd} = undef;
$self->{_shall_terminate} = 0;
$self->{_finishing_off} = undef;
$self->{_ovs_dbus_service_name} = $ENV{OVS_DBUS_SERVICE_NAME} // 'org.opensuse.os_autoinst.switch';

return $self;
}
Expand Down Expand Up @@ -601,6 +602,14 @@ sub is_qemu_running ($self) {
return undef;
}

sub is_ovs_dbus_service_running ($self) {
eval { defined &Net::DBus::system or require Net::DBus };
return 0 if $@;
my $bus = ($self->{_system_dbus} //= Net::DBus->system(nomainloop => 1));
my $service = eval { defined $bus->get_service('org.opensuse.os_autoinst.switch') };
return !$@ && $service;
}

# checks whether the worker is available
# note: This is used to check certain error conditions *before* starting a job to prevent incompletes and
# being able to propagate the brokenness to the web UIs.
Expand All @@ -623,7 +632,12 @@ sub check_availability ($self) {
}

# auto-detect worker address if not specified explicitly
return 'Unable to determine worker address (WORKER_HOSTNAME)' unless $self->settings->auto_detect_worker_address;
my $settings = $self->settings;
return 'Unable to determine worker address (WORKER_HOSTNAME)' unless $settings->auto_detect_worker_address;

# check org.opensuse.os_autoinst.switch if it is a MM-capable worker slot
return "D-Bus service '$self->{_ovs_dbus_service_name}' is not running"
if $settings->has_class('tap') && !$self->is_ovs_dbus_service_running;

return undef;
}
Expand Down
5 changes: 5 additions & 0 deletions lib/OpenQA/Worker/Settings.pm
Original file line number Diff line number Diff line change
Expand Up @@ -132,4 +132,9 @@ sub is_local_worker ($self) {
return $self->{_local} = 1;
}

sub has_class ($self, $worker_class) {
my $c = $self->{_worker_classes} //= {map { $_ => 1 } split(',', $self->global_settings->{WORKER_CLASS} // '')};
return exists $c->{$worker_class};
}

1;
22 changes: 22 additions & 0 deletions t/24-worker-overall.t
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,15 @@ my $load_avg_file = simulate_load('0.93 0.95 10.25 2/2207 1212', 'worker-overall
use Mojo::Base -base;
has availability_error => 'Cache service info error: Connection refused';
}
{
package Test::FakeDBus; # uncoverable statement count:2
use Mojo::Base -base, -signatures;
has mock_service_value => 1;
sub get_service ($self, $service_name) { $self->mock_service_value }
}

my $dbus_mock = Test::MockModule->new('Net::DBus', no_auto => 1);
$dbus_mock->define(system => sub (@) { Test::FakeDBus->new });
my $cache_service_client_mock = Test::MockModule->new('OpenQA::CacheService::Client');
$cache_service_client_mock->redefine(info => sub { Test::FakeCacheServiceClientInfo->new });

Expand Down Expand Up @@ -546,6 +554,20 @@ subtest 'checking worker address' => sub {
is $global_settings->{WORKER_HOSTNAME}, 'localhost', '"localhost" assumed as WORKER_HOSTNAME for local worker';
};

subtest 'check availability of Open vSwitch related D-Bus service' => sub {
delete $worker->settings->{_worker_classes};
$worker->settings->global_settings->{WORKER_CLASS} = 'foo,tap,bar';
ok $worker->settings->has_class('tap'), 'worker has tap class';
is $worker->check_availability, undef, 'worker considered available if D-Bus service available';

$worker->{_system_dbus}->mock_service_value(undef);
like $worker->check_availability, qr/D-Bus/, 'worker considered broken if D-Bus service not available';

delete $worker->settings->{_worker_classes};
$worker->settings->global_settings->{WORKER_CLASS} = 'foo,bar';
is $worker->check_availability, undef, 'worker considered always available if not a tap worker';
};

subtest 'handle client status changes' => sub {
my $fake_client = OpenQA::Worker::WebUIConnection->new('some-host', {apikey => 'foo', apisecret => 'bar'});
my $fake_client_2
Expand Down

0 comments on commit c2016a2

Please sign in to comment.