Skip to content

Commit

Permalink
From: Chet Burgess <chester.burgess@ticketmaster.com>
Browse files Browse the repository at this point in the history 
Attached is a patch that converts all usage of reporting errors via c_failure
to using the c->error call instead. This results in a nice stack trace like
error output that shows the flow of errors.

The main spine program still uses the presence of c_failure to determine if
there was some type of parsing failure in creating of the Spine::Data object.
The constructor for Spine::Data handles setting c_failure if a failure has
occurred during object creation. 

This patch also adds significant style cleanup to all files affected by the
core of the patch.

This patch closes issues 29 and 23.

Minor typo fixes by Phil Dibowitz <phil@ipom.com>

Signed-off-by: Phil Dibowitz <phil@ipom.com>
  • Loading branch information
pdibowitz committed Aug 5, 2008
1 parent eddc629 commit c824472
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 45 deletions.
38 changes: 24 additions & 14 deletions lib/Spine/Data.pm
View file
Expand Up @@ -69,7 +69,8 @@ sub new {

if (not defined($data_object->{c_release}))
{
print STDERR "Spine::Data::new(): we require the config release number!";
print STDERR 'Spine::Data::new(): we require the config release '
. 'number!';
return undef;
}

Expand All @@ -88,8 +89,9 @@ sub new {
PARSE/complete));

# XXX Right now, the driver script handles error reporting
$data_object->_data();

unless ($data_object->_data() == SPINE_SUCCESS) {
$data_object->{c_failure} = 1;
}
return $data_object;
}

Expand All @@ -103,13 +105,15 @@ sub _data
chdir($self->{c_croot});

unless ($self->populate() == SPINE_SUCCESS) {
$self->{c_failure} = 'Failure to populate: ' . $self->{c_failure};
$self->error('Failure to run populate()!', 'crit');
$rc = SPINE_FAILURE;
}

unless ($rc == SPINE_SUCCESS and $self->parse() == SPINE_SUCCESS) {
$self->{c_failure} = 'Failure to parse: ' . $self->{c_failure};
$rc = SPINE_FAILURE;
unless ($rc != SPINE_SUCCESS) {
unless ($self->parse() == SPINE_SUCCESS) {
$self->error('Failure to run parse()!', 'crit');
$rc = SPINE_FAILURE;
}
}

# Clean up our Template instance to make certain that we don't have any
Expand Down Expand Up @@ -166,7 +170,8 @@ sub populate
# errors + failures encountered by plugins
if ($rc != 0) {
$DATA_POPULATED = SPINE_FAILURE;
$self->{c_failure} = "DISCOVERY/populate failed!";
$self->error('DISCOVERY/populate: Failed to run at least one hook!',
'crit');
return SPINE_FAILURE;
}

Expand All @@ -186,7 +191,8 @@ sub populate

unless ($rc == 0) {
$DATA_POPULATED = SPINE_FAILURE;
$self->{c_failure} = "DISCOVERY/policy-selection failed!";
$self->error('DISCOVERY/policy-selection: Failed to run at least one'
. 'hook!', 'crit');
return SPINE_FAILURE;
}

Expand All @@ -209,7 +215,7 @@ sub parse
# Make sure our discovery phases has been run first since we need a bunch
# of that info for out parsing. Most notably the descend order.
unless ($self->populate() == SPINE_SUCCESS) {
$self->{c_failure} = "Can't parse, runtime discovery failed somehow!";
$self->error('PARSE: failed to run populate()!', 'crit');
goto parse_failure;
}

Expand All @@ -223,7 +229,8 @@ sub parse
my $rc = $point->run_hooks($self);

if ($rc != 0) {
$self->{c_failure} = "PARSE/initialize failed!";
$self->error('PARSE/initialize: Failed to run at least one hook!',
'crit');
goto parse_failure;
}

Expand All @@ -237,7 +244,8 @@ sub parse
$rc = $point->run_hooks($self);

if ($rc != 0) {
$self->{c_failure} = "Failed to run at least one PARSE/pre-descent hook";
$self->error('PARSE/pre-descent: Failed to run at least one hook!',
'crit');
goto parse_failure;
}

Expand All @@ -251,7 +259,8 @@ sub parse
$rc = $point->run_hooks($self);

if ($rc != 0) {
$self->{c_failure} = "Failed to run at least one PARSE/post-descent hook";
$self->error('PARSE/post-descent: Failed to run at least one '
. 'hook!', 'crit');
goto parse_failure;
}

Expand All @@ -263,7 +272,8 @@ sub parse
$rc = $point->run_hooks($self);

if ($rc != 0) {
$self->{c_failure} = "Failed to run at least one PARSE/complete hook";
$self->error('PARSE/complete: Failed to run at least one hook!',
'crit');
goto parse_failure;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/Spine/Plugin/RPMPackageManager.pm
View file
Expand Up @@ -91,7 +91,7 @@ sub apt_exec
{
unless (Spine::Util::mkdir_p(catfile($overlay, $dir)))
{
$c->{c_failure} = "Failed to create directory \"$dir\"";
$c->error("Failed to create directory $dir", 'err');
return PLUGIN_ERROR;
}
}
Expand Down
75 changes: 61 additions & 14 deletions lib/Spine/Plugin/SystemInfo.pm
View file
Expand Up @@ -71,7 +71,6 @@ sub get_sysinfo
{
my $c = shift;
my ($ip_address, $bcast, $netmask, $netcard);
my $ifconfig = $c->getval('ifconfig_bin');
my $iface = $c->getval('primary_iface');

$c->cprint('retrieving system information', 3);
Expand All @@ -87,20 +86,55 @@ sub get_sysinfo
my %devs = @{$c->getvals('network_device_map')};

# We walk the PCI bus to determine which network card we have
open(PCI, '/sbin/lspci |');
while (<PCI>) {
my $lspci = $c->getval('lspci_bin') || qq(/sbin/lspci);
my $fh;
if (-f $lspci and -x $lspci)
{
$fh = new IO::File("$lspci |");
}
else
{
$c->error("$lspci not found or not executable!", 'err');
return PLUGIN_FATAL;
}

if (not $fh)
{
$c->error("Failed to run $lspci: $!", 'err');
return PLUGIN_FATAL;
}

foreach my $line (<$fh>)
{
next unless m/Ethernet/;
# FIXME This is kind of dumb. We don't provide any kind of
# interface to driver mapping and we really should
while (my ($re, $card) = each(%devs)) {
$netcard = $card if m/$re/;
}
}
$fh->close();
$netcard = 'unknown' unless $netcard;
close (PCI);

my $cmd = $ifconfig . ' eth' . $iface;
foreach my $line (`$cmd`)
my $ifconfig = $c->getval('ifconfig_bin') || qq(/sbin/ifconfig);
my $cmd = $ifconfig . ' eth' . $iface;
if (-f $ifconfig and -x $ifconfig)
{
$fh->open("$cmd |");
}
else
{
$c->error("$ifconfig not found or not executable!", 'err');
return PLUGIN_FATAL;
}

if (not $fh)
{
$c->error("Failed to run $cmd: $!", 'err');
return PLUGIN_FATAL;
}

foreach my $line (<$fh>)
{
if ($line =~
m/
Expand All @@ -114,6 +148,7 @@ sub get_sysinfo
$netmask = $3;
}
}
$fh->close();
}

$c->{c_platform} = $platform;
Expand All @@ -139,7 +174,7 @@ sub get_netinfo
my $nobj = new NetAddr::IP($c->getval('c_ip_address'));

unless (defined($nobj)) {
$c->{c_failure} = "No IP address found for $c->{c_hostname_f}";
$c->error("No IP address found for $c->{c_hostname_f}", 'err');
return PLUGIN_FATAL;
}

Expand Down Expand Up @@ -169,7 +204,7 @@ sub get_netinfo

unless (defined $c->{c_subnet})
{
$c->{c_failure} = "network for $c->{c_ip_address} is not defined";
$c->error("network for $c->{c_ip_address} is not defined", 'err');
return PLUGIN_FATAL;
}

Expand All @@ -194,6 +229,7 @@ sub is_virtual
{

my $c = shift;
my $xen_indicator = $c->getval('xen_indicator') || qq(/proc/xen);

$c->{c_is_virtual} = 0;

Expand All @@ -207,11 +243,22 @@ sub is_virtual
else
{
# We actually have to parse the lspci output now.
my $fh = new IO::File('/sbin/lspci -n |');
my $lspci = $c->getval('lspci_bin') || qq(/sbin/lspci);
my $fh;

if (-f $lspci and -x $lspci)
{
$fh = new IO::File("$lspci -n |");
}
else
{
$c->error("$lspci not found or not executable!", 'err');
return PLUGIN_FATAL;
}

if (not $fh)
{
$c->{c_failure} = "Failed to run /sbin/lspci: $!";
$c->error("Failed to run $lspci: $!", 'err');
return PLUGIN_FATAL;
}

Expand Down Expand Up @@ -409,7 +456,7 @@ sub get_num_procs
my $nprocs = 0;

unless (defined($cpuinfo)) {
$c->{c_failure} = 'Failed to open /proc/cpuinfo';
$c->error('Failed to open /proc/cpuinfo', 'err');
return PLUGIN_FATAL;
}

Expand Down Expand Up @@ -439,17 +486,17 @@ sub get_hardware_platform

if (-f $dmidecode and -x $dmidecode)
{
$fh = new IO::File('$dmidecode |');
$fh = new IO::File("$dmidecode |");
}
else
{
$c->{c_failure} = "$dmidecode not found or not executable.";
$c->error("$dmidecode not found or not executable!" , 'err');
return PLUGIN_FATAL;
}

if (not $fh)
{
$c->{c_failure} = "Failed to run dmidecode: $!";
$c->error("Failed to run $dmidecode: $!", 'err');
return PLUGIN_FATAL;
}

Expand Down
41 changes: 25 additions & 16 deletions spine
View file
Expand Up @@ -152,7 +152,8 @@ if (not defined($CONFIG))

if (not -f DEFAULT_CONFIGFILE)
{
print STDERR "No config file found at default $cfile. Using hardcoded defaults.\n";
print STDERR "No config file found at default $cfile. "
. "Using hardcoded defaults.\n";

$CONFIG = DEFAULT_CONFIG;
}
Expand Down Expand Up @@ -394,23 +395,25 @@ my $c = Spine::Data->new(hostname => $hostname,
#
# rtilder Tue Jun 27 12:23:39 PDT 2006
if (not defined($c)) {
print STDERR "spine initialization: Errors encountered parsing data tree.\n";
print STDERR 'spine initialization: Errors encountered parsing '
. "data tree.\n";
goto failure;
}

# Our data object will contain the c_failure key if we
# have had a critical parsing error.
if ($c->getval('c_failure'))
{
print STDERR "spine initialization: Errors encountered parsing data tree.\n";
$c->error("failure: " . $c->getval('c_failure'), 'crit');
print STDERR 'spine initialization: Errors encountered parsing '
. "data tree.\n";
goto failure;
}

# A quick sanity check, just to be sure
if ($c->{c_release} != $source->release())
{
print STDERR "Requested release \"$c->{c_release}\" doesn't match release parsed\n";
print STDERR "Requested release \"$c->{c_release}\" doesn't match "
. "release parsed\n";
print STDERR 'from config source "', $source->release(), "\"\n";
goto failure;
}
Expand All @@ -426,14 +429,14 @@ $state->data($c);
my $lock_fh = get_lock();
unless ($lock_fh)
{
$c->error("could not get exclusive lock", 'crit');
$c->error('could not get exclusive lock', 'crit');
goto failure;
}

PHASE: foreach my $pork ( ( ['PREPARE', "Failed to prepare for emission!"],
['EMIT', "Failed to emit configuration!"],
['APPLY', "Failed to apply configuration!"],
['CLEAN', "Failed to clean up!"] ) ) {
PHASE: foreach my $pork ( ( ['PREPARE', 'Failed to prepare for emission!'],
['EMIT', 'Failed to emit configuration!'],
['APPLY', 'Failed to apply configuration!'],
['CLEAN', 'Failed to clean up!'] ) ) {
my ($phase, $msg) = @{$pork};

my $point = $registry->get_hook_point($phase);
Expand Down Expand Up @@ -663,7 +666,8 @@ sub get_source
Path => $options->{croot});

if (not defined($source)) {
print STDERR "Failed to initialize config root from command line: $Spine::ConfigSource::FileSystem::ERROR\n";
print STDERR 'Failed to initialize config root from command line:'
. " $Spine::ConfigSource::FileSystem::ERROR\n";
print STDERR "Falling back to configuration file settings.\n";
}
}
Expand All @@ -685,7 +689,8 @@ sub get_source
eval "require $source_type";

if ($@) {
print STDERR "Failed to load the $source_type configuration source: $@\n";
print STDERR "Failed to load the $source_type configuration "
. "source: $@\n";
return undef;
}

Expand All @@ -711,7 +716,8 @@ sub get_source
no strict 'refs';
eval { $foo = ${$source_type . '::ERROR'} };

print STDERR "Failure to initialize configuration source $source_type: $foo\n";
print STDERR 'Failure to initialize configuration source '
. "$source_type: $foo\n";
use strict 'refs';
return undef;
}
Expand Down Expand Up @@ -758,20 +764,23 @@ sub get_release
}

if (not defined($release)) {
print STDERR "Failed while checking for newer configuration releases: $source->{error}\n";
print STDERR 'Failed while checking for newer configuration '
. "releases: $source->{error}\n";
goto release_failed;
}

if ($release) {
if (not defined($source->retrieve($release))) {
print STDERR "Failed to retrieve latest configuration release: $source->{error}\n";
print STDERR 'Failed to retrieve latest configuration release: '
. "$source->{error}\n";
goto release_failed;
}
}

if (not defined($source->config_root()))
{
print STDERR "Couldn't mount or find the configuration source: $source->{error}\n";
print STDERR 'Couldn\'t mount or find the configuration source: '
. "$source->{error}\n";
goto release_failed;
}

Expand Down

0 comments on commit c824472

Please sign in to comment.