Some proposed changes to Manual Import functionality

Lukas Krejci lkrejci at redhat.com
Thu Apr 25 09:18:41 UTC 2013


While I understand why we see the issue, I am not sure I agree with the 
solutions proposed.

I think we should follow the same logic we use elsewhere - the user is always 
right, unless impossible ;)

The new behavior as proposed would disallow a manual import if the resource 
was in the discovery queue, am I right? I do think that would be really bad.

Also, I do not think it makes sense for the plugin developers to distinguish 
between auto-discovered and manually added resources. It is still the same 
type of underlying resources, only the discovery logic is not clever enough to 
discover all cases.

Also, if we had different RK for manually added and auto-discovered resources, 
what would happen if the user manually imported one and then found an 
equivalent resource in the discovery queue and imported that, too? Is the user 
supposed to know that those two are the same and that the code just wasn't 
clever enough to find they're equal (which it would if they had the same RK)?

I think the simple table below would provide clear enough rules for handling 
manual import that would make sense for the user:

Inventory           | Manual Add | Result
===============================================================================
N/A                 | Rm         | Rm added
-------------------------------------------------------------------------------
Ri in NEW           | Rm         | Rm overwrites Ri and moves it to COMMITTED
-------------------------------------------------------------------------------
Ri in IGNORED       | Rm         | Rm overwrites Ri and moves it to COMMITTED
-------------------------------------------------------------------------------
Ri in UNINVENTORIED | Rm         | Rm updates Ri and moves it to COMMITTED
-------------------------------------------------------------------------------
Ri in COMMITTED     | Rm         | Error during manual add
-------------------------------------------------------------------------------

Ri represents a resource in the inventory, Rm the resource with the same 
resource key that is being manually imported.

Notice that if the resource in the inventory is NEW or IGNORED, meaning that 
it only is in the discovery queue, the manually added resource takes 
precedence over the resource in the discovery queue and completely rewrites it 
during manuall add. 

This is different for the UNINVENTORIED resources, which we keep around in the 
inventory to keep their histories. To keep that logic, the manual add of such 
resource would basically behave the same as if the resource was rediscovered 
automatically and reimported from the discovery queue.

Trying to manually add a resource that is already in the inventory and is 
committed to it is of course an error because the same thing can be done in a 
more "ordinary" way by simply updating the plugin/resource configs of the 
existing resource.

I think this provides a quite clean behavior that is consistent with how the 
rest of the system behaves already and therefore provides the "path of least 
surprises" to the user.

My apologies if you meant something similar in the original email but because 
I was not exactly sure what your thoughts were about the import logic in i), I 
thought I'd rather be explicit in what I mean even at the expense of repeating 
your arguments ;)

Lukas

On Wednesday, April 24, 2013 21:39:39 Larry O'Leary wrote:
> What I meant by making the keys more unique is that perhaps a manual add
> explicitly prefix its keys with "!!manual!!" or some other magic word.
> This would have prevented this issue and not require/expect a merge of
> the configuration.
> 
> On Wed, 2013-04-24 at 22:23 -0400, Simeon Pinder wrote:
> > My mistake I should have been clearer. I meant to communicate that all
> > steps i, ii an iii should all be implemented as part of the fix.
> > 
> > You are right that each plugin could do more to make sure that manual add
> > and automatic discovery keys should be more unique.  In this case the
> > resource key is the installation directory for tomcat, which is normally
> > unique enough except in this one case.  The larger issue is that even
> > after the InventoryManager(pc) does detect existing resources that it's
> > still the wrong place to initiate configuration/inventory status updates
> > for an existing resource.
> > 
> > ----- Original Message -----
> > 
> > > From: "Larry O'Leary" <loleary at redhat.com>
> > > To: rhq-devel at lists.fedorahosted.org
> > > Sent: Wednesday, April 24, 2013 6:24:02 PM
> > > Subject: Re: Some proposed changes to Manual Import functionality
> > > 
> > > On Wed, 2013-04-24 at 14:59 -0400, Simeon Pinder wrote:
> > > > Hi All,
> > > > 
> > > > After discussions with Jay, I wanted to describe some changes that I
> > > > think
> > > > we should make in support of 'Manual Import' functionality in the
> > > > plugin
> > > > container and potentially on the server side.
> > > > 
> > > > The issue:
> > > > With BZ 883825 we stumbled into odd behavior while trying to test out
> > > > 'Manual Import' of Tomcat instances. What wasn't immediately apparent
> > > > was
> > > > that those same Tomcat instances were already silently indexed by RHQ
> > > > in
> > > > the 'Discovery Queue'.  So that while attempting to 'Manual Import'
> > > > these
> > > > new instances with a new plugin configuration, the plugin container
> > > > would
> > > > take the information from the 'new' import and detect that we'd
> > > > already
> > > > had that in inventory(the auto-discovered version) and use the invalid
> > > > configuration from that version. The result was that a user submitting
> > > > a
> > > > Manual Import with valid credentials, was getting rejected because RHQ
> > > > was
> > > > silently swapping in the wrong config with no credentials. While the
> > > > situation just described is ugly, the wider issue is that it occurred
> > > > because we were never clear about what should happen when one tries to
> > > > 'Manual Import' a new resource resulting in a resource key that
> > > > matches an
> > > > existing resource.  Essentially such a behavior, if allowed, would
> > > > result
> > > > in a Resource configuration/status update which was not the original
> > > > point.
> > > > 
> > > > To summarize, RHQ/JON does not do enough to detect and deter when
> > > > 'Manual
> > > > Import' is being used to inadvertently update the state of a Resource
> > > > already indexed by RHQ.  Current suggestions are to:
> > > > 
> > > > i)Update the RHQ code to better detect and prevent such behavior with
> > > > appropriate error messages
> > > > 
> > > >   - modify InventoryManager to error out when 'Manual Import' is
> > > >   called and
> > > >   an existing RHQ resource is already indexed.  The new logic should
> > > >   handle Resources in all InventoryStatus.* states better since
> > > >   changes to
> > > >   existing Resources should only be handled via our existing Resource
> > > >   update mechanism.  This means better agent side and server side logs
> > > >   messages when this situation is attempted in the future.  I'm also
> > > >   looking into throwing better UI errors after "Manual Import"
> > > >   attempts.
> > > >   Right now we only really support Plugin Configuration exceptions(bad
> > > >   creds) or Plugin Container exceptions of which this doesn't fit in
> > > >   either category well.
> > > > 
> > > > ii) update released/docs/support information to better describe this
> > > > situation and workaround
> > > > 
> > > >  - this just means documenting this behavior and how to avoid getting
> > > >  into
> > > >  this state.  An administrator attempting to inventory resources could
> > > >  get
> > > >  into this state and it's confusing to debug/detect if auto discovery
> > > >  is
> > > >  doing it's job well.
> > > > 
> > > > iii)clarify the javadoc on the remote api documentation so that plugin
> > > > developers know to not attempt to handle such unintentional updates.
> > > > 
> > > >  - Trying to support 'Manual Import', without filtering out existing
> > > >  Resources that match the same identity, is a subtle point that we
> > > >  haven't
> > > >  gotten right in the past.  The DiscoveryBossRemote public api javadoc
> > > >  implies that we will handle cases where 'Manual Import' specifies a
> > > >  plugin config that maps to an existing resource.  We need to update
> > > >  this
> > > >  api description here to be clearer about which situations should not
> > > >  be
> > > >  handled by 'Manual Import'.
> > > > 
> > > > This last point is tricky because there's no simple way to tell a
> > > > plugin
> > > > developers how to detect if the plugin configuration will map to an
> > > > existing Resource already.  There is perhaps a CLI query that they can
> > > > put
> > > > together but it's not a great solution.  Logging error messages to the
> > > > server and agent logs also isn't a great solution here from a CLI
> > > > users
> > > > standpoint.
> > > > 
> > > > I think we should actively discourage this usage of 'Manual Import'
> > > > that
> > > > can overlap with Resource update. Are there any other opinions here?
> > > > 
> > > > -Simeon
> > > 
> > > Perhaps manually added resources get a special key? Obviously this would
> > > still require work/understanding from the plug-in developer to ensure
> > > that they support manual add and that it is done in a way makes the
> > > special key meaningful. It would be nice if we could make this happen at
> > > the pc level.
> > > 
> > > Outside of that, I would think that i) above is really the only option.
> > > If we don't allow duplicate resources (one from discovery and one from
> > > manual add) then we need to tell the user that the add failed due to a
> > > resource with the same key already existing. I am not really sure what
> > > should happen though when a manually added resource makes it into
> > > inventory and then discovery discovers a new one with the same key
> > > (server scan ran and discovered resource, and then user uninventoried it
> > > and manually added before another discovery ran).
> > > 
> > > --
> > > Larry O'Leary
> > > https://plus.google.com/u/0/112645929986009801513
> > > 
> > > _______________________________________________
> > > rhq-devel mailing list
> > > rhq-devel at lists.fedorahosted.org
> > > https://lists.fedorahosted.org/mailman/listinfo/rhq-devel
> > 
> > _______________________________________________
> > rhq-devel mailing list
> > rhq-devel at lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/rhq-devel
> 
> _______________________________________________
> rhq-devel mailing list
> rhq-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/rhq-devel


More information about the rhq-devel mailing list