Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: set logger for controller-runtime #1211

Merged
merged 1 commit into from
Aug 8, 2024
Merged

fix: set logger for controller-runtime #1211

merged 1 commit into from
Aug 8, 2024

Conversation

yeahdongcn
Copy link
Contributor

Closes #1208

📑 Description

Set logger for controller-runtime.

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

ℹ Additional Information

Testing done:

  • make docker-build -> ok
  • After applying the newly built Docker image in k8sgpt-operator, this log [controller-runtime] log.SetLogger(...) was never called; logs will not be displayed. is no longer present.

@yeahdongcn yeahdongcn requested review from a team as code owners August 1, 2024 07:50
cf250024
cf250024 approved these changes Aug 2, 2024
View reviewed changes
Copy link

@cf250024 cf250024 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the changes and it works for me. Thanks.

@cf250024 cf250024 mentioned this pull request Aug 2, 2024
Closed
4 tasks
@cf250024
Copy link

cf250024 commented Aug 6, 2024

Can we put new code in pkg/server/serve.go instead of cmd/serve/serve.go?
I'm thinking this part is more related to server that cli. I tried this option prior your PR and it also works.

@AlexsJones
Copy link
Member

Can we put new code in pkg/server/serve.go instead of cmd/serve/serve.go? I'm thinking this part is more related to server that cli. I tried this option prior your PR and it also works.

I would be supportive of that also

AlexsJones
AlexsJones requested changes Aug 7, 2024
View reviewed changes
Copy link
Member

@AlexsJones AlexsJones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move this into the pkg/server/serve.go

@yeahdongcn
fix: set logger for controller-runtime
29a029b 
Signed-off-by: Xiaodong Ye <yeahdongcn@gmail.com>
@yeahdongcn
Copy link
Contributor Author

Please move this into the pkg/server/serve.go

Done. @cf250024 @AlexsJones

@AlexsJones AlexsJones merged commit 8e37369 into k8sgpt-ai:main Aug 8, 2024
8 checks passed
@github-actions github-actions bot mentioned this pull request Aug 7, 2024
Merged
ronaldpetty pushed a commit to ronaldpetty/k8sgpt that referenced this pull request Aug 17, 2024
@yeahdongcn @ronaldpetty
fix: set logger for controller-runtime (k8sgpt-ai#1211)
d4cf0ca 
Signed-off-by: Xiaodong Ye <yeahdongcn@gmail.com>
@BrewTestBot BrewTestBot mentioned this pull request Sep 23, 2024
Merged
AlexsJones pushed a commit that referenced this pull request Oct 24, 2024
@yeahdongcn @AlexsJones
fix: set logger for controller-runtime (#1211)
bdb0f2c 
Signed-off-by: Xiaodong Ye <yeahdongcn@gmail.com>
Signed-off-by: AlexsJones <alexsimonjones@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cf250024 cf250024 cf250024 approved these changes

@AlexsJones AlexsJones AlexsJones approved these changes

Assignees
No one assigned
Labels
None yet
Projects
Backlog
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: The K8sGPT docker image can't log in the container provisioned with K8sGPT CRD.
3 participants
@yeahdongcn @cf250024 @AlexsJones