diff --git a/pkg/deploy/lattice/listener_synthesizer.go b/pkg/deploy/lattice/listener_synthesizer.go index 4a4d29db..e35ece05 100644 --- a/pkg/deploy/lattice/listener_synthesizer.go +++ b/pkg/deploy/lattice/listener_synthesizer.go @@ -45,19 +45,15 @@ func (l *listenerSynthesizer) Synthesize(ctx context.Context) error { return err } - // deletes are deferred to the later logic comparing existing listeners - // to current listeners. - if !listener.IsDeleted { - status, err := l.listenerMgr.Upsert(ctx, listener, svc) - if err != nil { - listenerErr = errors.Join(listenerErr, - fmt.Errorf("failed ListenerManager.Upsert %s-%s due to err %s", - listener.Spec.K8SRouteName, listener.Spec.K8SRouteNamespace, err)) - continue - } - - listener.Status = &status + status, err := l.listenerMgr.Upsert(ctx, listener, svc) + if err != nil { + listenerErr = errors.Join(listenerErr, + fmt.Errorf("failed ListenerManager.Upsert %s-%s due to err %s", + listener.Spec.K8SRouteName, listener.Spec.K8SRouteNamespace, err)) + continue } + + listener.Status = &status } if listenerErr != nil { @@ -86,8 +82,8 @@ func (l *listenerSynthesizer) Synthesize(ctx context.Context) error { func (l *listenerSynthesizer) shouldDelete(listenerToFind *model.Listener, stackListeners []*model.Listener) bool { for _, candidate := range stackListeners { if candidate.Spec.Port == listenerToFind.Spec.Port && candidate.Spec.Protocol == listenerToFind.Spec.Protocol { - // found a match, delete if match is deleted - return candidate.IsDeleted + // found a match, do not delete + return false } } // there is no matching listener diff --git a/pkg/deploy/lattice/listener_synthesizer_test.go b/pkg/deploy/lattice/listener_synthesizer_test.go index b12aff71..7aa421c5 100644 --- a/pkg/deploy/lattice/listener_synthesizer_test.go +++ b/pkg/deploy/lattice/listener_synthesizer_test.go @@ -44,37 +44,6 @@ func Test_SynthesizeListenerCreate(t *testing.T) { assert.Nil(t, err) } -func Test_SynthesizeListenerDeleteNoOp(t *testing.T) { - c := gomock.NewController(t) - defer c.Finish() - ctx := context.TODO() - mockListenerMgr := NewMockListenerManager(c) - - stack := core.NewDefaultStack(core.StackID{Name: "foo", Namespace: "bar"}) - - svc := &model.Service{ - ResourceMeta: core.NewResourceMeta(stack, "AWS:VPCServiceNetwork::Service", "stack-svc-id"), - Status: &model.ServiceStatus{Id: "svc-id"}, - } - assert.NoError(t, stack.AddResource(svc)) - - l := &model.Listener{ - ResourceMeta: core.NewResourceMeta(stack, "AWS:VPCServiceNetwork::Listener", "l-id"), - Spec: model.ListenerSpec{ - StackServiceId: "stack-svc-id", - }, - IsDeleted: true, // <-- the bit that matters - } - assert.NoError(t, stack.AddResource(l)) - - // since we aren't returning any resources, we interpret that to mean the listener is already deleted - mockListenerMgr.EXPECT().List(ctx, gomock.Any()).Return([]*vpclattice.ListenerSummary{}, nil) - - ls := NewListenerSynthesizer(gwlog.FallbackLogger, mockListenerMgr, stack) - err := ls.Synthesize(ctx) - assert.Nil(t, err) -} - func Test_SynthesizeListenerCreateWithReconcile(t *testing.T) { c := gomock.NewController(t) defer c.Finish() diff --git a/pkg/deploy/lattice/rule_manager.go b/pkg/deploy/lattice/rule_manager.go index 25991ac4..48f34479 100644 --- a/pkg/deploy/lattice/rule_manager.go +++ b/pkg/deploy/lattice/rule_manager.go @@ -97,9 +97,24 @@ func (r *defaultRuleManager) buildLatticeRule(modelRule *model.Rule) (*vpclattic updateMatchFromRule(&httpMatch, modelRule) gro.Match = &vpclattice.RuleMatch{HttpMatch: &httpMatch} - if len(modelRule.Spec.Action.TargetGroups) > 0 { + // check if we have at least one valid target group + var hasValidTargetGroup bool + for _, tg := range modelRule.Spec.Action.TargetGroups { + if tg.LatticeTgId != model.InvalidBackendRefTgId { + hasValidTargetGroup = true + break + } + } + + if hasValidTargetGroup { var latticeTGs []*vpclattice.WeightedTargetGroup for _, ruleTg := range modelRule.Spec.Action.TargetGroups { + // skip any invalid TGs - eventually VPC Lattice may support weighted fixed response + // and this logic can be more in line with the spec + if ruleTg.LatticeTgId == model.InvalidBackendRefTgId { + continue + } + latticeTG := vpclattice.WeightedTargetGroup{ TargetGroupIdentifier: aws.String(ruleTg.LatticeTgId), Weight: aws.Int64(ruleTg.Weight), @@ -114,7 +129,7 @@ func (r *defaultRuleManager) buildLatticeRule(modelRule *model.Rule) (*vpclattic }, } } else { - r.log.Debugf("There are no target groups, defaulting to 404 Fixed response") + r.log.Debugf("There are no valid target groups, defaulting to 404 Fixed response") gro.Action = &vpclattice.RuleAction{ FixedResponse: &vpclattice.FixedResponseAction{ StatusCode: aws.Int64(404), @@ -237,7 +252,7 @@ func (r *defaultRuleManager) updateIfNeeded( _, err := r.cloud.Lattice().UpdateRuleWithContext(ctx, &uri) if err != nil { - return model.RuleStatus{}, fmt.Errorf("Failed UpdateRule %d for %s, %s due to %s", + return model.RuleStatus{}, fmt.Errorf("failed UpdateRule %d for %s, %s due to %s", ruleToUpdate.Priority, latticeListenerId, latticeSvcId, err) } @@ -375,7 +390,7 @@ func (r *defaultRuleManager) Delete(ctx context.Context, ruleId string, serviceI _, err := r.cloud.Lattice().DeleteRuleWithContext(ctx, &deleteInput) if err != nil { - return fmt.Errorf("Failed DeleteRule %s/%s/%s due to %s", serviceId, listenerId, ruleId, err) + return fmt.Errorf("failed DeleteRule %s/%s/%s due to %s", serviceId, listenerId, ruleId, err) } r.log.Infof("Success DeleteRule %s/%s/%s", serviceId, listenerId, ruleId) diff --git a/pkg/deploy/lattice/rule_manager_test.go b/pkg/deploy/lattice/rule_manager_test.go index 28d535c3..3d210bbd 100644 --- a/pkg/deploy/lattice/rule_manager_test.go +++ b/pkg/deploy/lattice/rule_manager_test.go @@ -65,6 +65,67 @@ func Test_Create(t *testing.T) { }, } + rInvalidBR := &model.Rule{ + Spec: model.RuleSpec{ + Priority: 1, + Action: model.RuleAction{ + TargetGroups: []*model.RuleTargetGroup{ + { + LatticeTgId: model.InvalidBackendRefTgId, + Weight: 1, + }, + }, + }, + PathMatchPrefix: true, + PathMatchValue: "/foo", + }, + } + + rTwoInvalidBR := &model.Rule{ + Spec: model.RuleSpec{ + Priority: 1, + Action: model.RuleAction{ + TargetGroups: []*model.RuleTargetGroup{ + { + LatticeTgId: model.InvalidBackendRefTgId, + Weight: 1, + }, + { + LatticeTgId: model.InvalidBackendRefTgId, + Weight: 1, + }, + }, + }, + PathMatchPrefix: true, + PathMatchValue: "/foo", + }, + } + + rOneValidBR := &model.Rule{ + Spec: model.RuleSpec{ + Priority: 1, + Method: "POST", + Action: model.RuleAction{ + TargetGroups: []*model.RuleTargetGroup{ + { + LatticeTgId: model.InvalidBackendRefTgId, + Weight: 1, + }, + { + LatticeTgId: model.InvalidBackendRefTgId, + Weight: 1, + }, + { + LatticeTgId: "tg-id", + Weight: 1, + }, + }, + }, + PathMatchPrefix: true, + PathMatchValue: "/foo", + }, + } + t.Run("test create", func(t *testing.T) { mockLattice.EXPECT().GetRulesAsList(ctx, gomock.Any()).Return( []*vpclattice.GetRuleOutput{}, nil) @@ -183,6 +244,55 @@ func Test_Create(t *testing.T) { assert.Nil(t, err) assert.Equal(t, "existing-arn", ruleStatus.Arn) }) + + t.Run("test create - invalid backendRefs", func(t *testing.T) { + mockLattice.EXPECT().GetRulesAsList(ctx, gomock.Any()).Return( + []*vpclattice.GetRuleOutput{}, nil).Times(2) + + mockLattice.EXPECT().CreateRuleWithContext(ctx, gomock.Any()).DoAndReturn( + func(ctx context.Context, input *vpclattice.CreateRuleInput, i ...interface{}) (*vpclattice.CreateRuleOutput, error) { + assert.Equal(t, int64(404), aws.Int64Value(input.Action.FixedResponse.StatusCode)) + + return &vpclattice.CreateRuleOutput{ + Arn: aws.String("arn"), + Id: aws.String("id"), + Name: aws.String("name"), + }, nil + }).Times(2) + + rm := NewRuleManager(gwlog.FallbackLogger, cloud) + ruleStatus, err := rm.Upsert(ctx, rInvalidBR, l, svc) + assert.Nil(t, err) + assert.Equal(t, "arn", ruleStatus.Arn) + + // result should be the same so long as all backendRefs are invalid + ruleStatus, err = rm.Upsert(ctx, rTwoInvalidBR, l, svc) + assert.Nil(t, err) + assert.Equal(t, "arn", ruleStatus.Arn) + }) + + t.Run("test create - one valid backendRef, two invalid", func(t *testing.T) { + mockLattice.EXPECT().GetRulesAsList(ctx, gomock.Any()).Return( + []*vpclattice.GetRuleOutput{}, nil) + + mockLattice.EXPECT().CreateRuleWithContext(ctx, gomock.Any()).DoAndReturn( + func(ctx context.Context, input *vpclattice.CreateRuleInput, i ...interface{}) (*vpclattice.CreateRuleOutput, error) { + assert.Equal(t, "POST", aws.StringValue(input.Match.HttpMatch.Method)) + assert.Equal(t, 1, len(input.Action.Forward.TargetGroups)) + assert.Equal(t, "tg-id", aws.StringValue(input.Action.Forward.TargetGroups[0].TargetGroupIdentifier)) + + return &vpclattice.CreateRuleOutput{ + Arn: aws.String("arn"), + Id: aws.String("id"), + Name: aws.String("name"), + }, nil + }) + + rm := NewRuleManager(gwlog.FallbackLogger, cloud) + ruleStatus, err := rm.Upsert(ctx, rOneValidBR, l, svc) + assert.Nil(t, err) + assert.Equal(t, "arn", ruleStatus.Arn) + }) } func Test_CreateWithTempPriority(t *testing.T) { diff --git a/pkg/deploy/lattice/rule_synthesizer.go b/pkg/deploy/lattice/rule_synthesizer.go index da628055..9a742ed2 100644 --- a/pkg/deploy/lattice/rule_synthesizer.go +++ b/pkg/deploy/lattice/rule_synthesizer.go @@ -49,6 +49,12 @@ func (r *ruleSynthesizer) resolveRuleTgIds(ctx context.Context, modelRule *model } if rtg.StackTargetGroupId != "" { + if rtg.StackTargetGroupId == model.InvalidBackendRefTgId { + r.log.Debugf("Rule TG has an invalid backendref, setting TG id to invalid") + rtg.LatticeTgId = model.InvalidBackendRefTgId + continue + } + r.log.Debugf("Fetching TG %d from the stack (ID %s)", i, rtg.StackTargetGroupId) stackTg := &model.TargetGroup{} diff --git a/pkg/deploy/lattice/rule_synthesizer_test.go b/pkg/deploy/lattice/rule_synthesizer_test.go index 0fe1a82b..5b5cffe1 100644 --- a/pkg/deploy/lattice/rule_synthesizer_test.go +++ b/pkg/deploy/lattice/rule_synthesizer_test.go @@ -166,6 +166,9 @@ func Test_resolveRuleTgs(t *testing.T) { { StackTargetGroupId: "stack-tg-id", }, + { + StackTargetGroupId: model.InvalidBackendRefTgId, + }, }, }, }, @@ -197,4 +200,5 @@ func Test_resolveRuleTgs(t *testing.T) { assert.Equal(t, "svc-export-tg-id", r.Spec.Action.TargetGroups[0].LatticeTgId) assert.Equal(t, "tg-id", r.Spec.Action.TargetGroups[1].LatticeTgId) + assert.Equal(t, model.InvalidBackendRefTgId, r.Spec.Action.TargetGroups[2].LatticeTgId) } diff --git a/pkg/gateway/model_build_listener.go b/pkg/gateway/model_build_listener.go index f6fb12ac..b0769837 100644 --- a/pkg/gateway/model_build_listener.go +++ b/pkg/gateway/model_build_listener.go @@ -5,8 +5,6 @@ import ( "errors" "fmt" model "github.com/aws/aws-application-networking-k8s/pkg/model/lattice" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/types" gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" ) @@ -27,9 +25,6 @@ func (t *latticeServiceModelBuildTask) extractListenerInfo( t.log.Debugf("Building Listener for Route %s-%s", t.route.Name(), t.route.Namespace()) gw, err := t.getGateway(ctx) if err != nil { - if apierrors.IsNotFound(err) && !t.route.DeletionTimestamp().IsZero() { - return 0, string(protocol), nil // ok if we're deleting the route - } return 0, "", err } @@ -83,6 +78,10 @@ func (t *latticeServiceModelBuildTask) buildListeners(ctx context.Context, stack if len(t.route.Spec().ParentRefs()) == 0 { t.log.Debugf("No ParentRefs on route %s-%s, nothing to do", t.route.Name(), t.route.Namespace()) } + if !t.route.DeletionTimestamp().IsZero() { + t.log.Debugf("Route %s-%s is deleted, skipping listener build", t.route.Name(), t.route.Namespace()) + return nil + } for _, parentRef := range t.route.Spec().ParentRefs() { if parentRef.Name != t.route.Spec().ParentRefs()[0].Name { @@ -112,7 +111,6 @@ func (t *latticeServiceModelBuildTask) buildListeners(ctx context.Context, stack t.log.Debugf("Added listener %s-%s to the stack (ID %s)", modelListener.Spec.K8SRouteName, modelListener.Spec.K8SRouteNamespace, modelListener.ID()) - modelListener.IsDeleted = !t.route.DeletionTimestamp().IsZero() } return nil diff --git a/pkg/gateway/model_build_rule.go b/pkg/gateway/model_build_rule.go index 46c55beb..e3918750 100644 --- a/pkg/gateway/model_build_rule.go +++ b/pkg/gateway/model_build_rule.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" @@ -31,6 +30,7 @@ const ( ) func (t *latticeServiceModelBuildTask) buildRules(ctx context.Context, stackListenerId string) error { + // note we only build rules for non-deleted routes t.log.Debugf("Processing %d rules", len(t.route.Spec().Rules())) for i, rule := range t.route.Spec().Rules() { @@ -254,9 +254,16 @@ func (t *latticeServiceModelBuildTask) getTargetGroupsForRuleAction(ctx context. // generate the actual target group model for the backendRef _, tg, err := t.brTgBuilder.Build(ctx, t.route, backendRef, t.stack) if err != nil { - return nil, err + ibre := &InvalidBackendRefError{} + if !errors.As(err, &ibre) { + return nil, err + } + + t.log.Infof("Invalid backendRef found on route %s", t.route.Name()) + ruleTG.StackTargetGroupId = model.InvalidBackendRefTgId + } else { + ruleTG.StackTargetGroupId = tg.ID() } - ruleTG.StackTargetGroupId = tg.ID() } tgList = append(tgList, &ruleTG) diff --git a/pkg/gateway/model_build_rule_test.go b/pkg/gateway/model_build_rule_test.go index 7faa9547..42afeb65 100644 --- a/pkg/gateway/model_build_rule_test.go +++ b/pkg/gateway/model_build_rule_test.go @@ -31,6 +31,13 @@ type dummyTgBuilder struct { } func (d *dummyTgBuilder) Build(ctx context.Context, route core.Route, backendRef core.BackendRef, stack core.Stack) (core.Stack, *model.TargetGroup, error) { + if backendRef.Name() == "invalid" { + return stack, nil, &InvalidBackendRefError{ + BackendRef: backendRef, + Reason: "not valid", + } + } + // just need to provide a TG with an ID id := fmt.Sprintf("tg-%d", d.i) d.i++ @@ -78,6 +85,13 @@ func Test_RuleModelBuild(t *testing.T) { }, Weight: &weight2, } + var invalidBackendRef = gwv1beta1.BackendRef{ + BackendObjectReference: gwv1beta1.BackendObjectReference{ + Name: "invalid", + Kind: &serviceKind, + }, + Weight: &weight2, + } var backendRef1Namespace1 = gwv1beta1.BackendRef{ BackendObjectReference: gwv1beta1.BackendObjectReference{ Name: "targetgroup2", @@ -1361,6 +1375,98 @@ func Test_RuleModelBuild(t *testing.T) { }, }, }, + { + name: "invalid backendRef", + wantErrIsNil: true, + route: core.NewHTTPRoute(gwv1beta1.HTTPRoute{ + ObjectMeta: apimachineryv1.ObjectMeta{ + Name: "service1", + Namespace: "default", + }, + Spec: gwv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gwv1beta1.CommonRouteSpec{ + ParentRefs: []gwv1beta1.ParentReference{ + { + Name: "gw1", + SectionName: &httpSectionName, + }, + }, + }, + Rules: []gwv1beta1.HTTPRouteRule{ + { + BackendRefs: []gwv1beta1.HTTPBackendRef{ + { + BackendRef: invalidBackendRef, + }, + }, + }, + }, + }, + }), + expectedSpec: []model.RuleSpec{ + { + StackListenerId: "listener-id", + Action: model.RuleAction{ + TargetGroups: []*model.RuleTargetGroup{ + { + StackTargetGroupId: model.InvalidBackendRefTgId, + Weight: int64(*invalidBackendRef.Weight), + }, + }, + }, + }, + }, + }, + + { + name: "valid and invalid backendRef", + wantErrIsNil: true, + route: core.NewHTTPRoute(gwv1beta1.HTTPRoute{ + ObjectMeta: apimachineryv1.ObjectMeta{ + Name: "service1", + Namespace: "default", + }, + Spec: gwv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gwv1beta1.CommonRouteSpec{ + ParentRefs: []gwv1beta1.ParentReference{ + { + Name: "gw1", + SectionName: &httpSectionName, + }, + }, + }, + Rules: []gwv1beta1.HTTPRouteRule{ + { + BackendRefs: []gwv1beta1.HTTPBackendRef{ + { + BackendRef: invalidBackendRef, + }, + { + BackendRef: backendRef1, + }, + }, + }, + }, + }, + }), + expectedSpec: []model.RuleSpec{ + { + StackListenerId: "listener-id", + Action: model.RuleAction{ + TargetGroups: []*model.RuleTargetGroup{ + { + StackTargetGroupId: model.InvalidBackendRefTgId, + Weight: int64(*invalidBackendRef.Weight), + }, + { + StackTargetGroupId: "tg-0", + Weight: int64(*backendRef1.Weight), + }, + }, + }, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/gateway/model_build_targetgroup.go b/pkg/gateway/model_build_targetgroup.go index 7c0450cb..6d9a4846 100644 --- a/pkg/gateway/model_build_targetgroup.go +++ b/pkg/gateway/model_build_targetgroup.go @@ -24,6 +24,15 @@ import ( model "github.com/aws/aws-application-networking-k8s/pkg/model/lattice" ) +type InvalidBackendRefError struct { + BackendRef core.BackendRef + Reason string +} + +func (e *InvalidBackendRefError) Error() string { + return e.Reason +} + type SvcExportTargetGroupModelBuilder interface { // used during standard model build Build(ctx context.Context, svcExport *anv1alpha1.ServiceExport) (core.Stack, error) @@ -247,7 +256,7 @@ func (t *backendRefTargetGroupModelBuildTask) buildTargetGroup(ctx context.Conte } t.log.Debugf("Added target group for backendRef %s to the stack %s", t.backendRef.Name(), stackTG.ID()) - stackTG.IsDeleted = !t.route.DeletionTimestamp().IsZero() + stackTG.IsDeleted = !t.route.DeletionTimestamp().IsZero() // should always be false if !stackTG.IsDeleted { t.buildTargets(ctx, stackTG.ID()) } @@ -260,16 +269,10 @@ func (t *backendRefTargetGroupModelBuildTask) buildTargets(ctx context.Context, t.log.Debugf("Service import does not manage targets, returning") return nil } - backendRefNsName := getBackendRefNsName(t.route, t.backendRef) svc := &corev1.Service{} if err := t.client.Get(ctx, backendRefNsName, svc); err != nil { - if apierrors.IsNotFound(err) && !t.route.DeletionTimestamp().IsZero() { - t.log.Infof("Ignoring not found error for service %s on deleted route %s", - t.backendRef.Name(), t.route.Name()) - } else { - return fmt.Errorf("error finding backend service %s due to %s", backendRefNsName, err) - } + return fmt.Errorf("error finding backend service %s due to %s", backendRefNsName, err) } targetsBuilder := NewTargetsBuilder(t.log, t.client, t.stack) @@ -283,20 +286,21 @@ func (t *backendRefTargetGroupModelBuildTask) buildTargets(ctx context.Context, // Now, Only k8sService and serviceImport creation deletion use this function to build TargetGroupSpec, serviceExport does not use this function to create TargetGroupSpec func (t *backendRefTargetGroupModelBuildTask) buildTargetGroupSpec(ctx context.Context) (model.TargetGroupSpec, error) { + // note we only build target groups for backendRefs on non-deleted routes backendKind := string(*t.backendRef.Kind()) t.log.Debugf("buildTargetGroupSpec, kind %s", backendKind) vpc := config.VpcID eksCluster := config.ClusterName - routeIsDeleted := !t.route.DeletionTimestamp().IsZero() - backendRefNsName := getBackendRefNsName(t.route, t.backendRef) svc := &corev1.Service{} if err := t.client.Get(ctx, backendRefNsName, svc); err != nil { - if routeIsDeleted && apierrors.IsNotFound(err) { - t.log.Infof("Ignoring not found error for service %s on deleted route %s", - t.backendRef.Name(), t.route.Name()) - } else if !routeIsDeleted { + if apierrors.IsNotFound(err) { + return model.TargetGroupSpec{}, &InvalidBackendRefError{ + BackendRef: t.backendRef, + Reason: fmt.Sprintf("service %s on route %s not found, backendRef invalid", backendRefNsName.Name, t.route.Name()), + } + } else { return model.TargetGroupSpec{}, fmt.Errorf("error finding backend service %s due to %s", backendRefNsName, err) } @@ -307,14 +311,7 @@ func (t *backendRefTargetGroupModelBuildTask) buildTargetGroupSpec(ctx context.C if svc != nil { ipAddressType, err = buildTargetGroupIpAddressType(svc) if err != nil { - if routeIsDeleted { - // Ignore error for deletion request - t.log.Debugf("Unable to determine IP address type for deleted route, using default") - ipAddressType = vpclattice.IpAddressTypeIpv4 - } else { - // we care that there's an error if we are not deleting - return model.TargetGroupSpec{}, err - } + return model.TargetGroupSpec{}, err } } diff --git a/pkg/gateway/model_build_targetgroup_test.go b/pkg/gateway/model_build_targetgroup_test.go index 8e53fe1c..0f075773 100644 --- a/pkg/gateway/model_build_targetgroup_test.go +++ b/pkg/gateway/model_build_targetgroup_test.go @@ -28,7 +28,7 @@ import ( model "github.com/aws/aws-application-networking-k8s/pkg/model/lattice" ) -func Test_TGModelByServicExportBuild(t *testing.T) { +func Test_TGModelByServiceExportBuild(t *testing.T) { config.VpcID = "vpc-id" config.ClusterName = "cluster-name" @@ -418,47 +418,6 @@ func Test_TGModelByHTTPRouteBuild(t *testing.T) { wantErrIsNil: false, wantIPv6TargetGroup: false, }, - { - name: "Create LatticeService where backend serviceimport does NOT exist", - route: core.NewHTTPRoute(gwv1beta1.HTTPRoute{ - ObjectMeta: metav1.ObjectMeta{ - Name: "service4", - Namespace: "ns1", - Finalizers: []string{"gateway.k8s.aws/resources"}, - }, - Spec: gwv1beta1.HTTPRouteSpec{ - CommonRouteSpec: gwv1beta1.CommonRouteSpec{ - ParentRefs: []gwv1beta1.ParentReference{ - { - Name: "gateway1", - Namespace: namespacePtr("ns1"), - }, - }, - }, - Rules: []gwv1beta1.HTTPRouteRule{ - { - BackendRefs: []gwv1beta1.HTTPBackendRef{ - { - BackendRef: gwv1beta1.BackendRef{ - BackendObjectReference: gwv1beta1.BackendObjectReference{ - Name: "service4-tg1", - Namespace: namespacePtr("ns31"), - Kind: kindPtr("ServiceImport"), - }, - }, - }, - }, - }, - }, - }, - }), - svcExist: false, - wantError: nil, - wantName: "service1", - wantIsDeleted: false, - wantErrIsNil: false, - wantIPv6TargetGroup: false, - }, { name: "Lattice Service with IPv6 Target Group", route: core.NewHTTPRoute(gwv1beta1.HTTPRoute{ @@ -551,6 +510,10 @@ func Test_TGModelByHTTPRouteBuild(t *testing.T) { _, stackTg, err := builder.Build(ctx, tt.route, httpBackendRef, stack) if !tt.wantErrIsNil { + ibre := &InvalidBackendRefError{} + if !tt.svcExist { + assert.ErrorAs(t, err, &ibre) + } assert.NotNil(t, err) return } @@ -590,6 +553,8 @@ func Test_TGModelByHTTPRouteBuild(t *testing.T) { } } +// service imports do not do a full TG build, just a reference +// see model_build_rule.go#getTargetGroupsForRuleAction func Test_ServiceImportToTGBuildReturnsError(t *testing.T) { config.VpcID = "vpc-id" config.ClusterName = "cluster-name" diff --git a/pkg/model/lattice/listener.go b/pkg/model/lattice/listener.go index 6b8eaa8e..09f62326 100644 --- a/pkg/model/lattice/listener.go +++ b/pkg/model/lattice/listener.go @@ -8,7 +8,6 @@ type Listener struct { core.ResourceMeta `json:"-"` Spec ListenerSpec `json:"spec"` Status *ListenerStatus `json:"status,omitempty"` - IsDeleted bool `json:"isdeleted"` } type ListenerSpec struct { diff --git a/pkg/model/lattice/rule.go b/pkg/model/lattice/rule.go index 1fe19dd3..aad0e383 100644 --- a/pkg/model/lattice/rule.go +++ b/pkg/model/lattice/rule.go @@ -14,7 +14,8 @@ type Rule struct { } const ( - MaxRulePriority = 100 + MaxRulePriority = 100 + InvalidBackendRefTgId = "INVALID" ) type RuleSpec struct { diff --git a/test/pkg/test/framework.go b/test/pkg/test/framework.go index ec856987..9cf64048 100644 --- a/test/pkg/test/framework.go +++ b/test/pkg/test/framework.go @@ -467,9 +467,9 @@ func (env *Framework) GetTargets(ctx context.Context, targetGroup *vpclattice.Ta *target.Status == vpclattice.TargetStatusHealthy) }) - g.Expect(retrievedTargets).Should(HaveLen(len(targetIps))) + g.Expect(targetIps).Should(HaveLen(len(podIps))) found = retrievedTargets - }).WithPolling(time.Minute).WithTimeout(7 * time.Minute).Should(Succeed()) + }).WithPolling(15 * time.Second).WithTimeout(7 * time.Minute).Should(Succeed()) return found } @@ -478,7 +478,7 @@ func (env *Framework) GetAllTargets(ctx context.Context, targetGroup *vpclattice } func GetTargets(targetGroup *vpclattice.TargetGroupSummary, deployment *appsv1.Deployment, env *Framework, ctx context.Context) ([]string, []*vpclattice.TargetSummary) { - env.Log.Infoln("Trying to retrieve registered targets for targetGroup", targetGroup.Name) + env.Log.Infoln("Trying to retrieve registered targets for targetGroup", *targetGroup.Name) env.Log.Infoln("deployment.Spec.Selector.MatchLabels:", deployment.Spec.Selector.MatchLabels) podList := &corev1.PodList{} expectedMatchingLabels := make(map[string]string, len(deployment.Spec.Selector.MatchLabels)) diff --git a/test/suites/integration/deregister_targets_test.go b/test/suites/integration/delete_target_groups_test.go similarity index 74% rename from test/suites/integration/deregister_targets_test.go rename to test/suites/integration/delete_target_groups_test.go index f08147e2..8d8cd1cb 100644 --- a/test/suites/integration/deregister_targets_test.go +++ b/test/suites/integration/delete_target_groups_test.go @@ -1,6 +1,8 @@ package integration import ( + "github.com/aws/aws-application-networking-k8s/pkg/aws/services" + "github.com/aws/aws-sdk-go/aws" "log" "time" @@ -16,7 +18,7 @@ import ( "github.com/aws/aws-application-networking-k8s/test/pkg/test" ) -var _ = Describe("Deregister Targets", Ordered, func() { +var _ = Describe("Delete Unused Target Groups", Ordered, func() { var ( deployments = make([]*appsv1.Deployment, 2) services = make([]*v1.Service, 2) @@ -26,11 +28,11 @@ var _ = Describe("Deregister Targets", Ordered, func() { BeforeAll(func() { deployments[0], services[0] = testFramework.NewHttpApp(test.HTTPAppOptions{ - Name: "target-deregistration-test-1", + Name: "tg-delete-test-1", Namespace: k8snamespace, }) deployments[1], services[1] = testFramework.NewHttpApp(test.HTTPAppOptions{ - Name: "target-deregistration-test-2", + Name: "tg-delete-test-2", Namespace: k8snamespace, }) pathMatchHttpRoute = testFramework.NewPathMatchHttpRoute( @@ -66,14 +68,14 @@ var _ = Describe("Deregister Targets", Ordered, func() { } }) - It("Kubernetes Service deletion deregisters targets", func() { + It("Kubernetes Service deletion deletes target groups", func() { testFramework.ExpectDeleted(ctx, services[0]) - verifyNoTargetsForTargetGroup(targetGroups[0]) + verifyTargetGroupDeleted(targetGroups[0]) }) - It("Kubernetes Deployment deletion triggers targets de-registering", func() { + It("Kubernetes Deployment deletion deletes target groups", func() { testFramework.ExpectDeleted(ctx, deployments[1]) - verifyNoTargetsForTargetGroup(targetGroups[1]) + verifyTargetGroupDeleted(targetGroups[1]) }) AfterAll(func() { @@ -89,16 +91,21 @@ var _ = Describe("Deregister Targets", Ordered, func() { }) }) -func verifyNoTargetsForTargetGroup(targetGroup *vpclattice.TargetGroupSummary) { +func verifyTargetGroupDeleted(targetGroup *vpclattice.TargetGroupSummary) { Eventually(func(g Gomega) { - log.Println("Verifying VPC lattice Targets deregistered") - targets, err := testFramework.LatticeClient.ListTargetsAsList(ctx, &vpclattice.ListTargetsInput{ + log.Println("Verifying VPC lattice target group deleted ", *targetGroup.Id) + tg, err := testFramework.LatticeClient.GetTargetGroupWithContext(ctx, &vpclattice.GetTargetGroupInput{ TargetGroupIdentifier: targetGroup.Id, }) - g.Expect(err).To(BeNil()) - log.Println("targets:", targets) - for _, target := range targets { - g.Expect(*target.Status).To(Equal(vpclattice.TargetStatusDraining)) + if err != nil && services.IsNotFoundError(err) { + return } + + // showing up as "deleting" is also fine + if aws.StringValue(tg.Status) == vpclattice.TargetGroupStatusDeleteInProgress { + return + } + + g.Expect(true).To(BeFalse()) }).Should(Succeed()) }