Skip to content

Conversation

@rutvijmehta-harness
Copy link
Contributor

@rutvijmehta-harness rutvijmehta-harness commented May 15, 2023

Bug:

Created 2 dockerfiles where we set the below environment variables and built them in parallel:
testimg:1.0 : ENV GOPATH=$HOME/go
testimg2:2.0 : ENV GOPATH=$HOME/go2

Parallel Step 1:
image

Parallel Step 2:
image

Reproduced the bug where both testimg and testimg2 were the same (since the environment variable set was the same) after they were built. Ideally the should be different:
Commands used:
testimg:1.0:
docker run --name=testcontainer1 rutvijspit/testimg:1.0
docker exec testcontainer1 sh -c /usr/bin/env | grep GOPATH

testimg2:2.0:
docker run --name=testcontainer2 rutvijspit/testimg2:2.0
docker exec testcontainer2 sh -c /usr/bin/env | grep GOPATH

image

After the fix:

Parallel Step 1:
image

Parallel Step 2:
image

Verification that different environement variable is set:
image

docker.go Outdated
return "", errors.New("unable to fetch digest")
}

func getUniqueBuildName(repo, buildName string) string {

Choose a reason for hiding this comment

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

What if build is triggered for a run without git clone?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved away from this approach. We now generate a temporary tag using a random string.

@bradrydzewski
Copy link
Member

bradrydzewski commented May 16, 2023

Looks good. I would recommend one minor change.

I would recommend using crypto/rand to ensure you have a truly random string. The challenge with math/rand is that it is deterministic, and if you seed with time, there is still the possibility of conflicts.

import "crypto/rand"

// random generates a random string
func random() string {
	result := make([]rune, 32)
	runes := []rune("0123456789abcdef")
	x := int64(len(runes))
	for i := range result {
		num, err := rand.Int(rand.Reader, big.NewInt(x))
		if err != nil {
			panic("error creating random number") // should never happen
		}
		result[i] = runes[num.Int64()]
	}
	return string(result)
}

alternatively you can use a library that handles this. In other Drone projects we use the below library since it is lightweight and feature complete, with zero external dependencies:

import "github.com/dchest/uniuri"

s := uniuri.New()

@rutvijmehta-harness
Copy link
Contributor Author

Thanks @bradrydzewski for the review. I've used github.com/dchest/uniuri to generate the temporary tag.

@dgarg-harness dgarg-harness merged commit 001de45 into drone-plugins:master May 16, 2023
@rutvijmehta-harness rutvijmehta-harness added the bug Something isn't working label May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants