-
Notifications
You must be signed in to change notification settings - Fork 57
validate resource exists #593
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
Conversation
|
SAP employees are expected to use their SAP-email address for commits related to their work. Our compliance check has detected usage of an email other than a SAP one by a SAP employee. Please update your pull request accordingly. If you think this is wrong or need any assistance, please contact [email protected]. |
| var smError *sm.ServiceManagerError | ||
| if ok := errors.As(err, &smError); ok { | ||
| if smError.StatusCode == http.StatusNotFound { | ||
| condition := metav1.Condition{ |
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.
maybe we should check header
x-cf-routererror | unknown_route
in case SM is down?
same for instance
| Status: metav1.ConditionFalse, | ||
| ObservedGeneration: serviceBinding.Generation, | ||
| Reason: common.ResourceNotFound, | ||
| Message: fmt.Sprintf("Binding %s not found for this cluster or namespace; or it is not managed by this operator-access instance.", serviceInstance.Status.InstanceID), |
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.
put it in const with placeholder for resource type
| if utils.IsMarkedForDeletion(serviceBinding.ObjectMeta) { | ||
| return r.delete(ctx, serviceBinding, serviceInstance) | ||
| } else if len(serviceBinding.Status.BindingID) > 0 { | ||
| if _, err := smClient.GetBindingByID(serviceBinding.Status.BindingID, nil); err != nil { |
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.
maybe we don't need it for binding?
because the instance will be not ready already.. we reduce a lot of calls...
|
|
||
| if utils.IsMarkedForDeletion(serviceInstance.ObjectMeta) { | ||
| return r.deleteInstance(ctx, serviceInstance) | ||
| } else if len(serviceInstance.Status.InstanceID) > 0 { |
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.
if it will be after hash check we will not bring it twice first time after update
| if err != nil { | ||
| log.Error(err, "failed to get sm client") | ||
| return utils.HandleOperationFailure(ctx, r.Client, serviceInstance, common.Unknown, err) | ||
| } |
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.
move to line 109 before use same for binding
No description provided.