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

Fix up CRD auto-generated code #5432

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
13ab0b3
Fix the name of flytepropeller in invocation of script
eapolinario May 29, 2024
0690d8a
Pull in k8s.io/code-generator v0.24.1 and uncomment line
eapolinario May 29, 2024
161a1f6
Implement `DeepCopyInto` for structs that have proto fields
eapolinario May 29, 2024
e341bb6
Fix lint errors
eapolinario May 30, 2024
49731c7
Revert "Pull in k8s.io/code-generator v0.24.1 and uncomment line"
eapolinario May 30, 2024
113266b
Merge remote-tracking branch 'origin' into regenerate-CRD
eapolinario Jun 6, 2024
c80ad37
Fix Branch
eapolinario Jun 6, 2024
2583a89
Fix ExecutionError
eapolinario Jun 7, 2024
4a22d29
Fix RawOutputDataConfig, ExecutionConfig, WorkflowExecutionIdentifier…
eapolinario Jun 7, 2024
e6d2e01
Renew FlyteWorkflow autogenerated code
eapolinario Jun 8, 2024
e1536c4
Add DeepCopyReferenceConstructor to ReferenceConstructor interface
eapolinario Jun 8, 2024
c3abe42
Add Tests to cover FlyteWorkflow - specifically SecurityContext
eapolinario Jun 8, 2024
c70d923
Use the SecurityContext and Error wrappers
eapolinario Jun 8, 2024
076695b
Fix lint errors and tests
eapolinario Jun 10, 2024
23539c2
Update golden files to account for correct json field `failedNodeId`
eapolinario Jun 11, 2024
54b63fa
Remove duplicate definition of SecurityContext
eapolinario Jun 11, 2024
b6741a2
Use ExecutionError in WorkflowNodeStatus, ArrayNodeStatus, and Dynami…
eapolinario Jun 11, 2024
10f9fb3
Remove copy of WorfklowStatus DeepCopyInto
eapolinario Jun 11, 2024
c000c33
Remove commented test
eapolinario Jun 11, 2024
7800ab2
Merge remote-tracking branch 'origin' into regenerate-CRD
eapolinario Jun 11, 2024
8272457
Handle null in ArrayNodeStatus, DynamicNodeStatus, and WorkflowNodeSt…
eapolinario Jun 11, 2024
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: 4 additions & 0 deletions flyteadmin/pkg/common/mocks/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ func (t *TestDataStore) Delete(ctx context.Context, reference storage.DataRefere
return t.DeleteCb(ctx, reference)
}

func (t *TestDataStore) DeepCopyReferenceConstructor() storage.ReferenceConstructor {
return nil
}

func GetMockStorageClient() *storage.DataStore {
mockStorageClient := TestDataStore{
Store: make(map[storage.DataReference][]byte),
Expand Down
2 changes: 1 addition & 1 deletion flyteadmin/pkg/workflowengine/impl/prepare_execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func addPermissions(securityCtx *core.SecurityContext, roleNameKey string, flyte
if securityCtx == nil || securityCtx.RunAs == nil {
return
}
flyteWf.SecurityContext = *securityCtx
flyteWf.SecurityContext = v1alpha1.SecurityContext{SecurityContext: *securityCtx}
if len(securityCtx.RunAs.IamRole) > 0 {
if flyteWf.Annotations == nil {
flyteWf.Annotations = map[string]string{}
Expand Down
2 changes: 1 addition & 1 deletion flytepropeller/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ cross_compile:
go build -o bin/cross/kubectl-flyte ./cmd/kubectl-flyte/main.go

op_code_generate:
@RESOURCE_NAME=flyteworkflow OPERATOR_PKG=github.com/flyteorg/flytepropeller ./hack/update-codegen.sh
@RESOURCE_NAME=flyteworkflow OPERATOR_PKG=github.com/flyteorg/flyte/flytepropeller ./hack/update-codegen.sh

benchmark:
mkdir -p ./bin/benchmark
Expand Down
2 changes: 1 addition & 1 deletion flytepropeller/hack/update-codegen.sh
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ bash ${CODEGEN_PKG}/generate-groups.sh "deepcopy,client,informer,lister" \
${OPERATOR_PKG}/pkg/client \
${OPERATOR_PKG}/pkg/apis \
${RESOURCE_NAME}:v1alpha1 \
--output-base "$(dirname ${BASH_SOURCE})/../../../.." \
--output-base "$(dirname ${BASH_SOURCE})/../../../../.." \
--go-header-file ${SCRIPT_ROOT}/hack/boilerplate.go.txt

# To use your own boilerplate text use:
Expand Down
42 changes: 34 additions & 8 deletions flytepropeller/pkg/apis/flyteworkflow/v1alpha1/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"bytes"

"github.com/golang/protobuf/jsonpb"
"google.golang.org/protobuf/proto"

"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
)
Expand Down Expand Up @@ -31,9 +32,7 @@

// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *BooleanExpression) DeepCopyInto(out *BooleanExpression) {
*out = *in
// We do not manipulate the object, so its ok
// Once we figure out the autogenerate story we can replace this
out.BooleanExpression = proto.Clone(in.BooleanExpression).(*core.BooleanExpression)
}

type IfBlock struct {
Expand All @@ -49,11 +48,35 @@
return in.ThenNode
}

type Error struct {
*core.Error
}

func (in *Error) UnmarshalJSON(b []byte) error {
in.Error = &core.Error{}
return jsonpb.Unmarshal(bytes.NewReader(b), in.Error)

Check warning on line 57 in flytepropeller/pkg/apis/flyteworkflow/v1alpha1/branch.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/apis/flyteworkflow/v1alpha1/branch.go#L55-L57

Added lines #L55 - L57 were not covered by tests
}

func (in Error) MarshalJSON() ([]byte, error) {
if in.Error == nil {
return nilJSON, nil

Check warning on line 62 in flytepropeller/pkg/apis/flyteworkflow/v1alpha1/branch.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/apis/flyteworkflow/v1alpha1/branch.go#L60-L62

Added lines #L60 - L62 were not covered by tests
}
var buf bytes.Buffer
if err := marshaler.Marshal(&buf, in.Error); err != nil {
return nil, err

Check warning on line 66 in flytepropeller/pkg/apis/flyteworkflow/v1alpha1/branch.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/apis/flyteworkflow/v1alpha1/branch.go#L64-L66

Added lines #L64 - L66 were not covered by tests
}
return buf.Bytes(), nil

Check warning on line 68 in flytepropeller/pkg/apis/flyteworkflow/v1alpha1/branch.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/apis/flyteworkflow/v1alpha1/branch.go#L68

Added line #L68 was not covered by tests
}

func (in *Error) DeepCopyInto(out *Error) {
out.Error = proto.Clone(in.Error).(*core.Error)
}

type BranchNodeSpec struct {
If IfBlock `json:"if"`
ElseIf []*IfBlock `json:"elseIf,omitempty"`
Else *NodeID `json:"else,omitempty"`
ElseFail *core.Error `json:"elseFail,omitempty"`
If IfBlock `json:"if"`
ElseIf []*IfBlock `json:"elseIf,omitempty"`
Else *NodeID `json:"else,omitempty"`
ElseFail *Error `json:"elseFail,omitempty"`
}

func (in *BranchNodeSpec) GetIf() ExecutableIfBlock {
Expand All @@ -73,5 +96,8 @@
}

func (in *BranchNodeSpec) GetElseFail() *core.Error {
return in.ElseFail
if in.ElseFail == nil {
return nil
}
return in.ElseFail.Error
}
74 changes: 71 additions & 3 deletions flytepropeller/pkg/apis/flyteworkflow/v1alpha1/branch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"google.golang.org/protobuf/proto"

"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
)
Expand All @@ -30,8 +31,10 @@ func TestBranchNodeSpecMethods(t *testing.T) {
boolExpr := &core.BooleanExpression{}

// Creating an Error instance for testing
errorMessage := &core.Error{
Message: "Test error",
errorMessage := &Error{
Error: &core.Error{
Message: "Test error",
},
}

ifNode := NodeID("ifNode")
Expand Down Expand Up @@ -71,8 +74,73 @@ func TestBranchNodeSpecMethods(t *testing.T) {
assert.Equal(t, boolExpr, elifs[0].GetCondition())
assert.Equal(t, &elifNode, elifs[0].GetThenNode())

assert.Equal(t, errorMessage, branchNodeSpec.GetElseFail())
assert.True(t, proto.Equal(errorMessage.Error, branchNodeSpec.GetElseFail()))

branchNodeSpec.ElseFail = nil
assert.Nil(t, branchNodeSpec.GetElseFail())
}

func TestWrappedBooleanExpressionDeepCopy(t *testing.T) {
// 1. Set up proto
protoBoolExpr := &core.BooleanExpression{
Expr: &core.BooleanExpression_Comparison{
Comparison: &core.ComparisonExpression{
Operator: core.ComparisonExpression_GT,
LeftValue: &core.Operand{
Val: &core.Operand_Primitive{
Primitive: &core.Primitive{
Value: &core.Primitive_Integer{
Integer: 10,
},
},
},
},
RightValue: &core.Operand{
Val: &core.Operand_Primitive{
Primitive: &core.Primitive{
Value: &core.Primitive_Integer{
Integer: 11,
},
},
},
},
},
},
}

// 2. Define the wrapper object
boolExpr := BooleanExpression{
BooleanExpression: protoBoolExpr,
}

// 3. Deep copy the wrapper object
copyBoolExpr := boolExpr.DeepCopy()

// 4. Compare the pointers and the actual values
// Assert that the pointers are different
assert.True(t, boolExpr.BooleanExpression != copyBoolExpr.BooleanExpression)

// Assert that the values stored in the proto messages are equal
assert.True(t, proto.Equal(boolExpr.BooleanExpression, copyBoolExpr.BooleanExpression))
}

func TestWrappedErrorDeepCopy(t *testing.T) {
// 1. Set up proto
protoError := &core.Error{
Message: "an error",
}

// 2. Define the wrapper object
error := Error{
Error: protoError,
}

// 3. Deep copy the wrapper object
errorCopy := error.DeepCopy()

// 4. Assert that the pointers are different
assert.True(t, error.Error != errorCopy.Error)

// 5. Assert that the values stored in the proto messages are equal
assert.True(t, proto.Equal(error.Error, errorCopy.Error))
}
3 changes: 2 additions & 1 deletion flytepropeller/pkg/apis/flyteworkflow/v1alpha1/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"

"github.com/golang/protobuf/jsonpb"
"google.golang.org/protobuf/proto"

"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
)
Expand All @@ -30,5 +31,5 @@ func (in *ExecutionError) MarshalJSON() ([]byte, error) {
}

func (in *ExecutionError) DeepCopyInto(out *ExecutionError) {
*out = *in
out.ExecutionError = proto.Clone(in.ExecutionError).(*core.ExecutionError)
}
18 changes: 18 additions & 0 deletions flytepropeller/pkg/apis/flyteworkflow/v1alpha1/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"testing"

"github.com/golang/protobuf/proto"
"github.com/stretchr/testify/assert"

"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
Expand All @@ -28,3 +29,20 @@ func TestExecutionErrorJSONMarshalling(t *testing.T) {
assert.Equal(t, execError.Message, newExecErr.ExecutionError.Message)
assert.Equal(t, execError.ErrorUri, newExecErr.ExecutionError.ErrorUri)
}

func TestExecutionErrorDeepCopy(t *testing.T) {
execError := &core.ExecutionError{
Code: "TestCode",
Message: "Test error message",
ErrorUri: "Test error uri",
}

execErr := &ExecutionError{ExecutionError: execError}
newExecErr := execErr.DeepCopy()

// 4. Compare the pointers and the actual values
// Assert that the pointers are different
assert.True(t, execErr.ExecutionError != newExecErr.ExecutionError)
// Assert that the values stored in the proto messages are equal
assert.True(t, proto.Equal(execErr.ExecutionError, newExecErr.ExecutionError))
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package v1alpha1

import (
"google.golang.org/protobuf/proto"
"k8s.io/apimachinery/pkg/api/resource"

"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/admin"
Expand All @@ -15,7 +16,7 @@ type RawOutputDataConfig struct {
}

func (in *RawOutputDataConfig) DeepCopyInto(out *RawOutputDataConfig) {
*out = *in
out.RawOutputDataConfig = proto.Clone(in.RawOutputDataConfig).(*admin.RawOutputDataConfig)
}

// This contains workflow-execution specifications and overrides.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"google.golang.org/protobuf/proto"
"k8s.io/apimachinery/pkg/api/resource"

"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/admin"
"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
)

func TestRawOutputConfig(t *testing.T) {
Expand All @@ -14,3 +17,55 @@ func TestRawOutputConfig(t *testing.T) {
}}
assert.Equal(t, "s3://bucket", r.OutputLocationPrefix)
}

func TestWrappedRawOutputConfigDeepCopy(t *testing.T) {
rawOutputDataConfig := RawOutputDataConfig{&admin.RawOutputDataConfig{
OutputLocationPrefix: "s3://bucket",
}}
rawOutputDataConfigCopy := rawOutputDataConfig.DeepCopy()

assert.True(t, rawOutputDataConfig.RawOutputDataConfig != rawOutputDataConfigCopy.RawOutputDataConfig)
assert.True(t, proto.Equal(rawOutputDataConfig.RawOutputDataConfig, rawOutputDataConfigCopy.RawOutputDataConfig))
}

func TestExecutionConfigDeepCopy(t *testing.T) {
// 1. Create an ExecutionConfig object (including the wrapper object - WorkflowExecutionIdentifier)
interruptible := true
executionConfig := &ExecutionConfig{
TaskPluginImpls: map[string]TaskPluginOverride{},
MaxParallelism: 32,
RecoveryExecution: WorkflowExecutionIdentifier{
WorkflowExecutionIdentifier: &core.WorkflowExecutionIdentifier{
Project: "project",
Domain: "domain",
Org: "organization",
Name: "name",
},
},
TaskResources: TaskResources{
Requests: TaskResourceSpec{
CPU: resource.Quantity{
Format: "1",
},
Memory: resource.Quantity{
Format: "1Gi",
},
},
},
Interruptible: &interruptible,
OverwriteCache: false,
EnvironmentVariables: map[string]string{
"key": "value",
},
}

// 2. Deep copy the wrapper object
executionConfigCopy := executionConfig.DeepCopy()

// 3. Assert that the pointers are different
assert.True(t, executionConfig.RecoveryExecution.WorkflowExecutionIdentifier != executionConfigCopy.RecoveryExecution.WorkflowExecutionIdentifier)

// 4. Assert that the values are the same
assert.True(t, proto.Equal(executionConfig.RecoveryExecution.WorkflowExecutionIdentifier, executionConfigCopy.RecoveryExecution.WorkflowExecutionIdentifier))
assert.Equal(t, executionConfig, executionConfigCopy)
}
13 changes: 13 additions & 0 deletions flytepropeller/pkg/apis/flyteworkflow/v1alpha1/gate.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"bytes"

"github.com/golang/protobuf/jsonpb"
"google.golang.org/protobuf/proto"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

different uris?


"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
)
Expand Down Expand Up @@ -36,6 +37,10 @@
return buf.Bytes(), nil
}

func (in *ApproveCondition) DeepCopyInto(out *ApproveCondition) {
out.ApproveCondition = proto.Clone(in.ApproveCondition).(*core.ApproveCondition)

Check warning on line 41 in flytepropeller/pkg/apis/flyteworkflow/v1alpha1/gate.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/apis/flyteworkflow/v1alpha1/gate.go#L40-L41

Added lines #L40 - L41 were not covered by tests
}

func (in *ApproveCondition) UnmarshalJSON(b []byte) error {
in.ApproveCondition = &core.ApproveCondition{}
return jsonpb.Unmarshal(bytes.NewReader(b), in.ApproveCondition)
Expand All @@ -62,6 +67,10 @@
return jsonpb.Unmarshal(bytes.NewReader(b), in.SignalCondition)
}

func (in *SignalCondition) DeepCopyInto(out *SignalCondition) {
out.SignalCondition = proto.Clone(in.SignalCondition).(*core.SignalCondition)

Check warning on line 71 in flytepropeller/pkg/apis/flyteworkflow/v1alpha1/gate.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/apis/flyteworkflow/v1alpha1/gate.go#L70-L71

Added lines #L70 - L71 were not covered by tests
}

type SleepCondition struct {
*core.SleepCondition
}
Expand All @@ -83,6 +92,10 @@
return jsonpb.Unmarshal(bytes.NewReader(b), in.SleepCondition)
}

func (in *SleepCondition) DeepCopyInto(out *SleepCondition) {
out.SleepCondition = proto.Clone(in.SleepCondition).(*core.SleepCondition)

Check warning on line 96 in flytepropeller/pkg/apis/flyteworkflow/v1alpha1/gate.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/apis/flyteworkflow/v1alpha1/gate.go#L95-L96

Added lines #L95 - L96 were not covered by tests
}

type GateNodeSpec struct {
Kind ConditionKind `json:"kind"`
Approve *ApproveCondition `json:"approve,omitempty"`
Expand Down
Loading
Loading