diff --git a/docker/sandbox-bundled/manifests/complete-agent.yaml b/docker/sandbox-bundled/manifests/complete-agent.yaml index 761469bd83..ea6321200d 100644 --- a/docker/sandbox-bundled/manifests/complete-agent.yaml +++ b/docker/sandbox-bundled/manifests/complete-agent.yaml @@ -816,7 +816,7 @@ type: Opaque --- apiVersion: v1 data: - haSharedSecret: aVh1N3lZb0F1c2l0NHVuRg== + haSharedSecret: ZXlJVkhWYjdIMHhjamZadA== proxyPassword: "" proxyUsername: "" kind: Secret @@ -1413,7 +1413,7 @@ spec: metadata: annotations: checksum/config: 8f50e768255a87f078ba8b9879a0c174c3e045ffb46ac8723d2eedbe293c8d81 - checksum/secret: 042e6b21a3852a65952e0701cd9667e53bfef57590eea4d116b261472f29a882 + checksum/secret: 94a4c448ea7ad0892283bc4cfc6c506c83c9c5fe998587f4b2c55194c6a674e3 labels: app: docker-registry release: flyte-sandbox diff --git a/docker/sandbox-bundled/manifests/complete.yaml b/docker/sandbox-bundled/manifests/complete.yaml index e80cc05d20..3437469a1c 100644 --- a/docker/sandbox-bundled/manifests/complete.yaml +++ b/docker/sandbox-bundled/manifests/complete.yaml @@ -798,7 +798,7 @@ type: Opaque --- apiVersion: v1 data: - haSharedSecret: Q2EyanRtd1JjWmVKS2tHMw== + haSharedSecret: OW1PbDdRY0t4RllhM3Nybg== proxyPassword: "" proxyUsername: "" kind: Secret @@ -1362,7 +1362,7 @@ spec: metadata: annotations: checksum/config: 8f50e768255a87f078ba8b9879a0c174c3e045ffb46ac8723d2eedbe293c8d81 - checksum/secret: 01201329c98a1417f04feeef00dc21e67cf73d99ac9b99486ce5788eca0c282c + checksum/secret: 1f30487909a5b2db21b8f92a734fcb321ab30f01694f4257333026e00d512053 labels: app: docker-registry release: flyte-sandbox diff --git a/docker/sandbox-bundled/manifests/dev.yaml b/docker/sandbox-bundled/manifests/dev.yaml index fe04e4c059..f0e2a866af 100644 --- a/docker/sandbox-bundled/manifests/dev.yaml +++ b/docker/sandbox-bundled/manifests/dev.yaml @@ -499,7 +499,7 @@ metadata: --- apiVersion: v1 data: - haSharedSecret: N3dIemE2TnF1b3l1SWdNTw== + haSharedSecret: MWVqaUwzWDZtUWY4TDdscA== proxyPassword: "" proxyUsername: "" kind: Secret @@ -934,7 +934,7 @@ spec: metadata: annotations: checksum/config: 8f50e768255a87f078ba8b9879a0c174c3e045ffb46ac8723d2eedbe293c8d81 - checksum/secret: ca7423805c2fd3a98507c790af575a6e5389f50d6baa09bd8c49cb59c4452340 + checksum/secret: 53219c6f309435a180b4635448e130a2ec19b63b379a881dde73bf8ae957a1ad labels: app: docker-registry release: flyte-sandbox diff --git a/flyteadmin/go.mod b/flyteadmin/go.mod index cfc2bfa010..836bc69979 100644 --- a/flyteadmin/go.mod +++ b/flyteadmin/go.mod @@ -48,7 +48,6 @@ require ( github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.9.0 github.com/wI2L/jsondiff v0.5.0 - github.com/wolfeidau/humanhash v1.1.0 go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.47.0 go.opentelemetry.io/otel v1.24.0 golang.org/x/net v0.27.0 diff --git a/flyteadmin/go.sum b/flyteadmin/go.sum index 31a1714ed7..7c9c02881f 100644 --- a/flyteadmin/go.sum +++ b/flyteadmin/go.sum @@ -1301,8 +1301,6 @@ github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6Kllzaw github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= github.com/wI2L/jsondiff v0.5.0 h1:RRMTi/mH+R2aXcPe1VYyvGINJqQfC3R+KSEakuU1Ikw= github.com/wI2L/jsondiff v0.5.0/go.mod h1:qqG6hnK0Lsrz2BpIVCxWiK9ItsBCpIZQiv0izJjOZ9s= -github.com/wolfeidau/humanhash v1.1.0 h1:06KgtyyABJGBbrfMONrW7S+b5TTYVyrNB/jss5n7F3E= -github.com/wolfeidau/humanhash v1.1.0/go.mod h1:jkpynR1bfyfkmKEQudIC0osWKynFAoayRjzH9OJdVIg= github.com/xdg/scram v0.0.0-20180814205039-7eeb5667e42c/go.mod h1:lB8K/P019DLNhemzwFU4jHLhdvlE6uDZjXFejJXr49I= github.com/xdg/stringprep v0.0.0-20180714160509-73f8eece6fdc/go.mod h1:Jhud4/sHMO4oL310DaZAKk9ZaJ08SJfe+sJh0HrGL1Y= github.com/xdg/stringprep v1.0.0/go.mod h1:Jhud4/sHMO4oL310DaZAKk9ZaJ08SJfe+sJh0HrGL1Y= diff --git a/flyteadmin/pkg/async/schedule/aws/workflow_executor.go b/flyteadmin/pkg/async/schedule/aws/workflow_executor.go index 523fdd077e..c4a5d75d14 100644 --- a/flyteadmin/pkg/async/schedule/aws/workflow_executor.go +++ b/flyteadmin/pkg/async/schedule/aws/workflow_executor.go @@ -15,7 +15,7 @@ import ( "github.com/flyteorg/flyte/flyteadmin/pkg/async" scheduleInterfaces "github.com/flyteorg/flyte/flyteadmin/pkg/async/schedule/interfaces" - "github.com/flyteorg/flyte/flyteadmin/pkg/common/naming" + "github.com/flyteorg/flyte/flyteadmin/pkg/common" "github.com/flyteorg/flyte/flyteadmin/pkg/errors" "github.com/flyteorg/flyte/flyteadmin/pkg/manager/interfaces" runtimeInterfaces "github.com/flyteorg/flyte/flyteadmin/pkg/runtime/interfaces" @@ -129,7 +129,7 @@ func generateExecutionName(launchPlan *admin.LaunchPlan, kickoffTime time.Time) Name: launchPlan.Id.Name, }) randomSeed := kickoffTime.UnixNano() + int64(hashedIdentifier) - return naming.GetExecutionName(randomSeed) + return common.GetExecutionName(randomSeed) } func (e *workflowExecutor) formulateExecutionCreateRequest( @@ -207,6 +207,7 @@ func (e *workflowExecutor) run() error { continue } executionRequest := e.formulateExecutionCreateRequest(launchPlan, scheduledWorkflowExecutionRequest.KickoffTime) + ctx = contextutils.WithWorkflowID(ctx, fmt.Sprintf(workflowIdentifierFmt, executionRequest.Project, executionRequest.Domain, executionRequest.Name)) err = e.resolveKickoffTimeArg(scheduledWorkflowExecutionRequest, launchPlan, executionRequest) diff --git a/flyteadmin/pkg/common/executions.go b/flyteadmin/pkg/common/executions.go index 4ac1ec7300..fbb5bdd6bd 100644 --- a/flyteadmin/pkg/common/executions.go +++ b/flyteadmin/pkg/common/executions.go @@ -1,9 +1,22 @@ package common import ( + "fmt" + + "k8s.io/apimachinery/pkg/util/rand" + "github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core" ) +const ExecutionIDLength = 20 +const ExecutionStringFormat = "a%s" + +/* #nosec */ +func GetExecutionName(seed int64) string { + rand.Seed(seed) + return fmt.Sprintf(ExecutionStringFormat, rand.String(ExecutionIDLength-1)) +} + var terminalExecutionPhases = map[core.WorkflowExecution_Phase]bool{ core.WorkflowExecution_SUCCEEDED: true, core.WorkflowExecution_FAILED: true, diff --git a/flyteadmin/pkg/common/executions_test.go b/flyteadmin/pkg/common/executions_test.go new file mode 100644 index 0000000000..628abd6e9d --- /dev/null +++ b/flyteadmin/pkg/common/executions_test.go @@ -0,0 +1,23 @@ +package common + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +const AllowedExecutionIDStartCharStr = "abcdefghijklmnopqrstuvwxyz" +const AllowedExecutionIDStr = "abcdefghijklmnopqrstuvwxyz1234567890" + +var AllowedExecutionIDStartChars = []rune(AllowedExecutionIDStartCharStr) +var AllowedExecutionIDChars = []rune(AllowedExecutionIDStr) + +func TestGetExecutionName(t *testing.T) { + randString := GetExecutionName(time.Now().UnixNano()) + assert.Len(t, randString, ExecutionIDLength) + assert.Contains(t, AllowedExecutionIDStartChars, rune(randString[0])) + for i := 1; i < len(randString); i++ { + assert.Contains(t, AllowedExecutionIDChars, rune(randString[i])) + } +} diff --git a/flyteadmin/pkg/common/naming/execution_name.go b/flyteadmin/pkg/common/naming/execution_name.go deleted file mode 100644 index 01aa3fe8b6..0000000000 --- a/flyteadmin/pkg/common/naming/execution_name.go +++ /dev/null @@ -1,30 +0,0 @@ -package naming - -import ( - "fmt" - - "github.com/wolfeidau/humanhash" - "k8s.io/apimachinery/pkg/util/rand" - - "github.com/flyteorg/flyte/flyteadmin/pkg/runtime" - runtimeInterfaces "github.com/flyteorg/flyte/flyteadmin/pkg/runtime/interfaces" -) - -const ExecutionIDLength = 20 -const ExecutionIDLengthLimit = 63 -const ExecutionStringFormat = "a%s" - -var configProvider runtimeInterfaces.ApplicationConfiguration = runtime.NewApplicationConfigurationProvider() - -/* #nosec */ -func GetExecutionName(seed int64) string { - rand.Seed(seed) - config := configProvider.GetTopLevelConfig() - if config.FeatureGates.EnableFriendlyNames { - hashKey := []byte(rand.String(ExecutionIDLength)) - // Ignoring the error as it's guaranteed hash key longer than result in this context. - result, _ := humanhash.Humanize(hashKey, 4) - return result - } - return fmt.Sprintf(ExecutionStringFormat, rand.String(ExecutionIDLength-1)) -} diff --git a/flyteadmin/pkg/common/naming/execution_name_test.go b/flyteadmin/pkg/common/naming/execution_name_test.go deleted file mode 100644 index 91f04d3bf1..0000000000 --- a/flyteadmin/pkg/common/naming/execution_name_test.go +++ /dev/null @@ -1,78 +0,0 @@ -package naming - -import ( - "context" - "strings" - "testing" - "time" - - "github.com/stretchr/testify/assert" - - runtimeInterfaces "github.com/flyteorg/flyte/flyteadmin/pkg/runtime/interfaces" - runtimeMocks "github.com/flyteorg/flyte/flyteadmin/pkg/runtime/mocks" - "github.com/flyteorg/flyte/flyteadmin/scheduler/identifier" - "github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core" -) - -const AllowedExecutionIDAlphabetStr = "abcdefghijklmnopqrstuvwxyz" -const AllowedExecutionIDAlphanumericStr = "abcdefghijklmnopqrstuvwxyz1234567890" -const AllowedExecutionIDFriendlyNameStr = "abcdefghijklmnopqrstuvwxyz-" - -var AllowedExecutionIDAlphabets = []rune(AllowedExecutionIDAlphabetStr) -var AllowedExecutionIDAlphanumerics = []rune(AllowedExecutionIDAlphanumericStr) -var AllowedExecutionIDFriendlyNameChars = []rune(AllowedExecutionIDFriendlyNameStr) - -func TestGetExecutionName(t *testing.T) { - originalConfigProvider := configProvider - defer func() { configProvider = originalConfigProvider }() - - mockConfigProvider := &runtimeMocks.MockApplicationProvider{} - configProvider = mockConfigProvider - - t.Run("general name", func(t *testing.T) { - appConfig := runtimeInterfaces.ApplicationConfig{ - FeatureGates: runtimeInterfaces.FeatureGates{ - EnableFriendlyNames: false, - }, - } - mockConfigProvider.SetTopLevelConfig(appConfig) - - randString := GetExecutionName(time.Now().UnixNano()) - assert.Len(t, randString, ExecutionIDLength) - assert.Contains(t, AllowedExecutionIDAlphabets, rune(randString[0])) - for i := 1; i < len(randString); i++ { - assert.Contains(t, AllowedExecutionIDAlphanumerics, rune(randString[i])) - } - }) - - t.Run("friendly name", func(t *testing.T) { - appConfig := runtimeInterfaces.ApplicationConfig{ - FeatureGates: runtimeInterfaces.FeatureGates{ - EnableFriendlyNames: true, - }, - } - mockConfigProvider.SetTopLevelConfig(appConfig) - - randString := GetExecutionName(time.Now().UnixNano()) - assert.LessOrEqual(t, len(randString), ExecutionIDLengthLimit) - for i := 0; i < len(randString); i++ { - assert.Contains(t, AllowedExecutionIDFriendlyNameChars, rune(randString[i])) - } - hyphenCount := strings.Count(randString, "-") - assert.Equal(t, 3, hyphenCount, "FriendlyName should contain exactly three hyphens") - words := strings.Split(randString, "-") - assert.Equal(t, 4, len(words), "FriendlyName should be split into exactly four words") - }) - - t.Run("deterministic name", func(t *testing.T) { - hashValue := identifier.HashScheduledTimeStamp(context.Background(), &core.Identifier{ - Project: "Project", - Domain: "Domain", - Name: "Name", - Version: "Version", - }, time.Time{}) - - name := GetExecutionName(int64(hashValue)) - assert.Equal(t, name, "carpet-juliet-kentucky-kentucky") - }) -} diff --git a/flyteadmin/pkg/manager/impl/util/shared.go b/flyteadmin/pkg/manager/impl/util/shared.go index ba8fc41760..8402451200 100644 --- a/flyteadmin/pkg/manager/impl/util/shared.go +++ b/flyteadmin/pkg/manager/impl/util/shared.go @@ -8,7 +8,6 @@ import ( "google.golang.org/grpc/codes" "github.com/flyteorg/flyte/flyteadmin/pkg/common" - "github.com/flyteorg/flyte/flyteadmin/pkg/common/naming" "github.com/flyteorg/flyte/flyteadmin/pkg/errors" "github.com/flyteorg/flyte/flyteadmin/pkg/manager/impl/shared" "github.com/flyteorg/flyte/flyteadmin/pkg/manager/impl/validation" @@ -26,7 +25,7 @@ func GetExecutionName(request *admin.ExecutionCreateRequest) string { if request.Name != "" { return request.Name } - return naming.GetExecutionName(time.Now().UnixNano()) + return common.GetExecutionName(time.Now().UnixNano()) } func GetTask(ctx context.Context, repo repoInterfaces.Repository, identifier *core.Identifier) ( diff --git a/flyteadmin/pkg/manager/impl/util/shared_test.go b/flyteadmin/pkg/manager/impl/util/shared_test.go index 114dbebdfb..b9b296971e 100644 --- a/flyteadmin/pkg/manager/impl/util/shared_test.go +++ b/flyteadmin/pkg/manager/impl/util/shared_test.go @@ -12,8 +12,8 @@ import ( "github.com/stretchr/testify/assert" "google.golang.org/grpc/codes" + "github.com/flyteorg/flyte/flyteadmin/pkg/common" commonMocks "github.com/flyteorg/flyte/flyteadmin/pkg/common/mocks" - "github.com/flyteorg/flyte/flyteadmin/pkg/common/naming" flyteAdminErrors "github.com/flyteorg/flyte/flyteadmin/pkg/errors" "github.com/flyteorg/flyte/flyteadmin/pkg/manager/impl/testutils" managerInterfaces "github.com/flyteorg/flyte/flyteadmin/pkg/manager/interfaces" @@ -42,7 +42,7 @@ func TestPopulateExecutionID(t *testing.T) { Domain: "domain", }) assert.NotEmpty(t, name) - assert.Len(t, name, naming.ExecutionIDLength) + assert.Len(t, name, common.ExecutionIDLength) } func TestPopulateExecutionID_ExistingName(t *testing.T) { diff --git a/flyteadmin/pkg/runtime/interfaces/application_configuration.go b/flyteadmin/pkg/runtime/interfaces/application_configuration.go index e3453db0f7..15ed271412 100644 --- a/flyteadmin/pkg/runtime/interfaces/application_configuration.go +++ b/flyteadmin/pkg/runtime/interfaces/application_configuration.go @@ -49,8 +49,7 @@ type PostgresConfig struct { } type FeatureGates struct { - EnableArtifacts bool `json:"enableArtifacts" pflag:",Enable artifacts feature."` - EnableFriendlyNames bool `json:"enableFriendlyNames" pflag:",Enable generation of friendly execution names feature."` + EnableArtifacts bool `json:"enableArtifacts" pflag:",Enable artifacts feature."` } // ApplicationConfig is the base configuration to start admin diff --git a/flyteadmin/scheduler/executor/executor_impl.go b/flyteadmin/scheduler/executor/executor_impl.go index e269d79b2a..5e4a8fcf8e 100644 --- a/flyteadmin/scheduler/executor/executor_impl.go +++ b/flyteadmin/scheduler/executor/executor_impl.go @@ -2,6 +2,7 @@ package executor import ( "context" + "strings" "time" "github.com/prometheus/client_golang/prometheus" @@ -11,7 +12,6 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/util/retry" - "github.com/flyteorg/flyte/flyteadmin/pkg/common/naming" "github.com/flyteorg/flyte/flyteadmin/scheduler/identifier" "github.com/flyteorg/flyte/flyteadmin/scheduler/repositories/models" "github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/admin" @@ -54,18 +54,22 @@ func (w *executor) Execute(ctx context.Context, scheduledTime time.Time, s model } // Making the identifier deterministic using the hash of the identifier and scheduled time - hashValue := identifier.HashScheduledTimeStamp(ctx, &core.Identifier{ + executionIdentifier, err := identifier.GetExecutionIdentifier(ctx, &core.Identifier{ Project: s.Project, Domain: s.Domain, Name: s.Name, Version: s.Version, }, scheduledTime) - executionName := naming.GetExecutionName(int64(hashValue)) + if err != nil { + logger.Errorf(ctx, "failed to generate execution identifier for schedule %+v due to %v", s, err) + return err + } + executionRequest := &admin.ExecutionCreateRequest{ Project: s.Project, Domain: s.Domain, - Name: executionName, + Name: "f" + strings.ReplaceAll(executionIdentifier.String(), "-", "")[:19], Spec: &admin.ExecutionSpec{ LaunchPlan: &core.Identifier{ ResourceType: core.ResourceType_LAUNCH_PLAN, @@ -93,7 +97,7 @@ func (w *executor) Execute(ctx context.Context, scheduledTime time.Time, s model // Do maximum of 30 retries on failures with constant backoff factor opts := wait.Backoff{Duration: 3000, Factor: 2.0, Steps: 30} - err := retry.OnError(opts, + err = retry.OnError(opts, func(err error) bool { // For idempotent behavior ignore the AlreadyExists error which happens if we try to schedule a launchplan // for execution at the same time which is already available in admin. diff --git a/flyteadmin/scheduler/identifier/identifier.go b/flyteadmin/scheduler/identifier/identifier.go index caf5c94296..5d386e8652 100644 --- a/flyteadmin/scheduler/identifier/identifier.go +++ b/flyteadmin/scheduler/identifier/identifier.go @@ -34,7 +34,7 @@ func GetScheduleName(ctx context.Context, s models.SchedulableEntity) string { // GetExecutionIdentifier returns UUID using the hashed value of the schedule identifier and the scheduledTime func GetExecutionIdentifier(ctx context.Context, identifier *core.Identifier, scheduledTime time.Time) (uuid.UUID, error) { - hashValue := HashScheduledTimeStamp(ctx, identifier, scheduledTime) + hashValue := hashScheduledTimeStamp(ctx, identifier, scheduledTime) b := make([]byte, 16) binary.LittleEndian.PutUint64(b, hashValue) return uuid.FromBytes(b) @@ -55,8 +55,8 @@ func hashIdentifier(ctx context.Context, identifier *core.Identifier) uint64 { return h.Sum64() } -// HashScheduledTimeStamp return the hash of the identifier and the scheduledTime -func HashScheduledTimeStamp(ctx context.Context, identifier *core.Identifier, scheduledTime time.Time) uint64 { +// hashScheduledTimeStamp return the hash of the identifier and the scheduledTime +func hashScheduledTimeStamp(ctx context.Context, identifier *core.Identifier, scheduledTime time.Time) uint64 { h := fnv.New64() _, err := h.Write([]byte(fmt.Sprintf(executionIDInputsFormat, identifier.Project, identifier.Domain, identifier.Name, identifier.Version, scheduledTime.Unix()))) diff --git a/go.mod b/go.mod index 8c8053def6..6c25974da0 100644 --- a/go.mod +++ b/go.mod @@ -179,7 +179,6 @@ require ( github.com/tidwall/pretty v1.2.0 // indirect github.com/tidwall/sjson v1.2.5 // indirect github.com/wI2L/jsondiff v0.5.0 // indirect - github.com/wolfeidau/humanhash v1.1.0 // indirect go.opencensus.io v0.24.0 // indirect go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.47.0 // indirect go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.46.1 // indirect diff --git a/go.sum b/go.sum index 63453bbd87..ac7b9f5987 100644 --- a/go.sum +++ b/go.sum @@ -1337,8 +1337,6 @@ github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6Kllzaw github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= github.com/wI2L/jsondiff v0.5.0 h1:RRMTi/mH+R2aXcPe1VYyvGINJqQfC3R+KSEakuU1Ikw= github.com/wI2L/jsondiff v0.5.0/go.mod h1:qqG6hnK0Lsrz2BpIVCxWiK9ItsBCpIZQiv0izJjOZ9s= -github.com/wolfeidau/humanhash v1.1.0 h1:06KgtyyABJGBbrfMONrW7S+b5TTYVyrNB/jss5n7F3E= -github.com/wolfeidau/humanhash v1.1.0/go.mod h1:jkpynR1bfyfkmKEQudIC0osWKynFAoayRjzH9OJdVIg= github.com/xdg/scram v0.0.0-20180814205039-7eeb5667e42c/go.mod h1:lB8K/P019DLNhemzwFU4jHLhdvlE6uDZjXFejJXr49I= github.com/xdg/stringprep v0.0.0-20180714160509-73f8eece6fdc/go.mod h1:Jhud4/sHMO4oL310DaZAKk9ZaJ08SJfe+sJh0HrGL1Y= github.com/xdg/stringprep v1.0.0/go.mod h1:Jhud4/sHMO4oL310DaZAKk9ZaJ08SJfe+sJh0HrGL1Y=