Skip to content

Commit

Permalink
chore(xds): improve error logging (#8136)
Browse files Browse the repository at this point in the history
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
  • Loading branch information
jakubdyszkiewicz authored Oct 25, 2023
1 parent 1eaf89b commit 152408b
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 52 deletions.
4 changes: 2 additions & 2 deletions pkg/core/permissions/matcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func MatchExternalServicesTrafficPermissions(
dataplane *core_mesh.DataplaneResource,
externalServices *core_mesh.ExternalServiceResourceList,
permissions *core_mesh.TrafficPermissionResourceList,
) ([]*core_mesh.ExternalServiceResource, error) {
) []*core_mesh.ExternalServiceResource {
var matchedExternalServices []*core_mesh.ExternalServiceResource

externalServicePermissions := BuildExternalServicesPermissionsMap(externalServices, permissions.Items)
Expand All @@ -73,7 +73,7 @@ func MatchExternalServicesTrafficPermissions(
matchedExternalServices = append(matchedExternalServices, externalService)
}
}
return matchedExternalServices, nil
return matchedExternalServices
}

type ExternalServicePermissions map[string]*core_mesh.TrafficPermissionResource
Expand Down
3 changes: 1 addition & 2 deletions pkg/core/permissions/matcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,7 @@ var _ = Describe("Match", func() {
tp := &core_mesh.TrafficPermissionResourceList{
Items: given.policies,
}
matchedEs, err := permissions.MatchExternalServicesTrafficPermissions(given.dataplane, es, tp)
Expect(err).ToNot(HaveOccurred())
matchedEs := permissions.MatchExternalServicesTrafficPermissions(given.dataplane, es, tp)

Expect(given.expected).To(HaveLen(len(matchedEs)))
for _, externalService := range matchedEs {
Expand Down
13 changes: 4 additions & 9 deletions pkg/plugins/runtime/gateway/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,7 @@ func (g *FilterChainGenerators) For(ctx xds_context.Context, info GatewayListene
// policies.
func gatewayListenerInfoFromProxy(
ctx context.Context, meshCtx *xds_context.MeshContext, proxy *core_xds.Proxy,
) (
[]GatewayListenerInfo, error,
) {
) []GatewayListenerInfo {
gateway := xds_topology.SelectGateway(meshCtx.Resources.Gateways().Items, proxy.Dataplane.Spec.Matches)

if gateway == nil {
Expand All @@ -119,7 +117,7 @@ func gatewayListenerInfoFromProxy(
"service", proxy.Dataplane.Spec.GetIdentifyingService(),
)

return nil, nil
return nil
}

log.V(1).Info(fmt.Sprintf("matched gateway %q to dataplane %q",
Expand Down Expand Up @@ -147,12 +145,9 @@ func gatewayListenerInfoFromProxy(

var listenerInfos []GatewayListenerInfo

matchedExternalServices, err := permissions.MatchExternalServicesTrafficPermissions(
matchedExternalServices := permissions.MatchExternalServicesTrafficPermissions(
proxy.Dataplane, externalServices, meshCtx.Resources.TrafficPermissions(),
)
if err != nil {
return nil, errors.Wrap(err, "unable to find external services matched by traffic permissions")
}

outboundEndpoints := core_xds.EndpointMap{}
for k, v := range meshCtx.EndpointMap {
Expand Down Expand Up @@ -197,7 +192,7 @@ func gatewayListenerInfoFromProxy(
})
}

return listenerInfos, nil
return listenerInfos
}

func (g Generator) Generate(ctx context.Context, xdsCtx xds_context.Context, proxy *core_xds.Proxy) (*core_xds.ResourceSet, error) {
Expand Down
8 changes: 1 addition & 7 deletions pkg/plugins/runtime/gateway/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,7 @@ func (p *plugin) Apply(ctx context.Context, meshContext xds_context.MeshContext,
if proxy.Dataplane == nil || !proxy.Dataplane.Spec.IsBuiltinGateway() {
return nil
}
l, err := gatewayListenerInfoFromProxy(
ctx, &meshContext, proxy,
)
if err != nil {
return err
}
proxy.RuntimeExtensions[metadata.PluginName] = l
proxy.RuntimeExtensions[metadata.PluginName] = gatewayListenerInfoFromProxy(ctx, &meshContext, proxy)
return nil
}

Expand Down
24 changes: 13 additions & 11 deletions pkg/xds/context/mesh_context_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"sort"
"strings"

"github.com/pkg/errors"

mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1"
"github.com/kumahq/kuma/pkg/core"
"github.com/kumahq/kuma/pkg/core/datasource"
Expand Down Expand Up @@ -79,12 +81,12 @@ func (m *meshContextBuilder) Build(ctx context.Context, meshName string) (MeshCo
func (m *meshContextBuilder) BuildIfChanged(ctx context.Context, meshName string, latestMeshCtx *MeshContext) (*MeshContext, error) {
mesh := core_mesh.NewMeshResource()
if err := m.rm.Get(ctx, mesh, core_store.GetByKey(meshName, core_model.NoMesh)); err != nil {
return nil, err
return nil, errors.Wrapf(err, "could not fetch mesh %s", meshName)
}

resources, err := m.fetchResources(ctx, mesh)
if err != nil {
return nil, err
return nil, errors.Wrap(err, "could not fetch resources")
}
m.resolveAddresses(resources)

Expand All @@ -103,7 +105,7 @@ func (m *meshContextBuilder) BuildIfChanged(ctx context.Context, meshName string

virtualOutboundView, err := m.vipsPersistence.GetByMesh(ctx, mesh.GetMeta().GetName())
if err != nil {
return nil, err
return nil, errors.Wrap(err, "could not fetch vips")
}
// resolve all the domains
domains, outbounds := xds_topology.VIPOutbounds(virtualOutboundView, m.topLevelDomain, m.vipPort)
Expand Down Expand Up @@ -210,7 +212,7 @@ func (m *meshContextBuilder) fetchResources(ctx context.Context, mesh *core_mesh
case core_mesh.MeshType:
meshes := &core_mesh.MeshResourceList{}
if err := m.rm.List(ctx, meshes, core_store.ListOrdered()); err != nil {
return Resources{}, err
return Resources{}, errors.Wrap(err, "could not list meshes")
}
otherMeshes := &core_mesh.MeshResourceList{}
for _, someMesh := range meshes.Items {
Expand All @@ -224,20 +226,20 @@ func (m *meshContextBuilder) fetchResources(ctx context.Context, mesh *core_mesh
case core_mesh.ZoneIngressType:
zoneIngresses := &core_mesh.ZoneIngressResourceList{}
if err := m.rm.List(ctx, zoneIngresses, core_store.ListOrdered()); err != nil {
return Resources{}, err
return Resources{}, errors.Wrap(err, "could not list zone ingresses")
}
resources.MeshLocalResources[typ] = zoneIngresses
case core_mesh.ZoneEgressType:
zoneEgresses := &core_mesh.ZoneEgressResourceList{}
if err := m.rm.List(ctx, zoneEgresses, core_store.ListOrdered()); err != nil {
return Resources{}, err
return Resources{}, errors.Wrap(err, "could not list zone egresses")
}
resources.MeshLocalResources[typ] = zoneEgresses
case system.ConfigType:
configs := &system.ConfigResourceList{}
var items []*system.ConfigResource
if err := m.rm.List(ctx, configs, core_store.ListOrdered()); err != nil {
return Resources{}, err
return Resources{}, errors.Wrap(err, "could not list configs")
}
for _, config := range configs.Items {
if configInHash(config.Meta.GetName(), mesh.Meta.GetName()) {
Expand All @@ -256,7 +258,7 @@ func (m *meshContextBuilder) fetchResources(ctx context.Context, mesh *core_mesh

insights := &core_mesh.ServiceInsightResourceList{}
if err := m.rm.List(ctx, insights, core_store.ListByMesh(mesh.Meta.GetName()), core_store.ListOrdered()); err != nil {
return Resources{}, err
return Resources{}, errors.Wrap(err, "could not list service insights")
}

resources.MeshLocalResources[typ] = insights
Expand All @@ -266,7 +268,7 @@ func (m *meshContextBuilder) fetchResources(ctx context.Context, mesh *core_mesh
return Resources{}, err
}
if err := m.rm.List(ctx, rlist, core_store.ListByMesh(mesh.Meta.GetName()), core_store.ListOrdered()); err != nil {
return Resources{}, err
return Resources{}, errors.Wrapf(err, "could not list %s", typ)
}
resources.MeshLocalResources[typ] = rlist
}
Expand All @@ -286,7 +288,7 @@ func (m *meshContextBuilder) fetchResources(ctx context.Context, mesh *core_mesh
return gateway.(*core_mesh.MeshGatewayResource).Spec.IsCrossMesh()
},
); err != nil {
return Resources{}, err
return Resources{}, errors.Wrap(err, "could not fetch cross mesh resources")
}
}
if _, ok := m.typeSet[core_mesh.DataplaneType]; ok {
Expand All @@ -302,7 +304,7 @@ func (m *meshContextBuilder) fetchResources(ctx context.Context, mesh *core_mesh
return dataplane.(*core_mesh.DataplaneResource).Spec.IsBuiltinGateway()
},
); err != nil {
return Resources{}, err
return Resources{}, errors.Wrap(err, "could not fetch cross mesh resources")
}
}

Expand Down
18 changes: 6 additions & 12 deletions pkg/xds/sync/dataplane_proxy_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,11 @@ func (p *DataplaneProxyBuilder) Build(ctx context.Context, key core_model.Resour
return nil, core_store.ErrorResourceNotFound(core_mesh.DataplaneType, key.Name, key.Mesh)
}

routing, destinations, err := p.resolveRouting(ctx, meshContext, dp)
if err != nil {
return nil, err
}
routing, destinations := p.resolveRouting(ctx, meshContext, dp)

matchedPolicies, err := p.matchPolicies(meshContext, dp, destinations)
if err != nil {
return nil, err
return nil, errors.Wrap(err, "could not match policies")
}

matchedPolicies.TrafficRoutes = routing.TrafficRoutes
Expand Down Expand Up @@ -69,7 +66,7 @@ func (p *DataplaneProxyBuilder) Build(ctx context.Context, key core_model.Resour
for k, pl := range core_plugins.Plugins().ProxyPlugins() {
err := pl.Apply(ctx, meshContext, proxy)
if err != nil {
return nil, errors.Wrapf(err, "Failed applying proxy plugin: %s", k)
return nil, errors.Wrapf(err, "failed applying proxy plugin: %s", k)
}
}
return proxy, nil
Expand All @@ -79,11 +76,8 @@ func (p *DataplaneProxyBuilder) resolveRouting(
ctx context.Context,
meshContext xds_context.MeshContext,
dataplane *core_mesh.DataplaneResource,
) (*core_xds.Routing, core_xds.DestinationMap, error) {
matchedExternalServices, err := permissions.MatchExternalServicesTrafficPermissions(dataplane, meshContext.Resources.ExternalServices(), meshContext.Resources.TrafficPermissions())
if err != nil {
return nil, nil, err
}
) (*core_xds.Routing, core_xds.DestinationMap) {
matchedExternalServices := permissions.MatchExternalServicesTrafficPermissions(dataplane, meshContext.Resources.ExternalServices(), meshContext.Resources.TrafficPermissions())

p.resolveVIPOutbounds(meshContext, dataplane)

Expand All @@ -105,7 +99,7 @@ func (p *DataplaneProxyBuilder) resolveRouting(
OutboundTargets: meshContext.EndpointMap,
ExternalServiceOutboundTargets: endpointMap,
}
return routing, destinations, nil
return routing, destinations
}

func (p *DataplaneProxyBuilder) resolveVIPOutbounds(meshContext xds_context.MeshContext, dataplane *core_mesh.DataplaneResource) {
Expand Down
18 changes: 9 additions & 9 deletions pkg/xds/sync/dataplane_watchdog.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func (d *DataplaneWatchdog) Cleanup() error {
func (d *DataplaneWatchdog) syncDataplane(ctx context.Context, metadata *core_xds.DataplaneMetadata) (SyncResult, error) {
meshCtx, err := d.MeshCache.GetMeshContext(ctx, d.key.Mesh)
if err != nil {
return SyncResult{}, err
return SyncResult{}, errors.Wrap(err, "could not get mesh context")
}

certInfo := d.EnvoyCpCtx.Secrets.Info(d.key)
Expand All @@ -133,7 +133,7 @@ func (d *DataplaneWatchdog) syncDataplane(ctx context.Context, metadata *core_xd
}
proxy, err := d.DataplaneProxyBuilder.Build(ctx, d.key, meshCtx)
if err != nil {
return SyncResult{}, err
return SyncResult{}, errors.Wrap(err, "could not build dataplane proxy")
}
networking := proxy.Dataplane.Spec.Networking
envoyAdminMTLS, err := d.getEnvoyAdminMTLS(ctx, networking.Address, networking.AdvertisedAddress)
Expand All @@ -147,7 +147,7 @@ func (d *DataplaneWatchdog) syncDataplane(ctx context.Context, metadata *core_xd
proxy.Metadata = metadata
changed, err := d.DataplaneReconciler.Reconcile(ctx, *envoyCtx, proxy)
if err != nil {
return SyncResult{}, err
return SyncResult{}, errors.Wrap(err, "could not reconcile")
}
d.lastHash = meshCtx.Hash

Expand All @@ -169,7 +169,7 @@ func (d *DataplaneWatchdog) syncIngress(ctx context.Context, metadata *core_xds.

aggregatedMeshCtxs, err := xds_context.AggregateMeshContexts(ctx, d.ResManager, d.MeshCache.GetMeshContext)
if err != nil {
return SyncResult{}, err
return SyncResult{}, errors.Wrap(err, "could not aggregate mesh contexts")
}

result := SyncResult{
Expand All @@ -186,7 +186,7 @@ func (d *DataplaneWatchdog) syncIngress(ctx context.Context, metadata *core_xds.

proxy, err := d.IngressProxyBuilder.Build(ctx, d.key, aggregatedMeshCtxs)
if err != nil {
return SyncResult{}, err
return SyncResult{}, errors.Wrap(err, "could not build ingress proxy")
}
networking := proxy.ZoneIngressProxy.ZoneIngressResource.Spec.GetNetworking()
envoyAdminMTLS, err := d.getEnvoyAdminMTLS(ctx, networking.GetAddress(), networking.GetAdvertisedAddress())
Expand All @@ -197,7 +197,7 @@ func (d *DataplaneWatchdog) syncIngress(ctx context.Context, metadata *core_xds.
proxy.Metadata = metadata
changed, err := d.IngressReconciler.Reconcile(ctx, *envoyCtx, proxy)
if err != nil {
return SyncResult{}, err
return SyncResult{}, errors.Wrap(err, "could not reconcile")
}
if changed {
result.Status = ChangedStatus
Expand All @@ -217,7 +217,7 @@ func (d *DataplaneWatchdog) syncEgress(ctx context.Context, metadata *core_xds.D

aggregatedMeshCtxs, err := xds_context.AggregateMeshContexts(ctx, d.ResManager, d.MeshCache.GetMeshContext)
if err != nil {
return SyncResult{}, err
return SyncResult{}, errors.Wrap(err, "could not aggregate mesh contexts")
}

result := SyncResult{
Expand All @@ -234,7 +234,7 @@ func (d *DataplaneWatchdog) syncEgress(ctx context.Context, metadata *core_xds.D

proxy, err := d.EgressProxyBuilder.Build(ctx, d.key, aggregatedMeshCtxs)
if err != nil {
return SyncResult{}, err
return SyncResult{}, errors.Wrap(err, "could not build egress proxy")
}
networking := proxy.ZoneEgressProxy.ZoneEgressResource.Spec.Networking
envoyAdminMTLS, err := d.getEnvoyAdminMTLS(ctx, networking.Address, "")
Expand All @@ -245,7 +245,7 @@ func (d *DataplaneWatchdog) syncEgress(ctx context.Context, metadata *core_xds.D
proxy.Metadata = metadata
changed, err := d.EgressReconciler.Reconcile(ctx, *envoyCtx, proxy)
if err != nil {
return SyncResult{}, err
return SyncResult{}, errors.Wrap(err, "could not reconcile")
}
if changed {
result.Status = ChangedStatus
Expand Down

0 comments on commit 152408b

Please sign in to comment.