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

[RFC] Don't replace proxy with random port in AddOrReplace #356

Closed
wants to merge 1 commit into from

Conversation

tinnet
Copy link

@tinnet tinnet commented Jan 12, 2022

When asking ProxyCollection to AddOrReplace a proxy with a random
listening port (:0), don't stop and replace an existing one with
the same name and upstream.

When asking ProxyCollection to `AddOrReplace` a proxy with a random
listening port (`:0`), don't stop and replace an existing one with
the same `name` and `upstream`.
@miry miry added this to the 2.4.0 milestone Jan 13, 2022
func TestAddOrReplaceProxyToCollectionWithRandomPort(t *testing.T) {
collection := toxiproxy.NewProxyCollection()
proxy1 := NewTestProxy("test", "localhost:20000")
proxy2 := NewTestProxy("test", "localhost:0")
Copy link
Contributor

Choose a reason for hiding this comment

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

As I understand the Listen for NewTestProxy is always "localhost:0". The second argument is upstream.

if err != nil {
return err
}
if port == "0" {
Copy link
Contributor

@miry miry Jan 13, 2022

Choose a reason for hiding this comment

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

I would add also empty port to same ( Listen = "" works the same as ":0")

// If the port in the address parameter is empty or "0", as in
// "127.0.0.1:" or "[::1]:0", a port number is automatically chosen.
// The Addr method of Listener can be used to discover the chosen
// port.

https://play.golang.com/p/qXvL6s63ZbE

func main() {
	var port string

	listens := []string{
		"",
		"localhost",
		"localhost:0",
		":0",
	}
	for _, listen := range listens {
		_, port, _ = net.SplitHostPort(listen)
		fmt.Printf("port for '%s': %v\n", listen, port)
	}

}

output:

port for '': 
port for 'localhost': 
port for 'localhost:0': 0
port for ':0': 0

@miry miry removed this from the 2.4.0 milestone Jan 13, 2022
@miry miry added the question label Jan 13, 2022
@miry miry modified the milestone: 2.4.0 Jan 20, 2022
@miry miry added the Toxiproxy label Jan 20, 2022
@miry
Copy link
Contributor

miry commented Jun 10, 2022

Clossing for now. Pls reopen if it is still required.

@miry miry closed this Jun 10, 2022
@miry miry deleted the mcn/port-0-in-addreplace branch June 10, 2022 09:49
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.

2 participants