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.

Sign up for GitHub

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

Jump to bottom

t3c parent.config fix for MSO non-topo mid cache group no parent cache groups #7471

Merged
merged 5 commits into from
May 3, 2023
Merged

t3c parent.config fix for MSO non-topo mid cache group no parent cache groups #7471

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f80b085
t3c parent.config fix for MSO non-topo mid cache group no parent cach…
Apr 28, 2023
76e382f
port tests to strategies.yaml
May 1, 2023
af411cf
add fix to changelog
May 1, 2023
6e9d308
parentdotconfig rename primSec struct to parentCacheGroups
May 1, 2023
1ac8abf
parentCacheGroups struct comment change
May 2, 2023
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- [#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.
- [#6385](https://github.com/apache/trafficcontrol/issues/6385) *Traffic Ops* Reserved consistentHashQueryParameters cause internal server error
- [#7471](https://github.com/apache/trafficcontrol/pull/7471) *Traffic Control Cache Config (t3c)* Fixed issue with MSO non topo origins from multiple cache groups.

### Removed
- [#7271](https://github.com/apache/trafficcontrol/pull/7271) Remove components in `infrastructre/docker/`, not in use as cdn-in-a-box performs the same functionality.
Expand Down
146 changes: 105 additions & 41 deletions lib/go-atscfg/parentdotconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,14 +165,19 @@ func MakeParentDotConfig(
}, nil
}

// primarySecondary contains the names of the primary and secondary parent cache groups.
type primarySecondary struct {
// parentCacheGroups holds parent cache groups names for OTF topologies.
// Primaries and Secondaries are from origin parent cache groups with
// assigned primary and secondary cache groups.
// The Null parent cache groups aren't considered unless the current server
// cache group has no primary/secondary parents selected.
type parentCacheGroups struct {
Primary string
Secondary string
Nulls []string
}

// createTopology creates an on the fly topology for this server and non topology delivery service.
func createTopology(server *Server, ds DeliveryService, nameTopologies map[TopologyName]tc.Topology, ocgmap map[OriginHost]primarySecondary) (string, tc.Topology, []string) {
func createTopology(server *Server, ds DeliveryService, nameTopologies map[TopologyName]tc.Topology, ocgmap map[OriginHost]parentCacheGroups) (string, tc.Topology, []string) {

topoName := ""
topo := tc.Topology{}
Expand Down Expand Up @@ -214,7 +219,7 @@ func createTopology(server *Server, ds DeliveryService, nameTopologies map[Topol
topo.Nodes = append(topo.Nodes, node)
} else {
// If mid cache group, insert fake edge cache group.
// This is incorrect if there are multiple MID tiers.
// This is technically incorrect if there are multiple MID tiers.
pind := 1
if strings.HasPrefix(server.Type, tc.MidTypePrefix) {
parents := []int{pind}
Expand All @@ -226,16 +231,30 @@ func createTopology(server *Server, ds DeliveryService, nameTopologies map[Topol
topo.Nodes = append(topo.Nodes, edgeNode)
}

hasprimsec := false
parents := []int{}
if cgprimsec.Primary != "" {
hasprimsec = true
parents = append(parents, pind)
pind++
}
if cgprimsec.Secondary != "" {
hasprimsec = true
parents = append(parents, pind)
pind++
}

// Only consider arbitrarily assigned parents if the current
// cache group does not have primary and secondary parents assigned.
if !hasprimsec && 0 < len(cgprimsec.Nulls) {
for _, _ = range cgprimsec.Nulls {
parents = append(parents, pind)
pind++
}
// Adding a '-1' in the parent indices indicates that this has non topology MSO parents, all should just go into primary
parents = append(parents, -1)
}

// create the cache group this server belongs to
node := tc.TopologyNode{
Cachegroup: *server.Cachegroup,
Expand All @@ -249,6 +268,11 @@ func createTopology(server *Server, ds DeliveryService, nameTopologies map[Topol
if cgprimsec.Secondary != "" {
topo.Nodes = append(topo.Nodes, tc.TopologyNode{Cachegroup: cgprimsec.Secondary})
}
if !hasprimsec {
for _, null := range cgprimsec.Nulls {
topo.Nodes = append(topo.Nodes, tc.TopologyNode{Cachegroup: null})
}
}
}
return topoName, topo, warns
}
Expand Down Expand Up @@ -324,7 +348,7 @@ func makeParentDotConfigData(
// serverParams are the parent.config params for this particular server
serverParams := getServerParentConfigParams(server, parentConfigParams)

parentCacheGroups := map[string]struct{}{}
parentCGs := map[string]struct{}{}
if cacheIsTopLevel {
for _, cg := range cacheGroups {
if cg.Type == nil {
Expand All @@ -337,7 +361,7 @@ func makeParentDotConfigData(
if *cg.Type != tc.CacheGroupOriginTypeName {
continue
}
parentCacheGroups[*cg.Name] = struct{}{}
parentCGs[*cg.Name] = struct{}{}
}
} else {
for _, cg := range cacheGroups {
Expand All @@ -350,10 +374,10 @@ func makeParentDotConfigData(

if *cg.Name == *server.Cachegroup {
if cg.ParentName != nil && *cg.ParentName != "" {
parentCacheGroups[*cg.ParentName] = struct{}{}
parentCGs[*cg.ParentName] = struct{}{}
}
if cg.SecondaryParentName != nil && *cg.SecondaryParentName != "" {
parentCacheGroups[*cg.SecondaryParentName] = struct{}{}
parentCGs[*cg.SecondaryParentName] = struct{}{}
}
break
}
Expand Down Expand Up @@ -393,7 +417,7 @@ func makeParentDotConfigData(
}
continue
}
if _, ok := parentCacheGroups[*sv.Cachegroup]; !ok {
if _, ok := parentCGs[*sv.Cachegroup]; !ok {
continue
}
if sv.Type != tc.OriginTypeName &&
Expand Down Expand Up @@ -425,6 +449,7 @@ func makeParentDotConfigData(
cgServerIDs[*server.ID] = struct{}{}

cgDSServers := filterDSS(dss, nil, cgServerIDs)

parentServerDSes := map[int]map[int]struct{}{} // map[serverID][dsID]
for _, dss := range cgDSServers {
if parentServerDSes[dss.Server] == nil {
Expand All @@ -446,7 +471,7 @@ func makeParentDotConfigData(
warnings = append(warnings, dsOriginWarns...)

// Note map cache group lists are ordered, prim first, sec second
ocgmap := map[OriginHost]primarySecondary{}
ocgmap := map[OriginHost]parentCacheGroups{}

for _, ds := range dses {

Expand Down Expand Up @@ -483,7 +508,7 @@ func makeParentDotConfigData(
if len(ocgmap) == 0 {
ocgmap = makeOCGMap(parentInfos)
if len(ocgmap) == 0 {
ocgmap[""] = primarySecondary{}
ocgmap[""] = parentCacheGroups{}
}
}

Expand Down Expand Up @@ -630,21 +655,29 @@ func (p parentInfo) ToAbstract() *ParentAbstractionServiceParent {
type parentInfos map[OriginHost]parentInfo

// Returns a map of parent cache groups names per origin host.
func makeOCGMap(opis map[OriginHost][]parentInfo) map[OriginHost]primarySecondary {
ocgnames := map[OriginHost]primarySecondary{}
func makeOCGMap(opis map[OriginHost][]parentInfo) map[OriginHost]parentCacheGroups {
ocgnames := map[OriginHost]parentCacheGroups{}

type primSec struct {
isPrim bool
isSec bool
}

for host, pis := range opis {
cgnames := make(map[string]bool)
cgnames := map[string]primSec{}
for _, pi := range pis {
cgnames[string(pi.Cachegroup)] = pi.PrimaryParent
cgnames[string(pi.Cachegroup)] =
primSec{isPrim: pi.PrimaryParent, isSec: pi.SecondaryParent}
}

ps := primarySecondary{}
for cg, isPrim := range cgnames {
if isPrim {
ps := parentCacheGroups{}
for cg, primsec := range cgnames {
if primsec.isPrim {
ps.Primary = cg
} else {
} else if primsec.isSec {
ps.Secondary = cg
} else {
ps.Nulls = append(ps.Nulls, cg)
}
}
ocgnames[host] = ps
Expand Down Expand Up @@ -1311,9 +1344,9 @@ func getTopologyParents(
dsMergeGroups []string, // sorted parent merge groups for this ds
) ([]*ParentAbstractionServiceParent, []*ParentAbstractionServiceParent, []string, error) {
warnings := []string{}

// If it's the last tier, then the parent is the origin.
// Note this doesn't include MSO, whose final tier cachegroup points to the origin cachegroup.

if serverIsLastTier {
orgURI, orgWarns, err := getOriginURI(*ds.OrgServerFQDN) // TODO pass, instead of calling again
warnings = append(warnings, orgWarns...)
Expand Down Expand Up @@ -1346,27 +1379,54 @@ func getTopologyParents(
return nil, nil, warnings, errors.New("This server '" + *server.HostName + "' not in DS " + *ds.XMLID + " topology, skipping")
}

if len(svNode.Parents) == 0 {
numParents := len(svNode.Parents)

// No parents (or special case mso non topo)
if numParents == 0 || -1 == svNode.Parents[0] {
return nil, nil, warnings, errors.New("DS " + *ds.XMLID + " topology '" + *ds.Topology + "' is last tier, but NonLastTier called! Should never happen")
}
if numParents := len(svNode.Parents); numParents > 2 {
warnings = append(warnings, "DS "+*ds.XMLID+" topology '"+*ds.Topology+"' has "+strconv.Itoa(numParents)+" parent nodes, but Apache Traffic Server only supports Primary and Secondary (2) lists of parents. CacheGroup nodes after the first 2 will be ignored!")
}
if len(topology.Nodes) <= svNode.Parents[0] {
return nil, nil, warnings, errors.New("DS " + *ds.XMLID + " topology '" + *ds.Topology + "' node parent " + strconv.Itoa(svNode.Parents[0]) + " greater than number of topology nodes " + strconv.Itoa(len(topology.Nodes)) + ". Cannot create parents!")
}
if len(svNode.Parents) > 1 && len(topology.Nodes) <= svNode.Parents[1] {
warnings = append(warnings, "DS "+*ds.XMLID+" topology '"+*ds.Topology+"' node secondary parent "+strconv.Itoa(svNode.Parents[1])+" greater than number of topology nodes "+strconv.Itoa(len(topology.Nodes))+". Secondary parent will be ignored!")
}

parentCG := topology.Nodes[svNode.Parents[0]].Cachegroup
secondaryParentCG := ""
if len(svNode.Parents) > 1 && len(topology.Nodes) > svNode.Parents[1] {
secondaryParentCG = topology.Nodes[svNode.Parents[1]].Cachegroup
}
primaryCGs := map[string]struct{}{}
secondaryCG := ""

if parentCG == "" {
return nil, nil, warnings, errors.New("Server '" + *server.HostName + "' DS " + *ds.XMLID + " topology '" + *ds.Topology + "' cachegroup '" + *server.Cachegroup + "' topology node parent " + strconv.Itoa(svNode.Parents[0]) + " is not in the topology!")
// Check for special case of MSO non topology with no parent
// cache groups assigned to the server cache group.
if -1 == svNode.Parents[numParents-1] {
for _, pind := range svNode.Parents {
if -1 == pind {
break
}
parentCG := topology.Nodes[pind].Cachegroup
if parentCG != "" {
primaryCGs[parentCG] = struct{}{}
} else {
warnings = append(warnings, "Server '"+*server.HostName+"' DS "+*ds.XMLID+" topology '"+*ds.Topology+"' cachegroup '"+*server.Cachegroup+"' topology node parent "+strconv.Itoa(pind)+" is not in the topology!")
}
}
} else { // normal topology/mso configuration
if 2 < numParents && 0 <= svNode.Parents[0] {
warnings = append(warnings, "DS "+*ds.XMLID+" topology '"+*ds.Topology+"' has "+strconv.Itoa(numParents)+" parent nodes, but Apache Traffic Server only supports Primary and Secondary (2) lists of parents. CacheGroup nodes after the first 2 will be ignored!")
}

numNodes := len(topology.Nodes)

if numNodes <= svNode.Parents[0] {
return nil, nil, warnings, errors.New("DS " + *ds.XMLID + " topology '" + *ds.Topology + "' node parent " + strconv.Itoa(svNode.Parents[0]) + " greater than number of topology nodes " + strconv.Itoa(numNodes) + ". Cannot create parents!")
}
if 1 < numParents && numNodes <= svNode.Parents[1] {
warnings = append(warnings, "DS "+*ds.XMLID+" topology '"+*ds.Topology+"' node secondary parent "+strconv.Itoa(svNode.Parents[1])+" greater than number of topology nodes "+strconv.Itoa(numNodes)+". Secondary parent will be ignored!")
}

parentCG := topology.Nodes[svNode.Parents[0]].Cachegroup
if 1 < numParents && svNode.Parents[1] < numNodes {
secondaryCG = topology.Nodes[svNode.Parents[1]].Cachegroup
}

if parentCG == "" {
return nil, nil, warnings, errors.New("Server '" + *server.HostName + "' DS " + *ds.XMLID + " topology '" + *ds.Topology + "' cachegroup '" + *server.Cachegroup + "' topology node parent " + strconv.Itoa(svNode.Parents[0]) + " is not in the topology!")
}

primaryCGs[parentCG] = struct{}{}
}

parentStrs := []*ParentAbstractionServiceParent{}
Expand Down Expand Up @@ -1403,16 +1463,15 @@ func getTopologyParents(
if sv.Type != tc.OriginTypeName && !hasRequiredCapabilities(serverCapabilities[*sv.ID], dsRequiredCapabilities[*ds.ID]) {
continue
}
if *sv.Cachegroup == parentCG {
if _, ok := primaryCGs[*sv.Cachegroup]; ok {
parentStr, err := serverParentStr(&sv.Server, sv.Params)
if err != nil {
return nil, nil, warnings, errors.New("getting server parent string: " + err.Error())
}
if parentStr != nil { // will be nil if server is not_a_parent (possibly other reasons)
parentStrs = append(parentStrs, parentStr)
}
}
if *sv.Cachegroup == secondaryParentCG {
} else if *sv.Cachegroup == secondaryCG {
parentStr, err := serverParentStr(&sv.Server, sv.Params)
if err != nil {
return nil, nil, warnings, errors.New("getting server parent string: " + err.Error())
Expand All @@ -1422,7 +1481,12 @@ func getTopologyParents(
}

if 0 < len(dsMergeGroups) && 0 < len(secondaryParentStrs) {
if util.ContainsStr(dsMergeGroups, parentCG) && util.ContainsStr(dsMergeGroups, secondaryParentCG) {
parentCG := ""
for key, _ := range primaryCGs {
parentCG = key
break
}
if util.ContainsStr(dsMergeGroups, parentCG) && util.ContainsStr(dsMergeGroups, secondaryCG) {
parentStrs = append(parentStrs, secondaryParentStrs...)
secondaryParentStrs = nil
}
Expand Down
Loading