Skip to content

Commit

Permalink
Revert "Improve execution name readability" (#5740)
Browse files Browse the repository at this point in the history
* Revert "fix: Use deterministic execution names in scheduler (#5724)"

This reverts commit a058fd1.

* Revert "Improve execution name readability (#5637)"

This reverts commit 2ed2408.

* nit

Signed-off-by: Kevin Su <[email protected]>

* make helm

Signed-off-by: Kevin Su <[email protected]>

---------

Signed-off-by: Kevin Su <[email protected]>
  • Loading branch information
pingsutw authored Sep 26, 2024
1 parent d523f21 commit c0cb4d1
Show file tree
Hide file tree
Showing 17 changed files with 61 additions and 136 deletions.
4 changes: 2 additions & 2 deletions docker/sandbox-bundled/manifests/complete-agent.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,7 @@ type: Opaque
---
apiVersion: v1
data:
haSharedSecret: aVh1N3lZb0F1c2l0NHVuRg==
haSharedSecret: ZXlJVkhWYjdIMHhjamZadA==
proxyPassword: ""
proxyUsername: ""
kind: Secret
Expand Down Expand Up @@ -1413,7 +1413,7 @@ spec:
metadata:
annotations:
checksum/config: 8f50e768255a87f078ba8b9879a0c174c3e045ffb46ac8723d2eedbe293c8d81
checksum/secret: 042e6b21a3852a65952e0701cd9667e53bfef57590eea4d116b261472f29a882
checksum/secret: 94a4c448ea7ad0892283bc4cfc6c506c83c9c5fe998587f4b2c55194c6a674e3
labels:
app: docker-registry
release: flyte-sandbox
Expand Down
4 changes: 2 additions & 2 deletions docker/sandbox-bundled/manifests/complete.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,7 @@ type: Opaque
---
apiVersion: v1
data:
haSharedSecret: Q2EyanRtd1JjWmVKS2tHMw==
haSharedSecret: OW1PbDdRY0t4RllhM3Nybg==
proxyPassword: ""
proxyUsername: ""
kind: Secret
Expand Down Expand Up @@ -1362,7 +1362,7 @@ spec:
metadata:
annotations:
checksum/config: 8f50e768255a87f078ba8b9879a0c174c3e045ffb46ac8723d2eedbe293c8d81
checksum/secret: 01201329c98a1417f04feeef00dc21e67cf73d99ac9b99486ce5788eca0c282c
checksum/secret: 1f30487909a5b2db21b8f92a734fcb321ab30f01694f4257333026e00d512053
labels:
app: docker-registry
release: flyte-sandbox
Expand Down
4 changes: 2 additions & 2 deletions docker/sandbox-bundled/manifests/dev.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ metadata:
---
apiVersion: v1
data:
haSharedSecret: N3dIemE2TnF1b3l1SWdNTw==
haSharedSecret: MWVqaUwzWDZtUWY4TDdscA==
proxyPassword: ""
proxyUsername: ""
kind: Secret
Expand Down Expand Up @@ -934,7 +934,7 @@ spec:
metadata:
annotations:
checksum/config: 8f50e768255a87f078ba8b9879a0c174c3e045ffb46ac8723d2eedbe293c8d81
checksum/secret: ca7423805c2fd3a98507c790af575a6e5389f50d6baa09bd8c49cb59c4452340
checksum/secret: 53219c6f309435a180b4635448e130a2ec19b63b379a881dde73bf8ae957a1ad
labels:
app: docker-registry
release: flyte-sandbox
Expand Down
1 change: 0 additions & 1 deletion flyteadmin/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions flyteadmin/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
5 changes: 3 additions & 2 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/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"
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand Down
13 changes: 13 additions & 0 deletions flyteadmin/pkg/common/executions.go
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
23 changes: 23 additions & 0 deletions flyteadmin/pkg/common/executions_test.go
Original file line number Diff line number Diff line change
@@ -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]))
}
}
30 changes: 0 additions & 30 deletions flyteadmin/pkg/common/naming/execution_name.go

This file was deleted.

78 changes: 0 additions & 78 deletions flyteadmin/pkg/common/naming/execution_name_test.go

This file was deleted.

3 changes: 1 addition & 2 deletions flyteadmin/pkg/manager/impl/util/shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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) (
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, naming.ExecutionIDLength)
assert.Len(t, name, common.ExecutionIDLength)
}

func TestPopulateExecutionID_ExistingName(t *testing.T) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 9 additions & 5 deletions flyteadmin/scheduler/executor/executor_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package executor

import (
"context"
"strings"
"time"

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

0 comments on commit c0cb4d1

Please sign in to comment.