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.

Sign up for GitHub

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

Append UID to config name #69

Merged
merged 1 commit into from
May 28, 2024
Merged

Append UID to config name #69

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion cmd/action/ci/ci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package ci

import (
"fmt"
"math/rand"
ArangoGutierrez marked this conversation as resolved.
Show resolved Hide resolved
"os"

"github.com/NVIDIA/holodeck/api/holodeck/v1alpha1"
Expand Down Expand Up @@ -61,5 +62,19 @@ func setCfgName(cfg *v1alpha1.Environment) {
if len(sha) > 8 {
sha = sha[:8]
}
cfg.Name = fmt.Sprintf("ci%s-%s", attempt, sha)
// uid is unique for each run
uid := generateUID()

cfg.Name = fmt.Sprintf("ci%s-%s-%s", attempt, sha, uid)
}

func generateUID() string {
const charset = "abcdefghijklmnopqrstuvwxyz0123456789"

b := make([]byte, 8)
for i := range b {
b[i] = charset[rand.Intn(len(charset))]
Copy link
Member

Choose a reason for hiding this comment

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

Question: Is the lint error reported here or at the import?

Copy link
Member

Choose a reason for hiding this comment

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

It would be reported here. Tested in the device plugin:

 ✗ make check
golangci-lint run ./...
tests/e2e/framework/framework.go:149:53: G404: Use of weak random number generator (math/rand instead of crypto/rand) (gosec)
                f.UniqueName = fmt.Sprintf("%s-%08x", f.BaseName, rand.Int31())
                                                                  ^
tests/e2e/framework/util.go:50:22: G404: Use of weak random number generator (math/rand instead of crypto/rand) (gosec)
        return strconv.Itoa(rand.Intn(10000))
                            ^
make: *** [Makefile:81: lint] Error 1

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Copy link
Member

@elezar elezar May 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I mean, do we want to add the nolint here and NOT at the import.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did a run without the comment on both

❯ golangci-lint run ./... |grep weak

and golangci-lint didn't complained

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have edited it to only have the lint comment on the code and not the import.

Copy link
Member

Choose a reason for hiding this comment

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

This project doesn't seem to have a .golangci file. Could that be why we're not seeing the issue?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I copied the one from device-plug repo thinking the same, and got no error

❯ golangci-lint --version
golangci-lint has version 1.59.0 built with go1.22.3 from 2059b18 on 2024-05-25T11:38:08Z

}

return string(b)
}
Loading