Skip to content

Don't pass AllowNoPollers=true when unsetting current and ramping version #746

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.

Sign up for GitHub

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

Open
carlydf wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Don't pass AllowNoPollers=true when unsetting current and ramping version #746

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bae8f8f
don't use deprecated version string in cleanup
carlydf Feb 3, 2026
471eb19
add to log to confirm build-docker-images is running my code
carlydf Feb 3, 2026
3c7c05e
don't pass AllowNoPollers=true
carlydf Feb 3, 2026
b99fa70
try with no workflow timeout
carlydf Feb 3, 2026
6e64ec9
set workflow timeout to 5m and explain
carlydf Feb 3, 2026
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
39 changes: 7 additions & 32 deletions features/deployment_versioning/aa_clean_old_deployments/feature.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ var Feature = harness.Feature{
}
return run, nil
},
StartWorkflowOptionsMutator: func(o *client.StartWorkflowOptions) {
// I observed this complete in 2 minutes 55 seconds when there were many Deployments
// to delete. If the deletions are happening frequently, this will likely be shorter.
// Giving a long timeout here to make sure it doesn't cause flakiness.
o.WorkflowExecutionTimeout = 5 * time.Minute
Comment on lines +26 to +29
Copy link
Author

@carlydf carlydf Feb 3, 2026

Choose a reason for hiding this comment

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

the next run completed in 279ms, so I think when there isn't much work to be done, this workflow completes quickly, but when there are many versions to delete, it takes a while. I think we should keep it at 5 minutes

},
}

func CleanOldDeployments(ctx workflow.Context) (string, error) {
Expand Down Expand Up @@ -60,10 +66,6 @@ func ListOldDeployments(ctx context.Context) ([]string, error) {
allDeployments = append(allDeployments, deployment.Name)
}
}

if err != nil {
return nil, err
}
Comment on lines -63 to -66
Copy link
Author

@carlydf carlydf Feb 3, 2026

Choose a reason for hiding this comment

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

err was always nil here, so this was not doing anything

return allDeployments, nil
}

Expand All @@ -87,44 +89,17 @@ func DeleteDeployment(ctx context.Context, deploymentName string) error {
DeploymentName: deploymentName,
Identity: "feature-deployment-deleter",
IgnoreMissingTaskQueues: true,
AllowNoPollers: true,
Copy link
Author

@carlydf carlydf Feb 3, 2026

Choose a reason for hiding this comment

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

this is the change that actually avoided the error

})
if err != nil {
// Try using unversioned string (needed if deployment was very old)
_, err = client.WorkflowService().SetWorkerDeploymentCurrentVersion(ctx, &workflowservice.SetWorkerDeploymentCurrentVersionRequest{
Namespace: ns,
DeploymentName: deploymentName,
Version: "__unversioned__",
BuildId: "__unversioned__",
Identity: "feature-deployment-deleter",
IgnoreMissingTaskQueues: true,
AllowNoPollers: true,
})
if err != nil {
return fmt.Errorf("failed to unset current version for deployment %s: %w", deploymentName, err)
}
Comment on lines -93 to -105
Copy link
Author

@carlydf carlydf Feb 3, 2026

Choose a reason for hiding this comment

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

this code is just not needed anymore, because all of the deployments in this test environment are new

return fmt.Errorf("failed to unset current version for deployment %s: %w", deploymentName, err)
}
_, err = client.WorkflowService().SetWorkerDeploymentRampingVersion(ctx, &workflowservice.SetWorkerDeploymentRampingVersionRequest{
Namespace: ns,
DeploymentName: deploymentName,
Identity: "feature-deployment-deleter",
IgnoreMissingTaskQueues: true,
AllowNoPollers: true,
})
if err != nil {
// Try using unversioned string (needed if deployment was very old)
_, err = client.WorkflowService().SetWorkerDeploymentRampingVersion(ctx, &workflowservice.SetWorkerDeploymentRampingVersionRequest{
Namespace: ns,
DeploymentName: deploymentName,
Version: "__unversioned__",
BuildId: "__unversioned__",
Identity: "feature-deployment-deleter",
IgnoreMissingTaskQueues: true,
AllowNoPollers: true,
})
if err != nil {
return fmt.Errorf("failed to unset ramping version for deployment %s: %w", deploymentName, err)
}
return fmt.Errorf("failed to unset ramping version for deployment %s: %w", deploymentName, err)
}

Expand Down
Loading