[Ansible-service-broker] Deprovision default behaviour

[Ryan Hallisey]

> So while sure adding this flag seems to fix one aspect of it, albeit a
> key aspect, it would be better to see a proposal on dev experience from
> a developer perspective. And try to address the issues outlined and
> discussed. That discussion might lead to this exact patch, but it will
> be seen in the broader picture of how to fix the dev experience.
>
Sounds good, let's go with a broader dev proposal.  Philip, is that
something you would be wiling to start? Here's a link to the template
https://github.com/openshift/ansible-service-broker/blob/master/docs/proposals/proposal-template.md

-Ryan

On Wed, Feb 14, 2018 at 11:49 AM, jesus m. rodriguez <jesusr at redhat.com>
wrote:

> On Wed, 2018-02-14 at 11:29 -0500, Ryan Hallisey wrote:
> > > > I don’t know if I feel comfortable with adding this logic to the
> > > > primary
> > > > code path for deprovision. This is mostly me being conservative
> > > > with
> >
> > adding
> > > > configuration values that can allow things to get deleted from
> > > > our
> >
> > storage.
> > > >
> > >
> > > The developer experience is not acceptable right now, but I have to
> > > agree with Shawn.
> > >
> >
> > Agreed, the dev experience is really rough with deprovision. For
> > instance,
> > if you are testing your provision playbook with the broker and your
> > deprovision playbook fails, then you can't test provision again
> > without
> > some manually steps.  But, I think Phillip's patch is a good way to
> > provide
> > significant improvement to the dev experience with minimal code
> > changes.
> > The meat of his code is 4 lines:
> >
> > ```go
> > if a.brokerConfig.ForceDelete {
> >   log.Infof("Deprovision failed. Attempting to clean up related
> > resources.
> > User should ensure clean up is successful")
> >   cleanupDeprovision(&instance, a.dao) // Return a 200 to the
> > service-catlog
> > }
> > ```
>
> I think the problem is not necessarily that it is a seemingly small
> patch. But that there is an overall bigger issue that the dev
> experience isn't completely thought out yet.
>
> So while sure adding this flag seems to fix one aspect of it, albeit a
> key aspect, it would be better to see a proposal on dev experience from
> a developer perspective. And try to address the issues outlined and
> discussed. That discussion might lead to this exact patch, but it will
> be seen in the broader picture of how to fix the dev experience.
>
> My hesitation with applying this right now is I'd rather see what the
> broader list of dev experience problems are. How we can address them?
> Do we have a single flag that enables a bunch of things? Do we have a
> set of dev flags that can be flipped for individual things? Etc.
>
> We already have a dev_broker flag. Now this patch is adding a
> force_delete flag. While it is to fix a dev experience problem, how do
> I know that flag is just for dev? What are the implications of enabling
> said flag in a production environment? It's the list of subtleties that
> I'd like to at least see or anticipate before addressing just one
> aspect of a problem.
>
> >
> > > Unless I missed it, let's backup and describe the high-level
> > > problem
> > > in a proposal and
> > > submit solutions there. That's been our approach thus far and I
> > > think
> > > it applies here well.
> >
> > To make this the default behaviour, I agree needs a proposal.
>
> I would rather have a proposal to discuss the dev experience and needs.
> Then have a series of PRs to address the needs that come from that
> proposal. Again this patch fixes only one aspect of the dev experience.
>
> jesus
>
> _______________________________________________
> Ansible-service-broker mailing list
> Ansible-service-broker at redhat.com
> https://www.redhat.com/mailman/listinfo/ansible-service-broker
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/ansible-service-broker/attachments/20180214/f0821502/attachment.htm>