Skip to content

Commit

Permalink
fix: reset to default settings if healthCheck is removed (#369) (#405)
Browse files Browse the repository at this point in the history
  • Loading branch information
scottlaiaws authored Sep 20, 2023
1 parent 3f89fd8 commit 0999983
Show file tree
Hide file tree
Showing 2 changed files with 266 additions and 50 deletions.
94 changes: 83 additions & 11 deletions pkg/deploy/lattice/target_group_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@ import (
"github.com/aws/aws-sdk-go/service/vpclattice"

"fmt"
"strings"

lattice_aws "github.com/aws/aws-application-networking-k8s/pkg/aws"
"github.com/aws/aws-application-networking-k8s/pkg/config"
"github.com/aws/aws-application-networking-k8s/pkg/latticestore"
latticemodel "github.com/aws/aws-application-networking-k8s/pkg/model/lattice"
"strings"
)

type TargetGroupManager interface {
Expand Down Expand Up @@ -73,22 +74,16 @@ func (s *defaultTargetGroupManager) Create(ctx context.Context, targetGroup *lat
latticeTGName := getLatticeTGName(targetGroup)
// check if exists
tgSummary, err := s.findTargetGroup(ctx, targetGroup)

if err != nil {
return latticemodel.TargetGroupStatus{TargetGroupARN: "", TargetGroupID: ""}, err
}

vpcLatticeSess := s.cloud.Lattice()

// this means Target Group already existed, so this is an update request
if tgSummary != nil {
if targetGroup.Spec.Config.HealthCheckConfig != nil {
_, err := vpcLatticeSess.UpdateTargetGroupWithContext(ctx, &vpclattice.UpdateTargetGroupInput{
HealthCheck: targetGroup.Spec.Config.HealthCheckConfig,
TargetGroupIdentifier: tgSummary.Id,
})
if err != nil {
return latticemodel.TargetGroupStatus{TargetGroupARN: "", TargetGroupID: ""}, err
}
}
return latticemodel.TargetGroupStatus{TargetGroupARN: aws.StringValue(tgSummary.Arn), TargetGroupID: aws.StringValue(tgSummary.Id)}, nil
return s.update(ctx, targetGroup, tgSummary)
}

glog.V(6).Infof("create targetgroup API here %v\n", targetGroup)
Expand Down Expand Up @@ -171,6 +166,38 @@ func (s *defaultTargetGroupManager) Get(ctx context.Context, targetGroup *lattic
return latticemodel.TargetGroupStatus{TargetGroupARN: "", TargetGroupID: ""}, errors.New("Non existing Target Group")
}

func (s *defaultTargetGroupManager) update(ctx context.Context, targetGroup *latticemodel.TargetGroup, tgSummary *vpclattice.TargetGroupSummary) (latticemodel.TargetGroupStatus, error) {
glog.V(6).Info("Target Group already existed. Updating instead")

vpcLatticeSess := s.cloud.Lattice()

healthCheckConfig := targetGroup.Spec.Config.HealthCheckConfig

targetGroupStatus := latticemodel.TargetGroupStatus{
TargetGroupARN: aws.StringValue(tgSummary.Arn),
TargetGroupID: aws.StringValue(tgSummary.Id),
}

if healthCheckConfig == nil {
glog.V(6).Info("healthCheck is empty. Resetting to default settings")

targetGroupProtocolVersion := targetGroup.Spec.Config.ProtocolVersion

healthCheckConfig = s.getDefaultHealthCheckConfig(targetGroupProtocolVersion)
}

_, err := vpcLatticeSess.UpdateTargetGroupWithContext(ctx, &vpclattice.UpdateTargetGroupInput{
HealthCheck: healthCheckConfig,
TargetGroupIdentifier: tgSummary.Id,
})

if err != nil {
return latticemodel.TargetGroupStatus{}, err
}

return targetGroupStatus, nil
}

func (s *defaultTargetGroupManager) Delete(ctx context.Context, targetGroup *latticemodel.TargetGroup) error {
glog.V(6).Infof("Manager: Deleting target group %v \n", targetGroup)

Expand Down Expand Up @@ -382,3 +409,48 @@ func (s *defaultTargetGroupManager) findTargetGroup(ctx context.Context, targetG
}
return nil, nil
}

// Get default health check configuration according to
// https://docs.aws.amazon.com/vpc-lattice/latest/ug/target-group-health-checks.html#health-check-settings
func (s *defaultTargetGroupManager) getDefaultHealthCheckConfig(targetGroupProtocolVersion string) *vpclattice.HealthCheckConfig {
var intResetValue int64 = 0

defaultMatcher := vpclattice.Matcher{
HttpCode: aws.String("200"),
}

defaultPath := "/"

defaultProtocol := vpclattice.TargetGroupProtocolHttp

if targetGroupProtocolVersion == "" {
targetGroupProtocolVersion = vpclattice.TargetGroupProtocolVersionHttp1
}

enabled := targetGroupProtocolVersion == vpclattice.TargetGroupProtocolVersionHttp1

healthCheckProtocolVersion := targetGroupProtocolVersion

if targetGroupProtocolVersion == vpclattice.TargetGroupProtocolVersionGrpc {
healthCheckProtocolVersion = vpclattice.HealthCheckProtocolVersionHttp1
}

return &vpclattice.HealthCheckConfig{
Enabled: &enabled,

Protocol: &defaultProtocol,
ProtocolVersion: &healthCheckProtocolVersion,

Path: &defaultPath,
Matcher: &defaultMatcher,

// Set to nil to use the port of the targets
Port: nil,

// Set to 0 to reset to default value
HealthCheckIntervalSeconds: &intResetValue,
HealthyThresholdCount: &intResetValue,
UnhealthyThresholdCount: &intResetValue,
HealthCheckTimeoutSeconds: &intResetValue,
}
}
222 changes: 183 additions & 39 deletions pkg/deploy/lattice/target_group_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,17 @@ package lattice
import (
"context"
"errors"
"reflect"
"testing"

"github.com/aws/aws-application-networking-k8s/pkg/config"
"github.com/aws/aws-application-networking-k8s/pkg/model/core"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/vpclattice"

mocks_aws "github.com/aws/aws-application-networking-k8s/pkg/aws"
mocks "github.com/aws/aws-application-networking-k8s/pkg/aws/services"
"github.com/aws/aws-application-networking-k8s/pkg/config"
"github.com/aws/aws-application-networking-k8s/pkg/model/core"
latticemodel "github.com/aws/aws-application-networking-k8s/pkg/model/lattice"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/vpclattice"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
)
Expand Down Expand Up @@ -170,48 +170,83 @@ func Test_CreateTargetGroup_TGFailed_Active(t *testing.T) {
}

// target group status is active before creation, no need to recreate
func Test_CreateTargetGroup_TGActive_ACTIVE(t *testing.T) {
c := gomock.NewController(t)
defer c.Finish()
ctx := context.TODO()
tgSpec := latticemodel.TargetGroupSpec{
Name: "test",
Config: latticemodel.TargetGroupConfig{
Protocol: vpclattice.TargetGroupProtocolHttps,
ProtocolVersion: vpclattice.TargetGroupProtocolVersionHttp1,
HealthCheckConfig: &vpclattice.HealthCheckConfig{
Enabled: aws.Bool(true),
func Test_CreateTargetGroup_TGActive_UpdateHealthCheck(t *testing.T) {
tests := []struct {
healthCheckConfig *vpclattice.HealthCheckConfig
wantErr bool
}{
{
healthCheckConfig: &vpclattice.HealthCheckConfig{
Enabled: aws.Bool(false),
},
wantErr: false,
},
{
healthCheckConfig: nil,
wantErr: false,
},
{
wantErr: true,
},
}
tgCreateInput := latticemodel.TargetGroup{
ResourceMeta: core.ResourceMeta{},
Spec: tgSpec,
}
mockVpcLatticeSess := mocks.NewMockLattice(c)

ctx := context.TODO()

arn := "12345678912345678912"
id := "12345678912345678912"
name := "test-https-http1"

beforeCreateStatus := vpclattice.TargetGroupStatusActive
tgSummary := vpclattice.TargetGroupSummary{
Arn: &arn,
Id: &id,
Name: &name,
Status: &beforeCreateStatus,
}
listTgOutput := []*vpclattice.TargetGroupSummary{&tgSummary}
for _, test := range tests {
c := gomock.NewController(t)
defer c.Finish()

mockCloud := mocks_aws.NewMockCloud(c)
mockVpcLatticeSess.EXPECT().ListTargetGroupsAsList(ctx, gomock.Any()).Return(listTgOutput, nil)
mockVpcLatticeSess.EXPECT().UpdateTargetGroupWithContext(ctx, gomock.Any()).Return(nil, nil)
mockCloud.EXPECT().Lattice().Return(mockVpcLatticeSess).AnyTimes()
tgManager := NewTargetGroupManager(mockCloud)
resp, err := tgManager.Create(ctx, &tgCreateInput)
mockVpcLatticeSess := mocks.NewMockLattice(c)
mockCloud := mocks_aws.NewMockCloud(c)
tgManager := NewTargetGroupManager(mockCloud)

assert.Nil(t, err)
assert.Equal(t, resp.TargetGroupARN, arn)
assert.Equal(t, resp.TargetGroupID, id)
tgSpec := latticemodel.TargetGroupSpec{
Name: "test",
Config: latticemodel.TargetGroupConfig{
Protocol: vpclattice.TargetGroupProtocolHttps,
ProtocolVersion: vpclattice.TargetGroupProtocolVersionHttp1,
HealthCheckConfig: test.healthCheckConfig,
},
}

tgCreateInput := latticemodel.TargetGroup{
ResourceMeta: core.ResourceMeta{},
Spec: tgSpec,
}

tgSummary := vpclattice.TargetGroupSummary{
Arn: &arn,
Id: &id,
Name: aws.String("test-https-http1"),
Status: aws.String(vpclattice.TargetGroupStatusActive),
Port: aws.Int64(80),
}

listTgOutput := []*vpclattice.TargetGroupSummary{&tgSummary}

mockVpcLatticeSess.EXPECT().ListTargetGroupsAsList(ctx, gomock.Any()).Return(listTgOutput, nil)

if test.wantErr {
mockVpcLatticeSess.EXPECT().UpdateTargetGroupWithContext(ctx, gomock.Any()).Return(nil, errors.New("error"))
} else {
mockVpcLatticeSess.EXPECT().UpdateTargetGroupWithContext(ctx, gomock.Any()).Return(nil, nil)
}

mockCloud.EXPECT().Lattice().Return(mockVpcLatticeSess).AnyTimes()

resp, err := tgManager.Create(ctx, &tgCreateInput)

if test.wantErr {
assert.NotNil(t, err)
} else {
assert.Nil(t, err)
assert.Equal(t, resp.TargetGroupARN, arn)
assert.Equal(t, resp.TargetGroupID, id)
}
}
}

// target group status is create-in-progress before creation, return Retry
Expand Down Expand Up @@ -1105,3 +1140,112 @@ func Test_Get(t *testing.T) {
}
}
}

func Test_defaultTargetGroupManager_getDefaultHealthCheckConfig(t *testing.T) {
var (
resetValue = aws.Int64(0)
defaultMatcher = &vpclattice.Matcher{
HttpCode: aws.String("200"),
}
defaultPath = aws.String("/")
defaultProtocol = aws.String(vpclattice.TargetGroupProtocolHttp)
)

type args struct {
targetGroupProtocolVersion string
}

tests := []struct {
name string
args args
want *vpclattice.HealthCheckConfig
}{
{
name: "HTTP1 default health check config",
args: args{
targetGroupProtocolVersion: vpclattice.TargetGroupProtocolVersionHttp1,
},
want: &vpclattice.HealthCheckConfig{
Enabled: aws.Bool(true),
HealthCheckIntervalSeconds: resetValue,
HealthCheckTimeoutSeconds: resetValue,
HealthyThresholdCount: resetValue,
UnhealthyThresholdCount: resetValue,
Matcher: defaultMatcher,
Path: defaultPath,
Port: nil,
Protocol: defaultProtocol,
ProtocolVersion: aws.String(vpclattice.HealthCheckProtocolVersionHttp1),
},
},
{
name: "empty target group protocol version default health check config",
args: args{
targetGroupProtocolVersion: "",
},
want: &vpclattice.HealthCheckConfig{
Enabled: aws.Bool(true),
HealthCheckIntervalSeconds: resetValue,
HealthCheckTimeoutSeconds: resetValue,
HealthyThresholdCount: resetValue,
UnhealthyThresholdCount: resetValue,
Matcher: defaultMatcher,
Path: defaultPath,
Port: nil,
Protocol: defaultProtocol,
ProtocolVersion: aws.String(vpclattice.HealthCheckProtocolVersionHttp1),
},
},
{
name: "HTTP2 default health check config",
args: args{
targetGroupProtocolVersion: vpclattice.TargetGroupProtocolVersionHttp2,
},
want: &vpclattice.HealthCheckConfig{
Enabled: aws.Bool(false),
HealthCheckIntervalSeconds: resetValue,
HealthCheckTimeoutSeconds: resetValue,
HealthyThresholdCount: resetValue,
UnhealthyThresholdCount: resetValue,
Matcher: defaultMatcher,
Path: defaultPath,
Port: nil,
Protocol: defaultProtocol,
ProtocolVersion: aws.String(vpclattice.HealthCheckProtocolVersionHttp2),
},
},
{
name: "GRPC default health check config",
args: args{
targetGroupProtocolVersion: vpclattice.TargetGroupProtocolVersionGrpc,
},
want: &vpclattice.HealthCheckConfig{
Enabled: aws.Bool(false),
HealthCheckIntervalSeconds: resetValue,
HealthCheckTimeoutSeconds: resetValue,
HealthyThresholdCount: resetValue,
UnhealthyThresholdCount: resetValue,
Matcher: defaultMatcher,
Path: defaultPath,
Port: nil,
Protocol: defaultProtocol,
ProtocolVersion: aws.String(vpclattice.HealthCheckProtocolVersionHttp1),
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c := gomock.NewController(t)
defer c.Finish()

mockCloud := mocks_aws.NewMockCloud(c)

s := NewTargetGroupManager(mockCloud)

if got := s.getDefaultHealthCheckConfig(tt.args.targetGroupProtocolVersion); !reflect.DeepEqual(got, tt.want) {
t.Errorf("defaultTargetGroupManager.getDefaultHealthCheckConfig() = %v, want %v", got, tt.want)
}
})
}
}

0 comments on commit 0999983

Please sign in to comment.