This is an automated email from the ASF dual-hosted git repository.
rawlin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
The following commit(s) were added to refs/heads/master by this push:
new bace6a5 Fix t3c parent.config abstraction bugs (#6615)
bace6a5 is described below
commit bace6a5fa581c856a680cdaf84ed4a3e35c6f0da
Author: Robert O Butts <[email protected]>
AuthorDate: Fri Mar 4 15:11:38 2022 -0700
Fix t3c parent.config abstraction bugs (#6615)
Fixes several issues with the recent parent.config strategies
abstraction, specifically around MSO parents not getting the right
go_direct, parent_is_proxy, and qstring directives.
---
lib/go-atscfg/parentdotconfig.go | 63 ++++++++++++++-------
lib/go-atscfg/parentdotconfig_test.go | 102 ++++++++++++++++++++++++++--------
2 files changed, 120 insertions(+), 45 deletions(-)
diff --git a/lib/go-atscfg/parentdotconfig.go b/lib/go-atscfg/parentdotconfig.go
index fbd3d5d..097ea90 100644
--- a/lib/go-atscfg/parentdotconfig.go
+++ b/lib/go-atscfg/parentdotconfig.go
@@ -187,11 +187,16 @@ func makeParentDotConfigData(
profileParentConfigParams, parentWarns :=
getProfileParentConfigParams(tcParentConfigParams)
warnings = append(warnings, parentWarns...)
- // profileParentConfigParams, err :=
tcParamsToParamsWithProfiles(tcParentConfigParams)
- // if err != nil {
- // return nil, warnings, errors.New("adding profiles to parent
config params: " + err.Error())
- // }
+ parentConfigParamsWithProfiles, err :=
tcParamsToParamsWithProfiles(tcParentConfigParams)
+ if err != nil {
+ return nil, warnings, errors.New("adding profiles to parent
config params: " + err.Error())
+ }
+
+ // parentConfigParams are the parent.config params for all profiles
(needed for parents)
+ parentConfigParams :=
parameterWithProfilesToMap(parentConfigParamsWithProfiles)
+
+ // serverParams are the parent.config params for this particular server
serverParams := getServerParentConfigParams(server,
profileParentConfigParams)
parentCacheGroups := map[string]struct{}{}
@@ -333,6 +338,7 @@ func makeParentDotConfigData(
servers,
&ds,
serverParams,
+ parentConfigParams,
nameTopologies,
serverCapabilities,
dsRequiredCapabilities,
@@ -440,7 +446,7 @@ func makeParentDotConfigData(
textLine.SecondaryMode = secondaryMode
textLine.RetryPolicy = dsParams.Algorithm //
TODO convert
textLine.IgnoreQueryStringInParentSelection =
!parentQStr
- textLine.GoDirect = false
+ textLine.GoDirect = true
// textLine += parents + secondaryParents + `
round_robin=` + dsParams.Algorithm + ` qstring=` + parentQStr + `
go_direct=false parent_is_proxy=false`
prWarns := []string{}
@@ -911,6 +917,7 @@ func getTopologyParentConfigLine(
servers []Server,
ds *DeliveryService,
serverParams map[string]string,
+ parentConfigParams []parameterWithProfilesMap, // all params with
configFile parent.config
nameTopologies map[TopologyName]tc.Topology,
serverCapabilities map[int]map[ServerCapability]struct{},
dsRequiredCapabilities map[int]map[ServerCapability]struct{},
@@ -960,7 +967,7 @@ func getTopologyParentConfigLine(
}
// txt += "dest_domain=" + orgURI.Hostname() + " port=" + orgURI.Port()
- parents, secondaryParents, parentWarnings, err :=
getTopologyParents(server, ds, servers, serverParams, topology,
serverPlacement.IsLastTier, serverCapabilities, dsRequiredCapabilities,
dsOrigins, dsParams.MergeGroups)
+ parents, secondaryParents, parentWarnings, err :=
getTopologyParents(server, ds, servers, parentConfigParams, topology,
serverPlacement.IsLastTier, serverCapabilities, dsRequiredCapabilities,
dsOrigins, dsParams.MergeGroups)
warnings = append(warnings, parentWarnings...)
if err != nil {
return nil, warnings, errors.New("getting topology parents for
'" + *ds.XMLID + "': skipping! " + err.Error())
@@ -984,7 +991,7 @@ func getTopologyParentConfigLine(
txt.RetryPolicy = getTopologyRoundRobin(ds, serverParams,
serverPlacement.IsLastCacheTier, dsParams.Algorithm)
// txt += ` round_robin=` + getTopologyRoundRobin(ds, serverParams,
serverPlacement.IsLastCacheTier, dsParams.Algorithm)
- txt.GoDirect = getTopologyGoDirect(ds, serverPlacement.IsLastTier)
+ txt.GoDirect = getTopologyGoDirect(ds, serverPlacement.IsLastTier,
serverPlacement.IsLastCacheTier)
// txt += ` go_direct=` + getTopologyGoDirect(ds,
serverPlacement.IsLastTier)
// TODO convert
@@ -1001,6 +1008,7 @@ func getTopologyParentConfigLine(
tqWarns := []string{}
txt.IgnoreQueryStringInParentSelection, tqWarns =
getTopologyQueryStringIgnore(ds, serverParams, serverPlacement.IsLastCacheTier,
dsParams.Algorithm, useQueryStringInParentSelection)
warnings = append(warnings, tqWarns...)
+
// txt += ` qstring=` + getTopologyQueryString(ds, serverParams,
serverPlacement.IsLastCacheTier, dsParams.Algorithm,
dsParams.QueryStringHandling)
// TODO ensure value is always !goDirect, and determine what to do if
not
@@ -1148,7 +1156,10 @@ func getTopologyRoundRobin(
return ParentAbstractionServiceRetryPolicyConsistentHash
}
-func getTopologyGoDirect(ds *DeliveryService, serverIsLastTier bool) bool {
+func getTopologyGoDirect(ds *DeliveryService, serverIsLastTier bool,
serverIsLastCacheTier bool) bool {
+ if serverIsLastCacheTier {
+ return true
+ }
if !serverIsLastTier {
return false
}
@@ -1171,14 +1182,19 @@ func getTopologyQueryStringIgnore(
warnings := []string{}
if serverIsLastTier {
if ds.MultiSiteOrigin != nil && *ds.MultiSiteOrigin &&
qStringHandling == nil && algorithm ==
ParentAbstractionServiceRetryPolicyConsistentHash && ds.QStringIgnore != nil &&
tc.QStringIgnore(*ds.QStringIgnore) == tc.QStringIgnoreUseInCacheKeyAndPassUp {
- return true, warnings
+ return false, warnings
}
- return false, warnings
+
+ if qStringHandling != nil {
+ return !(*qStringHandling), warnings
+ }
+
+ return true, warnings
}
if param := serverParams[ParentConfigParamQStringHandling]; param != ""
{
if useQStr := ParentSelectParamQStringHandlingToBool(param);
useQStr != nil {
- return *useQStr, warnings
+ return !(*useQStr), warnings
} else if param != "" {
warnings = append(warnings, "Server param
'"+ParentConfigParamQStringHandling+"' value '"+param+"' malformed, not using!")
}
@@ -1195,36 +1211,40 @@ func getTopologyQueryStringIgnore(
// serverParentageParams gets the Parameters used for parent= line, or
defaults if they don't exist
// Returns the Parameters used for parent= lines for the given server, and any
warnings.
-func serverParentageParams(sv *Server, serverParams map[string]string)
(profileCache, []string) {
+func serverParentageParams(sv *Server, params []parameterWithProfilesMap)
(profileCache, []string) {
warnings := []string{}
// TODO deduplicate with atstccfg/parentdotconfig.go
profileCache := defaultProfileCache()
if sv.TCPPort != nil {
profileCache.Port = *sv.TCPPort
}
- for paramName, paramVal := range serverParams {
- switch paramName {
+ for _, param := range params {
+ if _, ok := param.ProfileNames[*sv.Profile]; !ok {
+ continue
+ }
+ switch param.Name {
case ParentConfigCacheParamWeight:
- profileCache.Weight = paramVal
+ profileCache.Weight = param.Value
case ParentConfigCacheParamPort:
- if i, err := strconv.Atoi(paramVal); err != nil {
+ if i, err := strconv.Atoi(param.Value); err != nil {
warnings = append(warnings, "port param is not
an integer, skipping! : "+err.Error())
} else {
profileCache.Port = i
}
case ParentConfigCacheParamUseIP:
- profileCache.UseIP = paramVal == "1"
+ profileCache.UseIP = param.Value == "1"
case ParentConfigCacheParamRank:
- if i, err := strconv.Atoi(paramVal); err != nil {
+ if i, err := strconv.Atoi(param.Value); err != nil {
warnings = append(warnings, "rank param is not
an integer, skipping! : "+err.Error())
} else {
profileCache.Rank = i
}
case ParentConfigCacheParamNotAParent:
- profileCache.NotAParent = paramVal != "false"
+ profileCache.NotAParent = param.Value != "false"
}
}
+
return profileCache, warnings
}
@@ -1262,7 +1282,7 @@ func getTopologyParents(
server *Server,
ds *DeliveryService,
servers []Server,
- serverParams map[string]string, // all params with configFile
parent.confign
+ parentConfigParams []parameterWithProfilesMap, // all params with
configFile parent.config
topology tc.Topology,
serverIsLastTier bool,
serverCapabilities map[int]map[ServerCapability]struct{},
@@ -1290,6 +1310,7 @@ func getTopologyParents(
Port: orgPort,
Weight: DefaultParentWeight,
}
+
return []*ParentAbstractionServiceParent{parent}, nil,
warnings, nil
}
@@ -1332,7 +1353,7 @@ func getTopologyParents(
serversWithParams := []serverWithParams{}
for _, sv := range servers {
- serverParentParams, parentWarns := serverParentageParams(&sv,
serverParams)
+ serverParentParams, parentWarns := serverParentageParams(&sv,
parentConfigParams)
warnings = append(warnings, parentWarns...)
serversWithParams = append(serversWithParams, serverWithParams{
Server: sv,
diff --git a/lib/go-atscfg/parentdotconfig_test.go
b/lib/go-atscfg/parentdotconfig_test.go
index 8df15e0..b53e481 100644
--- a/lib/go-atscfg/parentdotconfig_test.go
+++ b/lib/go-atscfg/parentdotconfig_test.go
@@ -1132,35 +1132,24 @@ func TestMakeParentDotConfigTopologiesMSO(t *testing.T)
{
ds1.OrgServerFQDN = util.StrPtr("http://ds1.example.net";)
ds1.Topology = util.StrPtr("t0")
ds1.MultiSiteOrigin = util.BoolPtr(true)
+ ds1.ProfileName = util.StrPtr("dsprofile")
dses := []DeliveryService{*ds1}
parentConfigParams := []tc.Parameter{
tc.Parameter{
- Name: ParentConfigParamQStringHandling,
- ConfigFile: "parent.config",
- Value: "myQStringHandlingParam",
- Profiles: []byte(`["serverprofile"]`),
- },
- tc.Parameter{
Name: ParentConfigParamAlgorithm,
ConfigFile: "parent.config",
Value: tc.AlgorithmConsistentHash,
Profiles: []byte(`["serverprofile"]`),
},
- tc.Parameter{
- Name: ParentConfigParamQString,
- ConfigFile: "parent.config",
- Value: "myQstringParam",
- Profiles: []byte(`["serverprofile"]`),
- },
}
serverParams := []tc.Parameter{
tc.Parameter{
Name: "trafficserver",
ConfigFile: "package",
- Value: "7",
+ Value: "9",
Profiles: []byte(`["global"]`),
},
}
@@ -1251,6 +1240,71 @@ func TestMakeParentDotConfigTopologiesMSO(t *testing.T) {
if strings.Contains(txt, "myorigin1") {
t.Errorf("expected no origin1 without DeliveryServiceServer
assigned to this DS, actual: '%v'", txt)
}
+
+ if !strings.Contains(txt, "go_direct=true") {
+ t.Errorf("expected MSO Topologies to Origin to go_direct=true,
actual: '%v'", txt)
+ }
+
+ if !strings.Contains(txt, "parent_is_proxy=false") {
+ t.Errorf("expected MSO Topologies to Origin to
parent_is_proxy=false, actual: '%v'", txt)
+ }
+
+ t.Run("MSO topologoies default qstring=ignore", func(t *testing.T) {
+ cfg, err := MakeParentDotConfig(dses, server, servers,
topologies, serverParams, parentConfigParams, serverCapabilities,
dsRequiredCapabilities, cgs, dss, cdn, hdr)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !strings.Contains(cfg.Text, "qstring=ignore") {
+ t.Errorf("expected MSO Topologies to Origin to default
to qstring=ignore, actual: '%v'", cfg.Text)
+ }
+ })
+
+ t.Run("MSO topologoies param qstring=ignore", func(t *testing.T) {
+ parentConfigParamsWithQstr := append(parentConfigParams,
tc.Parameter{
+ Name: ParentConfigParamQString,
+ ConfigFile: "parent.config",
+ Value: "ignore",
+ Profiles: []byte(`["serverprofile"]`),
+ })
+
+ cfg, err := MakeParentDotConfig(dses, server, servers,
topologies, serverParams, parentConfigParamsWithQstr, serverCapabilities,
dsRequiredCapabilities, cgs, dss, cdn, hdr)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !strings.Contains(cfg.Text, "qstring=ignore") {
+ t.Errorf("expected MSO Topologies to Origin to default
to qstring=ignore, actual: '%v'", cfg.Text)
+ }
+ })
+
+ t.Run("MSO topologoies param qstring=consider", func(t *testing.T) {
+ parentConfigParamsWithQstr := append(parentConfigParams,
tc.Parameter{
+ Name: ParentConfigParamQStringHandling,
+ ConfigFile: "parent.config",
+ Value: "consider",
+ Profiles: []byte(`["` + *ds1.ProfileName + `"]`),
+ })
+
+ cfg, err := MakeParentDotConfig(dses, server, servers,
topologies, serverParams, parentConfigParamsWithQstr, serverCapabilities,
dsRequiredCapabilities, cgs, dss, cdn, hdr)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !strings.Contains(cfg.Text, "qstring=consider") {
+ t.Errorf("expected MSO Topologies to Origin with param
to qstring=consider, actual: '''%v''' warnings '''%+v'''", cfg.Text,
cfg.Warnings)
+ }
+ })
+
+ t.Run("MSO topologoies param ds qstring consider", func(t *testing.T) {
+ ds1.QStringIgnore =
util.IntPtr(int(tc.QStringIgnoreUseInCacheKeyAndPassUp))
+ dses := []DeliveryService{*ds1}
+
+ cfg, err := MakeParentDotConfig(dses, server, servers,
topologies, serverParams, parentConfigParams, serverCapabilities,
dsRequiredCapabilities, cgs, dss, cdn, hdr)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !strings.Contains(cfg.Text, "qstring=consider") {
+ t.Errorf("expected MSO Topologies to Origin with param
to qstring=consider, actual: '''%v''' warnings '''%+v'''", cfg.Text,
cfg.Warnings)
+ }
+ })
}
func TestMakeParentDotConfigTopologiesMSOWithCapabilities(t *testing.T) {
@@ -2878,17 +2932,6 @@ func TestMakeParentDotConfigHTTPSOriginTopology(t
*testing.T) {
}
}
-// warnsContains returns whether the given warnings has str as a substring of
any warning.
-// Note this is different than lib/go-util.ContainsStr, which only returns if
the array has the exact value as one of its values.
-func warningsContains(warnings []string, str string) bool {
- for _, warn := range warnings {
- if strings.Contains(warn, str) {
- return true
- }
- }
- return false
-}
-
func TestMakeParentDotConfigMergeParentGroupTopology(t *testing.T) {
hdr := &ParentConfigOpts{AddComments: true, HdrComment:
"myHeaderComment"}
@@ -3050,6 +3093,17 @@ func TestMakeParentDotConfigMergeParentGroupTopology(t
*testing.T) {
}
}
+// warningsContains returns whether the given warnings has str as a substring
of any warning.
+// Note this is different than lib/go-util.ContainsStr, which only returns if
the array has the exact value as one of its values.
+func warningsContains(warnings []string, str string) bool {
+ for _, warn := range warnings {
+ if strings.Contains(warn, str) {
+ return true
+ }
+ }
+ return false
+}
+
func makeTestParentServer() *Server {
server := &Server{}
server.ProfileID = util.IntPtr(42)
