Resource client patch for apply status #517
Conversation
|
Issues linked to changelog: |
…nto resource_client_patch
| if errors.IsNotExist(writeErr) { | ||
| logger.Debugf("did not write report for %v : %v because resource was not found", resourceToWrite.GetMetadata().Ref(), status) | ||
| delete(resourceErrs, resource) | ||
| continue | ||
| } | ||
|
|
||
| if writeErr != nil { |
There was a problem hiding this comment.
What is different about the 2 if statements here? I am unfamiliar with errors.IsNotExist(writeErr).
There was a problem hiding this comment.
that error is for the scenario when we try to patch a resource that no longer exists (e.g. a user deleted it). rather than report an error in the logs, we just remove it from our in memory report and swallow the non-error. for any other error type we will report the error
|
|
||
| buf := &bytes.Buffer{} | ||
| var marshaller jsonpb.Marshaler | ||
| marshaller.EnumsAsInts = false // prefer jsonpb over encoding/json marshaller since it renders enum as string not int (i.e., state is human-readable) |
There was a problem hiding this comment.
Currently we write out ints for the states. If we want to switch to writing strings we should call it out in the changelog. I assume that this change won't affect any tooling around statuses because the unmarshaller will handle it the same regardless.
There was a problem hiding this comment.
happy to call that out in the changelog. yeah we currently use the jsonpb to unmarshal so it can handle either format, so im just opting for better readability
| if apierrors.IsNotFound(err) { | ||
| return nil, errors.NewNotExistErr(namespace, name, err) | ||
| } | ||
| return nil, errors.Wrapf(err, "patching secret from kubernetes") |
There was a problem hiding this comment.
Clarify in this message that we're patching the status on the secret. Users tend to zero in on log messages that have things like "secret" in them so it's good to be as clear as possible.
There was a problem hiding this comment.
Might as well include "status" in the other error messages well though.
This reverts commit 618b1c9.
lowers the number of api calls made to k8s apiserver; helps with status scalability for gloo as a client not getting rate limited talking to the k8s apiserver.
only marked with WIP label for now to prevent merges until solo-io/gloo#7133 passes ci; in the meantime this is ready for reviewgloo ci is passing