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:
[email protected]
