Minor Non-Functional Updates (#7215)

* Cleanup the discovery chain compilation route handling

Nothing functionally should be different here. The real difference is that when creating new targets or handling route destinations we use the router config entries name and namespace instead of that of the top level request. Today they SHOULD always be the same but that may not always be the case. This hopefully also makes it easier to understand how the router entries are handled.

* Refactor a small bit of the service manager tests in oss

We used to use the stringHash function to compute part of the filename where things would get persisted to. This has been changed in the core code to calling the StringHash method on the ServiceID type. It just so happens that the new method will output the same value for anything in the default namespace (by design actually). However, logically this filename computation in the test should do the same thing as the core code itself so I updated it here.

Also of note is that newer enterprise-only tests for the service manager cannot use the old stringHash function at all because it will produce incorrect results for non-default namespaces.
This commit is contained in:
Matt Keeler 2020-02-05 10:06:11 -05:00 committed by GitHub
parent cb77fc6d01
commit 228da48f5d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 7 additions and 5 deletions

View File

@ -554,7 +554,7 @@ func (c *compiler) assembleChain() error {
dest := route.Destination
svc := defaultIfEmpty(dest.Service, c.serviceName)
destNamespace := defaultIfEmpty(dest.Namespace, c.evaluateInNamespace)
destNamespace := defaultIfEmpty(dest.Namespace, router.NamespaceOrDefault())
// Check to see if the destination is eligible for splitting.
var (
@ -579,13 +579,13 @@ func (c *compiler) assembleChain() error {
// If we have a router, we'll add a catch-all route at the end to send
// unmatched traffic to the next hop in the chain.
defaultDestinationNode, err := c.getSplitterOrResolverNode(c.newTarget(c.serviceName, "", "", ""))
defaultDestinationNode, err := c.getSplitterOrResolverNode(c.newTarget(router.Name, "", router.NamespaceOrDefault(), ""))
if err != nil {
return err
}
defaultRoute := &structs.DiscoveryRoute{
Definition: newDefaultServiceRoute(c.serviceName, c.evaluateInNamespace),
Definition: newDefaultServiceRoute(router.Name, router.NamespaceOrDefault()),
NextNode: defaultDestinationNode.MapKey(),
}
routeNode.Routes = append(routeNode.Routes, defaultRoute)

View File

@ -310,8 +310,10 @@ func TestServiceManager_PersistService_API(t *testing.T) {
EnterpriseMeta: *structs.DefaultEnterpriseMeta(),
}
svcFile := filepath.Join(a.Config.DataDir, servicesDir, stringHash(svc.ID))
configFile := filepath.Join(a.Config.DataDir, serviceConfigDir, stringHash(svc.ID))
svcID := svc.CompoundServiceID()
svcFile := filepath.Join(a.Config.DataDir, servicesDir, svcID.StringHash())
configFile := filepath.Join(a.Config.DataDir, serviceConfigDir, svcID.StringHash())
// Service is not persisted unless requested, but we always persist service configs.
require.NoError(a.AddService(svc, nil, false, "", ConfigSourceRemote))