Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Logging Cleanup - Part 3 #414

Merged
merged 6 commits into from
Sep 27, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
64 changes: 0 additions & 64 deletions pkg/aws/gomock_reflect_3295326204/prog.go

This file was deleted.

64 changes: 0 additions & 64 deletions pkg/aws/gomock_reflect_3994833575/prog.go

This file was deleted.

4 changes: 2 additions & 2 deletions pkg/deploy/externaldns/dnsendpoint_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ func (s *defaultDnsEndpointManager) Create(ctx context.Context, service *lattice
Name: service.Spec.Name + "-dns",
}
if service.Spec.CustomerDomainName == "" {
s.log.Debugf("Skipping creation of %s: detected no custom domain", namespacedName.String())
s.log.Debugf("Skipping creation of %s: detected no custom domain", namespacedName)
return nil
}
if service.Status == nil || service.Status.Dns == "" {
s.log.Debugf("Skipping creation of %s: DNS target not ready in svc status", namespacedName.String())
s.log.Debugf("Skipping creation of %s: DNS target not ready in svc status", namespacedName)
return nil
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/deploy/lattice/service_network_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package lattice
import (
"context"
"errors"
"fmt"

"github.com/golang/glog"

Expand Down Expand Up @@ -229,8 +230,7 @@ func (m *defaultServiceNetworkManager) Delete(ctx context.Context, snName string
}
resp, err := vpcLatticeSess.DeleteServiceNetworkWithContext(ctx, &deleteInput)
if err != nil {
m.log.Debugf("Failed to delete service network %s due to %s", snName, resp)
return errors.New(LATTICE_RETRY)
return fmt.Errorf("%w: failed to delete service network %s due to %s", RetryErr, snName, resp)
}

return err
Expand Down
67 changes: 31 additions & 36 deletions pkg/deploy/lattice/service_network_synthesizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,34 +3,39 @@ package lattice
import (
"context"
"errors"
"github.com/golang/glog"

"github.com/aws/aws-application-networking-k8s/pkg/model/core"
latticemodel "github.com/aws/aws-application-networking-k8s/pkg/model/lattice"
"sigs.k8s.io/controller-runtime/pkg/client"
gateway_api "sigs.k8s.io/gateway-api/apis/v1beta1"

"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-application-networking-k8s/pkg/utils/gwlog"
)

func NewServiceNetworkSynthesizer(client client.Client, serviceNetworkManager ServiceNetworkManager, stack core.Stack) *serviceNetworkSynthesizer {
func NewServiceNetworkSynthesizer(
log gwlog.Logger,
client client.Client,
serviceNetworkManager ServiceNetworkManager,
stack core.Stack,
) *serviceNetworkSynthesizer {
return &serviceNetworkSynthesizer{
Client: client,
log: log,
client: client,
serviceNetworkManager: serviceNetworkManager,
stack: stack,
}
}

type serviceNetworkSynthesizer struct {
client.Client

log gwlog.Logger
client client.Client
serviceNetworkManager ServiceNetworkManager
stack core.Stack
}

func (s *serviceNetworkSynthesizer) Synthesize(ctx context.Context) error {
var ret = ""

glog.V(6).Infof("Start synthesizing ServiceNetworks/Gateways ... \n")

if err := s.synthesizeTriggeredGateways(ctx); err != nil {
ret = LATTICE_RETRY
}
Expand All @@ -44,34 +49,33 @@ func (s *serviceNetworkSynthesizer) Synthesize(ctx context.Context) error {
if ret != "" {
return errors.New(ret)
Comment on lines 49 to 50
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mind to cleanup this error handling too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gave this a try, but after a couple of hours of dealing with e2e test failures, reverted it for the sake of timeboxing. It seems as though we have some reliance on this error handling logic, which I'd like to keep outside the scope of the logging changes

} else {
glog.V(6).Infof("Finish synthesizing ServiceNetworks/Gateways ... \n")
return nil
}

}

func (s *serviceNetworkSynthesizer) synthesizeTriggeredGateways(ctx context.Context) error {
var serviceNetworks []*latticemodel.ServiceNetwork
var ret = "" // only tracks the last error encountered, others are in the logs

s.stack.ListResources(&serviceNetworks)
glog.V(6).Infof("Start synthesizing Triggered ServiceNetworks/Gateways ...%v \n", serviceNetworks)

for _, resServiceNetwork := range serviceNetworks {
s.log.Debugf("Start synthesizing service network %s", resServiceNetwork.Spec.Name)
if resServiceNetwork.Spec.IsDeleted {
deleteRet := s.deleteServiceNetwork(ctx, resServiceNetwork)
if deleteRet != "" {
ret = deleteRet
}
} else {
glog.V(6).Infof("Synthesizing Gateway: Add %v\n", resServiceNetwork.Spec.Name)
s.log.Debugf("Synthesizing Gateway %s", resServiceNetwork.Spec.Name)
serviceNetworkStatus, err := s.serviceNetworkManager.Create(ctx, resServiceNetwork)

if err != nil {
glog.V(6).Infof("Synthesizing Gateway failed for gateway %v error =%v\n ", resServiceNetwork.Spec.Name, err)
s.log.Debugf("Synthesizing Gateway failed for gateway %s due to %s",
resServiceNetwork.Spec.Name, err)
ret = LATTICE_RETRY
} else {
glog.V(6).Infof("Synthesizing Gateway succeeded for gateway %v, status %v \n", resServiceNetwork.Spec.Name, serviceNetworkStatus)
s.log.Debugf("Synthesizing Gateway succeeded for gateway %s, status %s",
resServiceNetwork.Spec.Name, serviceNetworkStatus)
}
}
}
Expand All @@ -84,34 +88,32 @@ func (s *serviceNetworkSynthesizer) synthesizeTriggeredGateways(ctx context.Cont
}

func (s *serviceNetworkSynthesizer) deleteServiceNetwork(ctx context.Context, resServiceNetwork *latticemodel.ServiceNetwork) string {
glog.V(6).Infof("Synthesizing Gateway: Del %v\n", resServiceNetwork.Spec.Name)
s.log.Debugf("Synthesizing Gateway deletion for service network %s", resServiceNetwork.Spec.Name)

// TODO need to check if service network is referenced by gateway in other namespace
gwList := &gateway_api.GatewayList{}
s.Client.List(ctx, gwList)
s.client.List(ctx, gwList)
snUsedByGateway := false
for _, gw := range gwList.Items {
if gw.Name == resServiceNetwork.Spec.Name &&
gw.Namespace != resServiceNetwork.Spec.Namespace {
glog.V(6).Infof("Skip deleting gw %v, namespace %v, since it is still used", gw.Name, gw.Namespace)
s.log.Debugf("Skip deleting gateway %s-%s, since it is still used", gw.Name, gw.Namespace)
snUsedByGateway = true
break
}
}

if snUsedByGateway {
glog.V(6).Infof("Skipping deleting gw: %v since it is still used by gateway(s)",
resServiceNetwork.Spec.Name)

return ""
}

err := s.serviceNetworkManager.Delete(ctx, resServiceNetwork.Spec.Name)
if err != nil {
glog.V(6).Infof("Synthesizing Gateway delete failed for gateway %v error =%v\n ", resServiceNetwork.Spec.Name, err)
s.log.Debugf("Synthesizing Gateway delete failed for gateway %s due to %s",
resServiceNetwork.Spec.Name, err)
return LATTICE_RETRY
} else {
glog.V(6).Infof("Synthesizing Gateway: successfully deleted gateway %v\n", resServiceNetwork.Spec.Name)
s.log.Debugf("Synthesizing gateway deletion for service network %s succeeded", resServiceNetwork.Spec.Name)
}

return ""
Expand All @@ -121,18 +123,16 @@ func (s *serviceNetworkSynthesizer) synthesizeSDKServiceNetworks(ctx context.Con
var ret = ""
sdkServiceNetworks, err := s.serviceNetworkManager.List(ctx)
if err != nil {
glog.V(2).Infof("Synthesize failed on List lattice serviceNetworkes %v\n", err)
return err
}
glog.V(6).Infof("SDK List: %v \n", sdkServiceNetworks)

// handling delete those gateway in lattice DB, but not in K8S DB
// check local K8S cache
gwList := &gateway_api.GatewayList{}
s.Client.List(context.TODO(), gwList)
s.client.List(context.TODO(), gwList)

for _, sdkServiceNetwork := range sdkServiceNetworks {
glog.V(6).Infof("Synthesizing Gateway: checking if sdkServiceNetwork %v needed to be deleted \n", sdkServiceNetwork)
s.log.Debugf("Checking if service network %s needs to be deleted during gateway synthesis", sdkServiceNetwork)

toBeDeleted := false

Expand All @@ -149,23 +149,19 @@ func (s *serviceNetworkSynthesizer) synthesizeSDKServiceNetworks(ctx context.Con
}

if toBeDeleted {

glog.V(2).Infof("Synthesizing Gateway: Delete stale sdkServiceNetwork %v\n", sdkServiceNetwork)
s.log.Debugf("Deleting stale service network %s during gateway synthesis", sdkServiceNetwork)
err := s.serviceNetworkManager.Delete(ctx, sdkServiceNetwork)

if err != nil {
glog.V(6).Infof("Need to retry synthesizing err %v", err)
s.log.Debugf("Need to retry synthesizing service network %s due to err %s", sdkServiceNetwork, err)
ret = LATTICE_RETRY
}
} else {
glog.V(6).Infof("Skip deleting sdkServiceNetwork %v since some gateway(s) still reference it",
s.log.Debugf("Skip deleting service network %s since some gateway(s) still reference it",
sdkServiceNetwork)

}

}
if ret != "" {
glog.V(2).Infof("Need to retry synthesizing, reg = %v", ret)
return errors.New(ret)
} else {
return nil
Expand All @@ -174,7 +170,6 @@ func (s *serviceNetworkSynthesizer) synthesizeSDKServiceNetworks(ctx context.Con

func mapResServiceNetworkByResourceID(resServiceNetworks []*latticemodel.ServiceNetwork) map[string]*latticemodel.ServiceNetwork {
resServiceNetworkByID := make(map[string]*latticemodel.ServiceNetwork, len(resServiceNetworks))

for _, resServiceNetwork := range resServiceNetworks {
resServiceNetworkByID[resServiceNetwork.ID()] = resServiceNetwork
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/deploy/lattice/service_network_synthesizer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
mock_client "github.com/aws/aws-application-networking-k8s/mocks/controller-runtime/client"
"github.com/aws/aws-application-networking-k8s/pkg/gateway"
latticemodel "github.com/aws/aws-application-networking-k8s/pkg/model/lattice"
"github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog"
)

func Test_SynthesizeTriggeredGateways(t *testing.T) {
Expand Down Expand Up @@ -150,7 +151,7 @@ func Test_SynthesizeTriggeredGateways(t *testing.T) {
mockMeshManager.EXPECT().Create(ctx, mesh).Return(meshStatus, tt.meshManagerErr)
}

meshMeshSynthesizer := NewServiceNetworkSynthesizer(mock_client, mockMeshManager, stack)
meshMeshSynthesizer := NewServiceNetworkSynthesizer(gwlog.FallbackLogger, mock_client, mockMeshManager, stack)
err := meshMeshSynthesizer.synthesizeTriggeredGateways(ctx)
assert.Equal(t, tt.wantSynthesizerErr, err)
}
Expand Down Expand Up @@ -253,7 +254,7 @@ func Test_SythesizeSDKMeshs(t *testing.T) {
}

mockMeshManager.EXPECT().List(ctx).Return(sdkMeshsReturned, nil)
meshMeshSynthesizer := NewServiceNetworkSynthesizer(mock_client, mockMeshManager, nil)
meshMeshSynthesizer := NewServiceNetworkSynthesizer(gwlog.FallbackLogger, mock_client, mockMeshManager, nil)
err := meshMeshSynthesizer.synthesizeSDKServiceNetworks(ctx)
assert.Equal(t, tt.wantSynthesizerErr, err)
}
Expand Down
Loading