Skip to content

Commit 27bf5a6

Browse files
zijun726911Zijun Wang
zijun726911
and
Zijun Wang
authored
Fix Httproute deletion leaking Target Group issue (#276)
* ## Changes: - Changed model_build_targetgroup and model_build_service, make sure target group deletion request `model` triggered by httproute deletion could pass to target_group_synthesizer correctly - Changed HttpRoute deletion's vpclattice resource deletion order: Delete latticeService first then delete the targetGroup * Add more unit test for target_group_manager --------- Co-authored-by: Zijun Wang <zijunw@amazon.com>
1 parent 518899f commit 27bf5a6

12 files changed

+866
-57
lines changed

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ export KUBEBUILDER_ASSETS ?= ${HOME}/.kubebuilder/bin
22
export CLUSTER_NAME ?= $(shell kubectl config view --minify -o jsonpath='{.clusters[].name}' | rev | cut -d"/" -f1 | rev | cut -d"." -f1)
33
export CLUSTER_VPC_ID ?= $(shell aws eks describe-cluster --name $(CLUSTER_NAME) | jq -r ".cluster.resourcesVpcConfig.vpcId")
44
export AWS_ACCOUNT_ID ?= $(shell aws sts get-caller-identity --query Account --output text)
5-
5+
export REGION ?= $(shell aws configure get region)
66
# Image URL to use all building/pushing image targets
77
IMG ?= controller:latest
88
VERSION ?= $(shell git tag --sort=committerdate | tail -1)

‎pkg/deploy/lattice/target_group_manager.go‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package lattice
33
import (
44
"context"
55
"errors"
6+
"github.com/aws/aws-sdk-go/aws/awserr"
67
"github.com/golang/glog"
78

89
"github.com/aws/aws-sdk-go/aws"
@@ -160,13 +161,19 @@ func (s *defaultTargetGroupManager) Delete(ctx context.Context, targetGroup *lat
160161
}
161162
glog.V(6).Infof("TG manager list, listReq %v\n", listTargetsInput)
162163
listResp, err := vpcLatticeSess.ListTargetsAsList(ctx, &listTargetsInput)
163-
glog.V(6).Infof("manager delete, listResp %v, err: %v \n", listResp, err)
164+
glog.V(6).Infof("TG manager delete, listResp %v, err: %v \n", listResp, err)
164165
if err != nil {
165166
return err
166167
}
167168

168169
var targets []*vpclattice.Target
169170
for _, t := range listResp {
171+
if t.Status != nil && *t.Status != vpclattice.TargetStatusUnused {
172+
glog.V(6).Infof("TargetGroupManager: The targetGroup [%v] has non-Unused status target(s), which means this targetGroup is still in use by latticeService, it cannot be deleted now\n", targetGroup.Spec.LatticeID)
173+
// Before call the defaultTargetGroupManager.Delete(), we always call the latticeServiceManager.Delete() first,
174+
// *t.Status != vpclattice.TargetStatusUnused means previous delete latticeService still in the progress, we could wait for 20 seconds and then retry
175+
return errors.New(LATTICE_RETRY)
176+
}
170177
targets = append(targets, &vpclattice.Target{
171178
Id: t.Id,
172179
Port: t.Port,
@@ -200,7 +207,17 @@ func (s *defaultTargetGroupManager) Delete(ctx context.Context, targetGroup *lat
200207
}
201208

202209
delResp, err := vpcLatticeSess.DeleteTargetGroupWithContext(ctx, &deleteTGInput)
203-
210+
if err != nil {
211+
if aerr, ok := err.(awserr.Error); ok {
212+
switch aerr.Code() {
213+
case vpclattice.ErrCodeResourceNotFoundException:
214+
err = nil
215+
break
216+
default:
217+
glog.V(6).Infof("vpcLatticeSess.DeleteTargetGroupWithContext() return error %v \n", err)
218+
}
219+
}
220+
}
204221
glog.V(2).Infof("TGManager delTGInput >>>> %v delTGResp :%v, err %v \n", deleteTGInput, delResp, err)
205222

206223
return err

‎pkg/deploy/lattice/target_group_manager_test.go‎

Lines changed: 87 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"errors"
66
"github.com/aws/aws-application-networking-k8s/pkg/config"
77
"github.com/aws/aws-application-networking-k8s/pkg/model/core"
8+
"github.com/aws/aws-sdk-go/aws/awserr"
89
"github.com/aws/aws-sdk-go/service/vpclattice"
910
"testing"
1011

@@ -511,20 +512,24 @@ func Test_CreateTargetGroup_CreateTGFailed(t *testing.T) {
511512
assert.Equal(t, resp.TargetGroupID, "")
512513
}
513514

514-
// Case1: Deregister targets and delete target group work perfectly fine
515+
// Case1: Deregister unused status targets and delete target group work perfectly fine
515516
// Case2: Delete target group that no targets register on it
516517
// Case3: While deleting target group, deregister targets fails
517518
// Case4: While deleting target group, list targets fails
518519
// Case5: While deleting target group, deregister targets unsuccessfully
519520
// Case6: Delete target group fails
521+
// Case7: While deleting target group, that it has non-unused status targets, return LATTICE_RETRY
522+
// Case8: While deleting target group, the vpcLatticeSess.DeleteTargetGroupWithContext() return ResourceNotFoundException, delete target group success and return err nil
520523

521-
// Case1: Deregister targets and delete target group work perfectly fine
524+
// Case1: Deregister unused status targets and delete target group work perfectly fine
522525
func Test_DeleteTG_DeRegisterTargets_DeleteTargetGroup(t *testing.T) {
523526
sId := "123.456.7.890"
524527
sPort := int64(80)
528+
targetStatus := vpclattice.TargetStatusUnused
525529
targetsList := &vpclattice.TargetSummary{
526-
Id: &sId,
527-
Port: &sPort,
530+
Id: &sId,
531+
Port: &sPort,
532+
Status: &targetStatus,
528533
}
529534
targetsSuccessful := &vpclattice.Target{
530535
Id: &sId,
@@ -774,6 +779,84 @@ func Test_DeleteTG_DeRegisterTargets_DeleteTargetGroupFailed(t *testing.T) {
774779
assert.Equal(t, err, errors.New("DeleteTG_failed"))
775780
}
776781

782+
// Case7: While deleting target group, it has non-unused status targets, return LATTICE_RETRY
783+
func Test_DeleteTG_TargetsNonUnused(t *testing.T) {
784+
targetId := "123.456.7.890"
785+
targetPort := int64(80)
786+
targetStatus := vpclattice.TargetStatusHealthy
787+
targets := &vpclattice.TargetSummary{
788+
Id: &targetId,
789+
Port: &targetPort,
790+
Status: &targetStatus,
791+
}
792+
listTargetsOutput := []*vpclattice.TargetSummary{targets}
793+
794+
tgSpec := latticemodel.TargetGroupSpec{
795+
Name: "test",
796+
Config: latticemodel.TargetGroupConfig{},
797+
Type: "IP",
798+
IsDeleted: false,
799+
LatticeID: "123",
800+
}
801+
tgDeleteInput := latticemodel.TargetGroup{
802+
ResourceMeta: core.ResourceMeta{},
803+
Spec: tgSpec,
804+
Status: nil,
805+
}
806+
c := gomock.NewController(t)
807+
defer c.Finish()
808+
ctx := context.TODO()
809+
mockCloud := mocks_aws.NewMockCloud(c)
810+
mockVpcLatticeSess := mocks.NewMockLattice(c)
811+
mockVpcLatticeSess.EXPECT().ListTargetsAsList(ctx, gomock.Any()).Return(listTargetsOutput, nil)
812+
mockCloud.EXPECT().Lattice().Return(mockVpcLatticeSess)
813+
tgManager := NewTargetGroupManager(mockCloud)
814+
815+
err := tgManager.Delete(ctx, &tgDeleteInput)
816+
817+
assert.NotNil(t, err)
818+
assert.Equal(t, err, errors.New(LATTICE_RETRY))
819+
}
820+
821+
// Case8: While deleting target group, the vpcLatticeSess.DeleteTargetGroupWithContext() return ResourceNotFoundException, delete target group success and return err nil
822+
func Test_DeleteTG_vpcLatticeSessReturnResourceNotFound_DeleteTargetGroupSuccessAndErrIsNil(t *testing.T) {
823+
sId := "123.456.7.890"
824+
sPort := int64(80)
825+
targets := &vpclattice.TargetSummary{
826+
Id: &sId,
827+
Port: &sPort,
828+
}
829+
listTargetsOutput := []*vpclattice.TargetSummary{targets}
830+
deRegisterTargetsOutput := &vpclattice.DeregisterTargetsOutput{}
831+
deleteTargetGroupOutput := &vpclattice.DeleteTargetGroupOutput{}
832+
833+
tgSpec := latticemodel.TargetGroupSpec{
834+
Name: "test",
835+
Config: latticemodel.TargetGroupConfig{},
836+
Type: "IP",
837+
IsDeleted: false,
838+
LatticeID: "123",
839+
}
840+
tgDeleteInput := latticemodel.TargetGroup{
841+
ResourceMeta: core.ResourceMeta{},
842+
Spec: tgSpec,
843+
}
844+
c := gomock.NewController(t)
845+
defer c.Finish()
846+
ctx := context.TODO()
847+
mockCloud := mocks_aws.NewMockCloud(c)
848+
mockVpcLatticeSess := mocks.NewMockLattice(c)
849+
mockVpcLatticeSess.EXPECT().ListTargetsAsList(ctx, gomock.Any()).Return(listTargetsOutput, nil)
850+
mockVpcLatticeSess.EXPECT().DeregisterTargetsWithContext(ctx, gomock.Any()).Return(deRegisterTargetsOutput, nil)
851+
mockVpcLatticeSess.EXPECT().DeleteTargetGroupWithContext(ctx, gomock.Any()).Return(deleteTargetGroupOutput, awserr.New(vpclattice.ErrCodeResourceNotFoundException, "", nil))
852+
mockCloud.EXPECT().Lattice().Return(mockVpcLatticeSess).AnyTimes()
853+
tgManager := NewTargetGroupManager(mockCloud)
854+
855+
err := tgManager.Delete(ctx, &tgDeleteInput)
856+
857+
assert.Nil(t, err)
858+
}
859+
777860
func Test_ListTG_TGsExist(t *testing.T) {
778861
arn := "123456789"
779862
id := "123456789"

‎pkg/deploy/lattice/target_group_synthesizer.go‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,3 +359,84 @@ func (t *targetGroupSynthesizer) PostSynthesize(ctx context.Context) error {
359359
// nothing to do here
360360
return nil
361361
}
362+
363+
func (t *targetGroupSynthesizer) SynthesizeTriggeredTargetGroupsCreation(ctx context.Context) error {
364+
var resTargetGroups []*latticemodel.TargetGroup
365+
var returnErr = false
366+
t.stack.ListResources(&resTargetGroups)
367+
glog.V(6).Infof("SynthesizeTriggeredTargetGroupsCreation TargetGroups: [%v]\n", resTargetGroups)
368+
for _, resTargetGroup := range resTargetGroups {
369+
if resTargetGroup.Spec.IsDeleted {
370+
glog.V(6).Infof("In the SynthesizeTriggeredTargetGroupsCreation(), we only handle TG Creation request and skip any deletion request [%v] \n", resTargetGroup)
371+
continue
372+
}
373+
if resTargetGroup.Spec.Config.IsServiceImport {
374+
tgStatus, err := t.targetGroupManager.Get(ctx, resTargetGroup)
375+
if err != nil {
376+
glog.V(6).Infof("Error on t.targetGroupManager.Get for %v err %v\n", resTargetGroup, err)
377+
returnErr = true
378+
continue
379+
}
380+
// for serviceimport, the httproutename is ""
381+
t.latticeDataStore.AddTargetGroup(resTargetGroup.Spec.Name,
382+
resTargetGroup.Spec.Config.VpcID, tgStatus.TargetGroupARN, tgStatus.TargetGroupID,
383+
resTargetGroup.Spec.Config.IsServiceImport, "")
384+
glog.V(6).Infof("targetGroup Synthesized successfully for %s: %v\n", resTargetGroup.Spec.Name, tgStatus)
385+
} else { // handle TargetGroup creation request that triggered by httproute with backendref k8sService creation or serviceExport creation
386+
resTargetGroup.Spec.Config.VpcID = config.VpcID
387+
tgStatus, err := t.targetGroupManager.Create(ctx, resTargetGroup)
388+
if err != nil {
389+
glog.V(6).Infof("Error on t.targetGroupManager.Create for %v err %v\n", resTargetGroup, err)
390+
returnErr = true
391+
continue
392+
}
393+
//In the ModelBuildTask, it should already add a tg entry in the latticeDataStore,
394+
//in here, only UPDATE the entry with tgStatus.TargetGroupARN and tgStatus.TargetGroupID
395+
t.latticeDataStore.AddTargetGroup(resTargetGroup.Spec.Name,
396+
resTargetGroup.Spec.Config.VpcID, tgStatus.TargetGroupARN,
397+
tgStatus.TargetGroupID, resTargetGroup.Spec.Config.IsServiceImport,
398+
resTargetGroup.Spec.Config.K8SHTTPRouteName)
399+
glog.V(6).Infof("targetGroup Synthesized successfully for %v: %v\n", resTargetGroup.Spec, tgStatus)
400+
}
401+
}
402+
glog.V(6).Infof("Done -- SynthesizeTriggeredTargetGroupsCreation %v\n", resTargetGroups)
403+
if returnErr {
404+
return errors.New(LATTICE_RETRY)
405+
} else {
406+
return nil
407+
}
408+
409+
}
410+
411+
func (t *targetGroupSynthesizer) SynthesizeTriggeredTargetGroupsDeletion(ctx context.Context) error {
412+
var resTargetGroups []*latticemodel.TargetGroup
413+
var returnErr = false
414+
t.stack.ListResources(&resTargetGroups)
415+
glog.V(2).Infof("SynthesizeTriggeredTargetGroupsDeletion: TargetGroups ==[%v]\n", resTargetGroups)
416+
for _, resTargetGroup := range resTargetGroups {
417+
if !resTargetGroup.Spec.IsDeleted {
418+
glog.V(6).Infof("SynthesizeTriggeredTargetGroupsDeletion should ignore target group creation request for tg: [%v]\n", resTargetGroup)
419+
continue
420+
}
421+
if resTargetGroup.Spec.Config.IsServiceImport {
422+
// For delete TargetGroup request triggered by service import, we just delete the tg in the datastore
423+
t.latticeDataStore.DelTargetGroup(resTargetGroup.Spec.Name, resTargetGroup.Spec.Config.K8SHTTPRouteName, resTargetGroup.Spec.Config.IsServiceImport)
424+
} else {
425+
// For delete TargetGroup request triggered by k8s service, invoke vpc lattice api to delete it, if success, delete the tg in the datastore as well
426+
err := t.targetGroupManager.Delete(ctx, resTargetGroup)
427+
glog.V(6).Infof("err := t.targetGroupManager.Delete(ctx, resTargetGroup) err: %v\n", err)
428+
if err == nil {
429+
glog.V(6).Infof("Delete Target Group in SynthesizeTriggeredTargetGroupsDeletion: successfully deleted target group %v\n", resTargetGroup)
430+
t.latticeDataStore.DelTargetGroup(resTargetGroup.Spec.Name, resTargetGroup.Spec.Config.K8SHTTPRouteName, resTargetGroup.Spec.Config.IsServiceImport)
431+
} else {
432+
glog.V(6).Infof("Delete Target Group in SynthesizeTriggeredTargetGroupsDeletion: failed to delete target group %v, err %v \n", resTargetGroup, err)
433+
returnErr = true
434+
}
435+
}
436+
}
437+
if returnErr {
438+
return errors.New(LATTICE_RETRY)
439+
} else {
440+
return nil
441+
}
442+
}

0 commit comments

Comments
 (0)