New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug 1583064 - missing action causes many sandboxes #972
Conversation
If the unbind fails we really want to let them know it failed because it is probably something the APB developer needs to fix. But if the action is non-existent we really need to prevent the multiple sandboxes. If we can't find the action, there is no point in continuing. marking job as SUCCEEDED but setting the message to be the actual error string. This will only happen for the optional actions: bind & unbind. Update isJobInProgress to take a string to allow the proper ID to be passed in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
VISACK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the nit LGTM
| @@ -732,7 +732,7 @@ func (a AnsibleBroker) Deprovision( | |||
| return nil, err | |||
| } | |||
|
|
|||
| alreadyInProgress, jobToken, err := a.isJobInProgress(&instance, apb.JobMethodDeprovision) | |||
| alreadyInProgress, jobToken, err := a.isJobInProgress(instance.ID.String(), apb.JobMethodDeprovision) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
pkg/broker/job_state_subscriber.go
Outdated
| @@ -35,6 +36,28 @@ func (jss *JobStateSubscriber) Notify(msg JobMsg) { | |||
| if isBinding(msg) { | |||
| id = msg.BindingUUID | |||
| } | |||
|
|
|||
| log.Debugf("message: method: [%v] isNotFound? [%v] state error [%v]", msg.State.Method, msg.State.Error == runtime.ErrorActionNotFound.Error(), msg.State.Error) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: I think this might have been for testing is it still needed?
If the unbind fails we really want to let them know it failed because it
is probably something the APB developer needs to fix. But if the action
is non-existent we really need to prevent the multiple sandboxes.
If we can't find the action, there is no point in continuing.
marking job as SUCCEEDED but setting the message to be the actual error
string. This will only happen for the optional actions: bind & unbind.
Update isJobInProgress to take a string to allow the proper ID to be
passed in.