Bug 672652 - Kickstart "Kernel Options" and "Post Kernel Options" reject valid options
Summary: Kickstart "Kernel Options" and "Post Kernel Options" reject valid options
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Spacewalk
Classification: Community
Component: WebUI
Version: 1.6
Hardware: All
OS: All
medium
medium
Target Milestone: ---
Assignee: Jan Pazdziora (Red Hat)
QA Contact: Red Hat Satellite QA List
URL:
Whiteboard:
Depends On:
Blocks: space16
TreeView+ depends on / blocked
 
Reported: 2011-01-25 19:44 UTC by Roy Keene
Modified: 2011-12-22 16:49 UTC (History)
3 users (show)

Fixed In Version: spacewalk-java-1.6.83-1
Clone Of:
Environment:
Last Closed: 2011-12-22 16:49:17 UTC
Embargoed:


Attachments (Terms of Use)
Patch proposed (2.91 KB, patch)
2011-08-29 19:18 UTC, Marcelo Moreira de Mello
no flags Details | Diff

Description Roy Keene 2011-01-25 19:44:37 UTC
Description of problem:
A kickstart profile supports passing kernel options to the Anaconda kernel ("Kernel Options") and the installed OS kernel ("Post Kernel Options"), however they reject the valid options:
    console=tty0 console=ttyS0,9600n8

The interface replaces it with:
    console=ttyS0,9600n8

Which does something entirely different and is not want I want.

How reproducible:
Completely.

Steps to Reproduce:
1. Login to Spacewalk
2. Click "Systems"
3. Click "Kickstart"
4. Click "Profiles"
5. Click a profile label
6. Type "console=tty0 console=ttyS0,9600n8" in the text box next to "Kernel Options"
7. Click "Update Kickstart"
  
Actual results:
Textbox next to "Kernel Options" contains "console=ttyS0,9600n8 "


Expected results:
Textbox next to "Kernel Options" should contain "console=tty0 console=ttyS0,9600n8"

Additional info:
This bug seems like a mis-guided attempt to remove redundant kernel options.  The kernel command line is not parsed as a key-value pair.

Comment 1 Miroslav Suchý 2011-04-11 07:33:20 UTC
We did not have time for this one during Spacewalk 1.4 time frame. Mass moving to Spacewalk 1.5.

Comment 2 Miroslav Suchý 2011-04-11 07:37:08 UTC
We did not have time for this one during Spacewalk 1.4 time frame. Mass moving to Spacewalk 1.5.

Comment 3 Jan Pazdziora (Red Hat) 2011-07-20 11:51:31 UTC
Aligning under space16.

Comment 4 Marcelo Moreira de Mello 2011-08-25 05:27:52 UTC
Taking..

Comment 5 Marcelo Moreira de Mello 2011-08-27 05:37:05 UTC
Issue reproduced into Spacewalk 1.6 nightly build. 

  When using double quotes, the text is included but does not include it correctly. 


%post# Start post install kernel options update
/sbin/grubby --update-kernel=`/sbin/grubby --default-kernel` --args="console=ttyS0,115200" "console=tty0 "
# End post install kernel options update


 Working on it..

Comment 6 Marcelo Moreira de Mello 2011-08-27 06:16:10 UTC
java/code/src/com/redhat/rhn/frontend/action/kickstart/KickstartDetailsEditAction.java
============================================

278     /**
279      * Proccess Cobbler form values, pulling in the form
280      *      and pushing the values to cobbler
281      * @param ksdata the kickstart data
282      * @param form the form
283      * @param user the user
284      */
285     public static void processCobblerFormValues(KickstartData ksdata,
286                                             DynaActionForm form, User user) {
287         CobblerProfileEditCommand cmd = new CobblerProfileEditCommand(ksdata, user);
288 
289         cmd.setKernelOptions(StringUtils.defaultString(form.getString(KERNEL_OPTIONS)));
290         cmd.setPostKernelOptions(StringUtils.defaultString(
291                             form.getString(POST_KERNEL_OPTIONS)));
292         cmd.store();
293 
294         CobblerXMLRPCHelper helper = new CobblerXMLRPCHelper();
295         Profile prof = Profile.lookupById(helper.getConnection(user),
296                 ksdata.getCobblerId());
297         if (prof == null) {
298             return;
299         }

-----


137     /**
138      * Setup cobbler form values These include, kernel options and virt options
139      * @param ctx the RequestContext
140      * @param form The form
141      * @param data the kickstart data
142      */
143     public static void setupCobblerFormValues(RequestContext ctx,
144             DynaActionForm form, KickstartData data) {
145         CobblerXMLRPCHelper helper = new CobblerXMLRPCHelper();
146         Profile prof = Profile.lookupById(helper.getConnection(ctx.getLoggedInUser()),
147                 data.getCobblerId());
148         if (prof != null) {
149             form.set(KERNEL_OPTIONS, prof.getKernelOptionsString());
150             form.set(POST_KERNEL_OPTIONS, prof.getKernelPostOptionsString());
151         }
152         KickstartVirtualizationType type = data.getKickstartDefaults().
153                                                     getVirtualizationType();


# cobbler system report 
... 
Name                           : system-a-test.redhat.com:3
Comment                        : 
Gateway                        : 
Hostname                       : 
Image                          : 
Kernel Options                 : {'console': 'ttyS0,9600n8'}
Kernel Options (Post Install)  : {'console': 'ttyS0,9600n8'}

Comment 7 Marcelo Moreira de Mello 2011-08-27 23:01:23 UTC
code/src/org/cobbler/CobblerObject.java
328     /**
329      * gets the kernel post options in string form
330      * @return the string
331      */
332     public String getKernelPostOptionsString() {
333         return convertOptionsMap(getKernelPostOptions());
334     }
335 
336     private String convertOptionsMap(Map<String, Object> map) {
337         StringBuilder string = new StringBuilder();
338         for (Object key : map.keySet()) {
339             if (StringUtils.isEmpty((String)map.get(key))) {
340                 string.append(key + " ");
341             }
342             else {
343                 string.append(key + "=" + map.get(key) + " ");
344             }
345         }
346         return string.toString();
347     }


   The function above overwrite the values keeping always the second parameter with '=' signal

  Doing the test at webUI:

     Kernel Options: console=tty0 console=ttyS0,115200
     Post Kernel Options: console=ttyS0,115200 console=tty0

  Result: 

     Kernel Options: console=ttyS0,115200 
     Post Kernel Options: console=tty0

Comment 8 Marcelo Moreira de Mello 2011-08-27 23:17:07 UTC
336     private String convertOptionsMap(Map<String, Object> map) {
337         StringBuilder string = new StringBuilder();
338         for (Object key : map.keySet()) {
339             if (StringUtils.isEmpty((String)map.get(key))) {
340                 string.append(key + " ");
341             }
342             else {
343                 string.append(key + "=" + map.get(key) + " ");
344             }
345         }
346         return string.toString();
347     }

  fuction convertOptionsMap()  is using Map object which cannot allow have duplicate keys


  at webUI:

     Kernel Options: console console=ttyS0,115200
     
  Result: 

     Kernel Options: console=ttyS0,115200

 OR

    
  at webUI:

     Kernel Options: console=ttyS0,115200 console
     
  Result: 

     Kernel Options: console

 
 When using different cases, it is included into the Map

 
  at webUI:

     Kernel Options: Console=tty0 console=ttyS0,115200
     
  Result: 

     Kernel Options: Console=tty0 console=ttyS0,115200

Comment 9 Marcelo Moreira de Mello 2011-08-29 19:18:21 UTC
Created attachment 520451 [details]
Patch proposed

Hello, 

  Follow attached a patch which allow multiple keys when setting kernel_options and kernel_post_options at kickstart profile. 

  Patch already sent to approval at spacewalk-devel mail list. 

    Mail Thread: https://www.redhat.com/archives/spacewalk-devel/2011-August/msg00085.html

 Cheers, 
Marcelo Moreira de Mello

Comment 12 Jan Pazdziora (Red Hat) 2011-09-30 11:25:47 UTC
(In reply to comment #9)
> Created attachment 520451 [details]
> Patch proposed
> 
> Hello, 
> 
>   Follow attached a patch which allow multiple keys when setting kernel_options
> and kernel_post_options at kickstart profile. 
> 
>   Patch already sent to approval at spacewalk-devel mail list. 
> 
>     Mail Thread:
> https://www.redhat.com/archives/spacewalk-devel/2011-August/msg00085.html
> 
>  Cheers, 
> Marcelo Moreira de Mello

Marcelo, do you have a new patch ready which would address the concerns Tomáš raised in the mailing list thread?

Comment 13 Jan Pazdziora (Red Hat) 2011-12-02 15:33:21 UTC
I've applied the patch as proposed in commit 9 to Spacewalk master, 4c24029b5166df603ac3550b661094ef6888889d.

Comment 14 Tomas Lestach 2011-12-05 13:23:23 UTC
fixing checkstyle issues: cd6114a6363e54c537d2df56d4802befc51e43f9

Comment 15 Tomas Lestach 2011-12-09 13:52:04 UTC
fixing behaviour, when entering values like: 
justkey keyisequal= keyemptystring=""
+ some cosmetic issues

spacewalk.git: 7a2edb175d67963ccba6c7f2cac3806c21dabc5e

Comment 16 Tomas Lestach 2011-12-19 10:01:01 UTC
kernel options stored before update might be stored as strings => 'upgrade issue'

spacewalk.git: 73c0e06e29163e0d68a196e1f7c3488f2d0982a7

Comment 17 Milan Zázrivec 2011-12-22 16:49:17 UTC
Spacewalk 1.6 has been released.


Note You need to log in before you can comment on or make changes to this bug.