Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Improve execution name readability" #5740

Merged
merged 5 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@

import (
"context"
"strings"
"time"

"github.com/prometheus/client_golang/prometheus"
Expand All @@ -11,7 +12,6 @@
"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 @@
}

// 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

Check warning on line 66 in flyteadmin/scheduler/executor/executor_impl.go

View check run for this annotation

Codecov / codecov/patch

flyteadmin/scheduler/executor/executor_impl.go#L65-L66

Added lines #L65 - L66 were not covered by tests
}

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 @@

// 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
Loading