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 2020/10/08 21:35:59 UTC
[GitHub] [trafficcontrol] mitchell852 opened a new pull request #5128: strips out servers in the wrong cdn if requesting servers for a ds
mitchell852 opened a new pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128
<!--
************ STOP!! ************
If this Pull Request is intended to fix a security vulnerability, DO NOT submit it! Instead, contact
the Apache Software Foundation Security Team at security@trafficcontrol.apache.org and follow the
guidelines at https://www.apache.org/security/ regarding vulnerability disclosure.
-->
## What does this PR (Pull Request) do?
<!-- Explain the changes you made here. If this fixes an Issue, identify it by
replacing the text in the checkbox item with the Issue number e.g.
- [x] This PR fixes #9001 OR is not related to any Issue
^ This will automatically close Issue number 9001 when the Pull Request is
merged (The '#' is important).
Be sure you check the box properly, see the "The following criteria are ALL
met by this PR" section for details.
-->
- [ ] This PR fixes #REPLACE_ME OR is not related to any Issue <!-- You can check for an issue here: https://github.com/apache/trafficcontrol/issues -->
## Which Traffic Control components are affected by this PR?
<!-- Please delete all components from this list that are NOT affected by this
Pull Request. Also, feel free to add the name of a tool or script that is
affected but not on the list.
Additionally, if this Pull Request does NOT affect documentation, please
explain why documentation is not required. -->
- CDN in a Box
- Documentation
- Grove
- Traffic Control Client <!-- Please specify which; e.g. 'Python', 'Go', 'Java' -->
- Traffic Monitor
- Traffic Ops
- Traffic Ops ORT
- Traffic Portal
- Traffic Router
- Traffic Stats
- Traffic Vault
- CI tests
## What is the best way to verify this PR?
<!-- Please include here ALL the steps necessary to test your Pull Request. If
it includes tests (and most should), outline here the steps needed to run the
tests. If not, lay out the manual testing procedure and please explain why
tests are unnecessary for this Pull Request. -->
## If this is a bug fix, what versions of Traffic Control are affected?
<!-- If this PR fixes a bug, please list here all of the affected versions - to
the best of your knowledge. It's also pretty helpful to include a commit hash
of where 'master' is at the time this PR is opened (if it affects master),
because what 'master' means will change over time. For example, if this PR
fixes a bug that's present in master (at commit hash '1df853c8'), in v4.0.0,
and in the current 4.0.1 Release candidate (e.g. RC1), then this list would
look like:
- master (1df853c8)
- 4.0.0
- 4.0.1 (RC1)
If you don't know what other versions might have this bug, AND don't know how
to find the commit hash of 'master', then feel free to leave this section
blank (or, preferably, delete it entirely).
-->
## The following criteria are ALL met by this PR
<!-- Check the boxes to signify that the associated statement is true. To
"check a box", replace the space inside of the square brackets with an 'x'.
e.g.
- [ x] <- Wrong
- [x ] <- Wrong
- [] <- Wrong
- [*] <- Wrong
- [x] <- Correct!
-->
- [ ] This PR includes tests OR I have explained why tests are unnecessary
- [ ] This PR includes documentation OR I have explained why documentation is unnecessary
- [ ] This PR includes an update to CHANGELOG.md OR such an update is not necessary
- [ ] This PR includes any and all required license headers
- [ ] This PR ensures that database migration sequence is correct OR this PR does not include a database migration
- [ ] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://www.apache.org/security/) for details)
## Additional Information
<!-- If you would like to include any additional information on the PR for
potential reviewers please put it here.
Some examples of this would be:
- Before and after screenshots/gifs of the Traffic Portal if it is affected
- Links to other dependent Pull Requests
- References to relevant context (e.g. new/updates to dependent libraries,
mailing list records, blueprints)
Feel free to leave this section blank (or, preferably, delete it entirely).
-->
<!--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r505056826
##########
File path: traffic_ops/testing/api/v3/deliveryserviceservers_test.go
##########
@@ -87,6 +89,51 @@ func AssignServersToTopologyBasedDeliveryService(t *testing.T) {
t.Fatalf("creating deliveryserviceserver assignment for topology-based delivery service - expected: 400-level status code, actual: %d", reqInf.StatusCode)
}
}
+func AssignServersToNonTopologyBasedDeliveryServiceThatUsesMidTier(t *testing.T) {
+ params := url.Values{}
+ params.Set("xmlId", "ds1")
+ dsWithMid, _, err := TOSession.GetDeliveryServicesV30WithHdr(nil, params)
+ if err != nil {
+ t.Fatalf("cannot GET delivery service 'ds1': %s", err.Error())
+ }
+ if len(dsWithMid) != 1 {
+ t.Fatalf("expected one delivery service: 'ds1', actual: %v", len(dsWithMid))
+ }
+ if dsWithMid[0].Topology != nil {
+ t.Fatal("expected delivery service: 'ds1' to have a nil Topology, actual: non-nil")
+ }
+ serversResp, _, err := TOSession.GetServersWithHdr(nil, nil)
+ serversIds := []int{}
+ for _, s := range serversResp.Response {
+ if s.CDNID != nil && *s.CDNID == *dsWithMid[0].CDNID && s.Type == tc.CacheTypeEdge.String() {
+ serversIds = append(serversIds, *s.ID)
+ }
+ }
+ if len(serversIds) < 1 {
+ t.Fatalf("expected: at least one EDGE in cdn %s, actual: 0", *dsWithMid[0].CDNName)
+ }
+
+ _, _, err = TOSession.CreateDeliveryServiceServers(*dsWithMid[0].ID, serversIds, true)
+ if err != nil {
+ t.Errorf("POST delivery service servers: %v", err)
+ }
+
+ params = url.Values{"dsId": []string{strconv.Itoa(*dsWithMid[0].ID)}}
+ dsServersResp, _, err := TOSession.GetServersWithHdr(¶ms, nil)
+ dsServerIds := []int{}
+ for _, dss := range dsServersResp.Response {
Review comment:
Another case that should check `err` before trying to use `serversResp`
##########
File path: traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
##########
@@ -274,6 +274,18 @@ func GetDSNameFromID(tx *sql.Tx, id int) (tc.DeliveryServiceName, bool, error) {
return name, true, nil
}
+// GetDSCDNIdFromID loads the DeliveryService's cdn ID from the database, from the delivery service ID. Returns whether the delivery service was found, and any error.
+func GetDSCDNIdFromID(tx *sql.Tx, id int) (int, bool, error) {
+ var cdnID int
Review comment:
`cdnID` is a good name because we know that it refers to the ID of a CDN. `id` is less clear, the variable name should mention what kind of ID it is
##########
File path: traffic_ops/testing/api/v3/deliveryserviceservers_test.go
##########
@@ -87,6 +89,51 @@ func AssignServersToTopologyBasedDeliveryService(t *testing.T) {
t.Fatalf("creating deliveryserviceserver assignment for topology-based delivery service - expected: 400-level status code, actual: %d", reqInf.StatusCode)
}
}
+func AssignServersToNonTopologyBasedDeliveryServiceThatUsesMidTier(t *testing.T) {
+ params := url.Values{}
+ params.Set("xmlId", "ds1")
+ dsWithMid, _, err := TOSession.GetDeliveryServicesV30WithHdr(nil, params)
+ if err != nil {
+ t.Fatalf("cannot GET delivery service 'ds1': %s", err.Error())
+ }
+ if len(dsWithMid) != 1 {
+ t.Fatalf("expected one delivery service: 'ds1', actual: %v", len(dsWithMid))
+ }
+ if dsWithMid[0].Topology != nil {
+ t.Fatal("expected delivery service: 'ds1' to have a nil Topology, actual: non-nil")
+ }
+ serversResp, _, err := TOSession.GetServersWithHdr(nil, nil)
+ serversIds := []int{}
+ for _, s := range serversResp.Response {
Review comment:
Should check `err` before trying to use `serversResp`
##########
File path: traffic_ops/testing/api/v3/deliveryserviceservers_test.go
##########
@@ -87,6 +89,51 @@ func AssignServersToTopologyBasedDeliveryService(t *testing.T) {
t.Fatalf("creating deliveryserviceserver assignment for topology-based delivery service - expected: 400-level status code, actual: %d", reqInf.StatusCode)
}
}
+func AssignServersToNonTopologyBasedDeliveryServiceThatUsesMidTier(t *testing.T) {
+ params := url.Values{}
+ params.Set("xmlId", "ds1")
+ dsWithMid, _, err := TOSession.GetDeliveryServicesV30WithHdr(nil, params)
+ if err != nil {
+ t.Fatalf("cannot GET delivery service 'ds1': %s", err.Error())
+ }
+ if len(dsWithMid) != 1 {
+ t.Fatalf("expected one delivery service: 'ds1', actual: %v", len(dsWithMid))
+ }
+ if dsWithMid[0].Topology != nil {
+ t.Fatal("expected delivery service: 'ds1' to have a nil Topology, actual: non-nil")
+ }
+ serversResp, _, err := TOSession.GetServersWithHdr(nil, nil)
+ serversIds := []int{}
+ for _, s := range serversResp.Response {
+ if s.CDNID != nil && *s.CDNID == *dsWithMid[0].CDNID && s.Type == tc.CacheTypeEdge.String() {
+ serversIds = append(serversIds, *s.ID)
+ }
+ }
+ if len(serversIds) < 1 {
+ t.Fatalf("expected: at least one EDGE in cdn %s, actual: 0", *dsWithMid[0].CDNName)
+ }
+
+ _, _, err = TOSession.CreateDeliveryServiceServers(*dsWithMid[0].ID, serversIds, true)
+ if err != nil {
+ t.Errorf("POST delivery service servers: %v", err)
Review comment:
The rest of the function uses `t.Fatalf()`, why use `t.Errorf()` here?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 edited a comment on pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
mitchell852 edited a comment on pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#issuecomment-708838464
> Servers API tests fail
>
> ```go
> [user@computer api]$ time go test ./v3 -test.v -cfg=../conf/traffic-ops-test.conf -fixtures=tc-fixtures.json -test.run '^TestServers$'
> INFO: traffic_ops_test.go:71: 2020-10-14T17:00:48.15509479-06:00: Using Config values:
> TO Config File: ../conf/traffic-ops-test.conf
> TO Fixtures: tc-fixtures.json
> TO URL: https://localhost:6443
> TO Session Timeout In Secs: 60
> DB Server: localhost
> DB User: traffic_ops
> DB Name: traffic_ops
> DB Ssl: false
> UseIMS: true
> === RUN TestServers
> cdns_test.go:89: Response: {[{cdn was created. success}]}
...
> phys_locations_test.go:91: Response: {[{physLocation was created. success}]}
> phys_locations_test.go:91: Response: {[{physLocation was created. success}]}
> phys_locations_test.go:91: Response: {[{physLocation was created. success}]}
> topologies_test.go:56: Response: &{{a topology my-topology [{0 parentCachegroup [] <nil>} {0 secondaryCachegroup [] <nil>} {0 cachegroup3 [0 1] <nil>}] <nil>} {[{topology was created. success}]}}
> topologies_test.go:56: Response: &{{another topology another-topology [{0 parentCachegroup [] <nil>} {0 cachegroup1 [0] <nil>} {0 secondaryCachegroup [] <nil>} {0 cachegroup2 [2] <nil>}] <nil>} {[{topology was created. success}]}}
> topologies_test.go:56: Response: &{{A topology with secondary parents secondary-parents [{0 parentCachegroup [] <nil>} {0 cachegroup1 [0 2] <nil>} {0 secondaryCachegroup [] <nil>} {0 fallback1 [2 0] <nil>} {0 fallback2 [2 0] <nil>}] <nil>} {[{topology was created. success}]}}
> topologies_test.go:56: Response: &{{A 4-tier topology 4-tiers [{0 parentCachegroup [] <nil>} {0 parentCachegroup2 [0] <nil>} {0 cachegroup1 [1] <nil>} {0 secondaryCachegroup [1 0] <nil>} {0 fallback1 [3] <nil>}] <nil>} {[{topology was created. success}]}}
> topologies_test.go:56: Response: &{{This topology stems from 2 ancestors forked-topology [{0 topology-edge-cg-01 [2] <nil>} {0 topology-edge-cg-02 [6] <nil>} {0 topology-mid-cg-01 [3] <nil>} {0 topology-mid-cg-02 [4] <nil>} {0 topology-mid-cg-03 [5] <nil>} {0 topology-mid-cg-04 [] <nil>} {0 topology-mid-cg-05 [7] <nil>} {0 topology-mid-cg-06 [8] <nil>} {0 topology-mid-cg-07 [] <nil>}] <nil>} {[{topology was created. success}]}}
> topologies_test.go:56: Response: &{{a topology top-for-ds-req [{0 dtrc1 [] <nil>} {0 dtrc2 [0] <nil>} {0 dtrc3 [0] <nil>}] <nil>} {[{topology was created. success}]}}
> topologies_test.go:56: Response: &{{a topology top-for-ds-req2 [{0 dtrc1 [] <nil>} {0 dtrc2 [0] <nil>}] <nil>} {[{topology was created. success}]}}
> servers_test.go:246: Response: atlanta-edge-01 {[{Server created success}]}
> servers_test.go:246: Response: cdn2-test-edge {[{Server created success}]}
> servers_test.go:246: Response: influxdb02 {[{Server created success}]}
> servers_test.go:246: Response: atlanta-router-01 {[{Server created success}]}
> servers_test.go:246: Response: atlanta-edge-03 {[{Server created success}]}
> servers_test.go:246: Response: atlanta-edge-14 {[{Server created success}]}
> servers_test.go:246: Response: atlanta-edge-15 {[{Server created success}]}
> servers_test.go:246: Response: atlanta-mid-16 {[{Server created success}]}
> servers_test.go:246: Response: atlanta-mid-17 {[{Server created success}]}
> servers_test.go:246: Response: atlanta-org-1 {[{Server created success}]}
> servers_test.go:246: Response: atlanta-org-2 {[{Server created success}]}
> servers_test.go:246: Response: atlanta-mid-01 {[{Server created success}]}
> servers_test.go:246: Response: rascal01 {[{Server created success}]}
> servers_test.go:246: Response: edge1-cdn2 {[{Server created success}]}
> servers_test.go:246: Response: riak {[{Server created success}]}
> servers_test.go:246: Response: denver-mso-org-01 {[{Server created success}]}
> servers_test.go:246: Response: edge1-cdn1-cg3 {[{Server created success}]}
> servers_test.go:246: Response: edge2-cdn1-cg3 {[{Server created success}]}
> servers_test.go:246: Response: topology-edge-01 {[{Server created success}]}
> servers_test.go:246: Response: topology-edge-02 {[{Server created success}]}
> servers_test.go:246: Response: topology-mid-04 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-mid-01 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-mid-02 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-mid-03 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-edge-01 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-edge-02 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-edge-03 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-edge-04 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-edge-05 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-edge-06 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-edge-07 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-edge-08 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-mid-04 {[{Server created success}]}
> servers_test.go:753: type change update alerts: {Alerts:[]}
> servers_to_deliveryservice_assignment_test.go:74: alerts: {Alerts:[{Text:successfully assigned dses to server Level:success}]}
> servers_to_deliveryservice_assignment_test.go:77: response: [{DeliveryServiceNullableV14:{DeliveryServiceNullableV13:{DeliveryServiceNullableV12:{DeliveryServiceNullableV11:{Active:0xc002038478 AnonymousBlockingEnabled:0xc002038479 CacheURL:0xc00224cd00 CCRDNSTTL:0xc0020384a0 CDNID:0xc0020384b0 CDNName:0xc00224cd10 CheckPath:0xc00224cd20 DisplayName:0xc00224cd30 DNSBypassCNAME:<nil> DNSBypassIP:0xc00224cd40 DNSBypassIP6:0xc00224cd50 DNSBypassTTL:0xc0020384d0 DSCP:0xc0020384e0 EdgeHeaderRewrite:0xc00224cd70 GeoLimit:0xc0020384f0 GeoLimitCountries:0xc00224cd90 GeoLimitRedirectURL:<nil> GeoProvider:0xc0020384f8 GlobalMaxMBPS:0xc002038500 GlobalMaxTPS:0xc002038508 HTTPBypassFQDN:0xc00224cda0 ID:0xc002038510 InfoURL:0xc00224cdb0 InitialDispersion:0xc002038520 IPV6RoutingEnabled:0xc002038528 LastUpdated:0001-01-01 00:00:00 +0000 UTC LogsEnabled:0xc002038529 LongDesc:0xc00224cdc0 LongDesc1:0xc00224cde0 LongDesc2:0xc00224cdf0 MatchList:0xc002234e00 MaxDNSAnswers:0xc002038538 MidHead
erRewrite:0xc00224ce00 MissLat:0xc002038560 MissLong:0xc002038568 MultiSiteOrigin:0xc00203858a OriginShield:<nil> OrgServerFQDN:0xc00224ce10 ProfileDesc:<nil> ProfileID:<nil> ProfileName:<nil> Protocol:0xc002038590 QStringIgnore:0xc002038598 RangeRequestHandling:0xc0020385a0 RegexRemap:0xc00224ce20 RegionalGeoBlocking:0xc0020385af RemapText:0xc00224ce30 RoutingName:0xc00224ce40 Signed:true SSLKeyVersion:0xc0020385c8 TenantID:0xc0020385d0 Type:HTTP TypeID:0xc002038690 XMLID:0xc00224ce60 ExampleURLs:[http://ccr-ds1.ds1.test.cdn1.net https://ccr-ds1.ds1.test.cdn1.net]}} DeepCachingType:NEVER FQPacingRate:0xc0020386e8 SigningAlgorithm:0xc00224ce80 Tenant:0xc00224ce90 TRResponseHeaders:<nil> TRRequestHeaders:<nil>} ConsistentHashRegex:<nil> ConsistentHashQueryParams:[] MaxOriginConnections:0xc002038700} EcsEnabled:false RangeSliceBlockSize:<nil>}]
> servers_test.go:365: expected to get 1 server for Delivery Service: 322, actual: 2
> types_test.go:208: ---- DeleteTestTypes ----
> --- FAIL: TestServers (11.69s)
> FAIL
> FAIL github.com/apache/trafficcontrol/traffic_ops/testing/api/v3 12.020s
> FAIL
> ```
so strange. all passed for me before. checking.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 edited a comment on pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
mitchell852 edited a comment on pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#issuecomment-708838464
> servers_test.go:365: expected to get 1 server for Delivery Service: 322, actual: 2
> types_test.go:208: ---- DeleteTestTypes ----
> --- FAIL: TestServers (11.69s)
> FAIL
> FAIL github.com/apache/trafficcontrol/traffic_ops/testing/api/v3 12.020s
> FAIL
> ```
so strange. all passed for me before. checking.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 commented on a change in pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r505741605
##########
File path: traffic_ops/traffic_ops_golang/server/servers.go
##########
@@ -1009,7 +1013,12 @@ func getMidServers(edgeIDs []int, servers map[int]tc.ServerNullable, tx *sqlx.Tx
WHERE s.id IN (?)))
`
- query, args, err := sqlx.In(q, edgeIDs)
+ if cdnID > 0 {
+ q += ` AND s.cdn_id = ?`
+ filters = append(filters, cdnID)
+ }
Review comment:
for example, if i say "give me all the mids associated with these 5 edges", it possible those 5 edges are in different cdns, so in that case, don't even worry about the cdn. however, it turns out that the only use of this method is in the context of a DS, where you'd always want to filter by cdn...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#issuecomment-709541236
> yeah, looks like a bug. atlanta-mid-17 should not be in there as atlanta-mid-17 is in a different cdn than the ds...
Looking at https://github.com/apache/trafficcontrol/blob/ecb380648ae5854d5ee3cd72fbd32909091b9edb/traffic_ops/testing/api/v3/servers_test.go#L493-L508
the query parameter that is failing in the test is `topology`. In the database, `atlanta-mid-17` is there, so I think it can be safely added to the list.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5128: strips out servers in the wrong cdn if requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r504860567
##########
File path: traffic_ops/traffic_ops_golang/server/servers.go
##########
@@ -782,12 +782,17 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth
usesMids := false
queryAddition := ""
dsHasRequiredCapabilities := false
+ var dsCDNName tc.CDNName
if dsIDStr, ok := params[`dsId`]; ok {
// don't allow query on ds outside user's tenant
dsID, err := strconv.Atoi(dsIDStr)
if err != nil {
return nil, 0, errors.New("dsId must be an integer"), nil, http.StatusNotFound, nil
}
+ _, dsCDNName, _, err = dbhelpers.GetDSNameAndCDNFromID(tx.Tx, dsID)
+ if err != nil {
+ return nil, 0, errors.New("ds not found"), nil, http.StatusNotFound, nil
+ }
Review comment:
Right, but it always should check the CDN when the `dsId` query parameter exists, and all of the SQL for that part is in `deliveryServiceServersJoin` and `dssTopologiesJoinSubquery`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r505775463
##########
File path: traffic_ops/traffic_ops_golang/server/servers.go
##########
@@ -1009,7 +1013,12 @@ func getMidServers(edgeIDs []int, servers map[int]tc.ServerNullable, tx *sqlx.Tx
WHERE s.id IN (?)))
`
- query, args, err := sqlx.In(q, edgeIDs)
+ if cdnID > 0 {
+ q += ` AND s.cdn_id = ?`
+ filters = append(filters, cdnID)
+ }
Review comment:
> sometimes you don't want to filter by cdn...
Oh right, I forgot about that. I was in the `dsId` zone and was only thinking about that part.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman edited a comment on pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
zrhoffman edited a comment on pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#issuecomment-709373021
> honestly, i could not find that in the GHA output....
Neither could I. Your best bet is running the tests locally in CDN-in-a-Box.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 commented on a change in pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r505740139
##########
File path: traffic_ops/traffic_ops_golang/server/servers.go
##########
@@ -1009,7 +1013,12 @@ func getMidServers(edgeIDs []int, servers map[int]tc.ServerNullable, tx *sqlx.Tx
WHERE s.id IN (?)))
`
- query, args, err := sqlx.In(q, edgeIDs)
+ if cdnID > 0 {
+ q += ` AND s.cdn_id = ?`
+ filters = append(filters, cdnID)
+ }
Review comment:
sometimes you don't want to filter by cdn...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r505588906
##########
File path: traffic_ops/testing/api/v3/tc-fixtures.json
##########
@@ -2317,7 +2317,7 @@
"xmppPasswd": "X"
},
{
- "cachegroup": "parentCachegroup",
+ "cachegroup": "parentCachegroup2",
Review comment:
Why does the test fail when `atlanta-mid-17` is in `parentCachegroup`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 commented on pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#issuecomment-709484017
> So, look at whether `atlanta-mid-17` should be included in the result. If it should, add it to the array. If it should not, then there's a bug that should be fixed. I would recommend against simply moving it to a different cachegroup like [1026a81](https://github.com/apache/trafficcontrol/commit/1026a81bb621044a8e6c43b0670c4e5ab13597e6) does.
yeah, looks like a bug. atlanta-mid-17 should not be in there as atlanta-mid-17 is in a different cdn than the ds...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 commented on a change in pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r505663577
##########
File path: traffic_ops/testing/api/v3/tc-fixtures.json
##########
@@ -2317,7 +2317,7 @@
"xmppPasswd": "X"
},
{
- "cachegroup": "parentCachegroup",
+ "cachegroup": "parentCachegroup2",
Review comment:
looking. its hard imo to weed thru all these fixtures and map them to the test cases they are fulfilling. but maybe you're right. maybe it uncovered a different bug...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 commented on a change in pull request #5128: strips out servers in the wrong cdn if requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r504988536
##########
File path: traffic_ops/traffic_ops_golang/server/servers.go
##########
@@ -976,8 +981,13 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth
}
returnable := make([]tc.ServerNullable, 0, len(ids))
+
for _, id := range ids {
server := servers[id]
+ // if requesting servers for a given ds, need to only include those in the same cdn
+ if len(dsCDNName) > 0 && string(dsCDNName) != *server.CDNName {
+ continue
+ }
Review comment:
@zrhoffman - updated. how's that look?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 commented on pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#issuecomment-709348783
@zrhoffman - do you understand why the api tests are failing?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 commented on a change in pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r505686996
##########
File path: traffic_ops/testing/api/v3/tc-fixtures.json
##########
@@ -2317,7 +2317,7 @@
"xmppPasswd": "X"
},
{
- "cachegroup": "parentCachegroup",
+ "cachegroup": "parentCachegroup2",
Review comment:
reverting that.... parentCachegroup is fine for my test...now to figure out why it's breaking the topology test..
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r505018900
##########
File path: traffic_ops/traffic_ops_golang/server/servers.go
##########
@@ -987,19 +994,16 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth
return returnable, serverCount, nil, nil, http.StatusOK, &maxTime
}
-// getMidServers gets the mids used by the servers in this DS.
-//
-// Original comment from the Perl code:
-//
-// If the delivery service employs mids, we're gonna pull mid servers too by
-// pulling the cachegroups of the edges and finding those cachegroups parent
-// cachegroup... then we see which servers have cachegroup in parent cachegroup
-// list...that's how we find mids for the ds :)
-func getMidServers(edgeIDs []int, servers map[int]tc.ServerNullable, tx *sqlx.Tx) ([]int, error, error, int) {
+// getMidServers gets the mids used by the edges provided with an option to filter for a given cdn
+func getMidServers(edgeIDs []int, servers map[int]tc.ServerNullable, cdnID int, tx *sqlx.Tx) ([]int, error, error, int) {
Review comment:
servers unit test fails
```go
[user@computer server]$ go test
# github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/server [github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/server.test]
./servers_test.go:478:48: not enough arguments in call to getMidServers
have ([]int, map[int]tc.ServerNullable, *sqlx.Tx)
want ([]int, map[int]tc.ServerNullable, int, *sqlx.Tx)
FAIL github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/server [build failed]
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 edited a comment on pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
mitchell852 edited a comment on pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#issuecomment-708838464
> Servers API tests fail
>
> ```go
> [user@computer api]$ time go test ./v3 -test.v -cfg=../conf/traffic-ops-test.conf -fixtures=tc-fixtures.json -test.run '^TestServers$'
> INFO: traffic_ops_test.go:71: 2020-10-14T17:00:48.15509479-06:00: Using Config values:
> TO Config File: ../conf/traffic-ops-test.conf
> TO Fixtures: tc-fixtures.json
> TO URL: https://localhost:6443
> TO Session Timeout In Secs: 60
> DB Server: localhost
> DB User: traffic_ops
> DB Name: traffic_ops
> DB Ssl: false
> UseIMS: true
> === RUN TestServers
> cdns_test.go:89: Response: {[{cdn was created. success}]}
...
> servers_test.go:365: expected to get 1 server for Delivery Service: 322, actual: 2
> types_test.go:208: ---- DeleteTestTypes ----
> --- FAIL: TestServers (11.69s)
> FAIL
> FAIL github.com/apache/trafficcontrol/traffic_ops/testing/api/v3 12.020s
> FAIL
> ```
so strange. all passed for me before. checking.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r505731519
##########
File path: traffic_ops/traffic_ops_golang/server/servers_test.go
##########
@@ -475,7 +475,7 @@ func TestGetMidServers(t *testing.T) {
mock.ExpectBegin()
mock.ExpectQuery("SELECT").WillReturnRows(rows2)
- mid, userErr, sysErr, errCode := getMidServers(serverIDs, serverMap, db.MustBegin())
+ mid, userErr, sysErr, errCode := getMidServers(serverIDs, serverMap, 0, db.MustBegin())
Review comment:
This should be an actual CDN ID
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5128: strips out servers in the wrong cdn if requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r504755939
##########
File path: traffic_ops/traffic_ops_golang/server/servers.go
##########
@@ -782,12 +782,17 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth
usesMids := false
queryAddition := ""
dsHasRequiredCapabilities := false
+ var dsCDNName tc.CDNName
if dsIDStr, ok := params[`dsId`]; ok {
// don't allow query on ds outside user's tenant
dsID, err := strconv.Atoi(dsIDStr)
if err != nil {
return nil, 0, errors.New("dsId must be an integer"), nil, http.StatusNotFound, nil
}
+ _, dsCDNName, _, err = dbhelpers.GetDSNameAndCDNFromID(tx.Tx, dsID)
+ if err != nil {
+ return nil, 0, errors.New("ds not found"), nil, http.StatusNotFound, nil
+ }
Review comment:
We should not need to check which CDN the delivery service is in. The query just needs to be adjusted to make sure the servers match the delivery service's CDN.
##########
File path: traffic_ops/traffic_ops_golang/server/servers.go
##########
@@ -976,8 +981,13 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth
}
returnable := make([]tc.ServerNullable, 0, len(ids))
+
for _, id := range ids {
server := servers[id]
+ // if requesting servers for a given ds, need to only include those in the same cdn
+ if len(dsCDNName) > 0 && string(dsCDNName) != *server.CDNName {
+ continue
+ }
Review comment:
Is there a way to perform this check in the query itself?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 commented on a change in pull request #5128: strips out servers in the wrong cdn if requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r504857614
##########
File path: traffic_ops/traffic_ops_golang/server/servers.go
##########
@@ -782,12 +782,17 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth
usesMids := false
queryAddition := ""
dsHasRequiredCapabilities := false
+ var dsCDNName tc.CDNName
if dsIDStr, ok := params[`dsId`]; ok {
// don't allow query on ds outside user's tenant
dsID, err := strconv.Atoi(dsIDStr)
if err != nil {
return nil, 0, errors.New("dsId must be an integer"), nil, http.StatusNotFound, nil
}
+ _, dsCDNName, _, err = dbhelpers.GetDSNameAndCDNFromID(tx.Tx, dsID)
+ if err != nil {
+ return nil, 0, errors.New("ds not found"), nil, http.StatusNotFound, nil
+ }
Review comment:
well, it doesn't always check the cdn. for example:
GET /servers <-- doesn't check cdn
GET /servers?type=ORG <-- doesn't check cdn
GET /servers?dsId=X <-- does check cdn
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 commented on pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#issuecomment-709370401
> Look at the `expectedHostnames` array. If it encounters a host that is not in that array, it fails on line 458. In the excerpt of the API tests I posted:
>
> ```go
> servers_test.go:458: expected hostnames map[atlanta-mid-16:true edge1-cdn1-cg3:true edge2-cdn1-cg3:true], actual atlanta-mid-17 actual:
> ```
>
> It means that the only servers it expected in the response were
>
> * atlanta-mid-16
> * edge1-cdn1-cg3
> * edge2-cdn1-cg3
>
> but it found `atlanta-mid-17` as well, so it failed.
>
> That ` actual:` part at the end of the message is extra and should be removed (my bad).
>
> So, look at whether `atlanta-mid-17` should be included in the result. If it should, add it to the array. If it should not, then there's a bug that should be fixed. I would recommend against simply moving it to a different cachegroup like [1026a81](https://github.com/apache/trafficcontrol/commit/1026a81bb621044a8e6c43b0670c4e5ab13597e6) does.
honestly, i could not find that in the GHA output....
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 commented on a change in pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r505041603
##########
File path: traffic_ops/traffic_ops_golang/server/servers.go
##########
@@ -987,19 +994,16 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth
return returnable, serverCount, nil, nil, http.StatusOK, &maxTime
}
-// getMidServers gets the mids used by the servers in this DS.
-//
-// Original comment from the Perl code:
-//
-// If the delivery service employs mids, we're gonna pull mid servers too by
-// pulling the cachegroups of the edges and finding those cachegroups parent
-// cachegroup... then we see which servers have cachegroup in parent cachegroup
-// list...that's how we find mids for the ds :)
-func getMidServers(edgeIDs []int, servers map[int]tc.ServerNullable, tx *sqlx.Tx) ([]int, error, error, int) {
+// getMidServers gets the mids used by the edges provided with an option to filter for a given cdn
+func getMidServers(edgeIDs []int, servers map[int]tc.ServerNullable, cdnID int, tx *sqlx.Tx) ([]int, error, error, int) {
Review comment:
ah cool. thx
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman merged pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
zrhoffman merged pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 commented on a change in pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r505740139
##########
File path: traffic_ops/traffic_ops_golang/server/servers.go
##########
@@ -1009,7 +1013,12 @@ func getMidServers(edgeIDs []int, servers map[int]tc.ServerNullable, tx *sqlx.Tx
WHERE s.id IN (?)))
`
- query, args, err := sqlx.In(q, edgeIDs)
+ if cdnID > 0 {
+ q += ` AND s.cdn_id = ?`
+ filters = append(filters, cdnID)
+ }
Review comment:
i think you're missing the point here. ONLY filter by cdn if cdn is passed in...
sometimes you don't want to filter by cdn...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#issuecomment-709373021
> honestly, i could not find that in the GHA output....
Neither could I. Your best bet is running the tests locally.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 commented on pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#issuecomment-708838464
> Servers API tests fail
>
> ```go
> [user@computer api]$ time go test ./v3 -test.v -cfg=../conf/traffic-ops-test.conf -fixtures=tc-fixtures.json -test.run '^TestServers$'
> INFO: traffic_ops_test.go:71: 2020-10-14T17:00:48.15509479-06:00: Using Config values:
> TO Config File: ../conf/traffic-ops-test.conf
> TO Fixtures: tc-fixtures.json
> TO URL: https://localhost:6443
> TO Session Timeout In Secs: 60
> DB Server: localhost
> DB User: traffic_ops
> DB Name: traffic_ops
> DB Ssl: false
> UseIMS: true
> === RUN TestServers
> cdns_test.go:89: Response: {[{cdn was created. success}]}
> cdns_test.go:89: Response: {[{cdn was created. success}]}
> cdns_test.go:89: Response: {[{cdn was created. success}]}
> cdns_test.go:89: Response: {[{cdn was created. success}]}
> cdns_test.go:89: Response: {[{cdn was created. success}]}
> types_test.go:89: ---- CreateTestTypes ----
> user_test.go:103: Response: {[{user was created. success}]}
> user_test.go:103: Response: {[{user was created. success}]}
> user_test.go:103: Response: {[{user was created. success}]}
> user_test.go:103: Response: {[{user was created. success}]}
> user_test.go:103: Response: {[{user was created. success}]}
> user_test.go:103: Response: {[{user was created. success}]}
> user_test.go:103: Response: {[{user was created. success}]}
> parameters_test.go:75: Response: {[{param was created. success}]}
> parameters_test.go:75: Response: {[{param was created. success}]}
> parameters_test.go:75: Response: {[{param was created. success}]}
> parameters_test.go:75: Response: {[{param was created. success}]}
> parameters_test.go:75: Response: {[{param was created. success}]}
> parameters_test.go:75: Response: {[{param was created. success}]}
> parameters_test.go:75: Response: {[{param was created. success}]}
> parameters_test.go:75: Response: {[{param was created. success}]}
> parameters_test.go:75: Response: {[{param was created. success}]}
> parameters_test.go:75: Response: {[{param was created. success}]}
> parameters_test.go:75: Response: {[{param was created. success}]}
> parameters_test.go:75: Response: {[{param was created. success}]}
> parameters_test.go:75: Response: {[{param was created. success}]}
> parameters_test.go:75: Response: {[{param was created. success}]}
> parameters_test.go:75: Response: {[{param was created. success}]}
> parameters_test.go:75: Response: {[{param was created. success}]}
> parameters_test.go:75: Response: {[{param was created. success}]}
> parameters_test.go:75: Response: {[{param was created. success}]}
> parameters_test.go:75: Response: {[{param was created. success}]}
> parameters_test.go:75: Response: {[{param was created. success}]}
> profiles_test.go:162: Response: {[{profile was created. success}]}
> profiles_test.go:162: Response: {[{profile was created. success}]}
> profiles_test.go:162: Response: {[{profile was created. success}]}
> profiles_test.go:162: Response: {[{profile was created. success}]}
> profiles_test.go:162: Response: {[{profile was created. success}]}
> profiles_test.go:162: Response: {[{profile was created. success}]}
> profiles_test.go:162: Response: {[{profile was created. success}]}
> profiles_test.go:162: Response: {[{profile was created. success}]}
> profiles_test.go:162: Response: {[{profile was created. success}]}
> profiles_test.go:162: Response: {[{profile was created. success}]}
> profiles_test.go:162: Response: {[{profile was created. success}]}
> profiles_test.go:162: Response: {[{profile was created. success}]}
> profiles_test.go:162: Response: {[{profile was created. success}]}
> statuses_test.go:98: Response: {[{status was created. success}]}
> statuses_test.go:98: Response: {[{status was created. success}]}
> statuses_test.go:98: Response: {[{status was created. success}]}
> statuses_test.go:98: Response: {[{status was created. success}]}
> statuses_test.go:98: Response: {[{status was created. success}]}
> statuses_test.go:98: Response: {[{status was created. success}]}
> statuses_test.go:98: Response: {[{status was created. success}]}
> divisions_test.go:117: Response: {[{division was created. success}]}
> divisions_test.go:117: Response: {[{division was created. success}]}
> regions_test.go:91: Response: {[{region was created. success}]}
> regions_test.go:91: Response: {[{region was created. success}]}
> phys_locations_test.go:91: Response: {[{physLocation was created. success}]}
> phys_locations_test.go:91: Response: {[{physLocation was created. success}]}
> phys_locations_test.go:91: Response: {[{physLocation was created. success}]}
> topologies_test.go:56: Response: &{{a topology my-topology [{0 parentCachegroup [] <nil>} {0 secondaryCachegroup [] <nil>} {0 cachegroup3 [0 1] <nil>}] <nil>} {[{topology was created. success}]}}
> topologies_test.go:56: Response: &{{another topology another-topology [{0 parentCachegroup [] <nil>} {0 cachegroup1 [0] <nil>} {0 secondaryCachegroup [] <nil>} {0 cachegroup2 [2] <nil>}] <nil>} {[{topology was created. success}]}}
> topologies_test.go:56: Response: &{{A topology with secondary parents secondary-parents [{0 parentCachegroup [] <nil>} {0 cachegroup1 [0 2] <nil>} {0 secondaryCachegroup [] <nil>} {0 fallback1 [2 0] <nil>} {0 fallback2 [2 0] <nil>}] <nil>} {[{topology was created. success}]}}
> topologies_test.go:56: Response: &{{A 4-tier topology 4-tiers [{0 parentCachegroup [] <nil>} {0 parentCachegroup2 [0] <nil>} {0 cachegroup1 [1] <nil>} {0 secondaryCachegroup [1 0] <nil>} {0 fallback1 [3] <nil>}] <nil>} {[{topology was created. success}]}}
> topologies_test.go:56: Response: &{{This topology stems from 2 ancestors forked-topology [{0 topology-edge-cg-01 [2] <nil>} {0 topology-edge-cg-02 [6] <nil>} {0 topology-mid-cg-01 [3] <nil>} {0 topology-mid-cg-02 [4] <nil>} {0 topology-mid-cg-03 [5] <nil>} {0 topology-mid-cg-04 [] <nil>} {0 topology-mid-cg-05 [7] <nil>} {0 topology-mid-cg-06 [8] <nil>} {0 topology-mid-cg-07 [] <nil>}] <nil>} {[{topology was created. success}]}}
> topologies_test.go:56: Response: &{{a topology top-for-ds-req [{0 dtrc1 [] <nil>} {0 dtrc2 [0] <nil>} {0 dtrc3 [0] <nil>}] <nil>} {[{topology was created. success}]}}
> topologies_test.go:56: Response: &{{a topology top-for-ds-req2 [{0 dtrc1 [] <nil>} {0 dtrc2 [0] <nil>}] <nil>} {[{topology was created. success}]}}
> servers_test.go:246: Response: atlanta-edge-01 {[{Server created success}]}
> servers_test.go:246: Response: cdn2-test-edge {[{Server created success}]}
> servers_test.go:246: Response: influxdb02 {[{Server created success}]}
> servers_test.go:246: Response: atlanta-router-01 {[{Server created success}]}
> servers_test.go:246: Response: atlanta-edge-03 {[{Server created success}]}
> servers_test.go:246: Response: atlanta-edge-14 {[{Server created success}]}
> servers_test.go:246: Response: atlanta-edge-15 {[{Server created success}]}
> servers_test.go:246: Response: atlanta-mid-16 {[{Server created success}]}
> servers_test.go:246: Response: atlanta-mid-17 {[{Server created success}]}
> servers_test.go:246: Response: atlanta-org-1 {[{Server created success}]}
> servers_test.go:246: Response: atlanta-org-2 {[{Server created success}]}
> servers_test.go:246: Response: atlanta-mid-01 {[{Server created success}]}
> servers_test.go:246: Response: rascal01 {[{Server created success}]}
> servers_test.go:246: Response: edge1-cdn2 {[{Server created success}]}
> servers_test.go:246: Response: riak {[{Server created success}]}
> servers_test.go:246: Response: denver-mso-org-01 {[{Server created success}]}
> servers_test.go:246: Response: edge1-cdn1-cg3 {[{Server created success}]}
> servers_test.go:246: Response: edge2-cdn1-cg3 {[{Server created success}]}
> servers_test.go:246: Response: topology-edge-01 {[{Server created success}]}
> servers_test.go:246: Response: topology-edge-02 {[{Server created success}]}
> servers_test.go:246: Response: topology-mid-04 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-mid-01 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-mid-02 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-mid-03 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-edge-01 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-edge-02 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-edge-03 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-edge-04 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-edge-05 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-edge-06 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-edge-07 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-edge-08 {[{Server created success}]}
> servers_test.go:246: Response: dtrc-mid-04 {[{Server created success}]}
> servers_test.go:753: type change update alerts: {Alerts:[]}
> servers_to_deliveryservice_assignment_test.go:74: alerts: {Alerts:[{Text:successfully assigned dses to server Level:success}]}
> servers_to_deliveryservice_assignment_test.go:77: response: [{DeliveryServiceNullableV14:{DeliveryServiceNullableV13:{DeliveryServiceNullableV12:{DeliveryServiceNullableV11:{Active:0xc002038478 AnonymousBlockingEnabled:0xc002038479 CacheURL:0xc00224cd00 CCRDNSTTL:0xc0020384a0 CDNID:0xc0020384b0 CDNName:0xc00224cd10 CheckPath:0xc00224cd20 DisplayName:0xc00224cd30 DNSBypassCNAME:<nil> DNSBypassIP:0xc00224cd40 DNSBypassIP6:0xc00224cd50 DNSBypassTTL:0xc0020384d0 DSCP:0xc0020384e0 EdgeHeaderRewrite:0xc00224cd70 GeoLimit:0xc0020384f0 GeoLimitCountries:0xc00224cd90 GeoLimitRedirectURL:<nil> GeoProvider:0xc0020384f8 GlobalMaxMBPS:0xc002038500 GlobalMaxTPS:0xc002038508 HTTPBypassFQDN:0xc00224cda0 ID:0xc002038510 InfoURL:0xc00224cdb0 InitialDispersion:0xc002038520 IPV6RoutingEnabled:0xc002038528 LastUpdated:0001-01-01 00:00:00 +0000 UTC LogsEnabled:0xc002038529 LongDesc:0xc00224cdc0 LongDesc1:0xc00224cde0 LongDesc2:0xc00224cdf0 MatchList:0xc002234e00 MaxDNSAnswers:0xc002038538 MidHead
erRewrite:0xc00224ce00 MissLat:0xc002038560 MissLong:0xc002038568 MultiSiteOrigin:0xc00203858a OriginShield:<nil> OrgServerFQDN:0xc00224ce10 ProfileDesc:<nil> ProfileID:<nil> ProfileName:<nil> Protocol:0xc002038590 QStringIgnore:0xc002038598 RangeRequestHandling:0xc0020385a0 RegexRemap:0xc00224ce20 RegionalGeoBlocking:0xc0020385af RemapText:0xc00224ce30 RoutingName:0xc00224ce40 Signed:true SSLKeyVersion:0xc0020385c8 TenantID:0xc0020385d0 Type:HTTP TypeID:0xc002038690 XMLID:0xc00224ce60 ExampleURLs:[http://ccr-ds1.ds1.test.cdn1.net https://ccr-ds1.ds1.test.cdn1.net]}} DeepCachingType:NEVER FQPacingRate:0xc0020386e8 SigningAlgorithm:0xc00224ce80 Tenant:0xc00224ce90 TRResponseHeaders:<nil> TRRequestHeaders:<nil>} ConsistentHashRegex:<nil> ConsistentHashQueryParams:[] MaxOriginConnections:0xc002038700} EcsEnabled:false RangeSliceBlockSize:<nil>}]
> servers_test.go:365: expected to get 1 server for Delivery Service: 322, actual: 2
> types_test.go:208: ---- DeleteTestTypes ----
> --- FAIL: TestServers (11.69s)
> FAIL
> FAIL github.com/apache/trafficcontrol/traffic_ops/testing/api/v3 12.020s
> FAIL
> ```
so strange. all passed for me before. checking.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r505731519
##########
File path: traffic_ops/traffic_ops_golang/server/servers_test.go
##########
@@ -475,7 +475,7 @@ func TestGetMidServers(t *testing.T) {
mock.ExpectBegin()
mock.ExpectQuery("SELECT").WillReturnRows(rows2)
- mid, userErr, sysErr, errCode := getMidServers(serverIDs, serverMap, db.MustBegin())
+ mid, userErr, sysErr, errCode := getMidServers(serverIDs, serverMap, 0, db.MustBegin())
Review comment:
This should return an actual CDN ID
##########
File path: traffic_ops/traffic_ops_golang/server/servers.go
##########
@@ -1009,7 +1013,12 @@ func getMidServers(edgeIDs []int, servers map[int]tc.ServerNullable, tx *sqlx.Tx
WHERE s.id IN (?)))
`
- query, args, err := sqlx.In(q, edgeIDs)
+ if cdnID > 0 {
+ q += ` AND s.cdn_id = ?`
+ filters = append(filters, cdnID)
+ }
Review comment:
Checking if `cdnID` is greater than 0 should be unnecessary because servers are always in CDNs
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#issuecomment-709305013
`TestServers` API test gets a little further now but still fails. Last 10 lines:
```go
servers_test.go:246: Response: dtrc-mid-04 {[{Server created success}]}
servers_test.go:753: type change update alerts: {Alerts:[]}
servers_to_deliveryservice_assignment_test.go:74: alerts: {Alerts:[{Text:successfully assigned dses to server Level:success}]}
servers_to_deliveryservice_assignment_test.go:77: response: [{DeliveryServiceNullableV14:{DeliveryServiceNullableV13:{DeliveryServiceNullableV12:{DeliveryServiceNullableV11:{Active:0xc000249ad8 AnonymousBlockingEnabled:0xc000249ad9 CacheURL:0xc0020fa9a0 CCRDNSTTL:0xc000249b00 CDNID:0xc000249b10 CDNName:0xc0020fa9c0 CheckPath:0xc0020fa9e0 DisplayName:0xc0020fa9f0 DNSBypassCNAME:<nil> DNSBypassIP:0xc0020faa00 DNSBypassIP6:0xc0020faa10 DNSBypassTTL:0xc000249b30 DSCP:0xc000249b40 EdgeHeaderRewrite:0xc0020faa30 GeoLimit:0xc000249b50 GeoLimitCountries:0xc0020faa40 GeoLimitRedirectURL:<nil> GeoProvider:0xc000249b58 GlobalMaxMBPS:0xc000249b60 GlobalMaxTPS:0xc000249b68 HTTPBypassFQDN:0xc0020faa50 ID:0xc000249b70 InfoURL:0xc0020faa60 InitialDispersion:0xc000249b80 IPV6RoutingEnabled:0xc000249b88 LastUpdated:0001-01-01 00:00:00 +0000 UTC LogsEnabled:0xc000249b89 LongDesc:0xc0020faa70 LongDesc1:0xc0020faa80 LongDesc2:0xc0020faa90 MatchList:0xc00160a640 MaxDNSAnswers:0xc000249b98 MidHeader
Rewrite:0xc0020faaa0 MissLat:0xc000249bc0 MissLong:0xc000249bc8 MultiSiteOrigin:0xc000249bea OriginShield:<nil> OrgServerFQDN:0xc0020faab0 ProfileDesc:<nil> ProfileID:<nil> ProfileName:<nil> Protocol:0xc000249bf0 QStringIgnore:0xc000249bf8 RangeRequestHandling:0xc000249c00 RegexRemap:0xc0020faac0 RegionalGeoBlocking:0xc000249c0f RemapText:0xc0020faad0 RoutingName:0xc0020faae0 Signed:true SSLKeyVersion:0xc000249c28 TenantID:0xc000249c30 Type:HTTP TypeID:0xc000249c40 XMLID:0xc0020fab10 ExampleURLs:[http://ccr-ds1.ds1.test.cdn1.net https://ccr-ds1.ds1.test.cdn1.net]}} DeepCachingType:NEVER FQPacingRate:0xc000249c58 SigningAlgorithm:0xc0020fab40 Tenant:0xc0020fab50 TRResponseHeaders:<nil> TRRequestHeaders:<nil>} ConsistentHashRegex:<nil> ConsistentHashQueryParams:[] MaxOriginConnections:0xc000249c70} EcsEnabled:false RangeSliceBlockSize:<nil>}]
servers_test.go:458: expected hostnames map[atlanta-mid-16:true edge1-cdn1-cg3:true edge2-cdn1-cg3:true], actual atlanta-mid-17 actual:
types_test.go:208: ---- DeleteTestTypes ----
--- FAIL: TestServers (12.39s)
FAIL
FAIL github.com/apache/trafficcontrol/traffic_ops/testing/api/v3 15.930s
FAIL
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r505599105
##########
File path: traffic_ops/testing/api/v3/tc-fixtures.json
##########
@@ -2317,7 +2317,7 @@
"xmppPasswd": "X"
},
{
- "cachegroup": "parentCachegroup",
+ "cachegroup": "parentCachegroup2",
Review comment:
It shouldn't matter that it is used as part of a topology. `atlanta-mid-17` is in CDN `cdn2`. The delivery service used in the test has xmlId `ds-top` and is in CDN `cdn1`. So, `atlanta-mid-17` being returned in the response is a bug because it's in a different CDN, right?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 commented on a change in pull request #5128: strips out servers in the wrong cdn if requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r504857940
##########
File path: traffic_ops/traffic_ops_golang/server/servers.go
##########
@@ -976,8 +981,13 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth
}
returnable := make([]tc.ServerNullable, 0, len(ids))
+
for _, id := range ids {
server := servers[id]
+ // if requesting servers for a given ds, need to only include those in the same cdn
+ if len(dsCDNName) > 0 && string(dsCDNName) != *server.CDNName {
+ continue
+ }
Review comment:
yeah, i think you're right. best if done in the query but i had luck figuring that out. i'll look again.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#issuecomment-709363355
Look at the `expectedHostnames` array. If it encounters a host that is not in that array, it fails on line 458. In the excerpt of the API tests I posted:
```go
servers_test.go:458: expected hostnames map[atlanta-mid-16:true edge1-cdn1-cg3:true edge2-cdn1-cg3:true], actual atlanta-mid-17 actual:
```
It means that the only servers it expected in the response were
* atlanta-mid-16
* edge1-cdn1-cg3
* edge2-cdn1-cg3
but it found `atlanta-mid-17` as well, so it failed.
That ` actual:` part at the end of the message is extra and should be removed (my bad).
So, look at whether `atlanta-mid-17` should be included in the result. If it should, add it to the array. If it should not, then there's a bug that should be fixed. I would recommend against simply moving it to a different cachegroup like 1026a81bb6 does.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#issuecomment-708705354
Servers API tests fail
```go
[user@computer api]$ time go test ./v3 -test.v -cfg=../conf/traffic-ops-test.conf -fixtures=tc-fixtures.json -test.run '^TestServers$'
INFO: traffic_ops_test.go:71: 2020-10-14T17:00:48.15509479-06:00: Using Config values:
TO Config File: ../conf/traffic-ops-test.conf
TO Fixtures: tc-fixtures.json
TO URL: https://localhost:6443
TO Session Timeout In Secs: 60
DB Server: localhost
DB User: traffic_ops
DB Name: traffic_ops
DB Ssl: false
UseIMS: true
=== RUN TestServers
cdns_test.go:89: Response: {[{cdn was created. success}]}
cdns_test.go:89: Response: {[{cdn was created. success}]}
cdns_test.go:89: Response: {[{cdn was created. success}]}
cdns_test.go:89: Response: {[{cdn was created. success}]}
cdns_test.go:89: Response: {[{cdn was created. success}]}
types_test.go:89: ---- CreateTestTypes ----
user_test.go:103: Response: {[{user was created. success}]}
user_test.go:103: Response: {[{user was created. success}]}
user_test.go:103: Response: {[{user was created. success}]}
user_test.go:103: Response: {[{user was created. success}]}
user_test.go:103: Response: {[{user was created. success}]}
user_test.go:103: Response: {[{user was created. success}]}
user_test.go:103: Response: {[{user was created. success}]}
parameters_test.go:75: Response: {[{param was created. success}]}
parameters_test.go:75: Response: {[{param was created. success}]}
parameters_test.go:75: Response: {[{param was created. success}]}
parameters_test.go:75: Response: {[{param was created. success}]}
parameters_test.go:75: Response: {[{param was created. success}]}
parameters_test.go:75: Response: {[{param was created. success}]}
parameters_test.go:75: Response: {[{param was created. success}]}
parameters_test.go:75: Response: {[{param was created. success}]}
parameters_test.go:75: Response: {[{param was created. success}]}
parameters_test.go:75: Response: {[{param was created. success}]}
parameters_test.go:75: Response: {[{param was created. success}]}
parameters_test.go:75: Response: {[{param was created. success}]}
parameters_test.go:75: Response: {[{param was created. success}]}
parameters_test.go:75: Response: {[{param was created. success}]}
parameters_test.go:75: Response: {[{param was created. success}]}
parameters_test.go:75: Response: {[{param was created. success}]}
parameters_test.go:75: Response: {[{param was created. success}]}
parameters_test.go:75: Response: {[{param was created. success}]}
parameters_test.go:75: Response: {[{param was created. success}]}
parameters_test.go:75: Response: {[{param was created. success}]}
profiles_test.go:162: Response: {[{profile was created. success}]}
profiles_test.go:162: Response: {[{profile was created. success}]}
profiles_test.go:162: Response: {[{profile was created. success}]}
profiles_test.go:162: Response: {[{profile was created. success}]}
profiles_test.go:162: Response: {[{profile was created. success}]}
profiles_test.go:162: Response: {[{profile was created. success}]}
profiles_test.go:162: Response: {[{profile was created. success}]}
profiles_test.go:162: Response: {[{profile was created. success}]}
profiles_test.go:162: Response: {[{profile was created. success}]}
profiles_test.go:162: Response: {[{profile was created. success}]}
profiles_test.go:162: Response: {[{profile was created. success}]}
profiles_test.go:162: Response: {[{profile was created. success}]}
profiles_test.go:162: Response: {[{profile was created. success}]}
statuses_test.go:98: Response: {[{status was created. success}]}
statuses_test.go:98: Response: {[{status was created. success}]}
statuses_test.go:98: Response: {[{status was created. success}]}
statuses_test.go:98: Response: {[{status was created. success}]}
statuses_test.go:98: Response: {[{status was created. success}]}
statuses_test.go:98: Response: {[{status was created. success}]}
statuses_test.go:98: Response: {[{status was created. success}]}
divisions_test.go:117: Response: {[{division was created. success}]}
divisions_test.go:117: Response: {[{division was created. success}]}
regions_test.go:91: Response: {[{region was created. success}]}
regions_test.go:91: Response: {[{region was created. success}]}
phys_locations_test.go:91: Response: {[{physLocation was created. success}]}
phys_locations_test.go:91: Response: {[{physLocation was created. success}]}
phys_locations_test.go:91: Response: {[{physLocation was created. success}]}
topologies_test.go:56: Response: &{{a topology my-topology [{0 parentCachegroup [] <nil>} {0 secondaryCachegroup [] <nil>} {0 cachegroup3 [0 1] <nil>}] <nil>} {[{topology was created. success}]}}
topologies_test.go:56: Response: &{{another topology another-topology [{0 parentCachegroup [] <nil>} {0 cachegroup1 [0] <nil>} {0 secondaryCachegroup [] <nil>} {0 cachegroup2 [2] <nil>}] <nil>} {[{topology was created. success}]}}
topologies_test.go:56: Response: &{{A topology with secondary parents secondary-parents [{0 parentCachegroup [] <nil>} {0 cachegroup1 [0 2] <nil>} {0 secondaryCachegroup [] <nil>} {0 fallback1 [2 0] <nil>} {0 fallback2 [2 0] <nil>}] <nil>} {[{topology was created. success}]}}
topologies_test.go:56: Response: &{{A 4-tier topology 4-tiers [{0 parentCachegroup [] <nil>} {0 parentCachegroup2 [0] <nil>} {0 cachegroup1 [1] <nil>} {0 secondaryCachegroup [1 0] <nil>} {0 fallback1 [3] <nil>}] <nil>} {[{topology was created. success}]}}
topologies_test.go:56: Response: &{{This topology stems from 2 ancestors forked-topology [{0 topology-edge-cg-01 [2] <nil>} {0 topology-edge-cg-02 [6] <nil>} {0 topology-mid-cg-01 [3] <nil>} {0 topology-mid-cg-02 [4] <nil>} {0 topology-mid-cg-03 [5] <nil>} {0 topology-mid-cg-04 [] <nil>} {0 topology-mid-cg-05 [7] <nil>} {0 topology-mid-cg-06 [8] <nil>} {0 topology-mid-cg-07 [] <nil>}] <nil>} {[{topology was created. success}]}}
topologies_test.go:56: Response: &{{a topology top-for-ds-req [{0 dtrc1 [] <nil>} {0 dtrc2 [0] <nil>} {0 dtrc3 [0] <nil>}] <nil>} {[{topology was created. success}]}}
topologies_test.go:56: Response: &{{a topology top-for-ds-req2 [{0 dtrc1 [] <nil>} {0 dtrc2 [0] <nil>}] <nil>} {[{topology was created. success}]}}
servers_test.go:246: Response: atlanta-edge-01 {[{Server created success}]}
servers_test.go:246: Response: cdn2-test-edge {[{Server created success}]}
servers_test.go:246: Response: influxdb02 {[{Server created success}]}
servers_test.go:246: Response: atlanta-router-01 {[{Server created success}]}
servers_test.go:246: Response: atlanta-edge-03 {[{Server created success}]}
servers_test.go:246: Response: atlanta-edge-14 {[{Server created success}]}
servers_test.go:246: Response: atlanta-edge-15 {[{Server created success}]}
servers_test.go:246: Response: atlanta-mid-16 {[{Server created success}]}
servers_test.go:246: Response: atlanta-mid-17 {[{Server created success}]}
servers_test.go:246: Response: atlanta-org-1 {[{Server created success}]}
servers_test.go:246: Response: atlanta-org-2 {[{Server created success}]}
servers_test.go:246: Response: atlanta-mid-01 {[{Server created success}]}
servers_test.go:246: Response: rascal01 {[{Server created success}]}
servers_test.go:246: Response: edge1-cdn2 {[{Server created success}]}
servers_test.go:246: Response: riak {[{Server created success}]}
servers_test.go:246: Response: denver-mso-org-01 {[{Server created success}]}
servers_test.go:246: Response: edge1-cdn1-cg3 {[{Server created success}]}
servers_test.go:246: Response: edge2-cdn1-cg3 {[{Server created success}]}
servers_test.go:246: Response: topology-edge-01 {[{Server created success}]}
servers_test.go:246: Response: topology-edge-02 {[{Server created success}]}
servers_test.go:246: Response: topology-mid-04 {[{Server created success}]}
servers_test.go:246: Response: dtrc-mid-01 {[{Server created success}]}
servers_test.go:246: Response: dtrc-mid-02 {[{Server created success}]}
servers_test.go:246: Response: dtrc-mid-03 {[{Server created success}]}
servers_test.go:246: Response: dtrc-edge-01 {[{Server created success}]}
servers_test.go:246: Response: dtrc-edge-02 {[{Server created success}]}
servers_test.go:246: Response: dtrc-edge-03 {[{Server created success}]}
servers_test.go:246: Response: dtrc-edge-04 {[{Server created success}]}
servers_test.go:246: Response: dtrc-edge-05 {[{Server created success}]}
servers_test.go:246: Response: dtrc-edge-06 {[{Server created success}]}
servers_test.go:246: Response: dtrc-edge-07 {[{Server created success}]}
servers_test.go:246: Response: dtrc-edge-08 {[{Server created success}]}
servers_test.go:246: Response: dtrc-mid-04 {[{Server created success}]}
servers_test.go:753: type change update alerts: {Alerts:[]}
servers_to_deliveryservice_assignment_test.go:74: alerts: {Alerts:[{Text:successfully assigned dses to server Level:success}]}
servers_to_deliveryservice_assignment_test.go:77: response: [{DeliveryServiceNullableV14:{DeliveryServiceNullableV13:{DeliveryServiceNullableV12:{DeliveryServiceNullableV11:{Active:0xc002038478 AnonymousBlockingEnabled:0xc002038479 CacheURL:0xc00224cd00 CCRDNSTTL:0xc0020384a0 CDNID:0xc0020384b0 CDNName:0xc00224cd10 CheckPath:0xc00224cd20 DisplayName:0xc00224cd30 DNSBypassCNAME:<nil> DNSBypassIP:0xc00224cd40 DNSBypassIP6:0xc00224cd50 DNSBypassTTL:0xc0020384d0 DSCP:0xc0020384e0 EdgeHeaderRewrite:0xc00224cd70 GeoLimit:0xc0020384f0 GeoLimitCountries:0xc00224cd90 GeoLimitRedirectURL:<nil> GeoProvider:0xc0020384f8 GlobalMaxMBPS:0xc002038500 GlobalMaxTPS:0xc002038508 HTTPBypassFQDN:0xc00224cda0 ID:0xc002038510 InfoURL:0xc00224cdb0 InitialDispersion:0xc002038520 IPV6RoutingEnabled:0xc002038528 LastUpdated:0001-01-01 00:00:00 +0000 UTC LogsEnabled:0xc002038529 LongDesc:0xc00224cdc0 LongDesc1:0xc00224cde0 LongDesc2:0xc00224cdf0 MatchList:0xc002234e00 MaxDNSAnswers:0xc002038538 MidHeader
Rewrite:0xc00224ce00 MissLat:0xc002038560 MissLong:0xc002038568 MultiSiteOrigin:0xc00203858a OriginShield:<nil> OrgServerFQDN:0xc00224ce10 ProfileDesc:<nil> ProfileID:<nil> ProfileName:<nil> Protocol:0xc002038590 QStringIgnore:0xc002038598 RangeRequestHandling:0xc0020385a0 RegexRemap:0xc00224ce20 RegionalGeoBlocking:0xc0020385af RemapText:0xc00224ce30 RoutingName:0xc00224ce40 Signed:true SSLKeyVersion:0xc0020385c8 TenantID:0xc0020385d0 Type:HTTP TypeID:0xc002038690 XMLID:0xc00224ce60 ExampleURLs:[http://ccr-ds1.ds1.test.cdn1.net https://ccr-ds1.ds1.test.cdn1.net]}} DeepCachingType:NEVER FQPacingRate:0xc0020386e8 SigningAlgorithm:0xc00224ce80 Tenant:0xc00224ce90 TRResponseHeaders:<nil> TRRequestHeaders:<nil>} ConsistentHashRegex:<nil> ConsistentHashQueryParams:[] MaxOriginConnections:0xc002038700} EcsEnabled:false RangeSliceBlockSize:<nil>}]
servers_test.go:365: expected to get 1 server for Delivery Service: 322, actual: 2
types_test.go:208: ---- DeleteTestTypes ----
--- FAIL: TestServers (11.69s)
FAIL
FAIL github.com/apache/trafficcontrol/traffic_ops/testing/api/v3 12.020s
FAIL
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 commented on a change in pull request #5128: strips out servers in the wrong cdn if requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r504857940
##########
File path: traffic_ops/traffic_ops_golang/server/servers.go
##########
@@ -976,8 +981,13 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth
}
returnable := make([]tc.ServerNullable, 0, len(ids))
+
for _, id := range ids {
server := servers[id]
+ // if requesting servers for a given ds, need to only include those in the same cdn
+ if len(dsCDNName) > 0 && string(dsCDNName) != *server.CDNName {
+ continue
+ }
Review comment:
yeah, i think you're right. best if done in the query but i had no luck figuring that out. i'll look again.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 commented on a change in pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r505595315
##########
File path: traffic_ops/testing/api/v3/tc-fixtures.json
##########
@@ -2317,7 +2317,7 @@
"xmppPasswd": "X"
},
{
- "cachegroup": "parentCachegroup",
+ "cachegroup": "parentCachegroup2",
Review comment:
parentCachegroup was used as part of a topology so i just put it in another cg so not to mess w/ that test. but all passes now.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 commented on a change in pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r505110840
##########
File path: traffic_ops/testing/api/v3/deliveryserviceservers_test.go
##########
@@ -87,6 +89,51 @@ func AssignServersToTopologyBasedDeliveryService(t *testing.T) {
t.Fatalf("creating deliveryserviceserver assignment for topology-based delivery service - expected: 400-level status code, actual: %d", reqInf.StatusCode)
}
}
+func AssignServersToNonTopologyBasedDeliveryServiceThatUsesMidTier(t *testing.T) {
+ params := url.Values{}
+ params.Set("xmlId", "ds1")
+ dsWithMid, _, err := TOSession.GetDeliveryServicesV30WithHdr(nil, params)
+ if err != nil {
+ t.Fatalf("cannot GET delivery service 'ds1': %s", err.Error())
+ }
+ if len(dsWithMid) != 1 {
+ t.Fatalf("expected one delivery service: 'ds1', actual: %v", len(dsWithMid))
+ }
+ if dsWithMid[0].Topology != nil {
+ t.Fatal("expected delivery service: 'ds1' to have a nil Topology, actual: non-nil")
+ }
+ serversResp, _, err := TOSession.GetServersWithHdr(nil, nil)
+ serversIds := []int{}
+ for _, s := range serversResp.Response {
+ if s.CDNID != nil && *s.CDNID == *dsWithMid[0].CDNID && s.Type == tc.CacheTypeEdge.String() {
+ serversIds = append(serversIds, *s.ID)
+ }
+ }
+ if len(serversIds) < 1 {
+ t.Fatalf("expected: at least one EDGE in cdn %s, actual: 0", *dsWithMid[0].CDNName)
+ }
+
+ _, _, err = TOSession.CreateDeliveryServiceServers(*dsWithMid[0].ID, serversIds, true)
+ if err != nil {
+ t.Errorf("POST delivery service servers: %v", err)
Review comment:
good question
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 commented on a change in pull request #5128: strips out mids in the wrong cdn when requesting servers for a ds
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on a change in pull request #5128:
URL: https://github.com/apache/trafficcontrol/pull/5128#discussion_r505744422
##########
File path: traffic_ops/traffic_ops_golang/server/servers.go
##########
@@ -1009,7 +1013,12 @@ func getMidServers(edgeIDs []int, servers map[int]tc.ServerNullable, tx *sqlx.Tx
WHERE s.id IN (?)))
`
- query, args, err := sqlx.In(q, edgeIDs)
+ if cdnID > 0 {
+ q += ` AND s.cdn_id = ?`
+ filters = append(filters, cdnID)
+ }
Review comment:
this problem imo all stems back to the fact that servers from different cdns can live in the same cachegroup...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org