Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed t3c for layered profile order issue #7425

Merged
merged 4 commits into from
Mar 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions lib/go-atscfg/atscfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
43 changes: 23 additions & 20 deletions lib/go-atscfg/atscfg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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")
}
}

Expand Down
4 changes: 2 additions & 2 deletions lib/go-atscfg/parentdotconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"]`),
},
}

Expand Down