[1.16.x] Fix topology view when displaying mixed connect-native/normal services. (#18330)

Fix topology view when displaying mixed connect-native/normal services. (#13023)

* Fix topoloy intention with mixed connect-native/normal services.

If a service is registered twice, once with connect-native and once
without, the topology views would prune the existing intentions. This
change brings the code more in line with the transparent proxy behavior.

* Dedupe nodes in the ServiceTopology ui endpoint (like done with tags).

* Consider a service connect-native as soon as one instance is.

Co-authored-by: Florian Apolloner <florian@apolloner.eu>
This commit is contained in:
Chris S. Kim 2023-07-31 10:00:40 -04:00 committed by GitHub
parent 5429e56d1d
commit 8a3a96d075
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 100 additions and 11 deletions

3
.changelog/13023.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:improvement
ui: the topology view now properly displays services with mixed connect and non-connect instances.
```

View File

@ -4535,13 +4535,17 @@ func (s *Store) ServiceTopology(
maxIdx = idx
}
// Store downstreams with at least one instance in transparent proxy mode.
// Store downstreams with at least one instance in transparent proxy or connect native mode.
// This is to avoid returning downstreams from intentions when none of the downstreams are transparent proxies.
tproxyMap := make(map[structs.ServiceName]struct{})
proxyMap := make(map[structs.ServiceName]struct{})
for _, downstream := range unfilteredDownstreams {
if downstream.Service.Proxy.Mode == structs.ProxyModeTransparent {
sn := structs.NewServiceName(downstream.Service.Proxy.DestinationServiceName, &downstream.Service.EnterpriseMeta)
tproxyMap[sn] = struct{}{}
proxyMap[sn] = struct{}{}
}
if downstream.Service.Connect.Native {
sn := downstream.Service.CompoundServiceName()
proxyMap[sn] = struct{}{}
}
}
@ -4551,7 +4555,7 @@ func (s *Store) ServiceTopology(
if downstream.Service.Kind == structs.ServiceKindConnectProxy {
sn = structs.NewServiceName(downstream.Service.Proxy.DestinationServiceName, &downstream.Service.EnterpriseMeta)
}
if _, ok := tproxyMap[sn]; !ok && !downstream.Service.Connect.Native && downstreamSources[sn.String()] != structs.TopologySourceRegistration {
if _, ok := proxyMap[sn]; !ok && downstreamSources[sn.String()] != structs.TopologySourceRegistration {
// If downstream is not a transparent proxy or connect native, remove references
delete(downstreamSources, sn.String())
delete(downstreamDecisions, sn.String())
@ -4580,6 +4584,7 @@ func (s *Store) combinedServiceNodesTxn(tx ReadTxn, ws memdb.WatchSet, names []s
maxIdx uint64
resp structs.CheckServiceNodes
)
dedupMap := make(map[string]structs.CheckServiceNode)
for _, u := range names {
// Collect typical then connect instances
idx, csn, err := checkServiceNodesTxn(tx, ws, u.Name, false, &u.EnterpriseMeta, peerName)
@ -4589,7 +4594,9 @@ func (s *Store) combinedServiceNodesTxn(tx ReadTxn, ws memdb.WatchSet, names []s
if idx > maxIdx {
maxIdx = idx
}
resp = append(resp, csn...)
for _, item := range csn {
dedupMap[item.Node.Node+"/"+item.Service.ID] = item
}
idx, csn, err = checkServiceNodesTxn(tx, ws, u.Name, true, &u.EnterpriseMeta, peerName)
if err != nil {
@ -4598,7 +4605,12 @@ func (s *Store) combinedServiceNodesTxn(tx ReadTxn, ws memdb.WatchSet, names []s
if idx > maxIdx {
maxIdx = idx
}
resp = append(resp, csn...)
for _, item := range csn {
dedupMap[item.Node.Node+"/"+item.Service.ID] = item
}
}
for _, item := range dedupMap {
resp = append(resp, item)
}
return maxIdx, resp, nil
}

View File

@ -564,11 +564,25 @@ func summarizeServices(dump structs.ServiceDump, cfg *config.RuntimeConfig, dc s
sum := getService(psn)
svc := csn.Service
sum.Nodes = append(sum.Nodes, csn.Node.Node)
found := false
for _, existing := range sum.Nodes {
if existing == csn.Node.Node {
found = true
break
}
}
if !found {
sum.Nodes = append(sum.Nodes, csn.Node.Node)
}
sum.Kind = svc.Kind
sum.Datacenter = csn.Node.Datacenter
sum.InstanceCount += 1
sum.ConnectNative = svc.Connect.Native
// Consider a service connect native once at least one instance is
if svc.Connect.Native {
sum.ConnectNative = svc.Connect.Native
}
if svc.Kind == structs.ServiceKindConnectProxy {
sn := structs.NewServiceName(svc.Proxy.DestinationServiceName, &svc.EnterpriseMeta)
psn := structs.PeeredServiceName{Peer: peerName, ServiceName: sn}

View File

@ -1687,19 +1687,43 @@ func TestUIServiceTopology(t *testing.T) {
SkipNodeUpdate: true,
Service: &structs.NodeService{
Kind: structs.ServiceKindTypical,
ID: "cproxy",
ID: "cproxy-https",
Service: "cproxy",
Port: 1111,
Address: "198.18.1.70",
Tags: []string{"https"},
Connect: structs.ServiceConnect{Native: true},
},
Checks: structs.HealthChecks{
&structs.HealthCheck{
Node: "cnative",
CheckID: "cnative:cproxy",
CheckID: "cnative:cproxy-https",
Name: "cproxy-liveness",
Status: api.HealthPassing,
ServiceID: "cproxy",
ServiceID: "cproxy-https",
ServiceName: "cproxy",
},
},
},
"Service cproxy/http on cnative": {
Datacenter: "dc1",
Node: "cnative",
SkipNodeUpdate: true,
Service: &structs.NodeService{
Kind: structs.ServiceKindTypical,
ID: "cproxy-http",
Service: "cproxy",
Port: 1112,
Address: "198.18.1.70",
Tags: []string{"http"},
},
Checks: structs.HealthChecks{
&structs.HealthCheck{
Node: "cnative",
CheckID: "cnative:cproxy-http",
Name: "cproxy-liveness",
Status: api.HealthPassing,
ServiceID: "cproxy-http",
ServiceName: "cproxy",
},
},
@ -2125,6 +2149,42 @@ func TestUIServiceTopology(t *testing.T) {
FilteredByACLs: false,
},
},
{
name: "cbackend",
httpReq: func() *http.Request {
req, _ := http.NewRequest("GET", "/v1/internal/ui/service-topology/cbackend?kind=", nil)
return req
}(),
want: &ServiceTopology{
Protocol: "http",
TransparentProxy: false,
Upstreams: []*ServiceTopologySummary{},
Downstreams: []*ServiceTopologySummary{
{
ServiceSummary: ServiceSummary{
Name: "cproxy",
Datacenter: "dc1",
Tags: []string{"http", "https"},
Nodes: []string{"cnative"},
InstanceCount: 2,
ChecksPassing: 3,
ChecksWarning: 0,
ChecksCritical: 0,
ConnectNative: true,
EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(),
},
Intention: structs.IntentionDecisionSummary{
DefaultAllow: true,
Allowed: true,
HasPermissions: false,
HasExact: true,
},
Source: structs.TopologySourceSpecificIntention,
},
},
FilteredByACLs: false,
},
},
}
for _, tc := range tcs {