Bug 531122 - Activation keys do not populate correct in kickstart files - cobbler profiles loses previous keys when sync'ing
Summary: Activation keys do not populate correct in kickstart files - cobbler profiles...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Satellite 5
Classification: Red Hat
Component: Provisioning
Version: 530
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Justin Sherrill
QA Contact: Garik Khachikyan
URL:
Whiteboard:
Depends On:
Blocks: sat531-blockers
TreeView+ depends on / blocked
 
Reported: 2009-10-26 23:35 UTC by Xixi
Modified: 2018-10-27 15:59 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-04-23 07:10:24 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2010:0369 0 normal SHIPPED_LIVE Red Hat Network Satellite bug fix update 2010-04-23 07:01:54 UTC

Description Xixi 2009-10-26 23:35:37 UTC
Description of problem:
When customer updates a ks profile to add activation key, the cobbler profile associated with a system loses the previous keys, and ends up with <<inherit>>,new-key in the --activationkey= line.

Version-Release number of selected component (if applicable):
Red Hat Network (RHN) Satellite 5.3.0

System architecture(s):
arch=x86_64
release=5
flavor=server
base=rhel-x86_64-server-5

How reproducible:
Always.

Steps to Reproduce:
1. On Satellite 5.3.0, create a ks profile "test_profile" with 1 activation key associated with it (key optional but illustrates the issue further).
2. Run the following commmand (or something similar to associate a kickstart profile with a given system).
# cobbler system add --name "dhcp123-456" --profile=test_profile:1:Red-Hat --ip=11.11.22.57
This creates something like /tftpboot/pxelinux.cfg/0A0BF29D, grep for "ks=" and go to the ks url, you'll see something like --activationkey=1-f3b517fea5c6a1f926b308739e109c9e,1-ca77bb6a2bbd0d6b01a3f193f29b01c6, which is all good.
3. Create an(other) activation key.
4. Associate the new activation key with the kickstart profile in UI and save.
5. Check the cobbler profile created with step 2, again.
This time the ks profile at the same url will show --activationkey=<<inherit>>,1-newkey
 
Expected results:
Activation keys populated properly with both the default re-activation key AND the new activation key.

Actual Results:
See step 5. above.
User sees "<<inherit>>" in place of the system re-activation key and the new user created activation key.  I.e., it loses previous keys so they end up with default value "<<inherit>>"

Additional info:
Internal reproducer and more info to come.

Comment 1 Xixi 2009-10-26 23:46:31 UTC
Looks like everytime activation key associationss are modified for a ks profile (whether add or remove), it syncs to the cobbler profile.  Somewhere along the way, it loses the previous values, it may be CobblerObject.getRedHatManagementKey already doesn't return the right set from the data map initialization, or when modifying it... 

Relevant code snippets:

src/com/redhat/rhn/frontend/action/kickstart/ActivationKeysSubmitAction.java:
...
  60     protected void operateOnRemovedElements(List elements,

  61                                             HttpServletRequest request) {

  62         RequestContext ctx = new RequestContext(request);

  63 

  64         KickstartActivationKeysCommand cmd =

  65             new KickstartActivationKeysCommand(

  66                     ctx.getRequiredParam(RequestContext.KICKSTART_ID),

  67                     ctx.getCurrentUser());

  68 
...   

  82         cmd.removeTokensByIds(ids);

  83         cmd.store();

  84 

  85         return;

  86     }
...
  92     protected void operateOnAddedElements(List elements, HttpServletRequest request) {

  93         RequestContext ctx = new RequestContext(request);

  94 

  95         KickstartActivationKeysCommand cmd =

  96             new KickstartActivationKeysCommand(

  97                     ctx.getRequiredParam(RequestContext.KICKSTART_ID), 

  98                     ctx.getCurrentUser());
...
 113         cmd.addTokensByIds(ids);

 114         cmd.store();

 115 

 116         return;

 117     }
...

src/com/redhat/rhn/manager/kickstart/KickstartActivationKeysCommand.java:
...
  49     public void removeTokensByIds(ArrayList<Long> ids) {

  50         Set<String> keysToRemove = new HashSet<String>();

  51 

  52         for (Long id : ids) {

  53             Set<Token> tokenSetCopy = new HashSet<Token>();

  54             tokenSetCopy.addAll(this.getKickstartData().getDefaultRegTokens());

  55             for (Token token : tokenSetCopy) {

  56                 if (token.getId() == id) {

  57                     this.getKickstartData().getDefaultRegTokens().remove(token);

  58                     keysToRemove.add(ActivationKeyFactory.lookupByToken(token).getKey());

  59                 }

  60             }

  61         }

  62 

  63         Profile prof = Profile.lookupById(

  64                 CobblerXMLRPCHelper.getConnection(this.getUser()),

  65                 this.getKickstartData().getCobblerId());

  66         if (prof != null) {

  67             prof.syncRedHatManagementKeys(keysToRemove, Collections.EMPTY_SET);

  68         }

  69         prof.save();

  70     }
...
  76     public void addTokensByIds(ArrayList<Long> ids) {

  77         Set<String> keysToAdd = new HashSet<String>();

  78 

  79         for (Long id : ids) {

  80             Token token = TokenFactory.lookupById(id);

  81             this.getKickstartData().addDefaultRegToken(token);

  82             keysToAdd.add(ActivationKeyFactory.lookupByToken(token).getKey());

  83         }

  84         Profile prof = Profile.lookupById(

  85                 CobblerXMLRPCHelper.getConnection(this.getUser()),

  86                 this.getKickstartData().getCobblerId());

  87         if (prof != null) {

  88             prof.syncRedHatManagementKeys(Collections.EMPTY_SET, keysToAdd);

  89         }

  90         prof.save();

  91     }

  92 
...

src/org/cobbler/Profile.java:
...
  32 public class Profile extends CobblerObject {
...
 396       public void syncRedHatManagementKeys(Collection<String> keysToRemove,

 397                                                   Collection<String> keysToAdd) {

 398           super.syncRedHatManagementKeys(keysToRemove, keysToAdd);

 399           for (SystemRecord record :

 400                       SystemRecord.listByAssociatedProfile(client, this.getName())) {

 401               record.syncRedHatManagementKeys(keysToRemove, keysToAdd);

 402               record.save();

 403           }

 404       }
...

src/org/cobbler/CobblerObject.java:
...
  56     private static final String REDHAT_KEY = "redhat_management_key";
...
 473     public String getRedHatManagementKey() {

 474         return (String) dataMap.get(REDHAT_KEY);

 475     }
...
 481     public Set<String> getRedHatManagementKeySet() {

 482         String keys = StringUtils.defaultString(getRedHatManagementKey());

 483         String[] sets = (keys).split(",");

 484         Set set = new HashSet();

 485         set.addAll(Arrays.asList(sets));

 486         return set;

 487     }
...
 494     public void syncRedHatManagementKeys(Collection<String> keysToRemove,

 495                                             Collection<String> keysToAdd) {

 496         Set keySet = getRedHatManagementKeySet();

 497         keySet.removeAll(keysToRemove);

 498         keySet.addAll(keysToAdd);

 499         setRedHatManagementKey(keySet);

 500     }
...

Comment 4 Xixi 2009-10-26 23:51:29 UTC
For now, workaround is to manual change the cobbler-generated pxelinux file for the system and pointing it at the first (base?) profile.

Comment 5 Justin Sherrill 2010-02-26 18:53:05 UTC
4a13f480c34ea364fb285c80baf5bfcc832e6df1

fixed in spacewalk master.

Comment 6 Tomas Lestach 2010-03-11 16:25:52 UTC
satellite.git: 68422b1d6e83442c60ef5dd511795fe5ee0d9ec4

Comment 10 Garik Khachikyan 2010-03-29 08:55:59 UTC
# REOPEN

The issue is not fixed.

Reproducer was successful on initial packages of sat530

And proceeding the scenario in comment#0 on updated packages brings to following:
The cobbler update of new adding act. key to the kickstart profile TOTALLY overwrites *all* the previous act keys (including <<inherit>>) so the "--activationkey=<new_key>" looks like this was, but should be: "--activationkey=<<inherit>>,<old_key>,<new_key>"

@Justin: feel free contact me for the env. to reproduce the error. thanks.

Comment 11 Justin Sherrill 2010-03-30 16:04:44 UTC
Hey Garik,

It looks like the reproducer machine is running spacewalk-java-0.5.44-67.el4sat  but i don't think those changes landed until spacewalk-java-0.5.44-70.el4sat.

Can you retest with the newest spacewalk-java?

Thanks!

-Justin

Comment 12 Justin Sherrill 2010-03-31 15:39:28 UTC
Finally figured out the error you were referring to and it is related to creating the system record from cobbler directly.  If you do it from the UI it works fine.   The reason was that we weren't attempting to add all existing keys when updating a kickstart profile, only the new ones.  Made a change to add all the keys even if they are already there (dups are ignored).

Spacewalk commit: 
624f3267af6ab45f84753d1c4e5461ea19b0fc6d

Satellite commit:702746070eb6d8ba3824f475d4cebf7e59f1cb5d

Comment 14 Garik Khachikyan 2010-04-01 13:47:35 UTC
# VERIFIED

Checking the scenario in comment#0 also the case with creating Cobbler System Record passed okey.

Add/remove of the activation keys in the cobbler profiles are updating properly - nothing is getting lost now.

packages fixing the issue are:
---
python-cheetah-2.0.1-1
cobbler-1.6.6-5
spacewalk-web-0.5.23-32
spacewalk-java-0.5.44-73
spacewalk-backend-0.5.28-40.x.2
---

Comment 16 errata-xmlrpc 2010-04-23 07:10:24 UTC
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHBA-2010-0369.html


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