From da7455266023274dd1aca1fb9854a6bbe8985f67 Mon Sep 17 00:00:00 2001 From: Oliver Tappe Date: Sat, 24 May 2008 16:25:22 +0000 Subject: * heavily redesigned and improved the checking of attributes: instead of dying on the first error, we now collect all errors and return them. The caller can now decide what to do - die or just print a warning or whatever. * slxconfig now dies with the list of all attribute problems if there were any * slxconfig-demuxer prints warnings for all attribute problems that were found (checking for each system & client in turn) * adjusted desktop plugin to API changes concerning the attribute checks git-svn-id: http://svn.openslx.org/svn/openslx/openslx/trunk@1796 95ad53e4-c205-0410-b2fa-d234c58c8868 --- config-db/OpenSLX/AttributeRoster.pm | 57 +++++++++++++--------- config-db/slxconfig | 16 +++++- config-db/slxconfig-demuxer | 32 ++++++------ os-plugins/OpenSLX/OSPlugin/Base.pm | 5 +- os-plugins/OpenSLX/OSPlugin/Engine.pm | 41 +++++++++++++--- .../plugins/desktop/OpenSLX/OSPlugin/desktop.pm | 32 ++++++------ 6 files changed, 121 insertions(+), 62 deletions(-) diff --git a/config-db/OpenSLX/AttributeRoster.pm b/config-db/OpenSLX/AttributeRoster.pm index 61825f77..de7b5ced 100644 --- a/config-db/OpenSLX/AttributeRoster.pm +++ b/config-db/OpenSLX/AttributeRoster.pm @@ -498,22 +498,26 @@ sub getClientAttrs keys %AttributeInfo } -=item C +=item C Checks if the given stage3 attribute values are allowed (and make sense). -If all values are ok, this method returns 1 - if not, it dies with an -appropriate message. + +This method returns an array-ref of problems found. If there were no problems, +this methods returns undef. =cut -sub checkValues +sub findProblematicValues { - my $class = shift; - my $stage3Attrs = shift || {}; - my $vendorOSName = shift; + my $class = shift; + my $stage3Attrs = shift || {}; + my $vendorOSName = shift; + my $installedPlugins = shift; $class->_init() if !%AttributeInfo; + my @problems; + my %attrsByPlugin; foreach my $key (sort keys %{$stage3Attrs}) { my $value = $stage3Attrs->{$key}; @@ -531,30 +535,39 @@ sub checkValues || die _tr('attribute "%s" is unknown!', $key); my $regex = $attrInfo->{content_regex}; if ($regex && $value !~ $regex) { - die _tr( + push @problems, _tr( "the value '%s' for attribute %s is not allowed.\nAllowed values are: %s", $value, $key, $attrInfo->{content_descr} ); } } - # if no vendorOS-name has been provided, we can't initialize any plugins, - # so we are done - return 1 if !$vendorOSName; - - # now give each plugin a chance to check it's own attributes by itself - foreach my $pluginName (sort keys %attrsByPlugin) { - # create & start OSPlugin-engine for vendor-OS and current plugin - my $engine = OpenSLX::OSPlugin::Engine->new; - $engine->initialize($pluginName, $vendorOSName); - if (!$engine->{'plugin-path'}) { - warn _tr('unable to create engine for plugin "%s"!', $pluginName); - next; + # if no vendorOS-name has been provided or there are no plugins installed, + # we can't do any further checks + if ($vendorOSName && $installedPlugins) { + # now give each installed plugin a chance to check it's own attributes + # by itself + foreach my $pluginInfo (sort @$installedPlugins) { + my $pluginName = $pluginInfo->{plugin_name}; + vlog 2, "checking attrs of plugin: $pluginName\n"; + # create & start OSPlugin-engine for vendor-OS and current plugin + my $engine = OpenSLX::OSPlugin::Engine->new; + $engine->initialize($pluginName, $vendorOSName); + if (!$engine->{'plugin-path'}) { + warn _tr( + 'unable to create engine for plugin "%s"!', $pluginName + ); + next; + } + $engine->checkStage3AttrValues( + $attrsByPlugin{$pluginName}, \@problems + ); } - $engine->checkStage3AttrValues($attrsByPlugin{$pluginName}); } - return 1; + return if !@problems; + + return \@problems; } 1; diff --git a/config-db/slxconfig b/config-db/slxconfig index d4656e5e..02029a81 100755 --- a/config-db/slxconfig +++ b/config-db/slxconfig @@ -269,12 +269,24 @@ sub checkGivenStage3Attrs my $stage3Attrs = shift; my $vendorOSID = shift; + my $attrProblems; + if ($vendorOSID) { my $vendorOS = $openslxDB->fetchVendorOSByID($vendorOSID); - OpenSLX::AttributeRoster->checkValues($stage3Attrs, $vendorOS->{name}); + my @installedPlugins = $openslxDB->fetchInstalledPlugins($vendorOSID); + $attrProblems = OpenSLX::AttributeRoster->findProblematicValues( + $stage3Attrs, $vendorOS->{name}, \@installedPlugins + ); } else { - OpenSLX::AttributeRoster->checkValues($stage3Attrs); + $attrProblems = OpenSLX::AttributeRoster->findProblematicValues( + $stage3Attrs + ); + } + + if ($attrProblems) { + my $complaint = join "\n", @$attrProblems; + die $complaint; } return 1; diff --git a/config-db/slxconfig-demuxer b/config-db/slxconfig-demuxer index 6eb613a3..5d0928ae 100755 --- a/config-db/slxconfig-demuxer +++ b/config-db/slxconfig-demuxer @@ -641,14 +641,14 @@ sub writeClientConfigurationsForSystem $externalSystemID, $buildPath, $externalClientName ); - my $attrsOK = eval { - OpenSLX::AttributeRoster->checkValues( - $client->{attrs}, $info->{'vendor-os'}->{name} - ); - 1; - }; - if (!$attrsOK && $@) { - warn $@; + my $attrProblems = OpenSLX::AttributeRoster->findProblematicValues( + $client->{attrs}, $info->{'vendor-os'}->{name}, + $info->{'installed-plugins'} + ); + if ($attrProblems) { + my $complaint = join "\n", @$attrProblems; + $complaint =~ s{^}{client $client->{name}: }gms; + warn $complaint; } writeAttributesToFile($client, $attrFile); @@ -746,14 +746,14 @@ sub writeSystemConfiguration ) ); - my $attrsOK = eval { - OpenSLX::AttributeRoster->checkValues( - $info->{attrs}, $info->{'vendor-os'}->{name} - ); - 1; - }; - if (!$attrsOK && $@) { - warn $@; + my $attrProblems = OpenSLX::AttributeRoster->findProblematicValues( + $info->{attrs}, $info->{'vendor-os'}->{name}, + $info->{'installed-plugins'} + ); + if ($attrProblems) { + my $complaint = join "\n", @$attrProblems; + $complaint =~ s{^}{system $info->{name}: }gms; + warn $complaint; } my $attrFile = "$buildPath/initramfs/machine-setup"; diff --git a/os-plugins/OpenSLX/OSPlugin/Base.pm b/os-plugins/OpenSLX/OSPlugin/Base.pm index 8c314ff5..a07ec125 100644 --- a/os-plugins/OpenSLX/OSPlugin/Base.pm +++ b/os-plugins/OpenSLX/OSPlugin/Base.pm @@ -187,7 +187,8 @@ sub getDefaultAttrsForVendorOS Checks if the stage3 values given in B<$stage3Attrs> are allowed and makes sense. -If all values are ok, this method returns 1 - if not, it dies with an appropriate message. +This method returns an array-ref of problems found. If there were no problems, +this methods returns undef. Plugins may override this implementation to do checks that for instance look at the stage1 vendor-OS-attributes given in B<$vendorOSAttrs>. @@ -206,7 +207,7 @@ sub checkStage3AttrValues # this default implementation does no further checks (thus relying on the # attributte regex check that is done in the AttributeRoster) - return 1; + return; } =back diff --git a/os-plugins/OpenSLX/OSPlugin/Engine.pm b/os-plugins/OpenSLX/OSPlugin/Engine.pm index ada0af79..75ba3154 100644 --- a/os-plugins/OpenSLX/OSPlugin/Engine.pm +++ b/os-plugins/OpenSLX/OSPlugin/Engine.pm @@ -157,8 +157,8 @@ sub installPlugin # as the attrs may be changed by the plugin during installation, we # have to find a way to pass them back to this process (remember; # installation takes place in a forked process in order to do a chroot). - # We simply serialize the attributes into a temp and deserialize it - # in the calling process. + # We simply serialize the attributes into a temp file and deserialize + # it in the calling process. my $serializedAttrsFile = "$self->{'plugin-temp-path'}/serialized-attrs"; my $chrootedSerializedAttrsFile @@ -458,8 +458,8 @@ install/remove a plugin into/from a vendor-OS: Checks if the stage3 values given in B<$stage3Attrs> are allowed and make sense. -If all values are ok, this method returns 1 - if not, it dies with an -appropriate message. +If all values are ok, this method returns 1 - if not, it extends the given +problems array-ref with the problems that were found (and returns undef). This method chroots into the vendor-OS and then asks the plugin itself to check the attributes. @@ -470,16 +470,45 @@ sub checkStage3AttrValues { my $self = shift; my $stage3Attrs = shift; - + my $problemsOut = shift; + + # as the attrs may be changed by the plugin during installation, we + # have to find a way to pass them back to this process (remember; + # installation takes place in a forked process in order to do a chroot). + # We simply serialize the attributes into a temp file and deserialize + # it in the calling process. + my $serializedProblemsFile + = "$self->{'plugin-temp-path'}/serialized-problems"; + my $chrootedSerializedProblemsFile + = "$self->{'chrooted-plugin-temp-path'}/serialized-problems"; + + mkpath([ $self->{'plugin-repo-path'}, $self->{'plugin-temp-path'} ]); + + # HACK: do a dummy serialization here in order to get Storable + # completely loaded (otherwise it will complain in the chroot about + # missing modules). + store [], $serializedProblemsFile; + $self->_callChrootedFunctionForPlugin( sub { # let plugin check by itself - $self->{plugin}->checkStage3AttrValues( + my $problems = $self->{plugin}->checkStage3AttrValues( $stage3Attrs, $self->{'vendorOS-attrs'} ); + + # serialize list of problems (executed inside chroot) + store($problems, $chrootedSerializedProblemsFile) if $problems; } ); + # now retrieve (deserialize) the found problems and pass them on + my $problems = retrieve $serializedProblemsFile; + rmtree([ $self->{'plugin-temp-path'} ]); + if ($problems && ref($problems) eq 'ARRAY' && @$problems) { + push @$problemsOut, @$problems; + return; + } + return 1; } diff --git a/os-plugins/plugins/desktop/OpenSLX/OSPlugin/desktop.pm b/os-plugins/plugins/desktop/OpenSLX/OSPlugin/desktop.pm index 61787309..bfaee875 100644 --- a/os-plugins/plugins/desktop/OpenSLX/OSPlugin/desktop.pm +++ b/os-plugins/plugins/desktop/OpenSLX/OSPlugin/desktop.pm @@ -206,17 +206,19 @@ sub checkStage3AttrValues my $stage3Attrs = shift; my $vendorOSAttrs = shift; + my @problems; + my $manager = $stage3Attrs->{'desktop::manager'} || ''; if ($manager eq 'kdm') { if (!defined $vendorOSAttrs->{'desktop::kdm'}) { if (!$self->{distro}->isKDMInstalled()) { - die _tr( + push @problems, _tr( "KDM is not installed in vendor-OS, so using it as desktop manager wouldn't work!" ); } } elsif ($vendorOSAttrs->{'desktop::kdm'} == 0) { - die _tr( + push @problems, _tr( "desktop::kdm is 0, so using KDM as desktop manager is not allowed for this vendor-OS!" ); } @@ -224,13 +226,13 @@ sub checkStage3AttrValues elsif ($manager eq 'gdm') { if (!defined $vendorOSAttrs->{'desktop::gdm'}) { if (!$self->{distro}->isGDMInstalled()) { - die _tr( + push @problems, _tr( "GDM is not installed in vendor-OS, so using it as desktop manager wouldn't work!" ); } } elsif ($vendorOSAttrs->{'desktop::gdm'} == 0) { - die _tr( + push @problems, _tr( "desktop::gdm is 0, so using GDM as desktop manager is not allowed for this vendor-OS!" ); } @@ -238,13 +240,13 @@ sub checkStage3AttrValues elsif ($manager eq 'xdm') { if (!defined $vendorOSAttrs->{'desktop::xdm'}) { if (!$self->{distro}->isXDMInstalled()) { - die _tr( + push @problems, _tr( "XDM is not installed in vendor-OS, so using it as desktop manager wouldn't work!" ); } } elsif ($vendorOSAttrs->{'desktop::xdm'} == 0) { - die _tr( + push @problems, _tr( "desktop::xdm is 0, so using XDM as desktop manager is not allowed for this vendor-OS!" ); } @@ -254,13 +256,13 @@ sub checkStage3AttrValues if ($kind eq 'kde') { if (!defined $vendorOSAttrs->{'desktop::kde'}) { if (!$self->{distro}->isKDEInstalled()) { - die _tr( + push @problems, _tr( "KDE is not installed in vendor-OS, so using it as desktop kind wouldn't work!" ); } } elsif ($vendorOSAttrs->{'desktop::kde'} == 0) { - die _tr( + push @problems, _tr( "desktop::kde is 0, so using KDE as desktop kind is not allowed for this vendor-OS!" ); } @@ -268,13 +270,13 @@ sub checkStage3AttrValues elsif ($kind eq 'gnome') { if (!defined $vendorOSAttrs->{'desktop::gnome'}) { if (!$self->{distro}->isGNOMEInstalled()) { - die _tr( + push @problems, _tr( "GNOME is not installed in vendor-OS, so using it as desktop kind wouldn't work!" ); } } elsif ($vendorOSAttrs->{'desktop::gnome'} == 0) { - die _tr( + push @problems, _tr( "desktop::gnome is 0, so using GNOME as desktop kind is not allowed for this vendor-OS!" ); } @@ -282,13 +284,13 @@ sub checkStage3AttrValues elsif ($kind eq 'xfce') { if (!defined $vendorOSAttrs->{'desktop::xfce'}) { if (!$self->{distro}->isXFCEInstalled()) { - die _tr( + push @problems, _tr( "XFCE is not installed in vendor-OS, so using it as desktop kind wouldn't work!" ); } } elsif ($vendorOSAttrs->{'desktop::xfce'} == 0) { - die _tr( + push @problems, _tr( "desktop::xfce is 0, so using XFCE as desktop kind is not allowed for this vendor-OS!" ); } @@ -298,13 +300,15 @@ sub checkStage3AttrValues = split ',', $vendorOSAttrs->{'desktop::supported_themes'} || ''; my $theme = $stage3Attrs->{'desktop::theme'}; if (defined $theme && !grep { $_ eq $theme } @supportedThemes) { - die _tr( + push @problems, _tr( "desktop::theme '%s' does not refer to a supported theme!\nSupported themes are: %s", $theme, $vendorOSAttrs->{'desktop::supported_themes'} || '' ); } - return 1; + return if !@problems; + + return \@problems; } sub installationPhase -- cgit v1.2.3-55-g7522