On 03/31/2010 10:04 PM, Nathan Kinder wrote:
On 03/31/2010 09:52 PM, Endi Sukma Dewata wrote:
----- "Nathan Kinder"<nkinder@xxxxxxxxxx> wrote:
The admin server CGIs parse the start-slapd scripts to determine the
DS instance names. A recent format change to start-slapd caused this
parsing to break. These patches make the instance name easier to
parse from the script. One patch is for DS itself and one is for the
Admin Server.
ack - much better
Thanks, but I need to nak my own patch since it's imcomplete.
This isn't going to work well when upgrading an instance. We don't
regenerate the start-slapd script when running 'setup-ds.pl -u'. This
means that an upgraded instance will not work properly with any of the
admin server CGIs that need to parse the instance name from
start-slapd. This issue is already a problem not related to this patch,
but it seems we should fix it along with this issue.
I suppose the right thing to do is to make 'setup-ds.pl -u' generate a
new start-slapd script for the existing instances as well as a new
instance specific initconfig script if one doesn't exist. I think we
need to avoid wiping out an existing instance specific sysconfig script
since it may have been modified by an admin to add other stuff to it
(like KRB5_KTNAME for Kerberos). Do you see any problems with this
approach?
I've attached a new set of patches that implements the solution outlined
above.
-NGK
Sorry that my changes caused this problem. Peace... :)
Actually, a previous change of mine actually broke the upgrade case.
You just broke the admin server parsing code. :)
Would it be better to have a pure configuration file (not script) in the
instance directory that contains things like instance name, etc.? The
start-dirsrv is a script, and parsing a script without a proper parser is
risky. Same thing with the sysconfig scripts. Even a slight change in that
file could break some regular expressions in the Perl modules. What do you
think?
My previous change that breaks the upgrade case is trying to accomplish
this. We previously had all sorts of paths in the start-slapd scripts
which were parsed in various places. I created the instance specific
config files in /etc/sysconfig that now contain the paths and other
things needed to start an instance in an easily parsed format. The one
wrinkle is that we need to know the instance name to know which
sysconfig file to read since the file name is "dirsrv-<instance>". The
admin server tries to get the instance name from the start script to
find the sysconfig file so we can determine other things, such as the
pid file location. The only other way I can think of to get the
instance name would be to parse it out of the instance path that we use
to locate the start-slapd script. This is usually something like
"/usr/lib/dirsrv/slapd-<instance>". I think that my proposed change to
the start-slapd format is less error prone since there is no complex
parsing needed.
-NGK
Thanks.
--
Endi S. Dewata
--
389-devel mailing list
389-devel@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/389-devel
--
389-devel mailing list
389-devel@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/389-devel
>From 46b7eccd0859e7968f68fa7fc7dc55a1d19fb25f Mon Sep 17 00:00:00 2001
From: Nathan Kinder <nkinder@xxxxxxxxxx>
Date: Thu, 1 Apr 2010 11:12:02 -0700
Subject: [PATCH] Allow instance name to be parsed from start-slapd
The admin server CGIs need to be able to easily parse the
instance name from the start-slapd script. Recent format
changes have caused the existing parsing to break, so this
patch makes the parsing of the instance name easier.
To deal with the change in start-slapd format for an upgraded
instance, I have changed the setup code to regenerate all of
the instance scripts during an upgrade instead of simply adding
missing scripts. This is needed for any bug fix that modifies
a script template to work for an upgraded instance. I also
added code to write the instance sysconfig script during upgrade
if it doesn't exist already. We don't want to overwrite this
file if it already exists since it's designed for local changes
to be made to it.
---
ldap/admin/src/scripts/DSCreate.pm.in | 57 ++++++++++++++----------
ldap/admin/src/scripts/DSUpdate.pm.in | 9 +++-
ldap/admin/src/scripts/template-start-slapd.in | 3 +-
3 files changed, 42 insertions(+), 27 deletions(-)
diff --git a/ldap/admin/src/scripts/DSCreate.pm.in b/ldap/admin/src/scripts/DSCreate.pm.in
index 06b2d1f..09b7cd1 100644
--- a/ldap/admin/src/scripts/DSCreate.pm.in
+++ b/ldap/admin/src/scripts/DSCreate.pm.in
@@ -64,8 +64,8 @@ use Mozilla::LDAP::LDIF;
use Exporter;
@ISA = qw(Exporter);
-@EXPORT = qw(createDSInstance removeDSInstance setDefaults createInstanceScripts installSchema);
-@EXPORT_OK = qw(createDSInstance removeDSInstance setDefaults createInstanceScripts installSchema);
+@EXPORT = qw(createDSInstance removeDSInstance setDefaults createInstanceScripts makeOtherConfigFiles installSchema);
+@EXPORT_OK = qw(createDSInstance removeDSInstance setDefaults createInstanceScripts makeOtherConfigFiles installSchema);
use strict;
@@ -436,6 +436,7 @@ sub createConfigFile {
sub makeOtherConfigFiles {
my $inf = shift;
+ my $skip = shift;
my @errs;
my %maptable = (
"DS-ROOT" => $inf->{General}->{prefix},
@@ -461,13 +462,18 @@ sub makeOtherConfigFiles {
$src = "$inf->{General}->{prefix}@configdir@/slapd-collations.conf";
$dest = "$inf->{slapd}->{config_dir}/slapd-collations.conf";
+
$! = 0; # clear errno
- copy($src, $dest);
- if ($!) {
- return ('error_copying_file', $src, $dest, $!);
- }
- if (@errs = changeOwnerMode($inf, 4, $dest)) {
- return @errs;
+
+ #in skip mode, skip files that already exist
+ unless ($skip and -f $dest) {
+ copy($src, $dest);
+ if ($!) {
+ return ('error_copying_file', $src, $dest, $!);
+ }
+ if (@errs = changeOwnerMode($inf, 4, $dest)) {
+ return @errs;
+ }
}
# determine initconfig_dir
@@ -487,22 +493,25 @@ sub makeOtherConfigFiles {
$! = 0; # clear errno
- if (!open(SRC, "< $src")) {
- return ("error_opening_scripttmpl", $src, $!);
- }
- if (!open(DEST, "> $dest")) {
- return ("error_opening_scripttmpl", $dest, $!);
- }
- my $contents; # slurp entire file into memory
- read SRC, $contents, int(-s $src);
- close(SRC);
- while (my ($key, $val) = each %maptable) {
- $contents =~ s/\{\{$key\}\}/$val/g;
- }
- print DEST $contents;
- close(DEST);
- if (@errs = changeOwnerMode($inf, 4, $dest)) {
- return @errs;
+ # in skip mode, skip files that already exist
+ unless ($skip and -f $dest) {
+ if (!open(SRC, "< $src")) {
+ return ("error_opening_scripttmpl", $src, $!);
+ }
+ if (!open(DEST, "> $dest")) {
+ return ("error_opening_scripttmpl", $dest, $!);
+ }
+ my $contents; # slurp entire file into memory
+ read SRC, $contents, int(-s $src);
+ close(SRC);
+ while (my ($key, $val) = each %maptable) {
+ $contents =~ s/\{\{$key\}\}/$val/g;
+ }
+ print DEST $contents;
+ close(DEST);
+ if (@errs = changeOwnerMode($inf, 4, $dest)) {
+ return @errs;
+ }
}
return ();
diff --git a/ldap/admin/src/scripts/DSUpdate.pm.in b/ldap/admin/src/scripts/DSUpdate.pm.in
index 3792afe..3241a58 100644
--- a/ldap/admin/src/scripts/DSUpdate.pm.in
+++ b/ldap/admin/src/scripts/DSUpdate.pm.in
@@ -47,7 +47,7 @@ package DSUpdate;
use DSUtil;
use Inf;
use FileConn;
-use DSCreate qw(setDefaults createInstanceScripts);
+use DSCreate qw(setDefaults createInstanceScripts makeOtherConfigFiles);
use File::Basename qw(basename dirname);
@@ -315,7 +315,12 @@ sub updateDSInstance {
}
# upgrade instance scripts
- if (@errs = createInstanceScripts($inf, 1)) {
+ if (@errs = createInstanceScripts($inf, 0)) {
+ return @errs;
+ }
+
+ # add new or missing config files
+ if (@errs = makeOtherConfigFiles($inf, 1)) {
return @errs;
}
diff --git a/ldap/admin/src/scripts/template-start-slapd.in b/ldap/admin/src/scripts/template-start-slapd.in
index 444a37f..0c9ac63 100755
--- a/ldap/admin/src/scripts/template-start-slapd.in
+++ b/ldap/admin/src/scripts/template-start-slapd.in
@@ -5,6 +5,7 @@
# 0: Server started successfully
# 1: Server could not be started
# 2: Server already running
+INSTANCE={{SERV-ID}}
-@sbindir@/start-dirsrv -d {{INITCONFIG-DIR}} {{SERV-ID}} "$@"
+@sbindir@/start-dirsrv -d {{INITCONFIG-DIR}} $INSTANCE "$@"
exit $?
--
1.6.2.5
>From 12a4b6be267519edb3f1c28ea726e210c4b99bd0 Mon Sep 17 00:00:00 2001
From: Nathan Kinder <nkinder@xxxxxxxxxx>
Date: Wed, 31 Mar 2010 16:11:16 -0700
Subject: [PATCH] Change parsing of start-slapd for instance name
The format of start-slapd changed recently, causing the
parsing of the instance name from that script in the CGIs
to break. This corrects the parsing.
---
lib/libdsa/dsalib_location.c | 11 +++++------
1 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/lib/libdsa/dsalib_location.c b/lib/libdsa/dsalib_location.c
index 8cccd0f..7b4bfff 100644
--- a/lib/libdsa/dsalib_location.c
+++ b/lib/libdsa/dsalib_location.c
@@ -110,13 +110,10 @@ ds_get_run_dir()
fp = fopen(start_script, "r");
if (fp) {
while(fgets(line, BIG_LINE, fp)) {
- /* Find the line that calls start-dirsrv */
- if ((start = strstr(line, "start-dirsrv"))) {
+ /* Find the line with the instance name */
+ if ((start = strstr(line, "INSTANCE"))) {
/* skip any spaces after start-dirsrv */
- start += strlen("start-dirsrv");
- while (isspace(*start)) {
- start++;
- }
+ start += strlen("INSTANCE=");
/* find the end of the instance name */
p = start;
@@ -128,6 +125,8 @@ ds_get_run_dir()
if (strlen(start) > 0) {
inst_name = PR_smprintf("%s", start);
}
+
+ break;
}
}
fclose(fp);
--
1.6.2.5
--
389-devel mailing list
389-devel@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/389-devel
