[occm] Rand not being initialised any more with a seed

[pierreprinetti]

/kind bug

What happened:
#2100 changed the use of rand.Seed, which seeds the global PRNG, with rand.NewSource, that returns a new initialised PRNG. With that change, that line becomes a noop (because the generated PRNG is never assigned to a variable). As a result, as stated by the documentation of math/rand:

If Seed is not called, the generator behaves as if seeded by Seed(1)

As a consequence, all random numbers generated by the binary will always be the same in the same order, which doesn't look like the expected behaviour of the PR.

cloud-provider-openstack/cmd/openstack-cloud-controller-manager/main.go

Line 47 in 1bf5a4a

rand.NewSource(time.Now().UnixNano())

What you expected to happen:
rand.Seed(time.Now().UnixNano()) is left untouched, or is somehow replaced in the whole binary codebase by using the result of rand.NewSource(time.Now().UnixNano()).

How to reproduce it:

Anything else we need to know?:

Environment:

• openstack-cloud-controller-manager(or other related binary) version:
• OpenStack version:
• Others:

[pierreprinetti]

/cc zetaab

[pierreprinetti]

CC @zetaab

[zetaab]

rand.Seed is deprecated and golint did not execute successfully with that. Of course we could add exemption for that. However, I would like to see new way of doing things instead.

[pierreprinetti]

rand.NewSource will return a source. With your change, the new source is not assigned, so the program uses the unseeded global source (the same we were using before, just unseeded). If you want to use NewSource, you'd have to assign it the source to a variable:

s := rand.NewSource(n)

and then pass s to all consuming functions.

Note that as the docs say, when you don't use the global source you would also need to manage concurrency:

this source is not safe for concurrent use by multiple goroutines

Has rand.Seed been deprecated though? I haven't read that anywhere.

[zetaab]

golang/go#56319

https://pkg.go.dev/math/rand#Seed

[pierreprinetti]

Good to know, thanks.
However, two things remain true:

• this codebase uses Go v1.19. Is math/rand being seeded automatically (https://go-review.googlesource.com/c/go/+/443058) in v1.19?
• rand.Seed can't be replaced with rand.NewSource, which as I mentioned returns a new source rather than seeding the global source.

At a minimum, if the global source is seeded automatically in Go v1.19, we should remove the noop use of rand.NewSource because it is pretty confusing

[pierreprinetti]

hmm the deprecation notice you linked mentions that it's 1.20-only:

Prior to Go 1.20, the generator was seeded like Seed(1) at program startup

[pierreprinetti]

If that’s ok for you, I can take care of reverting the command while making the linter happy. WDYT?

[zetaab]

% grep -r rand *
cmd/openstack-cloud-controller-manager/main.go: "math/rand"
cmd/openstack-cloud-controller-manager/main.go: rand.NewSource(time.Now().UnixNano())
pkg/kms/encryption/aescbc/aescbc.go:    "crypto/rand"
pkg/kms/encryption/aescbc/aescbc.go:    if _, err := io.ReadFull(rand.Reader, iv); err != nil {
pkg/kms/encryption/aescbc/aescbc_test.go:       "crypto/rand"
pkg/kms/encryption/aescbc/aescbc_test.go:       _, _ = rand.Read(key)
pkg/csi/cinder/utils.go:func RunControllerandNodePublishServer(endpoint string, ids csi.IdentityServer, cs csi.ControllerServer, ns csi.NodeServer) {
pkg/csi/cinder/driver.go:       RunControllerandNodePublishServer(d.endpoint, d.ids, d.cs, d.ns)
pkg/ingress/controller/controller.go:   "crypto/rand"
pkg/ingress/controller/controller.go:   pfxData, err := pkcs12.Encode(rand.Reader, pk, cb[0], caCerts, "")
tests/sanity/cinder/fakecloud.go:       "math/rand"
tests/sanity/cinder/fakecloud.go:               ID:               randString(10),
tests/sanity/cinder/fakecloud.go:               ID:        randString(10),
tests/sanity/cinder/fakecloud.go:func randString(n int) string {
tests/sanity/cinder/fakecloud.go:               b[i] = letterBytes[rand.Intn(len(letterBytes))]

I am thinking is this math/rand seed even needed. There might be some library that needs that

seed were added by dims in #61

[zetaab]

@dims do you have idea why seed was added in #61 ?

[dims]

@zetaab at one point it was part of a kube-controller-manager i believe. I still see a lot of references:
https://cs.k8s.io/?q=rand.Seed%5C(time.Now%5C(%5C).UTC%5C(%5C).UnixNano%5C(%5C)%5C)&i=nope&files=&excludeFiles=&repos=

but it looks like it's been removed from k/k.

[zetaab]

@pierreprinetti there are still some references in k8s.io/cloud-provider to math/rand. So perhaps its best to add it back.

rand.Seed(time.Now().UTC().UnixNano()) and then linter fix is needed as well. And can you cherry-pick that to 1.26 and 1.25

[pierreprinetti]

perhaps its best to add it back

it will be ready for removal once the repo compiles with Go v1.20!

Thank you for your time, @zetaab.