Skip to content

Commit

Permalink
Improve execution name readability (#5637)
Browse files Browse the repository at this point in the history
Signed-off-by: wayner0628 <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Co-authored-by: Kevin Su <[email protected]>
Signed-off-by: pmahindrakar-oss <[email protected]>
  • Loading branch information
2 people authored and pmahindrakar-oss committed Sep 9, 2024
1 parent e7adb39 commit ef11183
Show file tree
Hide file tree
Showing 13 changed files with 112 additions and 60 deletions.
1 change: 1 addition & 0 deletions flyteadmin/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions flyteadmin/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
5 changes: 2 additions & 3 deletions flyteadmin/pkg/async/schedule/aws/workflow_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand Down
13 changes: 0 additions & 13 deletions flyteadmin/pkg/common/executions.go
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
23 changes: 0 additions & 23 deletions flyteadmin/pkg/common/executions_test.go

This file was deleted.

30 changes: 30 additions & 0 deletions flyteadmin/pkg/common/naming/execution_name.go
Original file line number Diff line number Diff line change
@@ -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))
}
64 changes: 64 additions & 0 deletions flyteadmin/pkg/common/naming/execution_name_test.go
Original file line number Diff line number Diff line change
@@ -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")
})

}
3 changes: 2 additions & 1 deletion flyteadmin/pkg/manager/impl/util/shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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) (
Expand Down
4 changes: 2 additions & 2 deletions flyteadmin/pkg/manager/impl/util/shared_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 4 additions & 17 deletions flyteadmin/scheduler/executor/executor_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package executor

import (
"context"
"strings"
"time"

"github.com/prometheus/client_golang/prometheus"
Expand All @@ -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"
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down

0 comments on commit ef11183

Please sign in to comment.