brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Svetoslav Neykov <s...@cloudsoft.io>
Subject Re: Proposal: Add appId optional paramater to deploy api
Date Wed, 26 Jul 2017 09:16:14 GMT
I think both proposals make sense. Even if implemented separately.

Wondering what's the right place to put the ID though. Isn't the tags a better place for that?
 I suppose it depends on whether we want YAML blueprints to have access to it.

Svet. 


> On 25.07.2017 г., at 21:56, Alex Heneveld <alex.heneveld@cloudsoftcorp.com> wrote:
> 
> Aled-
> 
> Should this be applicable to all POST/DELETE calls?  Imagine an
> `X-caller-request-uid` and a filter which caches them server side for a
> short period of time, blocking duplicates.
> 
> Solves an overlapping set of problems.  Your way deals with a
> "deploy-if-not-present" much later in time.
> 
> --A
> 
> On 25 July 2017 at 17:44, Aled Sage <aled.sage@gmail.com> wrote:
> 
>> Hi all,
>> 
>> I've been exploring adding support for `&deploymentUid=...` - please see
>> my work-in-progress PR [1].
>> 
>> Do people think that is a better or worse direction than supporting
>> `&appId=...` (which would likely be simpler code, but exposes the Brooklyn
>> internals more).
>> 
>> For `&appId=...`, we could either revert [2] (so we could set the id in
>> the EntitySpec), or we could inject it via a different (i.e. add a new)
>> internal way so that it isn't exposedon our Java api classes.
>> 
>> Thoughts?
>> 
>> Aled
>> 
>> [1] https://github.com/apache/brooklyn-server/pull/778
>> 
>> [2] https://github.com/apache/brooklyn-server/pull/687/commits/5
>> 5eb11fa91e9091447d56bb45116ccc3dc6009df
>> 
>> 
>> 
>> On 07/07/2017 18:28, Aled Sage wrote:
>> 
>>> Hi,
>>> 
>>> Taking a step back to justify why this kind of thing is really
>>> important...
>>> 
>>> This has come up because we want to call Brooklyn in a robust way from
>>> another system, and to handle a whole load of failure scenarios (e.g. that
>>> Brooklyn is temporarily down, connection fails at some point during the
>>> communication, the client in the other system goes down and another
>>> instance tries to pick up where it left off, etc).
>>> 
>>> Those kind of thing becomes much easier if you can make certain
>>> assumptions such as an API call being idempotent, or it guaranteeing to
>>> fail with a given error if that exact request has already been accepted.
>>> 
>>> ---
>>> 
>>> I much prefer the semantics of the call failing (with a meaningful error)
>>> if the app already exists - that will make retry a lot easier to do safely.
>>> 
>>> As for config keys on the app, in Duncan's use-case he'd much prefer to
>>> not mess with the user's YAML (e.g. to inject another config key before
>>> passing it to Brooklyn). It would be simpler in his case to supply in the
>>> url `?appId=...` or `?deploymentId=...`.
>>> 
>>> For using `deploymentId`, we could but that feels like more work. We'd
>>> want create a lookup of applications indexed by `deploymentId` as well as
>>> `appId`, and to fail if it already exists. Also, what if someone also
>>> defines a config key called `deploymentId` - would that be forbidden? Or
>>> would we name-space the config key with `org.apache.brooklyn.deploymentId`?
>>> Even with those concerns, I could be persuaded of the
>>> `org.apache.brooklyn.deploymentId` approach.
>>> 
>>> For "/application's ID is not meant to be user-supplied/", that has
>>> historically been the case but why should we stick to that? What matters is
>>> that the appId is definitely unique. That will be checked when creating the
>>> application entity. We could also include a regex check on the supplied id
>>> to make sure it looks reasonable (in case someone is already relying on app
>>> ids in weird ways like for filename generations, which would lead to a risk
>>> of script injection).
>>> 
>>> Aled
>>> 
>>> 
>>> On 07/07/2017 17:38, Svetoslav Neykov wrote:
>>> 
>>>> Hi Duncan,
>>>> 
>>>> I've solved this problem before by adding a caller generated config key
>>>> on the app (now it's also possible to tag them), then iterating over the
>>>> deployed apps, looking for the key.
>>>> 
>>>> An alternative which I'd like to mention is creating an async deploy
>>>> operation which immediately returns an ID generated by Brooklyn. There's
>>>> still a window where the client connection could fail though, however small
>>>> it is, so it doesn't fully solve your use case.
>>>> 
>>>> Your use case sounds reasonable so agree a solution to it would be nice
>>>> to have.
>>>> 
>>>> Svet.
>>>> 
>>>> 
>>>> On 7.07.2017 г., at 18:33, Duncan Grant <duncan.grant@cloudsoftcorp.com>
>>>>> wrote:
>>>>> 
>>>>> I'd like to propose adding an appId parameter to the deploy endpoint.
>>>>> This
>>>>> would be optional and would presumably reject any attempt to start a
>>>>> second
>>>>> app with the same id.  If set the appId would obviously be used in
>>>>> place of
>>>>> the generated id.
>>>>> 
>>>>> This proposal would be of use in scripting deployments in a distributed
>>>>> environment where deployment is not the first step in a number of
>>>>> asynchronous jobs and would give us a way of "connecting" those jobs
up.
>>>>> Hopefully it will help a lot in making things more robust for end-users.
>>>>> Currently, if the client’s connection to the Brooklyn server fails
while
>>>>> waiting for a response, it’s impossible to tell if the app was
>>>>> provisioned
>>>>> (e.g. how can you tell the difference between a likely-looking app, and
>>>>> another one deployed with an identical blueprint?). This would make it
>>>>> safe
>>>>> to either retry the deploy request, or to query for the app with the
>>>>> expected id to see if it exists.
>>>>> 
>>>>> Initially I'm hoping to use this in a downstream project but I think
>>>>> this
>>>>> would be useful to others.
>>>>> 
>>>>> If no one has objections I'll aim to implement this over the next
>>>>> couple of
>>>>> weeks.  On the other hand I'm totally open to suggestions of a better
>>>>> approach.
>>>>> 
>>>>> Thanks
>>>>> 
>>>>> Duncan Grant
>>>>> 
>>>> 
>>> 
>>> 
>> 


Mime
View raw message