cluster-api-provider-vsphere icon indicating copy to clipboard operation
cluster-api-provider-vsphere copied to clipboard

Logs do not show resource names

Open srm09 opened this issue 2 years ago • 8 comments

/kind bug

What steps did you take and what happened: Checking the log output for the capv manager pod, we see these entries

I0719 18:34:52.688028       1 vspheremachine_controller.go:159] capv-controller-manager/vspheremachine-controller "msg"="Waiting for Machine Controller to set OwnerRef on VSphereMachine"
I0719 18:34:52.688471       1 vspheremachine_controller.go:159] capv-controller-manager/vspheremachine-controller "msg"="Waiting for Machine Controller to set OwnerRef on VSphereMachine"
I0719 18:34:52.692036       1 vspheremachine_controller.go:159] capv-controller-manager/vspheremachine-controller "msg"="Waiting for Machine Controller to set OwnerRef on VSphereMachine"

What did you expect to happen: We expect the name of the VsphereMachine to be appended for easier debugging purposes.

I0719 18:34:52.692036       1 vspheremachine_controller.go:159] capv-controller-manager/vspheremachine-controller/<foo-machine> "msg"="Waiting for Machine Controller to set OwnerRef on VSphereMachine"

Anything else you would like to add: n/a

Environment:

  • Cluster-api-provider-vsphere version: v1.3.1
  • Kubernetes version: (use kubectl version):
  • OS (e.g. from /etc/os-release):

srm09 avatar Jul 19 '22 18:07 srm09

/help /good-first-issue

srm09 avatar Jul 19 '22 22:07 srm09

@srm09: This request has been marked as suitable for new contributors.

Guidelines

Please ensure that the issue body includes answers to the following questions:

  • Why are we solving this issue?
  • To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
  • Does this issue have zero to low barrier of entry?
  • How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-good-first-issue command.

In response to this:

/help /good-first-issue

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/test-infra repository.

k8s-ci-robot avatar Jul 19 '22 22:07 k8s-ci-robot

I will try to work on this. Be patient - it's my first issue :)

ameddin73 avatar Sep 16 '22 01:09 ameddin73

/assign @ameddin73

ameddin73 avatar Sep 16 '22 19:09 ameddin73

I couldn't get it to build. Looks like I don't have enough experience with VSphere :/ /unassign @ameddin73

ameddin73 avatar Sep 28 '22 15:09 ameddin73

@srm09 can you confirm that the issue is described properly?

  1. going through the code this is the only place I can find which seems to be textually close to the logs you provided (but it is doesn't match exactly)
  2. the exact match was last met in release v1.0.1 due to the fact that the direct construct r.Logger is used
  3. since then the message has been rephrased to match the recent text by @gab-satchi (in v1.1.0-alpha) and bug-fixed to use correct logger by you (in v1.3.0)
  4. though I did not reproduce the issue with missing OwnerRef in tests going through the logs that I was able to acquire gives us the following message
    I1001 18:34:39.335036   27303 vspheremachine_controller.go:299] "capv-controller-manager/vspheremachine-controller/vsphere-machine-reconciler-vgf9n/vsphere-machine-1: Cluster infrastructure is not ready yet"
    
    which comes from context logger which in order is set while building base context from the same logger that is used to print the OwnerRef message.

NB. The only place in this controller that is using non-contexted logger seem to be fetchCAPICluster(...) method but it's definitely not described in this task

AectannArd avatar Oct 01 '22 22:10 AectannArd

@srm09 could you have a look please?

AectannArd avatar Oct 04 '22 17:10 AectannArd

Seems that the issue shouldn't have been migrated to new release scope. It should have been closed.

AectannArd avatar Oct 13 '22 00:10 AectannArd

@AectannArd Good catch, the particular instance of the logger mentioned in the issue was fixed by the PR #1544 /close

srm09 avatar Oct 17 '22 19:10 srm09

@srm09: Closing this issue.

In response to this:

@AectannArd Good catch, the particular instance of the logger mentioned in the issue was fixed by the PR #1544 /close

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/test-infra repository.

k8s-ci-robot avatar Oct 17 '22 19:10 k8s-ci-robot