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.

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

[WIP] Add some debugging for kubeclient #508

Closed
wants to merge 1 commit into from

Conversation

jrafanie
Copy link
Member

Kubeclient uses RestClient for 4.x versions for the main connection but the http gem for watches. On master, kubeclient uses faraday for main connections. This commit provides a way to patch restclient/faraday and http to add logging of request/response.

TODO: We will need to provide a way to sanitize passwords and possibly other information.

Kubeclient uses RestClient for 4.x versions for the main connection
but the http gem for watches.  On master, kubeclient uses faraday for
main connections.  This commit provides a way to patch restclient/faraday
and http to add logging of request/response.

TODO: We will need to provide a way to sanitize passwords and possibly other
information.
@jrafanie
Copy link
Member Author

I'm seeing "body has already been consumed" locally with this patch, specifically, the WatchThread prepend:

[----] E, [2023-10-25T14:18:08.053516 #19540:f89918] ERROR -- evm: MIQ(ManageIQ::Providers::Openshift::ContainerManager::RefreshWorker::WatchThread#collector_thread) Watch thread for build_configs failed: body has already been consumed
[----] E, [2023-10-25T14:18:08.053723 #19540:f89918] ERROR -- evm: [HTTP::StateError]: body has already been consumed  Method:[block (2 levels) in <class:LogProxy>]
[----] E, [2023-10-25T14:18:08.053792 #19540:f89918] ERROR -- evm: /Users/joerafaniello/.gem/ruby/3.0.6/gems/http-5.1.1/lib/http/response/body.rb:67:in `stream!'
/Users/joerafaniello/.gem/ruby/3.0.6/gems/http-5.1.1/lib/http/response/body.rb:29:in `readpartial'
/Users/joerafaniello/.gem/ruby/3.0.6/gems/http-5.1.1/lib/http/response/body.rb:37:in `each'
/Users/joerafaniello/.gem/ruby/3.0.6/gems/kubeclient-4.11.0/lib/kubeclient/watch_stream.rb:25:in `each'
/Users/joerafaniello/Code/manageiq-providers-kubernetes/app/models/manageiq/providers/kubernetes/container_manager/refresh_worker/watch_thread.rb:56:in `collector_thread'
/Users/joerafaniello/Code/manageiq-providers-kubernetes/app/models/manageiq/providers/kubernetes/container_manager/refresh_worker/watch_thread.rb:26:in `block in start!'
D, [2023-10-25T14:18:08.054134 #19540] DEBUG -- : Audit-Id: 28cab2dd-30c1-4865-ae6b-2f53cd4c50bd

This still needs to be resolved.

if conn.respond_to?(:configure_faraday)
conn.configure_faraday { |faraday| faraday.response :logger }
else
RestClient.log = STDOUT
Copy link
Member

Choose a reason for hiding this comment

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

Should this be STDERR, to match the $stderr used in the prepended patch?

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 it's blowing up so I've shelved it for now.

@miq-bot
Copy link
Member

miq-bot commented Oct 31, 2023

Checked commit jrafanie@1237e0d with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
2 files checked, 6 offenses detected

app/models/manageiq/providers/kubernetes/container_manager.rb

lib/manageiq/providers/kubernetes/debug_kubeclient_watch.rb

@miq-bot miq-bot added the stale label Feb 5, 2024
@miq-bot
Copy link
Member

miq-bot commented Feb 5, 2024

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s).

module Kubernetes
module DebugKubeclientWatch
def build_client
super.use(logging: { logger: Logger.new($stderr) })
Copy link
Member

Choose a reason for hiding this comment

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

would a tap fix this?

i.e.:

Suggested change
super.use(logging: { logger: Logger.new($stderr) })
super.tap { |client| client.use(:logging => {:logger => Logger.new($stderr) }) }

... sorry. flyby ...

@miq-bot
Copy link
Member

miq-bot commented Jun 17, 2024

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s).

1 similar comment
@miq-bot
Copy link
Member

miq-bot commented Sep 23, 2024

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s).

@jrafanie
Copy link
Member Author

Stale, closing for now

@jrafanie jrafanie closed this Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants