Skip to content

Commit

Permalink
Sorting version summaries in DescribeWorkerDeployment calls (#7398)
Browse files Browse the repository at this point in the history
## What changed?
<!-- Describe what has changed in this PR -->
- Sorting version summaries in DescribeWorkerDeployment calls with the
newest version coming at the top.

## Why?
<!-- Tell your future self why have you made these changes -->
- Correctness and better user functionality.

## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
- Added a new functional test + modified an existing one.

## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->

## Documentation
<!-- Have you made sure this change doesn't falsify anything currently
stated in `docs/`? If significant
new behavior is added, have you described that in `docs/`? -->

## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
  • Loading branch information
Shivs11 authored Feb 28, 2025
1 parent a0c0c1e commit 719a92a
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 9 deletions.
6 changes: 6 additions & 0 deletions service/worker/workerdeployment/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"context"
"errors"
"fmt"
"sort"
"time"

"github.com/pborman/uuid"
Expand Down Expand Up @@ -1274,6 +1275,11 @@ func (d *ClientImpl) deploymentStateToDeploymentInfo(deploymentName string, stat
})
}

// Sort by create time, with the latest version first.
sort.Slice(workerDeploymentInfo.VersionSummaries, func(i, j int) bool {
return workerDeploymentInfo.VersionSummaries[i].CreateTime.AsTime().After(workerDeploymentInfo.VersionSummaries[j].CreateTime.AsTime())
})

return &workerDeploymentInfo, nil
}

Expand Down
68 changes: 59 additions & 9 deletions tests/worker_deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ func (s *WorkerDeploymentSuite) TestNamespaceDeploymentsLimit() {
s.pollFromDeploymentExpectFail(ctx, tv.WithDeploymentSeriesNumber(2))
}

func (s *WorkerDeploymentSuite) TestDescribeWorkerDeployment_TwoVersions() {
func (s *WorkerDeploymentSuite) TestDescribeWorkerDeployment_TwoVersions_Sorted() {
ctx, cancel := context.WithTimeout(context.Background(), time.Second*30)
defer cancel()
tv := testvars.New(s)
Expand All @@ -264,6 +264,15 @@ func (s *WorkerDeploymentSuite) TestDescribeWorkerDeployment_TwoVersions() {
secondVersion := tv.WithBuildIDNumber(2)

go s.pollFromDeployment(ctx, firstVersion)

// waiting for 1ms to start the second version later.
startTime := time.Now()
waitTime := 1 * time.Millisecond
s.EventuallyWithT(func(t *assert.CollectT) {
a := assert.New(t)
a.Greater(time.Since(startTime), waitTime)
}, 10*time.Second, 1000*time.Millisecond)

go s.pollFromDeployment(ctx, secondVersion)

s.EventuallyWithT(func(t *assert.CollectT) {
Expand All @@ -283,15 +292,13 @@ func (s *WorkerDeploymentSuite) TestDescribeWorkerDeployment_TwoVersions() {
if len(resp.GetWorkerDeploymentInfo().GetVersionSummaries()) < 2 {
return
}
a.NotNil(resp.GetWorkerDeploymentInfo().GetVersionSummaries()[0].GetVersion())
a.NotNil(resp.GetWorkerDeploymentInfo().GetVersionSummaries()[1].GetVersion())
// Verify that the version summaries are non-nil and sorted.
versionSummaries := resp.GetWorkerDeploymentInfo().GetVersionSummaries()

versions := []string{
resp.GetWorkerDeploymentInfo().GetVersionSummaries()[0].GetVersion(),
resp.GetWorkerDeploymentInfo().GetVersionSummaries()[1].GetVersion(),
}
a.Contains(versions, firstVersion.DeploymentVersionString())
a.Contains(versions, secondVersion.DeploymentVersionString())
a.NotNil(versionSummaries[0].GetVersion())
a.NotNil(versionSummaries[1].GetVersion())
a.Equal(versionSummaries[0].GetVersion(), secondVersion.DeploymentVersionString())
a.Equal(versionSummaries[1].GetVersion(), firstVersion.DeploymentVersionString())

a.NotNil(resp.GetWorkerDeploymentInfo().GetVersionSummaries()[0].GetCreateTime())
a.NotNil(resp.GetWorkerDeploymentInfo().GetVersionSummaries()[1].GetCreateTime())
Expand All @@ -300,6 +307,49 @@ func (s *WorkerDeploymentSuite) TestDescribeWorkerDeployment_TwoVersions() {
}, time.Second*10, time.Millisecond*1000)
}

func (s *WorkerDeploymentSuite) TestDescribeWorkerDeployment_MultipleVersions_Sorted() {
ctx, cancel := context.WithTimeout(context.Background(), time.Second*30)
defer cancel()
tv := testvars.New(s)

numVersions := 10

for i := 0; i < numVersions; i++ {
go s.pollFromDeployment(ctx, tv.WithBuildIDNumber(i))

// waiting for 1ms to start the next version later.
startTime := time.Now()
waitTime := 1 * time.Millisecond
s.EventuallyWithT(func(t *assert.CollectT) {
a := assert.New(t)
a.Greater(time.Since(startTime), waitTime)
}, 10*time.Second, 1000*time.Millisecond)
}

s.EventuallyWithT(func(t *assert.CollectT) {
a := assert.New(t)

resp, err := s.FrontendClient().DescribeWorkerDeployment(ctx, &workflowservice.DescribeWorkerDeploymentRequest{
Namespace: s.Namespace().String(),
DeploymentName: tv.DeploymentSeries(),
})
a.NoError(err)

a.NotNil(resp.GetWorkerDeploymentInfo().GetVersionSummaries())
a.Equal(numVersions, len(resp.GetWorkerDeploymentInfo().GetVersionSummaries()))

if len(resp.GetWorkerDeploymentInfo().GetVersionSummaries()) < numVersions {
return
}

// Verify that the version summaries are sorted.
versionSummaries := resp.GetWorkerDeploymentInfo().GetVersionSummaries()
for i := 0; i < numVersions-1; i++ {
a.Less(versionSummaries[i+1].GetCreateTime().AsTime(), versionSummaries[i].GetCreateTime().AsTime())
}
}, time.Second*10, time.Millisecond*1000)
}

func (s *WorkerDeploymentSuite) TestDescribeWorkerDeployment_SetCurrentVersion() {
ctx, cancel := context.WithTimeout(context.Background(), time.Second*30)
defer cancel()
Expand Down

0 comments on commit 719a92a

Please sign in to comment.