Skip to content

Conversation

@golangaccount
Copy link
Contributor

when goc server running in k8s
client register uri while be transform to is proxy address
in fact,there is no need to transform

add address uri check,make sure address parm is a complete uri

when goc server running  in k8s
client register uri while be transform to is proxy address
in fact,there is no need to transform

add address uri check,make sure address parm is a complete uri
@qiniu-bot qiniu-bot added size/M and removed size/S labels Jul 7, 2022
@golangaccount
Copy link
Contributor Author

/assign @CarlJi

@CarlJi
Copy link
Collaborator

CarlJi commented Jul 11, 2022

@golangaccount Thanks for your contribution.

The reason of this transform is try to find the proper IP especially in the local multi-interfaces scenario. And yes it's not elegant for the kubernetes scenario. To enhance it, we have solved this drawback in the goc v2 version by using web-socket protocol and welcome to have a try. https://github.com/qiniu/goc/tree/v2

For your PR, i am thinking that it's better to provide a switch in goc server to let user choose whether to do this transform or not than changing old behavior. since the default behavior should be compatibility with previous versions。

@golangaccount
Copy link
Contributor Author

cureent, i use goc v1.x in project,so i think has necessary to merge v1.x. yea,use this code,the nat network will not work well,i will add a parm in goc server to switch two conditions

add server runing parm network direct,
when direct,use user request parm uri,
default compare request parm uri host with client ip.
use direct mode,goc server can runing in k8s

change default mode port dispose rule
when port is empty,use 80 replace
because http default port is 80

add some rule to valid uri parm and
change some regist unit testing case
@qiniu-bot qiniu-bot added size/L and removed size/M labels Jul 11, 2022
@golangaccount
Copy link
Contributor Author

@CarlJi i edit the code,add a parm to control the network,if not set,it runing default

@CarlJi
Copy link
Collaborator

CarlJi commented Jul 15, 2022

Hi @golangaccount , if i understood your scenario correctly, switch off the ip revise during registering should meet your requirements。 so no need to change the logic of 'registry API', since it will affect the old behaviors。

@golangaccount
Copy link
Contributor Author

@CarlJi use bool can solve my problem,but there also has some question. if goc server is running, some client already regist,now i want to regist a new client with no revise,so i must set the value, use bool canot make sure the value is user set.

@CarlJi
Copy link
Collaborator

CarlJi commented Jul 18, 2022

while, if we want to let user be able to control this ability during registering from client side, i actually expect a new PR since it will also need to be changed for the goc build/install logic.

for your questions, i suggest to deploy multi goc servers. In my original design concepts, goc server should be lightweight and close to the services under tests。(PS: maybe it should be called goc proxy ... ) 😅

change goc server parm name,use ip_revise instead
@golangaccount
Copy link
Contributor Author

@CarlJi i change the parm,use ip_revise in goc server. i still retain request parm (iprevise) to change the server running style

@codecov
Copy link

codecov bot commented Jul 20, 2022

Codecov Report

Merging #286 (e04d051) into master (b4f0c87) will increase coverage by 0.14%.
The diff coverage is 74.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #286      +/-   ##
==========================================
+ Coverage   70.86%   71.00%   +0.14%     
==========================================
  Files          34       34              
  Lines        2011     2038      +27     
==========================================
+ Hits         1425     1447      +22     
- Misses        475      478       +3     
- Partials      111      113       +2     
Flag Coverage Δ
e2e-1.16.x ?
e2e-1.17.x ?
e2e-1.18.x ?
unittest-1.16.x 70.80% <74.28%> (-0.06%) ⬇️
unittest-1.17.x 70.90% <74.28%> (+0.04%) ⬆️
unittest-1.18.x 70.90% <74.28%> (+0.04%) ⬆️

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

Impacted Files Coverage Δ
cmd/server.go 45.45% <50.00%> (+1.01%) ⬆️
pkg/cover/server.go 83.84% <75.75%> (-1.95%) ⬇️
pkg/build/legacy.go 100.00% <0.00%> (+4.87%) ⬆️
pkg/build/gomodules.go 94.59% <0.00%> (+5.40%) ⬆️

@CarlJi
Copy link
Collaborator

CarlJi commented Jul 20, 2022

Hi @golangaccount , i am curious do you just use goc server alone?and how do you do instrumentation for your services without goc build/install ?

@golangaccount
Copy link
Contributor Author

@CarlJi i also use goc build/install, goc server --ip_revise=false can solve my k8s question(other code no need change).

when goc server running in k8s client register uri while be transform to is proxy address in fact,there is no need to transform

add address uri check,make sure address parm is a complete uri

@CarlJi
Copy link
Collaborator

CarlJi commented Jul 21, 2022

/lgtm
/approve

@qiniu-bot qiniu-bot added the lgtm label Jul 21, 2022
@qiniu-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CarlJi, golangaccount

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

@qiniu-bot qiniu-bot merged commit 5ccfc73 into qiniu:master Jul 21, 2022
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.

3 participants