-
Notifications
You must be signed in to change notification settings - Fork 79
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
Operator Enhancement #560
Operator Enhancement #560
Conversation
Signed-off-by: MUzairS15 <[email protected]>
Signed-off-by: MUzairS15 <[email protected]>
TODO: Replace logging with meshkit logger, redefine errors to be meshkit-errors. |
2.mov |
@@ -67,7 +68,9 @@ func (r *BrokerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr | |||
// Check if Broker controller deployed | |||
result, err := r.reconcileBroker(ctx, true, baseResource, req) | |||
if err != nil { | |||
return ctrl.Result{}, ErrReconcileBroker(err) | |||
err = ErrReconcileBroker(err) | |||
r.Log.Error(err, "broker reconcilation failed") |
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.
Reconciliation failures means what?
@@ -79,18 +82,24 @@ func (r *BrokerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr | |||
// Get broker endpoint | |||
err = brokerpackage.GetEndpoint(ctx, baseResource, r.Clientset, r.KubeConfig.Host) | |||
if err != nil { | |||
return ctrl.Result{}, ErrGetEndpoint(err) | |||
err = ErrGetEndpoint(err) | |||
r.Log.Error(err, "unable to get the broker endpoint") |
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.
meaning that the endpoint is undefined or that a network-based attempt failed?
} | ||
|
||
// Patch the broker resource | ||
patch, err := utils.Marshal(baseResource) | ||
if err != nil { | ||
return ctrl.Result{}, ErrUpdateResource(err) | ||
err = ErrUpdateResource(err) | ||
r.Log.Error(err, "unable to update broker resource") |
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.
Meaning that the resource couldn't be found or that it is present but the operator doesn't have permission or something else?
} | ||
|
||
err = r.Status().Patch(ctx, baseResource, client.RawPatch(types.MergePatchType, []byte(patch))) | ||
if err != nil { | ||
return ctrl.Result{}, ErrUpdateResource(err) | ||
err = ErrUpdateResource(err) | ||
r.Log.Error(err, "unable to update broker resource") |
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.
Meaning that the configuration was rejected from Kube API or that the resource isn't present or that the requested configuration was missing or invalid or what?
@@ -39,53 +39,53 @@ const ( | |||
|
|||
// Error definitions | |||
func ErrGetMeshsync(err error) error { | |||
return errors.New(ErrGetMeshsyncCode + ":" + "Unable to get meshsync resource") | |||
return errors.New(ErrGetMeshsyncCode + ":" + "Unable to get meshsync resource" + err.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.
Meaning that the custom resource was never created?
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.
As a user troubleshooting Meshery deployment, I'm not confident that I would know what to do when seeing these errors.
@innocentrda, your feedback here is helpful. |
Is there an adjoining addition to the https://docs.meshery.io/guides/troubleshooting/meshery-operator-meshsync guide? |
That's true. It's a good improvement to see those errors but being a bit detailed on why those errors and what to do can be helpful. |
Description
This PR fixes #
Notes for Reviewers
Signed commits