diff --git a/flyteadmin/go.mod b/flyteadmin/go.mod index ac743842505..b9eba5b83a3 100644 --- a/flyteadmin/go.mod +++ b/flyteadmin/go.mod @@ -48,6 +48,7 @@ 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/oauth2 v0.16.0 diff --git a/flyteadmin/go.sum b/flyteadmin/go.sum index dba9da2e86f..049add4bbce 100644 --- a/flyteadmin/go.sum +++ b/flyteadmin/go.sum @@ -1297,6 +1297,8 @@ 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 918402836be..13d2041f9da 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" + "github.com/flyteorg/flyte/flyteadmin/pkg/common/naming" "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) s Name: launchPlan.Id.Name, }) randomSeed := kickoffTime.UnixNano() + int64(hashedIdentifier) - return common.GetExecutionName(randomSeed) + return naming.GetExecutionName(randomSeed) } func (e *workflowExecutor) formulateExecutionCreateRequest( @@ -207,7 +207,6 @@ 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 fbb5bdd6bd9..4ac1ec73006 100644 --- a/flyteadmin/pkg/common/executions.go +++ b/flyteadmin/pkg/common/executions.go @@ -1,22 +1,9 @@ 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 deleted file mode 100644 index 628abd6e9da..00000000000 --- a/flyteadmin/pkg/common/executions_test.go +++ /dev/null @@ -1,23 +0,0 @@ -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 new file mode 100644 index 00000000000..01aa3fe8b6a --- /dev/null +++ b/flyteadmin/pkg/common/naming/execution_name.go @@ -0,0 +1,30 @@ +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 new file mode 100644 index 00000000000..22729dbb9ba --- /dev/null +++ b/flyteadmin/pkg/common/naming/execution_name_test.go @@ -0,0 +1,64 @@ +package naming + +import ( + "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" +) + +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") + }) + +} diff --git a/flyteadmin/pkg/manager/impl/util/shared.go b/flyteadmin/pkg/manager/impl/util/shared.go index 24c97f416a3..b1395697ef0 100644 --- a/flyteadmin/pkg/manager/impl/util/shared.go +++ b/flyteadmin/pkg/manager/impl/util/shared.go @@ -8,6 +8,7 @@ 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" @@ -25,7 +26,7 @@ func GetExecutionName(request admin.ExecutionCreateRequest) string { if request.Name != "" { return request.Name } - return common.GetExecutionName(time.Now().UnixNano()) + return naming.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 21a78997c30..75759485db8 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, common.ExecutionIDLength) + assert.Len(t, name, naming.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 ca6dc609231..8be59abe14f 100644 --- a/flyteadmin/pkg/runtime/interfaces/application_configuration.go +++ b/flyteadmin/pkg/runtime/interfaces/application_configuration.go @@ -49,7 +49,8 @@ type PostgresConfig struct { } type FeatureGates struct { - EnableArtifacts bool `json:"enableArtifacts" pflag:",Enable artifacts feature."` + EnableArtifacts bool `json:"enableArtifacts" pflag:",Enable artifacts feature."` + EnableFriendlyNames bool `json:"enableFriendlyNames" pflag:",Enable generation of friendly execution names 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 dffb98e1b6a..f3fd86c6cf0 100644 --- a/flyteadmin/scheduler/executor/executor_impl.go +++ b/flyteadmin/scheduler/executor/executor_impl.go @@ -2,7 +2,6 @@ package executor import ( "context" - "strings" "time" "github.com/prometheus/client_golang/prometheus" @@ -12,7 +11,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/util/retry" - "github.com/flyteorg/flyte/flyteadmin/scheduler/identifier" + "github.com/flyteorg/flyte/flyteadmin/pkg/common/naming" "github.com/flyteorg/flyte/flyteadmin/scheduler/repositories/models" "github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/admin" "github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core" @@ -53,23 +52,11 @@ 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 - executionIdentifier, err := identifier.GetExecutionIdentifier(ctx, core.Identifier{ - Project: s.Project, - Domain: s.Domain, - Name: s.Name, - Version: s.Version, - }, scheduledTime) - - if err != nil { - logger.Errorf(ctx, "failed to generate execution identifier for schedule %+v due to %v", s, err) - return err - } - + executionName := naming.GetExecutionName(time.Now().UnixNano()) executionRequest := &admin.ExecutionCreateRequest{ Project: s.Project, Domain: s.Domain, - Name: "f" + strings.ReplaceAll(executionIdentifier.String(), "-", "")[:19], + Name: executionName, Spec: &admin.ExecutionSpec{ LaunchPlan: &core.Identifier{ ResourceType: core.ResourceType_LAUNCH_PLAN, @@ -97,7 +84,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/go.mod b/go.mod index 6c25974da0d..8c8053def64 100644 --- a/go.mod +++ b/go.mod @@ -179,6 +179,7 @@ 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 3994ea66bfd..68eebb1fde8 100644 --- a/go.sum +++ b/go.sum @@ -1335,6 +1335,8 @@ 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=