You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2022/10/13 19:37:58 UTC

[GitHub] [trafficcontrol] traeak opened a new pull request, #7137: T3C parent.config simulate topology for non topo delivery services

traeak opened a new pull request, #7137:
URL: https://github.com/apache/trafficcontrol/pull/7137

   <!--
   Thank you for contributing! Please be sure to read our contribution guidelines: https://github.com/apache/trafficcontrol/blob/master/CONTRIBUTING.md
   If this closes or relates to an existing issue, please reference it using one of the following:
   
   Closes: #ISSUE
   Related: #ISSUE
   
   If this PR fixes a security vulnerability, DO NOT submit! Instead, contact
   the Apache Traffic Control Security Team at security@trafficcontrol.apache.org and follow the
   guidelines at https://apache.org/security regarding vulnerability disclosure.
   -->
   
   In order to simplify the t3c parentdotconfig.go code this change creates and assigns a temporary topology on the fly for non topology delivery services.
   
   <!-- **^ Add meaningful description above** --><hr/>
   
   ## Which Traffic Control components are affected by this PR?
   <!-- Please delete all components from this list that are NOT affected by this PR.
   Feel free to add the name of a tool or script that is affected but not on the list.
   -->
   - Traffic Control Cache Config (`t3c`, formerly ORT)
   
   ## What is the best way to verify this PR?
   <!-- Please include here ALL the steps necessary to test your PR.
   If your PR has tests (and most should), provide the steps needed to run the tests.
   If not, please provide step-by-step instructions to test the PR manually and explain why your PR does not need tests. -->
   
   Generate parent.config files using this version and an older version of T3C and compare results side by side.
   
   ## If this is a bugfix, which Traffic Control versions contained the bug?
   <!-- Delete this section if the PR is not a bugfix, or if the bug is only in the master branch.
   Examples:
   - 5.1.2
   - 5.1.3 (RC1)
    -->
   
   
   ## PR submission checklist
   - [x] This PR has tests <!-- If not, please delete this text and explain why this PR does not need tests. -->
   - [ ] This PR has documentation - code change only
   - [x] This PR has a CHANGELOG.md entry <!-- A fix for a bug from an ATC release, an improvement, or a new feature should have a changelog entry. -->
   - [ ] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://apache.org/security) for details)
   
   <!--
   Licensed to the Apache Software Foundation (ASF) under one
   or more contributor license agreements.  See the NOTICE file
   distributed with this work for additional information
   regarding copyright ownership.  The ASF licenses this file
   to you under the Apache License, Version 2.0 (the
   "License"); you may not use this file except in compliance
   with the License.  You may obtain a copy of the License at
   
       http://www.apache.org/licenses/LICENSE-2.0
   
   Unless required by applicable law or agreed to in writing,
   software distributed under the License is distributed on an
   "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
   KIND, either express or implied.  See the License for the
   specific language governing permissions and limitations
   under the License.
   -->
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] traeak commented on a diff in pull request #7137: T3C parent.config simulate topology for non topo delivery services

Posted by GitBox <gi...@apache.org>.
traeak commented on code in PR #7137:
URL: https://github.com/apache/trafficcontrol/pull/7137#discussion_r1003288096


##########
lib/go-atscfg/parentdotconfig.go:
##########
@@ -165,6 +165,11 @@ func MakeParentDotConfig(
 	}, nil
 }
 
+// Check if this ds type is edge only

Review Comment:
   comment changed and moved.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] rob05c commented on a diff in pull request #7137: T3C parent.config simulate topology for non topo delivery services

Posted by GitBox <gi...@apache.org>.
rob05c commented on code in PR #7137:
URL: https://github.com/apache/trafficcontrol/pull/7137#discussion_r997528717


##########
lib/go-atscfg/parentdotconfig.go:
##########
@@ -383,258 +390,112 @@ func makeParentDotConfigData(
 			continue
 		}
 
-		isMSO := ds.MultiSiteOrigin != nil && *ds.MultiSiteOrigin
+		// manufacture a topology for this DS.
+		if ds.Topology == nil || *ds.Topology == "" {
+			// only populate if there are non topology ds's
+			if len(ocgmap) == 0 {
+				ocgmap = makeOCGMap(parentInfos)
+				if len(ocgmap) == 0 {
+					ocgmap[""] = []string{}
+				}
+			}
 
-		// TODO put these in separate functions. No if-statement should be this long.
-		if ds.Topology != nil && *ds.Topology != "" {
-			pasvc, topoWarnings, err := getTopologyParentConfigLine(
-				server,
-				serversWithParams,
-				&ds,
-				serverParams,
-				parentConfigParams,
-				nameTopologies,
-				serverCapabilities,
-				dsRequiredCapabilities,
-				cacheGroups,
-				profileParentConfigParams,
-				isMSO,
-				atsMajorVersion,
-				dsOrigins[DeliveryServiceID(*ds.ID)],
-				opt.AddComments,
-			)
-			warnings = append(warnings, topoWarnings...)
+			orgFQDNStr := *ds.OrgServerFQDN
+			orgURI, orgWarns, err := getOriginURI(orgFQDNStr)
+			warnings = append(warnings, orgWarns...)
 			if err != nil {
-				// we don't want to fail generation with an error if one ds is malformed
-				warnings = append(warnings, err.Error()) // getTopologyParentConfigLine includes error context
+				warnings = append(warnings, "DS '"+*ds.XMLID+"' has malformed origin URI: '"+orgFQDNStr+"': skipping!"+err.Error())
 				continue
 			}
 
-			if pasvc != nil { // will be nil with no error if this server isn't in the Topology, or if it doesn't have the Required Capabilities
-				parentAbstraction.Services = append(parentAbstraction.Services, pasvc)
-			}
-		} else {
-			isLastCacheTier := noTopologyServerIsLastCacheForDS(server, &ds, cacheGroups)
-			serverPlacement := TopologyPlacement{
-				IsLastCacheTier:  isLastCacheTier,
-				IsFirstCacheTier: !isLastCacheTier || !ds.Type.UsesMidCache(),
-			}
-
-			dsParams, dswarns := getParentDSParams(ds, profileParentConfigParams, serverPlacement, isMSO)
-			warnings = append(warnings, dswarns...)
-
-			if cacheIsTopLevel {
-				parentQStr := false
-				if dsParams.QueryStringHandling == "" && dsParams.Algorithm == tc.AlgorithmConsistentHash && ds.QStringIgnore != nil && tc.QStringIgnore(*ds.QStringIgnore) == tc.QStringIgnoreUseInCacheKeyAndPassUp {
-					parentQStr = true
-				}
-
-				orgFQDNStr := *ds.OrgServerFQDN
-				// if this cache isn't the last tier, i.e. we're not going to the origin, use http not https
-				if !isLastCacheTier {
-					orgFQDNStr = strings.Replace(orgFQDNStr, `https://`, `http://`, -1)
-				}
-				orgURI, orgWarns, err := getOriginURI(orgFQDNStr)
-				warnings = append(warnings, orgWarns...)
-				if err != nil {
-					warnings = append(warnings, "DS '"+*ds.XMLID+"' has malformed origin URI: '"+orgFQDNStr+"': skipping!"+err.Error())
+			// use the topology name for the fqdn
+			topoName := orgURI.Hostname()
+			cgnames, ok := ocgmap[OriginHost(topoName)]
+			if !ok {
+				topoName = deliveryServicesAllParentsKey
+				cgnames, ok = ocgmap[OriginHost(topoName)]
+				if !ok {
+					warnings = append(warnings, "DS '"+*ds.XMLID+"' has no parent cache groups! Skipping!")
 					continue
 				}
+			}
 
-				pasvc := &ParentAbstractionService{}
-				pasvc.Name = *ds.XMLID
-
-				if ds.OriginShield != nil && *ds.OriginShield != "" {
-
-					policy := ParentAbstractionServiceRetryPolicyConsistentHash
-
-					if parentSelectAlg := serverParams[ParentConfigRetryKeysDefault.Algorithm]; strings.TrimSpace(parentSelectAlg) != "" {
-						paramPolicy := ParentSelectAlgorithmToParentAbstractionServiceRetryPolicy(parentSelectAlg)
-						if paramPolicy != ParentAbstractionServiceRetryPolicyInvalid {
-							policy = paramPolicy
-						} else {
-							warnings = append(warnings, "DS '"+*ds.XMLID+"' had malformed "+ParentConfigRetryKeysDefault.Algorithm+" parameter '"+parentSelectAlg+"', not using!")
-						}
-					}
-					pasvc.Comment = makeParentComment(opt.AddComments, *ds.XMLID, "")
-					pasvc.DestDomain = orgURI.Hostname()
-					pasvc.Port, err = strconv.Atoi(orgURI.Port())
-					if err != nil {
-						if strings.ToLower(orgURI.Scheme) == "https" {
-							pasvc.Port = 443
-						} else {
-							pasvc.Port = 80
-						}
-						warnings = append(warnings, "DS '"+*ds.XMLID+"' had malformed origin  port: '"+orgURI.Port()+"': using "+strconv.Itoa(pasvc.Port)+"! : "+err.Error())
-					}
-
-					fqdnPort := strings.Split(*ds.OriginShield, ":")
-					parent := &ParentAbstractionServiceParent{}
-					parent.FQDN = fqdnPort[0]
-					if len(fqdnPort) > 1 {
-						parent.Port, err = strconv.Atoi(fqdnPort[1])
-						if err != nil {
-							parent.Port = 80
-							warnings = append(warnings, "DS '"+*ds.XMLID+"' had malformed origin  port: '"+*ds.OriginShield+"': using "+strconv.Itoa(parent.Port)+"! : "+err.Error())
-						}
-					} else {
-						parent.Port = 80
-						warnings = append(warnings, "DS '"+*ds.XMLID+"' had no origin port: '"+*ds.OriginShield+"': using "+strconv.Itoa(parent.Port)+"!")
-					}
-					pasvc.Parents = append(pasvc.Parents, parent)
-					pasvc.RetryPolicy = policy
-					pasvc.GoDirect = true
-
-					// textLine += "dest_domain=" + orgURI.Hostname() + " port=" + orgURI.Port() + " parent=" + *ds.OriginShield + " " + algorithm + " go_direct=true\n"
-
-				} else if ds.MultiSiteOrigin != nil && *ds.MultiSiteOrigin {
-					pasvc.Comment = makeParentComment(opt.AddComments, *ds.XMLID, "")
-					pasvc.DestDomain = orgURI.Hostname()
-					pasvc.Port, err = strconv.Atoi(orgURI.Port())
-					if err != nil {
-						pasvc.Port = 80
-						warnings = append(warnings, "DS '"+*ds.XMLID+"' had malformed origin  port: '"+orgURI.Port()+"': using "+strconv.Itoa(pasvc.Port)+"! : "+err.Error())
-					}
-
-					// textLine += "dest_domain=" + orgURI.Hostname() + " port=" + orgURI.Port() + " "
-					if len(parentInfos) == 0 {
-					}
-
-					if len(parentInfos[OriginHost(orgURI.Hostname())]) == 0 {
-						// TODO error? emulates Perl
-						warnings = append(warnings, "DS "+*ds.XMLID+" has no parent servers")
-					}
-
-					parents, secondaryParents, secondaryMode, parentWarns := getMSOParentStrs(&ds, parentInfos[OriginHost(orgURI.Hostname())], atsMajorVersion, dsParams.Algorithm, dsParams.TryAllPrimariesBeforeSecondary)
-					warnings = append(warnings, parentWarns...)
-					pasvc.Parents = parents
-					pasvc.SecondaryParents = secondaryParents
-					pasvc.SecondaryMode = secondaryMode
-					pasvc.RetryPolicy = dsParams.Algorithm // TODO convert
-					pasvc.IgnoreQueryStringInParentSelection = !parentQStr
-					pasvc.GoDirect = true
-
-					// textLine += parents + secondaryParents + ` round_robin=` + dsParams.Algorithm + ` qstring=` + parentQStr + ` go_direct=false parent_is_proxy=false`
-
-					prWarns := dsParams.FillParentSvcRetries(cacheIsTopLevel, atsMajorVersion, pasvc)
-					warnings = append(warnings, prWarns...)
+			// Manufactured topology
+			topo := tc.Topology{Name: topoName}
 
-					parentAbstraction.Services = append(parentAbstraction.Services, pasvc)
+			if IsGoDirect(ds) {
+				node := tc.TopologyNode{
+					Cachegroup: *server.Cachegroup,
 				}
+				topo.Nodes = append(topo.Nodes, node)
 			} else {
-				queryStringHandling := ParentSelectParamQStringHandlingToBool(serverParams[ParentConfigParamQStringHandling]) // "qsh" in Perl
-				if queryStringHandling == nil && serverParams[ParentConfigParamQStringHandling] != "" {
-					warnings = append(warnings, "Server Parameter '"+ParentConfigParamQStringHandling+"' value '"+serverParams[ParentConfigParamQStringHandling]+"' malformed, not using!")
+				// If mid cache group, insert fake edge cache group.
+				// This is incorrect if there are multiple MID tiers.
+				pind := 1
+				if strings.HasPrefix(server.Type, tc.MidTypePrefix) {
+					parents := []int{pind}
+					pind++
+					edgeNode := tc.TopologyNode{
+						Cachegroup: "fake_edgecg",
+						Parents:    parents,
+					}
+					topo.Nodes = append(topo.Nodes, edgeNode)
 				}
 
-				roundRobin := ParentAbstractionServiceRetryPolicyConsistentHash
-				// roundRobin := `round_robin=consistent_hash`
-				goDirect := false
-				// goDirect := `go_direct=false`
-
-				parents, secondaryParents, secondaryMode, parentWarns := getParentStrs(&ds, dsRequiredCapabilities, parentInfos[deliveryServicesAllParentsKey], atsMajorVersion, dsParams.TryAllPrimariesBeforeSecondary)
-				warnings = append(warnings, parentWarns...)
-
-				pasvc := &ParentAbstractionService{}
-				pasvc.Name = *ds.XMLID
-
-				// peering ring check
-				if dsParams.UsePeering {
-					secondaryMode = ParentAbstractionServiceParentSecondaryModePeering
+				parents := []int{}
+				for ind := 0; ind < len(cgnames); ind++ {
+					parents = append(parents, pind)
+					pind++
 				}
 
-				orgFQDNStr := *ds.OrgServerFQDN
-				// if this cache isn't the last tier, i.e. we're not going to the origin, use http not https
-				if !isLastCacheTier {
-					orgFQDNStr = strings.Replace(orgFQDNStr, `https://`, `http://`, -1)
+				node := tc.TopologyNode{
+					Cachegroup: *server.Cachegroup,
+					Parents:    parents,
 				}
-				orgURI, orgWarns, err := getOriginURI(orgFQDNStr)
-				warnings = append(warnings, orgWarns...)
-				if err != nil {
-					warnings = append(warnings, "DS '"+*ds.XMLID+"' had malformed origin  URI: '"+*ds.OrgServerFQDN+"': skipping!"+err.Error())
-					continue
-				}
-
-				pasvc.Comment = makeParentComment(opt.AddComments, *ds.XMLID, "")
-
-				// TODO encode this in a DSType func, IsGoDirect() ?
-				if *ds.Type == tc.DSTypeHTTPNoCache || *ds.Type == tc.DSTypeHTTPLive || *ds.Type == tc.DSTypeDNSLive {
-					pasvc.DestDomain = orgURI.Hostname()
-					pasvc.Port, err = strconv.Atoi(orgURI.Port())
-					if err != nil {
-						if strings.ToLower(orgURI.Scheme) == "https" {
-							pasvc.Port = 443
-						} else {
-							pasvc.Port = 80
-						}
-						warnings = append(warnings, "DS '"+*ds.XMLID+"' had malformed origin  port: '"+orgURI.Port()+"': using "+strconv.Itoa(pasvc.Port)+"! : "+err.Error())
-					}
-
-					pasvc.GoDirect = true
-
-					pasvc.Parents = []*ParentAbstractionServiceParent{&ParentAbstractionServiceParent{
-						FQDN:   pasvc.DestDomain,
-						Port:   pasvc.Port,
-						Weight: 1.0,
-					}}
+				topo.Nodes = append(topo.Nodes, node)
 
-					// text += `dest_domain=` + orgURI.Hostname() + ` port=` + orgURI.Port() + ` go_direct=true` + "\n"
-				} else {
-
-					// check for profile psel.qstring_handling.  If this parameter is assigned to the server profile,
-					// then edges will use the qstring handling value specified in the parameter for all profiles.
-
-					// If there is no defined parameter in the profile, then check the delivery service profile.
-					// If psel.qstring_handling exists in the DS profile, then we use that value for the specified DS only.
-					// This is used only if not overridden by a server profile qstring handling parameter.
+				for _, cg := range cgnames {
+					topo.Nodes = append(topo.Nodes, tc.TopologyNode{Cachegroup: cg})
+				}
+			}
 
-					// TODO refactor this logic, hard to understand (transliterated from Perl)
-					dsQSH := queryStringHandling
-					if dsQSH == nil {
-						dsQSH = ParentSelectParamQStringHandlingToBool(dsParams.QueryStringHandling)
-						if dsQSH == nil && dsParams.QueryStringHandling != "" {
-							warnings = append(warnings, "Delivery Service parameter '"+ParentConfigParamQStringHandling+"' value '"+dsParams.QueryStringHandling+"' malformed, not using!")
-						}
+			nameTopologies[TopologyName(topoName)] = topo
+			ds.Topology = util.StrPtr(topoName)
+		}
 
-					}
-					parentQStr := dsQSH
-					if parentQStr == nil {
-						v := false
-						parentQStr = &v
-					}
-					if ds.QStringIgnore != nil && tc.QStringIgnore(*ds.QStringIgnore) == tc.QStringIgnoreUseInCacheKeyAndPassUp && dsQSH == nil {
-						v := true
-						parentQStr = &v
-					}
-					if parentQStr == nil {
-						b := !DefaultIgnoreQueryStringInParentSelection
-						parentQStr = &b
-					}
+		isMSO := ds.MultiSiteOrigin != nil && *ds.MultiSiteOrigin
 
-					pasvc.DestDomain = orgURI.Hostname()
-					pasvc.Port, err = strconv.Atoi(orgURI.Port())
-					if err != nil {
-						if strings.ToLower(orgURI.Scheme) == "https" {
-							pasvc.Port = 443
-						} else {
-							pasvc.Port = 80
-						}
-						warnings = append(warnings, "DS '"+*ds.XMLID+"' had malformed origin  port: '"+orgURI.Port()+"': using "+strconv.Itoa(pasvc.Port)+"! : "+err.Error())
-					}
-					pasvc.Parents = parents
-					pasvc.SecondaryParents = secondaryParents
-					pasvc.SecondaryMode = secondaryMode
-					pasvc.RetryPolicy = roundRobin
-					pasvc.GoDirect = goDirect
-					pasvc.IgnoreQueryStringInParentSelection = !*parentQStr
-					// text += `dest_domain=` + orgURI.Hostname() + ` port=` + orgURI.Port() + ` ` + parents + ` ` + secondaryParents + ` ` + roundRobin + ` ` + goDirect + ` qstring=` + parentQStr + "\n"
-				}
+		// TODO put these in separate functions. No if-statement should be this long.
+		if ds.Topology == nil || *ds.Topology == "" {

Review Comment:
   With the fake topology you're adding above, is it ever possible for this to be true? Should this block be removed?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] rob05c commented on a diff in pull request #7137: T3C parent.config simulate topology for non topo delivery services

Posted by GitBox <gi...@apache.org>.
rob05c commented on code in PR #7137:
URL: https://github.com/apache/trafficcontrol/pull/7137#discussion_r997367115


##########
lib/go-atscfg/parentdotconfig.go:
##########
@@ -165,6 +165,11 @@ func MakeParentDotConfig(
 	}, nil
 }
 
+// Check if this ds type is edge only

Review Comment:
   Symbol comments should be complete sentences starting with the symbol name and ending with a period, for GoDoc. Should be `// IsGoDirect checks if this ds type is edge only.`
   
   Also, would you object to moving this func to `atscfg.go`? Seems fairly generic, might be useful in other configs.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] rob05c commented on a diff in pull request #7137: T3C parent.config simulate topology for non topo delivery services

Posted by GitBox <gi...@apache.org>.
rob05c commented on code in PR #7137:
URL: https://github.com/apache/trafficcontrol/pull/7137#discussion_r997370334


##########
lib/go-atscfg/parentdotconfig.go:
##########
@@ -383,258 +390,112 @@ func makeParentDotConfigData(
 			continue
 		}
 
-		isMSO := ds.MultiSiteOrigin != nil && *ds.MultiSiteOrigin
+		// manufacture a topology for this DS.

Review Comment:
   Mind putting this in a func? It's a pretty big block, and does a very singular thing. IMO it'd make the code a lot easier to read and work with.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] traeak commented on a diff in pull request #7137: T3C parent.config simulate topology for non topo delivery services

Posted by GitBox <gi...@apache.org>.
traeak commented on code in PR #7137:
URL: https://github.com/apache/trafficcontrol/pull/7137#discussion_r1003289241


##########
lib/go-atscfg/parentdotconfig.go:
##########
@@ -383,258 +390,112 @@ func makeParentDotConfigData(
 			continue
 		}
 
-		isMSO := ds.MultiSiteOrigin != nil && *ds.MultiSiteOrigin
+		// manufacture a topology for this DS.
+		if ds.Topology == nil || *ds.Topology == "" {
+			// only populate if there are non topology ds's
+			if len(ocgmap) == 0 {
+				ocgmap = makeOCGMap(parentInfos)
+				if len(ocgmap) == 0 {
+					ocgmap[""] = []string{}
+				}
+			}
 
-		// TODO put these in separate functions. No if-statement should be this long.
-		if ds.Topology != nil && *ds.Topology != "" {
-			pasvc, topoWarnings, err := getTopologyParentConfigLine(
-				server,
-				serversWithParams,
-				&ds,
-				serverParams,
-				parentConfigParams,
-				nameTopologies,
-				serverCapabilities,
-				dsRequiredCapabilities,
-				cacheGroups,
-				profileParentConfigParams,
-				isMSO,
-				atsMajorVersion,
-				dsOrigins[DeliveryServiceID(*ds.ID)],
-				opt.AddComments,
-			)
-			warnings = append(warnings, topoWarnings...)
+			orgFQDNStr := *ds.OrgServerFQDN
+			orgURI, orgWarns, err := getOriginURI(orgFQDNStr)
+			warnings = append(warnings, orgWarns...)
 			if err != nil {
-				// we don't want to fail generation with an error if one ds is malformed
-				warnings = append(warnings, err.Error()) // getTopologyParentConfigLine includes error context
+				warnings = append(warnings, "DS '"+*ds.XMLID+"' has malformed origin URI: '"+orgFQDNStr+"': skipping!"+err.Error())
 				continue
 			}
 
-			if pasvc != nil { // will be nil with no error if this server isn't in the Topology, or if it doesn't have the Required Capabilities
-				parentAbstraction.Services = append(parentAbstraction.Services, pasvc)
-			}
-		} else {
-			isLastCacheTier := noTopologyServerIsLastCacheForDS(server, &ds, cacheGroups)
-			serverPlacement := TopologyPlacement{
-				IsLastCacheTier:  isLastCacheTier,
-				IsFirstCacheTier: !isLastCacheTier || !ds.Type.UsesMidCache(),
-			}
-
-			dsParams, dswarns := getParentDSParams(ds, profileParentConfigParams, serverPlacement, isMSO)
-			warnings = append(warnings, dswarns...)
-
-			if cacheIsTopLevel {
-				parentQStr := false
-				if dsParams.QueryStringHandling == "" && dsParams.Algorithm == tc.AlgorithmConsistentHash && ds.QStringIgnore != nil && tc.QStringIgnore(*ds.QStringIgnore) == tc.QStringIgnoreUseInCacheKeyAndPassUp {
-					parentQStr = true
-				}
-
-				orgFQDNStr := *ds.OrgServerFQDN
-				// if this cache isn't the last tier, i.e. we're not going to the origin, use http not https
-				if !isLastCacheTier {
-					orgFQDNStr = strings.Replace(orgFQDNStr, `https://`, `http://`, -1)
-				}
-				orgURI, orgWarns, err := getOriginURI(orgFQDNStr)
-				warnings = append(warnings, orgWarns...)
-				if err != nil {
-					warnings = append(warnings, "DS '"+*ds.XMLID+"' has malformed origin URI: '"+orgFQDNStr+"': skipping!"+err.Error())
+			// use the topology name for the fqdn
+			topoName := orgURI.Hostname()
+			cgnames, ok := ocgmap[OriginHost(topoName)]
+			if !ok {
+				topoName = deliveryServicesAllParentsKey
+				cgnames, ok = ocgmap[OriginHost(topoName)]
+				if !ok {
+					warnings = append(warnings, "DS '"+*ds.XMLID+"' has no parent cache groups! Skipping!")
 					continue
 				}
+			}
 
-				pasvc := &ParentAbstractionService{}
-				pasvc.Name = *ds.XMLID
-
-				if ds.OriginShield != nil && *ds.OriginShield != "" {
-
-					policy := ParentAbstractionServiceRetryPolicyConsistentHash
-
-					if parentSelectAlg := serverParams[ParentConfigRetryKeysDefault.Algorithm]; strings.TrimSpace(parentSelectAlg) != "" {
-						paramPolicy := ParentSelectAlgorithmToParentAbstractionServiceRetryPolicy(parentSelectAlg)
-						if paramPolicy != ParentAbstractionServiceRetryPolicyInvalid {
-							policy = paramPolicy
-						} else {
-							warnings = append(warnings, "DS '"+*ds.XMLID+"' had malformed "+ParentConfigRetryKeysDefault.Algorithm+" parameter '"+parentSelectAlg+"', not using!")
-						}
-					}
-					pasvc.Comment = makeParentComment(opt.AddComments, *ds.XMLID, "")
-					pasvc.DestDomain = orgURI.Hostname()
-					pasvc.Port, err = strconv.Atoi(orgURI.Port())
-					if err != nil {
-						if strings.ToLower(orgURI.Scheme) == "https" {
-							pasvc.Port = 443
-						} else {
-							pasvc.Port = 80
-						}
-						warnings = append(warnings, "DS '"+*ds.XMLID+"' had malformed origin  port: '"+orgURI.Port()+"': using "+strconv.Itoa(pasvc.Port)+"! : "+err.Error())
-					}
-
-					fqdnPort := strings.Split(*ds.OriginShield, ":")
-					parent := &ParentAbstractionServiceParent{}
-					parent.FQDN = fqdnPort[0]
-					if len(fqdnPort) > 1 {
-						parent.Port, err = strconv.Atoi(fqdnPort[1])
-						if err != nil {
-							parent.Port = 80
-							warnings = append(warnings, "DS '"+*ds.XMLID+"' had malformed origin  port: '"+*ds.OriginShield+"': using "+strconv.Itoa(parent.Port)+"! : "+err.Error())
-						}
-					} else {
-						parent.Port = 80
-						warnings = append(warnings, "DS '"+*ds.XMLID+"' had no origin port: '"+*ds.OriginShield+"': using "+strconv.Itoa(parent.Port)+"!")
-					}
-					pasvc.Parents = append(pasvc.Parents, parent)
-					pasvc.RetryPolicy = policy
-					pasvc.GoDirect = true
-
-					// textLine += "dest_domain=" + orgURI.Hostname() + " port=" + orgURI.Port() + " parent=" + *ds.OriginShield + " " + algorithm + " go_direct=true\n"
-
-				} else if ds.MultiSiteOrigin != nil && *ds.MultiSiteOrigin {
-					pasvc.Comment = makeParentComment(opt.AddComments, *ds.XMLID, "")
-					pasvc.DestDomain = orgURI.Hostname()
-					pasvc.Port, err = strconv.Atoi(orgURI.Port())
-					if err != nil {
-						pasvc.Port = 80
-						warnings = append(warnings, "DS '"+*ds.XMLID+"' had malformed origin  port: '"+orgURI.Port()+"': using "+strconv.Itoa(pasvc.Port)+"! : "+err.Error())
-					}
-
-					// textLine += "dest_domain=" + orgURI.Hostname() + " port=" + orgURI.Port() + " "
-					if len(parentInfos) == 0 {
-					}
-
-					if len(parentInfos[OriginHost(orgURI.Hostname())]) == 0 {
-						// TODO error? emulates Perl
-						warnings = append(warnings, "DS "+*ds.XMLID+" has no parent servers")
-					}
-
-					parents, secondaryParents, secondaryMode, parentWarns := getMSOParentStrs(&ds, parentInfos[OriginHost(orgURI.Hostname())], atsMajorVersion, dsParams.Algorithm, dsParams.TryAllPrimariesBeforeSecondary)
-					warnings = append(warnings, parentWarns...)
-					pasvc.Parents = parents
-					pasvc.SecondaryParents = secondaryParents
-					pasvc.SecondaryMode = secondaryMode
-					pasvc.RetryPolicy = dsParams.Algorithm // TODO convert
-					pasvc.IgnoreQueryStringInParentSelection = !parentQStr
-					pasvc.GoDirect = true
-
-					// textLine += parents + secondaryParents + ` round_robin=` + dsParams.Algorithm + ` qstring=` + parentQStr + ` go_direct=false parent_is_proxy=false`
-
-					prWarns := dsParams.FillParentSvcRetries(cacheIsTopLevel, atsMajorVersion, pasvc)
-					warnings = append(warnings, prWarns...)
+			// Manufactured topology
+			topo := tc.Topology{Name: topoName}
 
-					parentAbstraction.Services = append(parentAbstraction.Services, pasvc)
+			if IsGoDirect(ds) {
+				node := tc.TopologyNode{
+					Cachegroup: *server.Cachegroup,
 				}
+				topo.Nodes = append(topo.Nodes, node)
 			} else {
-				queryStringHandling := ParentSelectParamQStringHandlingToBool(serverParams[ParentConfigParamQStringHandling]) // "qsh" in Perl
-				if queryStringHandling == nil && serverParams[ParentConfigParamQStringHandling] != "" {
-					warnings = append(warnings, "Server Parameter '"+ParentConfigParamQStringHandling+"' value '"+serverParams[ParentConfigParamQStringHandling]+"' malformed, not using!")
+				// If mid cache group, insert fake edge cache group.
+				// This is incorrect if there are multiple MID tiers.
+				pind := 1
+				if strings.HasPrefix(server.Type, tc.MidTypePrefix) {
+					parents := []int{pind}
+					pind++
+					edgeNode := tc.TopologyNode{
+						Cachegroup: "fake_edgecg",
+						Parents:    parents,
+					}
+					topo.Nodes = append(topo.Nodes, edgeNode)
 				}
 
-				roundRobin := ParentAbstractionServiceRetryPolicyConsistentHash
-				// roundRobin := `round_robin=consistent_hash`
-				goDirect := false
-				// goDirect := `go_direct=false`
-
-				parents, secondaryParents, secondaryMode, parentWarns := getParentStrs(&ds, dsRequiredCapabilities, parentInfos[deliveryServicesAllParentsKey], atsMajorVersion, dsParams.TryAllPrimariesBeforeSecondary)
-				warnings = append(warnings, parentWarns...)
-
-				pasvc := &ParentAbstractionService{}
-				pasvc.Name = *ds.XMLID
-
-				// peering ring check
-				if dsParams.UsePeering {
-					secondaryMode = ParentAbstractionServiceParentSecondaryModePeering
+				parents := []int{}
+				for ind := 0; ind < len(cgnames); ind++ {
+					parents = append(parents, pind)
+					pind++
 				}
 
-				orgFQDNStr := *ds.OrgServerFQDN
-				// if this cache isn't the last tier, i.e. we're not going to the origin, use http not https
-				if !isLastCacheTier {
-					orgFQDNStr = strings.Replace(orgFQDNStr, `https://`, `http://`, -1)
+				node := tc.TopologyNode{
+					Cachegroup: *server.Cachegroup,
+					Parents:    parents,
 				}
-				orgURI, orgWarns, err := getOriginURI(orgFQDNStr)
-				warnings = append(warnings, orgWarns...)
-				if err != nil {
-					warnings = append(warnings, "DS '"+*ds.XMLID+"' had malformed origin  URI: '"+*ds.OrgServerFQDN+"': skipping!"+err.Error())
-					continue
-				}
-
-				pasvc.Comment = makeParentComment(opt.AddComments, *ds.XMLID, "")
-
-				// TODO encode this in a DSType func, IsGoDirect() ?
-				if *ds.Type == tc.DSTypeHTTPNoCache || *ds.Type == tc.DSTypeHTTPLive || *ds.Type == tc.DSTypeDNSLive {
-					pasvc.DestDomain = orgURI.Hostname()
-					pasvc.Port, err = strconv.Atoi(orgURI.Port())
-					if err != nil {
-						if strings.ToLower(orgURI.Scheme) == "https" {
-							pasvc.Port = 443
-						} else {
-							pasvc.Port = 80
-						}
-						warnings = append(warnings, "DS '"+*ds.XMLID+"' had malformed origin  port: '"+orgURI.Port()+"': using "+strconv.Itoa(pasvc.Port)+"! : "+err.Error())
-					}
-
-					pasvc.GoDirect = true
-
-					pasvc.Parents = []*ParentAbstractionServiceParent{&ParentAbstractionServiceParent{
-						FQDN:   pasvc.DestDomain,
-						Port:   pasvc.Port,
-						Weight: 1.0,
-					}}
+				topo.Nodes = append(topo.Nodes, node)
 
-					// text += `dest_domain=` + orgURI.Hostname() + ` port=` + orgURI.Port() + ` go_direct=true` + "\n"
-				} else {
-
-					// check for profile psel.qstring_handling.  If this parameter is assigned to the server profile,
-					// then edges will use the qstring handling value specified in the parameter for all profiles.
-
-					// If there is no defined parameter in the profile, then check the delivery service profile.
-					// If psel.qstring_handling exists in the DS profile, then we use that value for the specified DS only.
-					// This is used only if not overridden by a server profile qstring handling parameter.
+				for _, cg := range cgnames {
+					topo.Nodes = append(topo.Nodes, tc.TopologyNode{Cachegroup: cg})
+				}
+			}
 
-					// TODO refactor this logic, hard to understand (transliterated from Perl)
-					dsQSH := queryStringHandling
-					if dsQSH == nil {
-						dsQSH = ParentSelectParamQStringHandlingToBool(dsParams.QueryStringHandling)
-						if dsQSH == nil && dsParams.QueryStringHandling != "" {
-							warnings = append(warnings, "Delivery Service parameter '"+ParentConfigParamQStringHandling+"' value '"+dsParams.QueryStringHandling+"' malformed, not using!")
-						}
+			nameTopologies[TopologyName(topoName)] = topo
+			ds.Topology = util.StrPtr(topoName)
+		}
 
-					}
-					parentQStr := dsQSH
-					if parentQStr == nil {
-						v := false
-						parentQStr = &v
-					}
-					if ds.QStringIgnore != nil && tc.QStringIgnore(*ds.QStringIgnore) == tc.QStringIgnoreUseInCacheKeyAndPassUp && dsQSH == nil {
-						v := true
-						parentQStr = &v
-					}
-					if parentQStr == nil {
-						b := !DefaultIgnoreQueryStringInParentSelection
-						parentQStr = &b
-					}
+		isMSO := ds.MultiSiteOrigin != nil && *ds.MultiSiteOrigin
 
-					pasvc.DestDomain = orgURI.Hostname()
-					pasvc.Port, err = strconv.Atoi(orgURI.Port())
-					if err != nil {
-						if strings.ToLower(orgURI.Scheme) == "https" {
-							pasvc.Port = 443
-						} else {
-							pasvc.Port = 80
-						}
-						warnings = append(warnings, "DS '"+*ds.XMLID+"' had malformed origin  port: '"+orgURI.Port()+"': using "+strconv.Itoa(pasvc.Port)+"! : "+err.Error())
-					}
-					pasvc.Parents = parents
-					pasvc.SecondaryParents = secondaryParents
-					pasvc.SecondaryMode = secondaryMode
-					pasvc.RetryPolicy = roundRobin
-					pasvc.GoDirect = goDirect
-					pasvc.IgnoreQueryStringInParentSelection = !*parentQStr
-					// text += `dest_domain=` + orgURI.Hostname() + ` port=` + orgURI.Port() + ` ` + parents + ` ` + secondaryParents + ` ` + roundRobin + ` ` + goDirect + ` qstring=` + parentQStr + "\n"
-				}
+		// TODO put these in separate functions. No if-statement should be this long.
+		if ds.Topology == nil || *ds.Topology == "" {

Review Comment:
   Removed as it does seem like checks would call continue before this code would be hit.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] traeak commented on a diff in pull request #7137: T3C parent.config simulate topology for non topo delivery services

Posted by GitBox <gi...@apache.org>.
traeak commented on code in PR #7137:
URL: https://github.com/apache/trafficcontrol/pull/7137#discussion_r1003288538


##########
lib/go-atscfg/parentdotconfig.go:
##########
@@ -383,258 +390,112 @@ func makeParentDotConfigData(
 			continue
 		}
 
-		isMSO := ds.MultiSiteOrigin != nil && *ds.MultiSiteOrigin
+		// manufacture a topology for this DS.

Review Comment:
   moved to CreateTopologies function although there are quite a few args and return variables :(



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] rob05c merged pull request #7137: T3C parent.config simulate topology for non topo delivery services

Posted by GitBox <gi...@apache.org>.
rob05c merged PR #7137:
URL: https://github.com/apache/trafficcontrol/pull/7137


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org