openshift / origin-web-catalog

Apache License 2.0
18 stars 60 forks source link

origin-web-catalog does not honor updates_to on plans #533

Closed jmontleon closed 6 years ago

jmontleon commented 6 years ago

We specify in the dev plan that it can be updated to prod in the updates_to array in the dev plan and I can do that. (see comment at: openshift/origin-web-console#2350 for catalog output.)

However we don't specify that prod can be 'downgraded' to dev (the updates_to array is empty and omitted in the catalog output, but the UI still lets me try to do that.

@eriknelson @jmrodri @pmorie @spadgett This was in the proposal I was working off in our docs, though it looks not to be in the broker spec. https://github.com/openshift/ansible-service-broker/blob/master/docs/proposals/updates-first-pass.md#apb

I don't know how you'd all like to proceed.

spadgett commented 6 years ago

@jwforres

jmontleon commented 6 years ago

The UI appears to remove the Edit option when we fail to update

I see

Status:
    Failed 
Status Reason:
    Error updating ServiceInstance of ClusterServiceClass (K8S: "27793015fe45db2fbc1deb7372cc4036" ExternalName: "dh-rhscl-postgresql-apb") at ClusterServiceBroker "ansible-service-broker": Status: 400; ErrorMessage: <nil>; Description: plan update not possible; ResponseError: <nil> 

and no more option to edit. Would it be possible to retain the option to edit even though the update task failed?

jmrodri commented 6 years ago

Without a mechanism to validate possible plan transitions from the UI side, the broker can only handle the validation server-side by returning an error that it is not allowed. So @jmontleon point is valid that we will need to continue edit.

spadgett commented 6 years ago

@jeff-phillips-18 How hard is it to re-enable the back button to make additional changes on errors?

Depending on the size of this change, it might have to come post 3.7.

jeff-phillips-18 commented 6 years ago

Should be able to do that relatively easily. I think the issue here is allowing Edit on service instance's with a failed state. Can we tell the reason for failure? Should we allow edit of all failed instances?

spadgett commented 6 years ago

Is the service permanently failed if an update fails? That seems bad.

jeff-phillips-18 commented 6 years ago

Seems like it stays failed 😢

jeff-phillips-18 commented 6 years ago

@spadgett @jmrodri How should we proceed here? Allow edit of failed instances?

pmorie commented 6 years ago

Can someone clarify for me what the "updates_to" array is?

jmrodri commented 6 years ago

the updates_to identifies what plans can transition to what other plans. That is you can go from dev to prod and prevents prod going to dev.

jmrodri commented 6 years ago

Right now I'm looking through our catalog response and it seems we have an updates_to which is not spec compliant. So it makes sense as to why it is not being respected.

The larger issue is that if we use the current mechanism, of doing server-side validation, we will return an error if the plan transition is invalid. Which as stated https://github.com/openshift/origin-web-catalog/issues/533#issuecomment-339458438 the instance is marked as failed forever.

pmorie commented 6 years ago

updates_to is some internal implementation detail of the ansible broker, right?

spadgett commented 6 years ago

Closing as this is now tracked by https://bugzilla.redhat.com/show_bug.cgi?id=1507844