Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
7 changes: 1 addition & 6 deletions rollout/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,7 @@ func (c *executor) RollOut(ctx context.Context, input *types.RollOutInput) (last
TaskDefinition: c.td.TaskDefinitionArn,
}
if input.UpdateService {
updateInput.LoadBalancers = env.ServiceDefinitionInput.LoadBalancers
updateInput.NetworkConfiguration = env.ServiceDefinitionInput.NetworkConfiguration
updateInput.ServiceConnectConfiguration = env.ServiceDefinitionInput.ServiceConnectConfiguration
updateInput.ServiceRegistries = env.ServiceDefinitionInput.ServiceRegistries
updateInput.PlatformVersion = env.ServiceDefinitionInput.PlatformVersion
updateInput.VolumeConfigurations = env.ServiceDefinitionInput.VolumeConfigurations
applyServiceDefinitionToUpdateInput(updateInput, env.ServiceDefinitionInput)
}
if _, err := ecsCli.UpdateService(ctx, updateInput); err != nil {
l.Errorf("😨 failed to update service: %s", err)
Expand Down
26 changes: 17 additions & 9 deletions rollout/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,23 @@ func TestExecutor_Rollout(t *testing.T) {
taskMock.EXPECT().Start(gomock.Any()).Return(nil),
taskMock.EXPECT().Wait(gomock.Any()).Return(nil),
ecsMock.EXPECT().UpdateService(gomock.Any(), &ecs.UpdateServiceInput{
Cluster: &envars.Cluster,
Service: &envars.Service,
TaskDefinition: td.TaskDefinitionArn,
ServiceConnectConfiguration: envars.ServiceDefinitionInput.ServiceConnectConfiguration,
LoadBalancers: envars.ServiceDefinitionInput.LoadBalancers,
NetworkConfiguration: envars.ServiceDefinitionInput.NetworkConfiguration,
PlatformVersion: envars.ServiceDefinitionInput.PlatformVersion,
VolumeConfigurations: envars.ServiceDefinitionInput.VolumeConfigurations,
ServiceRegistries: envars.ServiceDefinitionInput.ServiceRegistries,
Cluster: &envars.Cluster,
CapacityProviderStrategy: envars.ServiceDefinitionInput.CapacityProviderStrategy,
Service: &envars.Service,
TaskDefinition: td.TaskDefinitionArn,
ServiceConnectConfiguration: envars.ServiceDefinitionInput.ServiceConnectConfiguration,
LoadBalancers: envars.ServiceDefinitionInput.LoadBalancers,
NetworkConfiguration: envars.ServiceDefinitionInput.NetworkConfiguration,
PlatformVersion: envars.ServiceDefinitionInput.PlatformVersion,
VolumeConfigurations: envars.ServiceDefinitionInput.VolumeConfigurations,
ServiceRegistries: envars.ServiceDefinitionInput.ServiceRegistries,
DeploymentConfiguration: envars.ServiceDefinitionInput.DeploymentConfiguration,
HealthCheckGracePeriodSeconds: envars.ServiceDefinitionInput.HealthCheckGracePeriodSeconds,
EnableECSManagedTags: &envars.ServiceDefinitionInput.EnableECSManagedTags,
PlacementConstraints: envars.ServiceDefinitionInput.PlacementConstraints,
PlacementStrategy: envars.ServiceDefinitionInput.PlacementStrategy,
PropagateTags: envars.ServiceDefinitionInput.PropagateTags,
VpcLatticeConfigurations: envars.ServiceDefinitionInput.VpcLatticeConfigurations,
}).
DoAndReturn(mocker.Ecs.UpdateService),
ecsMock.EXPECT().DescribeServices(gomock.Any(), gomock.Any(), gomock.Any()).
Expand Down
23 changes: 23 additions & 0 deletions rollout/service_sync.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package rollout

import (
"github.com/aws/aws-sdk-go-v2/service/ecs"
)

func applyServiceDefinitionToUpdateInput(updateInput *ecs.UpdateServiceInput, serviceInput *ecs.CreateServiceInput) {
// Preserve nil and empty slice distinctions from the service definition.
updateInput.CapacityProviderStrategy = serviceInput.CapacityProviderStrategy
updateInput.LoadBalancers = serviceInput.LoadBalancers
updateInput.NetworkConfiguration = serviceInput.NetworkConfiguration
updateInput.ServiceConnectConfiguration = serviceInput.ServiceConnectConfiguration
updateInput.ServiceRegistries = serviceInput.ServiceRegistries
updateInput.PlatformVersion = serviceInput.PlatformVersion
updateInput.VolumeConfigurations = serviceInput.VolumeConfigurations
updateInput.DeploymentConfiguration = serviceInput.DeploymentConfiguration
updateInput.HealthCheckGracePeriodSeconds = serviceInput.HealthCheckGracePeriodSeconds
updateInput.EnableECSManagedTags = &serviceInput.EnableECSManagedTags
updateInput.PlacementConstraints = serviceInput.PlacementConstraints
updateInput.PlacementStrategy = serviceInput.PlacementStrategy
updateInput.PropagateTags = serviceInput.PropagateTags
updateInput.VpcLatticeConfigurations = serviceInput.VpcLatticeConfigurations
}
53 changes: 53 additions & 0 deletions rollout/service_sync_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package rollout

import (
"testing"

"github.com/aws/aws-sdk-go-v2/service/ecs"
ecstypes "github.com/aws/aws-sdk-go-v2/service/ecs/types"
"github.com/stretchr/testify/assert"
)

func TestApplyServiceDefinitionToUpdateInput(t *testing.T) {
t.Run("preserves nil slice fields", func(t *testing.T) {
updateInput := &ecs.UpdateServiceInput{}
applyServiceDefinitionToUpdateInput(updateInput, &ecs.CreateServiceInput{})

assert.Nil(t, updateInput.CapacityProviderStrategy)
assert.Nil(t, updateInput.LoadBalancers)
assert.Nil(t, updateInput.ServiceRegistries)
assert.Nil(t, updateInput.VolumeConfigurations)
assert.Nil(t, updateInput.PlacementConstraints)
assert.Nil(t, updateInput.PlacementStrategy)
assert.Nil(t, updateInput.VpcLatticeConfigurations)

assert.Nil(t, updateInput.NetworkConfiguration)
assert.Nil(t, updateInput.ServiceConnectConfiguration)
assert.Nil(t, updateInput.PlatformVersion)
assert.Nil(t, updateInput.DeploymentConfiguration)
assert.Nil(t, updateInput.HealthCheckGracePeriodSeconds)
assert.Equal(t, ecstypes.PropagateTags(""), updateInput.PropagateTags)
})

t.Run("preserves explicit empty slice fields", func(t *testing.T) {
serviceInput := &ecs.CreateServiceInput{
CapacityProviderStrategy: []ecstypes.CapacityProviderStrategyItem{},
LoadBalancers: []ecstypes.LoadBalancer{},
ServiceRegistries: []ecstypes.ServiceRegistry{},
VolumeConfigurations: []ecstypes.ServiceVolumeConfiguration{},
PlacementConstraints: []ecstypes.PlacementConstraint{},
PlacementStrategy: []ecstypes.PlacementStrategy{},
VpcLatticeConfigurations: []ecstypes.VpcLatticeConfiguration{},
}
updateInput := &ecs.UpdateServiceInput{}
applyServiceDefinitionToUpdateInput(updateInput, serviceInput)

assert.Equal(t, serviceInput.CapacityProviderStrategy, updateInput.CapacityProviderStrategy)
assert.Equal(t, serviceInput.LoadBalancers, updateInput.LoadBalancers)
assert.Equal(t, serviceInput.ServiceRegistries, updateInput.ServiceRegistries)
assert.Equal(t, serviceInput.VolumeConfigurations, updateInput.VolumeConfigurations)
assert.Equal(t, serviceInput.PlacementConstraints, updateInput.PlacementConstraints)
assert.Equal(t, serviceInput.PlacementStrategy, updateInput.PlacementStrategy)
assert.Equal(t, serviceInput.VpcLatticeConfigurations, updateInput.VpcLatticeConfigurations)
})
}
62 changes: 62 additions & 0 deletions rollout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,68 @@ func integrationTest(t *testing.T, env *env.Envars, lbcount int, input *types.Ro
}
}

func TestCage_RollOut_UpdateServicePreservesNilOptionalSliceSettings(t *testing.T) {
ctx := context.TODO()
envars := test.DefaultEnvars()
mocker := test.NewMockContext()
td, _ := mocker.Ecs.RegisterTaskDefinition(ctx, envars.TaskDefinitionInput)

currentServiceInput := *envars.ServiceDefinitionInput
currentServiceInput.TaskDefinition = td.TaskDefinition.TaskDefinitionArn
currentServiceInput.LaunchType = ""
currentServiceInput.CapacityProviderStrategy = []ecstypes.CapacityProviderStrategyItem{
{CapacityProvider: aws.String("FARGATE"), Weight: 1},
}
currentServiceInput.ServiceRegistries = []ecstypes.ServiceRegistry{
{RegistryArn: aws.String("arn:aws:servicediscovery:us-west-2:123456789012:service/srv-123456")},
}
currentServiceInput.PlacementConstraints = []ecstypes.PlacementConstraint{
{Type: ecstypes.PlacementConstraintTypeDistinctInstance},
}
currentServiceInput.PlacementStrategy = []ecstypes.PlacementStrategy{
{Type: ecstypes.PlacementStrategyTypeSpread, Field: aws.String("attribute:ecs.availability-zone")},
}
_, _ = mocker.Ecs.CreateService(ctx, &currentServiceInput)

serviceBeforeRollout, _ := mocker.GetEcsService(envars.Service)
assert.NotEmpty(t, serviceBeforeRollout.CapacityProviderStrategy)
assert.NotEmpty(t, serviceBeforeRollout.LoadBalancers)
assert.NotEmpty(t, serviceBeforeRollout.ServiceRegistries)
assert.NotEmpty(t, serviceBeforeRollout.PlacementConstraints)
assert.NotEmpty(t, serviceBeforeRollout.PlacementStrategy)

nextServiceInput := currentServiceInput
nextServiceInput.CapacityProviderStrategy = nil
nextServiceInput.LoadBalancers = nil
nextServiceInput.ServiceRegistries = nil
nextServiceInput.PlacementConstraints = nil
nextServiceInput.PlacementStrategy = nil
envars.ServiceDefinitionInput = &nextServiceInput

c := &cage{di: di.NewDomain(func(b *di.B) {
b.Set(key.Env, envars)
b.Set(key.Ec2Cli, mocker.Ec2)
b.Set(key.EcsCli, mocker.Ecs)
b.Set(key.AlbCli, mocker.Alb)
b.Set(key.Logger, test.NewLogger())
b.Set(key.Time, test.NewFakeTime())
b.Set(key.TaskFactory, task.NewFactory(b.Future()))
})}

result, err := c.RollOut(ctx, &types.RollOutInput{UpdateService: true})
if err != nil {
t.Fatal(err)
}

updatedService, _ := mocker.GetEcsService(envars.Service)
assert.True(t, result.ServiceUpdated)
assert.Nil(t, updatedService.CapacityProviderStrategy)
assert.Nil(t, updatedService.LoadBalancers)
assert.Nil(t, updatedService.ServiceRegistries)
assert.Nil(t, updatedService.PlacementConstraints)
assert.Nil(t, updatedService.PlacementStrategy)
Comment thread
keroxp marked this conversation as resolved.
Outdated
}

func TestCage_Rollout_Failure(t *testing.T) {
t.Run("should error if DescribeServices failed", func(t *testing.T) {
ctrl := gomock.NewController(t)
Expand Down
6 changes: 6 additions & 0 deletions test/ecs.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ func (ctx *EcsServer) CreateService(c context.Context, input *ecs.CreateServiceI
}
ret := &types.Service{
ServiceName: input.ServiceName,
CapacityProviderStrategy: input.CapacityProviderStrategy,
RunningCount: 0,
LaunchType: input.LaunchType,
LoadBalancers: input.LoadBalancers,
Expand All @@ -37,6 +38,8 @@ func (ctx *EcsServer) CreateService(c context.Context, input *ecs.CreateServiceI
Status: &st,
ServiceArn: &idstr,
PlatformVersion: input.PlatformVersion,
PlacementConstraints: input.PlacementConstraints,
PlacementStrategy: input.PlacementStrategy,
ServiceRegistries: input.ServiceRegistries,
NetworkConfiguration: input.NetworkConfiguration,
Deployments: []types.Deployment{
Expand Down Expand Up @@ -116,7 +119,10 @@ func (ctx *EcsServer) UpdateService(c context.Context, input *ecs.UpdateServiceI
s.DesiredCount = nextDesiredCount
s.TaskDefinition = nextTaskDefinition
s.RunningCount = nextDesiredCount
s.CapacityProviderStrategy = input.CapacityProviderStrategy
s.PlatformVersion = input.PlatformVersion
s.PlacementConstraints = input.PlacementConstraints
s.PlacementStrategy = input.PlacementStrategy
s.ServiceRegistries = input.ServiceRegistries
s.NetworkConfiguration = input.NetworkConfiguration
s.LoadBalancers = input.LoadBalancers
Comment thread
keroxp marked this conversation as resolved.
Outdated
Expand Down