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

feat: improve request proxy log in yurthub component (#1772) #1783

Merged
merged 1 commit into from
Nov 14, 2023

Conversation

crazytaxii
Copy link
Contributor

What type of PR is this?

/kind feature

What this PR does / why we need it:

Improve request proxy log in yurthub component.

Which issue(s) this PR fixes:

Fixes #1772

@openyurt-bot openyurt-bot added the kind/feature kind/feature label Nov 8, 2023
Copy link

sonarqubecloud bot commented Nov 8, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@openyurt-bot
Copy link
Collaborator

Welcome @crazytaxii! It looks like this is your first PR to openyurtio/openyurt 🎉

@openyurt-bot openyurt-bot added the size/XS size/XS: 0-9 label Nov 8, 2023
@YTGhost
Copy link
Member

YTGhost commented Nov 8, 2023

E1108 15:02:44.317556   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:02:46.317393   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:02:48.316606   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:02:50.316611   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:02:52.316575   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:02:54.317090   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:02:56.316766   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:02:58.316611   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:03:00.317606   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:03:02.317576   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:03:04.317566   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:03:06.317298   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:03:08.316631   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:03:10.316608   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:03:12.317595   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:03:14.317588   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:03:16.317438   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:03:18.316640   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:03:20.317399   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:03:22.316664   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:03:24.316602   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:03:26.316572   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:03:28.317489   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:03:30.317208   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:03:32.317452   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:03:34.317015   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:03:36.317172   12757 manager.go:139] hub certificates are not ready: ca.crt file
E1108 15:03:36.317207   12757 manager.go:139] hub certificates are not ready: ca.crt file
    manager_test.go:142: certificates are not ready, timed out waiting for the condition
--- FAIL: TestReady (60.00s)
FAIL
coverage: 81.1% of statements
FAIL	github.com/openyurtio/openyurt/pkg/yurthub/certificate/manager	60.018s

@rambohe-ch The error in this unit test has nothing to do with what this pr modifies, maybe we can extend the wait time for this unit test a bit? And I think adding the /retest ability for ci is necessary, and then we can get this issue out of the way as well.

@rambohe-ch
Copy link
Member

@rambohe-ch The error in this unit test has nothing to do with what this pr modifies, maybe we can extend the wait time for this unit test a bit? And I think adding the /retest ability for ci is necessary, and then we can get this issue out of the way as well.

@YTGhost Thanks for your advice, i think it's a good idea. i will add a command like /rerun for re-running all failed actions, at the same time, in order to get rid of abusing this feature, only users with the reviewer role or higher have access to trigger it.

@YTGhost
Copy link
Member

YTGhost commented Nov 9, 2023

/rerun

Copy link

codecov bot commented Nov 9, 2023

Codecov Report

Merging #1783 (bb49fa6) into master (c1a4760) will decrease coverage by 0.06%.
Report is 1 commits behind head on master.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1783      +/-   ##
==========================================
- Coverage   52.19%   52.14%   -0.06%     
==========================================
  Files         172      172              
  Lines       20884    20884              
==========================================
- Hits        10901    10889      -12     
- Misses       9021     9032      +11     
- Partials      962      963       +1     
Flag Coverage Δ
unittests 52.14% <100.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
pkg/yurthub/proxy/util/util.go 62.30% <100.00%> (ø)

... and 1 file with indirect coverage changes

Copy link
Member

@YTGhost YTGhost left a comment

Choose a reason for hiding this comment

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

/lgtm

@crazytaxii
Copy link
Contributor Author

/assign @rambohe-ch

@openyurt-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: crazytaxii, rambohe-ch, YTGhost

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openyurt-bot openyurt-bot added the approved approved label Nov 14, 2023
@rambohe-ch rambohe-ch merged commit 3636ec0 into openyurtio:master Nov 14, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved approved kind/feature kind/feature lgtm lgtm size/XS size/XS: 0-9
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature request]Improve request proxy log in yurthub component.
4 participants