Bug 591863

Description of problem:

1. KickstartFormatter.java doesn't benefit from --logfile option for section %pre/%post, but instead it uses bash style redirection, which leads in case of different script language (python, perl) kickstart pre/post scripts to fail.

2. KickstartFormatter.java doesn't add new %post section before post_install_kernel_options and koan_environment setup, which leads to adding unrelated commands to preceding post script.

3. Small cosmetic typo - MOTD_FOOTER containing a line "# end of generated kickstart file" is being inserted before user define post install scripts and koan environment setup, it should be rather written at the end of kickstart file.


Version-Release number of selected component (if applicable):

spacewalk-java-0.5.44-75 (currently latest) and prior


How reproducible:

always


Steps to Reproduce:

1. the first issue isn't problem until the post script is written in diferrent script language than bash as post_install_kernel_options and koan_environment setup consist from lines of simple commands which bash normally executes line by line, but perl and python don't.

  a) create a kickstart profile with a post script like the following:

"Scripting Language:" /usr/bin/python
"Script Contents"
import shutil
shutil.copy2( '/proc/cmdline', '/root/cmdline' )

  b) check the box with "Log custom post scripts" in "Kickstart Details" -> "Details"

  c) kickstart the system (I would reccomend to uncheck the option "reboot" in "Kickstar Details" -> "Advanced Options" before kickstarting system to be able to switch to 3rd terminal and see the script error output)

  d) Switch to 3rd terminal by hitting alt-f3 (or ctrl-alt-f3 in graphical mode)

  e) you will see the following error:
   File "/tmp/ks-script-XXXXXX", line 2
      import shutil
          ^
SyntaxError: invalid syntax

2. the second issue is related to the previous one, if the KickstartFormatter.java didn't add the bash style redirection (the box with "Log custom post scripts" in "Kickstart Details" -> "Details" has to be unchecked), the extra lines with post_install_kernel_options and koan_environment setup would be added to python script and it would fail on them too. The steps are the same as previous.


Actual results:

The python script fails with "SyntaxError: invalid syntax", and neither /root/cmdline nor /root/ks-post.log are created.


Expected results:

Successful execution of post script.


Additional info:

I see the two first bugs as a little bit design error. The problem is that one functionality is written on two different places and in two different languages - java and python. The java code responsible for generation of the kikstart file through webui should use python code from pykickstart library (pykickstart library is used by anaconda to parse kickstart files and it also contains functions to write kickstart files) instead of implementing the same functionality twice, which leads to behaviour not conforming with anaconda parsing during kickstart process and as time goes it is more prone to bugs (this case) as one has to check code updates being made in anaconda/pykickstart library altogether with changes made to java code.

----------------------------------------------------------
snippet of the wrongly generated kickstart file (extra blank lines were removed):

# MOTD
echo >> /etc/motd
echo "RHN Satellite kickstart on $(date +'%Y-%m-%d')" >> /etc/motd
echo >> /etc/motd

# end of generated kickstart file

%post --interpreter /usr/bin/python
(
import shutil
shutil.copy2( '/proc/cmdline', '/root/cmdline' )
) >> /root/ks-post.log 2>&1

# Start koan environment setup
echo "export COBBLER_SERVER=<HOSTNAME>" > /etc/profile.d/cobbler.sh
echo "setenv COBBLER_SERVER <HOSTNAME>" > /etc/profile.d/cobbler.csh
# End koan environment setup

wget "http://<HOSTNAME>/cblr/svc/op/ks/profile/it850403:1:RedHat" -O /root/cobbler.ks
wget "http://<HOSTNAME>/cblr/svc/op/trig/mode/post/profile/it850403:1:RedHat" -O /dev/null
----------------------------------------------------------

----------------------------------------------------------
related source code for 1st bug:

=== <snip spacewalk-java-0.5.44/code/src/com/redhat/rhn/manager/kickstart/KickstartFormatter.java ===
    private static final String END_POST = ") >> /root/ks-post.log 2>&1\n";
    private static final String END_PRE = ") >> /tmp/ks-pre.log 2>&1\n";
    private static final String  BEGIN_PRE_POST_LOG = "(" + NEWLINE;

    private String getPrePost(String typeIn) {
...
        if (typeIn.equals(KickstartScript.TYPE_POST) &&
                ksdata.getPostLog()) {
            retval.append(BEGIN_PRE_POST_LOG);
        }
        if (typeIn.equals(KickstartScript.TYPE_PRE) &&
                ksdata.getPreLog()) {
            retval.append(BEGIN_PRE_POST_LOG);
        }
        retval.append(kss.getDataContents() + NEWLINE);
        if (typeIn.equals(KickstartScript.TYPE_POST) &&
                ksdata.getPostLog()) {
            retval.append(END_POST);
        }
        if (typeIn.equals(KickstartScript.TYPE_PRE) &&
                ksdata.getPreLog()) {
            retval.append(END_PRE);
        }
...
    }
=== snip> ===

The above code should be removed (except line "retval.append(kss.getDataContents() + NEWLINE);") and --logfile parameter for %pre/%post used instead (the code for %pre/%post line generation was stripped out from the above code snippet) to support also python/perl scripts. Anaconda supports logging to separate files by executing extracted script from kickstart file in a distinct process with output redirection, please, see the related related anaconda sources:

anaconda-11.1.2.209/kickstart.py: class AnacondaKSScript(Script): def run(self, chroot, serial, intf = None)
----------------------------------------------------------

----------------------------------------------------------
related code for the 2nd bug:

=== <snip spacewalk-java-0.5.44/code/src/com/redhat/rhn/manager/kickstart/KickstartFormatter.java ===
    public String getFileData() {
...
        buf.append(getPrePost(KickstartScript.TYPE_POST));
        buf.append(NEWLINE);
        addCobblerSnippet(buf, "post_install_kernel_options");
        addCobblerSnippet(buf, "koan_environment");
        buf.append("$kickstart_done");
        buf.append(NEWLINE);
        String retval = buf.toString();
        log.debug("fileData.retval:");
        log.debug(retval);
        return retval;
    }
=== snip> ===

between lines:

        buf.append(getPrePost(KickstartScript.TYPE_POST));
        buf.append(NEWLINE);

and

        addCobblerSnippet(buf, "post_install_kernel_options");

should be a line:

        buf.append("%post");
----------------------------------------------------------

One another strange thing I found, but not related to Satellite, but rather to anaconda (in fact pykickstart library) is that the Satellite currently implements old syntax, that is that every section %pre/%packages/%post ends at start of other %pre/%packages/%post section or ends at the end of file, but in the newest anaconda, which can be found in Fedora every section ends with corresponding "%end" directive. So why the current/newest anaconda for RHEL5 contains only language translation, which mentions %end directive, but does not contain implementation itself? :

$ grep -R %end pykickstart-0.43.8/
pykickstart-0.43.8/po/de.po:#~ "%s does not end with %%end.  This syntax has been deprecated.  It may be "
pykickstart-0.43.8/po/de.po:#~ "%s endet nicht mit %%end. Diese Syntax ist veraltet. Sie wird vielleicht "
pykickstart-0.43.8/po/de_CH.po:#~ "%s does not end with %%end.  This syntax has been deprecated.  It may be "
pykickstart-0.43.8/po/de_CH.po:#~ "%s endet nicht mit %%end. Diese Syntax ist veraltet. Sie wird vielleicht "
$