Some proposed changes to Manual Import functionality

Jay Shaughnessy jshaughn at redhat.com
Thu Apr 25 21:07:28 UTC 2013


Perhaps. But it seems odd to me that a GUI user would manual add 
something that was autodiscovered and is sitting in the discovery 
queue.  It sounds more like an automation use case.   I'd estimate about 
2-4 times the work to support the back-door import/update behavior.  
Anything is doable.

On 4/25/2013 1:34 PM, Lukas Krejci wrote:
> I approached the problem from the POV of a GUI user:
> A GUI user needs to fill out a possibly quite large plugin configuration. If
> that data accidentally describes a resource that already exists, the last
> things I would like to do is to throw away the (possibly changed) data I've
> put in, go to a different place in the GUI and repeat my steps (e.g. if I only
> need to change a couple of fields in a plugin config of an ignored resource).
>
> Thinking about your arguments, I think your proposal makes great sense for an
> API user, where we need to be precise and strict and therefore the API should
> strictly distinguish between a manual add and update.
>
> For the poor GUI guy though, the more helpful and forgiving the UI is, the
> better - hence my table, where only 1 case results in an error instead of 3
> out of 4.
>
> Lukas
>
> On Thursday, April 25, 2013 11:08:25 Simeon Pinder wrote:
>> ----- Original Message -----
>>
>>> From: "Lukas Krejci" <lkrejci at redhat.com>
>>> To: rhq-devel at lists.fedorahosted.org
>>> Sent: Thursday, April 25, 2013 5:18:41 AM
>>> Subject: Re: Some proposed changes to Manual Import functionality
>>>
>>> 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.
>> Actually we already prevent manual import if the resource is in the
>> discovery queue.  The code, as written, is almost guaranteed to use the
>> wrong pluginConfiguration despite being passed a correct one via the UI(AD
>> pluginConfig version got in first).  The proposal is to tighten up the code
>> so that this is actually enforced consistently and take a clearer stance in
>> the documentation and on the server side about what behavior manual import
>> should have.  Which I think it the core issue that there is still some back
>> and forth about.
>>> 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.
>> I agree, but since both discoveries can and do happen i) indepdendently of
>> each other ii)at different times in the plugin container then we're asking
>> both processes to be able to update the server state from the agent side
>> which I don't particularly like, to get this right.  Additionally, by
>> design, we've already implied which of these two processes(auto-discovery)
>> should take priority.  Most of the time we want autodiscovery to do exactly
>> what it's doing for resources, it's just manual import that needs to be
>> clearer about what it should do when it encounters a resource that's
>> already indexed.  The final point here is that the user is already engaged
>> in the process and has already made some decisions to IGNORE/DELETED these
>> same resources for some reason before.  I think we need to steer them back
>> to the existing process here instead to overwriting.
>>> 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)?
>> Again, this isn't really clearly spelled out either but most of the time
>> it's the AD queue that we want the customer to go with.  Also it's already
>> a well understood and interactive part of the current resource update
>> workflow.  By interactive here I mean that it's commonly understood that
>> updating the plugin configuration is to be done by the UI or CLI Resource*
>> api methods.
>>> 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 ;)
>> * FROM mazz's clarification I swapped DELETED in instead of UNINVENTORIED.
>>
>> Glad you added the table. I should have done that with the initial e-mail.
>> :) However we're not in agreement about whether your described process is
>> clean to the customer.  More specificially, if I've IGNORED a resource
>> explicitly, I don't want those resources to slip back into my COMMITTED
>> resource queue because I happen to put together a pluginConfiguration RK
>> combination that's been seen before.  I would expect errors in the UI
>> telling me that said resource is already indexed by RHQ but IGNORED(and
>> that I should navigate to that resource and address changes/updates from
>> there... kind of a thing.). However I think for DELETED that the flow you
>> described does make sense.
>>
>> This returns me back to an earlier point that I tried to make that Manual
>> Import really should not be in the business of updating resources and it
>> should have clearer semantics. As I understand it, manual import has two
>> clear use cases a)the resource truly isn't autodiscoverable or
>> b)you want to import resources that are not actually running.
>> To modify the behavior so that's it's a better backdoor to update existing
>> resources, when we already have an established process for resource update
>> doesn't seem like the right thing to me.  It would be cleaner and clearer
>> if Manual Import stayed out of resource update and errors out if it can't.
>> But let's hope we get some more feedback about this issue.
>>> 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
>>> _______________________________________________
>>> 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