Some proposed changes to Manual Import functionality

Thomas Segismont tsegismo at redhat.com
Tue May 7 14:04:13 UTC 2013


Hi Simeon,

What about a mixed approach? We could show an error message as you 
suggest and in the message box give the user the opportunity to confirm 
he wants to override the auto-discovered resource.

Cheers

Le 03/05/2013 02:39, Simeon Pinder a écrit :
> Hi all,
>
>    We got a few different responses to the RFC on correct Manual Import semantics but did not really arrive at consensus.  I've summarized the existing arguments into a clearer layout.
>
> https://docs.jboss.org/author/display/RHQ45/Manual+Add+Facet+semantics
>
>    Your additional comments/suggestions are appreciated so that we can agree on what makes the most sense for our customers moving forward.
>
> -Simeon
>
> ----- Original Message -----
>> From: "Jay Shaughnessy" <jshaughn at redhat.com>
>> To: rhq-devel at lists.fedorahosted.org
>> Sent: Thursday, April 25, 2013 5:07:28 PM
>> Subject: Re: Some proposed changes to Manual Import functionality
>>
>>
>> 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
>>
>> _______________________________________________
>> 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