Some proposed changes to Manual Import functionality

Simeon Pinder spinder at redhat.com
Fri May 3 00:39:50 UTC 2013


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
> 


More information about the rhq-devel mailing list