Fix the assignment of a delivery service to a server, to respect capabilities (#7878)
* Fix the assignment of a delivery service to a server, to respect capabilities
* adding changelog
* fixing Traffic Portal to account for the correct field names
* adding org server edge case
Co-authored-by: Chatterjee, Srijeet <srijeet_chatterjee@comcast.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8e59c61..8fb537f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,7 @@
### Fixed
- [#7846](https://github.com/apache/trafficcontrol/pull/7846) *Traffic Portal* Increase State character limit
+- [#7878](https://github.com/apache/trafficcontrol/pull/7878) *Traffic Ops, Traffic Portal* Fixed the case where TO was failing to assign delivery services to a server, due to a bug in the way the list of preexisting delivery services was being returned.
## [8.0.0] - 2023-09-20
### Added
diff --git a/lib/go-tc/deliveryservice_servers.go b/lib/go-tc/deliveryservice_servers.go
index 773a60c..93d5fdc 100644
--- a/lib/go-tc/deliveryservice_servers.go
+++ b/lib/go-tc/deliveryservice_servers.go
@@ -430,6 +430,7 @@
MgmtIPGateway: server.MgmtIPGateway,
MgmtIPNetmask: server.MgmtIPNetmask,
OfflineReason: server.OfflineReason,
+ Profiles: server.ProfileNames,
PhysicalLocation: util.CoalesceToDefault(server.PhysLocation),
PhysicalLocationID: util.CoalesceToDefault(server.PhysLocationID),
Rack: server.Rack,
diff --git a/traffic_ops/testing/api/v3/servers_id_deliveryservices_test.go b/traffic_ops/testing/api/v3/servers_id_deliveryservices_test.go
index 9720972..592df8d 100644
--- a/traffic_ops/testing/api/v3/servers_id_deliveryservices_test.go
+++ b/traffic_ops/testing/api/v3/servers_id_deliveryservices_test.go
@@ -182,7 +182,7 @@
return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) {
serverDeliveryServices, _, err := TOSession.GetServerIDDeliveryServicesWithHdr(serverID, nil)
assert.RequireNoError(t, err, "Error getting Server Delivery Services: %v", err)
- assert.RequireEqual(t, expectedDSCount, len(serverDeliveryServices), "Expected one Delivery Service returned Got: %d", len(serverDeliveryServices))
+ assert.RequireEqual(t, expectedDSCount, len(serverDeliveryServices), "Expected %d Delivery Service returned Got: %d", expectedDSCount, len(serverDeliveryServices))
for i := 0; i < len(expectedDSID); i++ {
validateServersDeliveryServices(expectedDSID[i])(t, toclientlib.ReqInf{}, serverDeliveryServices, tc.Alerts{}, nil)
}
diff --git a/traffic_ops/testing/api/v4/servers_id_deliveryservices_test.go b/traffic_ops/testing/api/v4/servers_id_deliveryservices_test.go
index 49e38eb..594fd52 100644
--- a/traffic_ops/testing/api/v4/servers_id_deliveryservices_test.go
+++ b/traffic_ops/testing/api/v4/servers_id_deliveryservices_test.go
@@ -193,7 +193,7 @@
return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) {
serverDeliveryServices, _, err := TOSession.GetServerIDDeliveryServices(serverID, client.RequestOptions{})
assert.RequireNoError(t, err, "Error getting Server Delivery Services: %v - alerts: %+v", err, serverDeliveryServices.Alerts)
- assert.RequireEqual(t, expectedDSCount, len(serverDeliveryServices.Response), "Expected Two Delivery Service returned Got: %d", len(serverDeliveryServices.Response))
+ assert.RequireEqual(t, expectedDSCount, len(serverDeliveryServices.Response), "Expected %d Delivery Service returned Got: %d", expectedDSCount, len(serverDeliveryServices.Response))
for i := 0; i < len(expectedDSID); i++ {
validateServersDeliveryServices(expectedDSID[i])(t, toclientlib.ReqInf{}, serverDeliveryServices.Response, tc.Alerts{}, nil)
diff --git a/traffic_ops/testing/api/v5/cachegroupsdeliveryservices_test.go b/traffic_ops/testing/api/v5/cachegroupsdeliveryservices_test.go
index 2ded463..919fdf3 100644
--- a/traffic_ops/testing/api/v5/cachegroupsdeliveryservices_test.go
+++ b/traffic_ops/testing/api/v5/cachegroupsdeliveryservices_test.go
@@ -42,10 +42,8 @@
EndpointID: GetCacheGroupId(t, "cachegroup3"),
ClientSession: TOSession,
RequestBody: []int{
- GetDeliveryServiceId(t, "ds1")(),
- GetDeliveryServiceId(t, "ds2")(),
GetDeliveryServiceId(t, "ds3")(),
- GetDeliveryServiceId(t, "ds3")(),
+ GetDeliveryServiceId(t, "ds4")(),
GetDeliveryServiceId(t, "DS5")(),
},
Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateCGDSServerAssignments()),
diff --git a/traffic_ops/testing/api/v5/servers_id_deliveryservices_test.go b/traffic_ops/testing/api/v5/servers_id_deliveryservices_test.go
index 307dda3..1ab7e5b 100644
--- a/traffic_ops/testing/api/v5/servers_id_deliveryservices_test.go
+++ b/traffic_ops/testing/api/v5/servers_id_deliveryservices_test.go
@@ -192,7 +192,7 @@
return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) {
serverDeliveryServices, _, err := TOSession.GetServerIDDeliveryServices(serverID, client.RequestOptions{})
assert.RequireNoError(t, err, "Error getting Server Delivery Services: %v - alerts: %+v", err, serverDeliveryServices.Alerts)
- assert.RequireEqual(t, expectedDSCount, len(serverDeliveryServices.Response), "Expected Two Delivery Service returned Got: %d", len(serverDeliveryServices.Response))
+ assert.RequireEqual(t, expectedDSCount, len(serverDeliveryServices.Response), "Expected %d Delivery Service returned Got: %d", expectedDSCount, len(serverDeliveryServices.Response))
for i := 0; i < len(expectedDSID); i++ {
validateServersDeliveryServices(expectedDSID[i])(t, toclientlib.ReqInf{}, serverDeliveryServices.Response, tc.Alerts{}, nil)
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go b/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go
index b0695cb..f9b0e7d 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go
@@ -945,7 +945,7 @@
}
where += `
-ds.id in (
+(ds.id in (
SELECT deliveryService FROM deliveryservice_server WHERE server = :server
) OR ds.id in (
SELECT id FROM deliveryservice
@@ -955,7 +955,18 @@
SELECT name FROM cachegroup
WHERE id = (
SELECT cachegroup FROM server WHERE id = :server
- ))))
+ )))))
+AND
+((
+(SELECT (t.name = 'ORG') FROM type t JOIN server s ON s.type = t.id WHERE s.id = :server)
+OR
+(SELECT COALESCE(ARRAY_AGG(ssc.server_capability), '{}')
+FROM server_server_capability ssc
+WHERE ssc."server" = :server)
+@>
+(
+SELECT COALESCE(ds.required_capabilities, '{}')
+)))
`
tenantIDs, err := tenant.GetUserTenantIDListTx(tx, user.TenantID)
diff --git a/traffic_ops/traffic_ops_golang/server/servers_assignment.go b/traffic_ops/traffic_ops_golang/server/servers_assignment.go
index 73d4532..da9c9e7 100644
--- a/traffic_ops/traffic_ops_golang/server/servers_assignment.go
+++ b/traffic_ops/traffic_ops_golang/server/servers_assignment.go
@@ -305,7 +305,7 @@
for id, caps := range dsCaps {
for _, dsrc := range caps {
if !util.ContainsStr(sCaps, dsrc) {
- return errors.New(fmt.Sprintf("cache %s cannot assign delivery service %d without having the required delivery service capabilities: %v", serverName, id, dsCaps)), nil, http.StatusBadRequest
+ return errors.New(fmt.Sprintf("cache %s cannot assign delivery service %d without having the required delivery service capabilities: %v", serverName, id, caps)), nil, http.StatusBadRequest
}
}
}
diff --git a/traffic_portal/app/src/common/modules/table/deliveryServiceServers/TableAssignDSServersController.js b/traffic_portal/app/src/common/modules/table/deliveryServiceServers/TableAssignDSServersController.js
index ce6bbbf..20ffd24 100644
--- a/traffic_portal/app/src/common/modules/table/deliveryServiceServers/TableAssignDSServersController.js
+++ b/traffic_portal/app/src/common/modules/table/deliveryServiceServers/TableAssignDSServersController.js
@@ -33,7 +33,7 @@
},
{
headerName: "Cache Group",
- field: "cachegroup",
+ field: "cacheGroup",
},
{
headerName: "Profile(s)",
