Multiple instances of act running simultaneously #6058
Replies: 3 comments 1 reply
-
|
A quick experiment yielded an answer: the container name generation is, indeed, resulting in identical container names being used for concurrent runs of So one possible solve is to add a flag that enables adding a (quasi-)"randomized" component into the container name to avoid this type of issue. This value would need to remain constant for the duration of the execution of the specific Regardless, this raises another conundrum when taken into context with the flag The goal here is to have Thoughts? I'll poke around a bit and see if I can come up with a sensible PR. I'd definitely appreciate input from others, though! |
Beta Was this translation helpful? Give feedback.
-
|
Sure enough - a small, localized tweak to the container naming code and all's well! I'm in the process of creating the PR. Importantly, it does take into account whether the container is meant to be reused, which will cause the container name to become predictable again. As a result, this shouldn't break anything out of the gate. For the impatient, this is the patch: diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go
index 5d42771..d474b55 100644
--- a/pkg/runner/run_context.go
+++ b/pkg/runner/run_context.go
@@ -12,6 +12,7 @@ import (
"errors"
"fmt"
"io"
+ "math/big"
"os"
"path/filepath"
"regexp"
@@ -28,6 +29,9 @@ import (
"github.com/opencontainers/selinux/go-selinux"
)
+var randomStringCharset = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789")
+var randomStringCharsetLength = len(randomStringCharset)
+
// RunContext contains info about current job
type RunContext struct {
Name string
@@ -89,8 +93,56 @@ func (rc *RunContext) GetEnv() map[string]string {
return rc.Env
}
+func (rc *RunContext) randomString(length int) (string, error) {
+ if length <= 0 {
+ return "", errors.New("The string length must be >= 0")
+ }
+
+ length64 := int64(randomStringCharsetLength)
+ target := make([]rune, length)
+ for i := range target {
+ p, err := rand.Int(rand.Reader, big.NewInt(length64))
+ if err != nil {
+ return "", err
+ }
+ target[i] = randomStringCharset[p.Int64()]
+ }
+ return string(target), nil
+}
+
func (rc *RunContext) jobContainerName() string {
- return createContainerName("act", rc.String())
+ // Let's assume a fixed, consistent "unique identifier" for
+ // the container name. If the container is not meant to be
+ // reusable, we will try to generate a unique identifier
+ // that can replace this constant one.
+ id := "reusable"
+ if !rc.Config.ReuseContainers {
+ // To allow multiple concurrent instances of ACT, we need to use
+ // a unique identifier that will differentiate all containers
+ // spawned by this instance from those spawned by others. If we
+ // don't do that, then weird conflicts will arise due to possible
+ // container name collsions. We use a secure random string of
+ // alphanumeric characters for this purpose. The string will be
+ // of the same length as the fixed-identifier used when reusable
+ // containers are desired, minus 2 characters, for consistency.
+ //
+ // The 2 character difference eliminate the chance that the random
+ // string be the same as the "fixed" ID
+ length := len(id) + 2
+
+ // This function will try to generate a secure random string. If
+ // that fails for some reason, it will use the default math/rand
+ // as its source of randomness.
+ if newID, err := rc.randomString(length); err != nil {
+ // TODO: Should we log this error? How (without a lot of hoops)?
+ // If we had some sort of issue with the randomness,
+ // let's just use the PID as the "randomness"
+ id = fmt.Sprintf("%08X", os.Getpid())
+ } else {
+ id = newID
+ }
+ }
+ return createContainerName("act", id, rc.String())
}
// networkName return the name of the network which will be created by `act` automatically for job,
Cheers! |
Beta Was this translation helpful? Give feedback.
-
|
I've since revised these changes, but the gist of it remains the same. |
Beta Was this translation helpful? Give feedback.
-
|
This is addressed by #6062 |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
Hi!
Our container tree is made up of 50-odd container images, including inter-dependencies, etc. Long story 😄
We have a script that rebuilds them all in the correct order, respecting those dependencies. When running this script against GitHub, all's well b/c we can have tons of workers running in parallel as required.
We've taken to using
actto perform builds locally on our development machines. This required some refactoring of the workflows and actions, but is generally working well.... except for the concurrency bit. As a result, we're forced to run a single instance ofactat a time, b/c if we run a second one we start getting all sorts of weird container conflicts and docker errors.It's worth noting that our workflows can also result in multiple parallel builds within the same job. This is working fine when using a single
actinstance. The problems come when we try to run a second one.I'm wondering if the container names/ids/whatnot that are generated when
actdoes its thing are colliding somehow, and this in turn causes these issues.Has anyone encountered this?
Beta Was this translation helpful? Give feedback.
All reactions