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

TODO and the comment is Inconsistent with RandomSuffix #87184

Open
tanjunchen opened this issue Jan 14, 2020 · 11 comments
Open

TODO and the comment is Inconsistent with RandomSuffix #87184

tanjunchen opened this issue Jan 14, 2020 · 11 comments

Comments

@tanjunchen
Copy link
Member

@tanjunchen tanjunchen commented Jan 14, 2020

ref:#86763

// RandomSuffix provides a random string to append to pods,services,rcs.
// TODO: Allow service names to have the same form as names
//       for pods and replication controllers so we don't
//       need to use such a function and can instead
//       use the UUID utility function.
func RandomSuffix() string {
	return strconv.Itoa(rand.Intn(10000))
}
return strconv.Itoa(rand.Intn(10000))  

Will return a fixed value instead of a random string.
TODO and the comment is Inconsistent with RandomSuffix.
This pr refer to change https://github.com/kubernetes/kubernetes/pull/71170/files#diff-eb7b79470992813ea1905e96c298b47bL2056

@tanjunchen

This comment has been minimized.

Copy link
Member Author

@tanjunchen tanjunchen commented Jan 14, 2020

@tanjunchen

This comment has been minimized.

Copy link
Member Author

@tanjunchen tanjunchen commented Jan 14, 2020

/cc @neolit123
/cc @ash2k

@neolit123

This comment has been minimized.

Copy link
Member

@neolit123 neolit123 commented Jan 14, 2020

/sig network

the function can use:
k8s.io/apimachinery/pkg/util/uuid

but you should ask #sig-network if this is OK.

@athenabot

This comment has been minimized.

Copy link

@athenabot athenabot commented Jan 14, 2020

/triage unresolved

Comment /remove-triage unresolved when the issue is assessed and confirmed.

🤖 I am a bot run by vllry. 👩‍🔬

@liggitt

This comment has been minimized.

Copy link
Member

@liggitt liggitt commented Jan 14, 2020

return strconv.Itoa(rand.Intn(10000))  

Will return a fixed value instead of a random string.

Why do you say that? Each invocation returns a different result.

@gavinfish

This comment has been minimized.

Copy link
Contributor

@gavinfish gavinfish commented Jan 14, 2020

return strconv.Itoa(rand.Intn(10000))  

Will return a fixed value instead of a random string.

Why do you say that? Each invocation returns a different result.

Quoted from https://gobyexample.com/random-numbers:

The default number generator is deterministic, so it’ll produce the same sequence of numbers each time by default. To produce varying sequences, give it a seed that changes.

Go playground: https://play.golang.org/p/WdOMBLOOu9k

@liggitt

This comment has been minimized.

Copy link
Member

@liggitt liggitt commented Jan 14, 2020

it produces the same sequence, not the same number

https://play.golang.org/p/2ibUPSy5v5X

@ash2k

This comment has been minimized.

Copy link
Member

@ash2k ash2k commented Jan 14, 2020

To produce varying sequences, give it a seed that changes.

That is exactly what is done in main(). If not, it's a bug. When I made #71170 I checked that it's the case.

@tanjunchen

This comment has been minimized.

Copy link
Member Author

@tanjunchen tanjunchen commented Jan 15, 2020

i see from #71170 (comment)

@tanjunchen

This comment has been minimized.

Copy link
Member Author

@tanjunchen tanjunchen commented Jan 15, 2020

it also can see from https://github.com/kubernetes/kubernetes/pull/71170/files#diff-6413e1dbafd6dd63b5d907ea3ae6f6f9R99
so i will remove

Will return a fixed value instead of a random string.
@tanjunchen

This comment has been minimized.

Copy link
Member Author

@tanjunchen tanjunchen commented Jan 15, 2020

now , can we consider UUID?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.