Fix versioning override with AutoUpgrade behavior (#1765)

temporalio · Jan 8, 2025 · 938dcad · 938dcad
1 parent 4dd46b8
commit 938dcad
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 5 deletions.
diff --git a/contrib/tools/workflowcheck/determinism/testdata/src/a/ignore.go b/contrib/tools/workflowcheck/determinism/testdata/src/a/ignore.go
@@ -15,8 +15,9 @@ func IgnoreAboveLine() { // want IgnoreAboveLine:"calls non-deterministic functi
 	fmt.Println("Do not ignore this")
 }
 
-//workflowcheck:ignore
 // IgnoreEntireFunction can have a Godoc comment too
+//
+//workflowcheck:ignore
 func IgnoreEntireFunction() {
 	fmt.Print("Do not ignore this")
 	fmt.Printf("Ignore this")

diff --git a/internal/internal_workflow_execution_options.go b/internal/internal_workflow_execution_options.go
@@ -105,6 +105,15 @@ func workflowExecutionOptionsMaskToProto(mask []string) *fieldmaskpb.FieldMask {
 }
 
 func workerDeploymentToProto(d Deployment) *deploymentpb.Deployment {
+	// Server 1.26.2 requires a nil Deployment pointer, and not just a pointer to an empty Deployment,
+	// to indicate that there is no Deployment.
+	// It is a server error to override versioning behavior to AutoUpgrade while providing a Deployment,
+	// and we need to replace it by nil. See https://github.com/temporalio/sdk-go/issues/1764.
+	//
+	// Future server versions may relax this requirement.
+	if (Deployment{}) == d {
+		return nil
+	}
 	return &deploymentpb.Deployment{
 		SeriesName: d.SeriesName,
 		BuildId:    d.BuildID,

diff --git a/internal/workflow_test.go b/internal/workflow_test.go
@@ -61,8 +61,8 @@ func TestGetChildWorkflowOptions(t *testing.T) {
 		},
 		ParentClosePolicy: enums.PARENT_CLOSE_POLICY_REQUEST_CANCEL,
 		VersioningIntent:  VersioningIntentDefault,
-		StaticSummary: "child workflow summary",
-		StaticDetails: "child workflow details",
+		StaticSummary:     "child workflow summary",
+		StaticDetails:     "child workflow details",
 	}
 
 	// Require test options to have non-zero value for each field. This ensures that we update tests (and the
@@ -84,7 +84,7 @@ func TestGetActivityOptions(t *testing.T) {
 		RetryPolicy:            newTestRetryPolicy(),
 		DisableEagerExecution:  true,
 		VersioningIntent:       VersioningIntentDefault,
-		Summary: 			  "activity summary",
+		Summary:                "activity summary",
 	}
 
 	assertNonZero(t, opts)

diff --git a/mocks/Client.go b/mocks/Client.go
diff --git a/test/deployment_test.go b/test/deployment_test.go
@@ -344,10 +344,11 @@ func (ts *DeploymentTestSuite) TestUpdateWorkflowExecutionOptions() {
 	// Two workers:
 	// 1) 1.0 with WaitSignalToStartVersionedOne (setCurrent)
 	// 2) 2.0 with WaitSignalToStartVersionedTwo
-	// Three workflows:
+	// Four workflows:
 	// 1) started with "1.0", manual override to "2.0", finish "2.0"
 	// 2) started with "1.0", manual override to "2.0", remove override, finish "1.0"
 	// 3) started with "1.0", no override, finishes with "1.0" unaffected
+	// 4) started with "1.0", manual override to auto-upgrade, finishes with "2.0"
 
 	worker1 := worker.New(ts.client, ts.taskQueueName, worker.Options{
 		BuildID:                 "1.0",
@@ -401,6 +402,11 @@ func (ts *DeploymentTestSuite) TestUpdateWorkflowExecutionOptions() {
 	handle3, err := ts.client.ExecuteWorkflow(ctx, ts.startWorkflowOptions("3"), "WaitSignalToStartVersioned")
 	ts.NoError(err)
 
+	handle4, err := ts.client.ExecuteWorkflow(ctx, ts.startWorkflowOptions("4"), "WaitSignalToStartVersioned")
+	ts.NoError(err)
+
+	ts.waitForWorkflowRunning(ctx, handle4)
+
 	options, err := ts.client.UpdateWorkflowExecutionOptions(ctx, client.UpdateWorkflowExecutionOptionsRequest{
 		WorkflowId: handle1.GetID(),
 		RunId:      handle1.GetRunID(),
@@ -445,9 +451,31 @@ func (ts *DeploymentTestSuite) TestUpdateWorkflowExecutionOptions() {
 	ts.NoError(err)
 	ts.Equal(options.VersioningOverride, client.VersioningOverride{})
 
+	// Add autoUpgrade to handle4
+	options, err = ts.client.UpdateWorkflowExecutionOptions(ctx, client.UpdateWorkflowExecutionOptionsRequest{
+		WorkflowId: handle4.GetID(),
+		RunId:      handle4.GetRunID(),
+		WorkflowExecutionOptionsChanges: client.WorkflowExecutionOptionsChanges{
+			VersioningOverride: &client.VersioningOverride{
+				Behavior: workflow.VersioningBehaviorAutoUpgrade,
+			},
+		},
+	})
+	ts.NoError(err)
+	ts.Equal(options.VersioningOverride.Deployment.BuildID, "")
+
+	_, err = ts.client.DeploymentClient().SetCurrent(ctx, client.DeploymentSetCurrentOptions{
+		Deployment: client.Deployment{
+			BuildID:    "2.0",
+			SeriesName: seriesName,
+		},
+	})
+	ts.NoError(err)
+
 	ts.NoError(ts.client.SignalWorkflow(ctx, handle1.GetID(), handle1.GetRunID(), "start-signal", "prefix"))
 	ts.NoError(ts.client.SignalWorkflow(ctx, handle2.GetID(), handle2.GetRunID(), "start-signal", "prefix"))
 	ts.NoError(ts.client.SignalWorkflow(ctx, handle3.GetID(), handle3.GetRunID(), "start-signal", "prefix"))
+	ts.NoError(ts.client.SignalWorkflow(ctx, handle4.GetID(), handle4.GetRunID(), "start-signal", "prefix"))
 
 	// Wait for all wfs to finish
 	var result string
@@ -462,6 +490,11 @@ func (ts *DeploymentTestSuite) TestUpdateWorkflowExecutionOptions() {
 	ts.NoError(handle3.Get(ctx, &result))
 	// no override
 	ts.True(IsVersionOne(result))
+
+	ts.NoError(handle4.Get(ctx, &result))
+	// override + autoUpgrade
+	ts.True(IsVersionTwo(result))
+
 }
 
 func (ts *DeploymentTestSuite) TestListDeployments() {