This is an automated email from the ASF dual-hosted git repository.
zrhoffman 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 3f94fdc375 Fixed t3c for layered profile order issue (#7425)
3f94fdc375 is described below
commit 3f94fdc375b6a2f57f9b965c442fad693bf09886
Author: Joe Pappano <[email protected]>
AuthorDate: Fri Mar 31 16:07:29 2023 -0400
Fixed t3c for layered profile order issue (#7425)
* Fixed issue with layered profile ordering
* fixed test for layered profiles to work with reverse iteration
* fixed formatting issues and added changelog entry
* Updated test to work with reverse iteration
---
CHANGELOG.md | 1 +
lib/go-atscfg/atscfg.go | 10 ++++----
lib/go-atscfg/atscfg_test.go | 43 +++++++++++++++++++----------------
lib/go-atscfg/parentdotconfig_test.go | 4 ++--
4 files changed, 31 insertions(+), 27 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 22f3ec2a70..e02c85e7e5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -95,6 +95,7 @@ The format is based on [Keep a
Changelog](http://keepachangelog.com/en/1.0.0/).
- [#6775](https://github.com/apache/trafficcontrol/issues/6775) *Traffic Ops*
Invalid "orgServerFqdn" in Delivery Service creation/update causes Internal
Server Error
- [#6695](https://github.com/apache/trafficcontrol/issues/6695) *Traffic
Control Cache Config (t3c)* Directory creation was erroneously reporting an
error when actually succeeding.
- [#7411](https://github.com/apache/trafficcontrol/pull/7411) *Traffic Control
Cache Config (t3c)* Fixed issue with wrong parent ordering with MSO
non-topology delivery services.
+- [#7425](https://github.com/apache/trafficcontrol/pull/7425) *Traffic Control
Cache Config (t3c)* Fixed issue with layered profile iteration being done in
the wrong order.
## [7.0.0] - 2022-07-19
### Added
diff --git a/lib/go-atscfg/atscfg.go b/lib/go-atscfg/atscfg.go
index 91dbdde4cf..457b662455 100644
--- a/lib/go-atscfg/atscfg.go
+++ b/lib/go-atscfg/atscfg.go
@@ -848,13 +848,13 @@ func layerProfilesFromMap(profileNames []string, params
[]parameterWithProfilesM
}
layeredParamMap := map[ParamKey]tc.Parameter{}
-
- for _, profileName := range profileNames {
- profileParams := allProfileParams[profileName]
+ // profileNames is ordered, we need to iterate through this backwards
+ // because we need subsequent params on other profiles to override
previous ones,
+ // this will provide the proper "layering" that we want.
+ for i := len(profileNames) - 1; i >= 0; i-- {
+ profileParams := allProfileParams[profileNames[i]]
for _, param := range profileParams {
paramkey := getParamKey(param)
- // because profileNames is ordered, this will cause
subsequent params
- // on other profiles to override previous ones,
"layering" like we want.
layeredParamMap[paramkey] = param
}
}
diff --git a/lib/go-atscfg/atscfg_test.go b/lib/go-atscfg/atscfg_test.go
index db0f6bb523..4075b3f41d 100644
--- a/lib/go-atscfg/atscfg_test.go
+++ b/lib/go-atscfg/atscfg_test.go
@@ -185,18 +185,21 @@ func TestLayerProfiles(t *testing.T) {
}
// per the params, on the layered profiles "FOO,BAR,BAZ" (in that
order):
- // (1) beta in BAR should override alpha FOO,
- // because they share the ConfigFile+Name key and BAR is later in
the layering
- // (2) gamma should be added, but not override
+ // we iterate through params in reverse order because FOO will have the
highest priority
+ // then BAR then BAZ, any duplicate params that exist with the same
ConfigFile+Name
+ // in subsequent profiles can be overwritten by the next higher profile
+ // (1) alpha in FOO, should override beta in BAR
+ // because they share the ConfigFile+Name key and FOO is later in
the layering
+ // (2) gamma should be added, but not overridden
// because the key is ConfigFile+Name, which isn't on any previous
profile
- // (3) epsilon in BAZ should override delta in BAR
- // because they share the ConfigFile+Name key and BAZ is later in
the layering
+ // (3) delta in BAR should override epsilon in BAZ
+ // because they share the ConfigFile+Name key and BAR is later in
the layering
// - this tests the parameters being in a different order than (1)
// (4) zeta should be added, but not override
// because the key is ConfigFile+Name, which isn't on any previous
profile
// - this tests the name matching but not config file, reverse of
(2).
- // (5) theta in BAZ should override eta and iota in FOO and BAR
- // because they share the ConfigFile+Name key and BAZ is last in the
layering
+ // (5) iota in FOO should override theta in BAZ and eta and BAR
+ // because they share the ConfigFile+Name key and FOO is last in the
layering
// - this tests multiple overrides
layeredParams, err := LayerProfiles(profileNames, allParams)
@@ -209,32 +212,32 @@ func TestLayerProfiles(t *testing.T) {
vals[param.Value] = struct{}{}
}
- if _, ok := vals["alpha"]; ok {
- t.Errorf("expected: param 'beta' to override 'alpha', actual:
alpha in layered parameters")
+ if _, ok := vals["beta"]; ok {
+ t.Errorf("expected: param 'alpha' to override 'beta', actual:
beta in layered parameters")
}
- if _, ok := vals["beta"]; !ok {
- t.Errorf("expected: param 'beta' to override 'alpha', actual:
beta not in layered parameters")
+ if _, ok := vals["alpha"]; !ok {
+ t.Errorf("expected: param 'alpha' to override 'beta', actual:
alpha not in layered parameters")
}
if _, ok := vals["gamma"]; !ok {
t.Errorf("expected: param 'gamma' with no ConfigFile+Name in
another profile to be in layered parameters, actual: gamma not in layered
parameters")
}
- if _, ok := vals["delta"]; ok {
- t.Errorf("expected: param 'epsilon' to override 'delta' in
prior profile, actual: delta in layered parameters")
+ if _, ok := vals["epsilon"]; ok {
+ t.Errorf("expected: param 'delta' to override 'epsilon' in
prior profile, actual: epsilon in layered parameters")
}
- if _, ok := vals["epsilon"]; !ok {
- t.Errorf("expected: param 'epsilon' to override 'delta' in
prior profile, actual: epsilon not in layered parameters")
+ if _, ok := vals["delta"]; !ok {
+ t.Errorf("expected: param 'delta' to override 'epsilon' in
prior profile, actual: delta not in layered parameters")
}
if _, ok := vals["zeta"]; !ok {
t.Errorf("expected: param 'zeta' with no ConfigFile+Name in
another profile to be in layered parameters, actual: zeta not in layered
parameters")
}
- if _, ok := vals["theta"]; !ok {
- t.Errorf("expected: param 'theta' to override 'eta' and 'iota'
in prior profile, actual: theta not in layered parameters")
+ if _, ok := vals["iota"]; !ok {
+ t.Errorf("expected: param 'iota' to override 'eta' and 'theta'
in prior profile, actual: iota not in layered parameters")
}
if _, ok := vals["eta"]; ok {
- t.Errorf("expected: param 'theta' to override 'eta' and 'iota'
in prior profile, actual: eta in layered parameters")
+ t.Errorf("expected: param 'iota' to override 'eta' and 'theta'
in prior profile, actual: eta in layered parameters")
}
- if _, ok := vals["iota"]; ok {
- t.Errorf("expected: param 'theta' to override 'eta' and 'iota'
in prior profile, actual: iota in layered parameters")
+ if _, ok := vals["theta"]; ok {
+ t.Errorf("expected: param 'iota' to override 'eta' and 'theta'
in prior profile, actual: theta in layered parameters")
}
}
diff --git a/lib/go-atscfg/parentdotconfig_test.go
b/lib/go-atscfg/parentdotconfig_test.go
index a37a9c885b..6dc100dbd9 100644
--- a/lib/go-atscfg/parentdotconfig_test.go
+++ b/lib/go-atscfg/parentdotconfig_test.go
@@ -3734,13 +3734,13 @@ func
TestMakeParentDotConfigTopologiesServerMultipleProfileParams(t *testing.T)
Name: ParentConfigCacheParamWeight,
ConfigFile: "parent.config",
Value: "100",
- Profiles: []byte(`["serverprofile0"]`),
+ Profiles: []byte(`["serverprofile1"]`),
},
tc.Parameter{
Name: ParentConfigCacheParamWeight,
ConfigFile: "parent.config",
Value: "200",
- Profiles: []byte(`["serverprofile1"]`),
+ Profiles: []byte(`["serverprofile0"]`),
},
}
