馃尡 Fix log keys, use upper case for logs #10613
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/L
|
/assign @fabriziopandini @chrischdi |
|
/test pull-cluster-api-e2e-main |
|
/retest |
|
/lgtm one optional question/idea: shouldn't we be always able now to use |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 10293cc098cd28332c7937564a5f724df467ed6a
|
internal/controllers/machinedeployment/machinedeployment_sync.go
Outdated
Show resolved
Hide resolved
There is no guarantee that the Kind is set in typed objects (only for Unstructured & PartialObjectMeta) IIRC it depends on if a live client or the cache is used to retrieve the object. But we just shouldn't depend on it (only for Unstructured & PartialObjectMeta) Bonus info: The reason why kind must be set for Unstructured & PartialObjectMeta is because otherwise there would be no way to tell what kind they have. This doesn't apply for typed objects. |
|
^^ @chrischdi I hope I didn't use it incorrectly in this PR :) |
Signed-off-by: Stefan B眉ringer buringerst@vmware.com
k8s-ci-robot
removed
the
lgtm
|
Added one small fix to be sure that we have a GVK during cluster deletion. I checked the logs earlier and the GVK is actually filled out, but we just shouldn't rely on GVK being set for typed objects For client.Object's this also usually looks like this |
|
@sbueringer: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
| @@ -488,7 +488,7 @@ func (r *MachinePoolReconciler) infraMachineToMachinePoolMapper(ctx context.Cont | |||
| if labels.IsMachinePoolOwned(o) { | |||
| machinePool, err := utilexp.GetMachinePoolByLabels(ctx, r.Client, o.GetNamespace(), o.GetLabels()) | |||
| if err != nil { | |||
| log.Error(err, "failed to get MachinePool for InfraMachine", "infraMachine", klog.KObj(o), "labels", o.GetLabels()) | |||
| log.Error(err, "Failed to get MachinePool for InfraMachine", o.GetObjectKind().GroupVersionKind().Kind, klog.KObj(o), "labels", o.GetLabels()) | |||
There was a problem hiding this comment.
Btw this is only safe because this func is used with an unstructured informer. So we can infer that kind must be set
|
/test pull-cluster-api-e2e-main |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 1a399804ffc3109bccd6a1701936bb0fedf3dc6b
|
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fabriziopandini The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
Signed-off-by: Stefan B眉ringer buringerst@vmware.com
What this PR does / why we need it:
Maybe wouldn't have done it if I had realized earlier that we have >1k usages of WithValues / Info / Error, but here we go.
Went over all of those and made sure we follow log conventions
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Related to #9447