From adbdd1ef6fcb1c9e0f19be9db8ec4a591c09ae32 Mon Sep 17 00:00:00 2001 From: "Chatterjee, Srijeet" Date: Mon, 30 Oct 2023 10:57:32 -0600 Subject: [PATCH] * 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 --- CHANGELOG.md | 1 + lib/go-tc/deliveryservice_servers.go | 1 + .../api/v3/servers_id_deliveryservices_test.go | 2 +- .../api/v4/servers_id_deliveryservices_test.go | 2 +- .../api/v5/cachegroupsdeliveryservices_test.go | 4 +--- .../api/v5/servers_id_deliveryservices_test.go | 2 +- .../deliveryservice/servers/servers.go | 15 +++++++++++++-- .../server/servers_assignment.go | 2 +- .../TableAssignDSServersController.js | 2 +- 9 files changed, 21 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3044683b48..47f45fd116 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). ### 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 773a60ccc0..93d5fdc0b5 100644 --- a/lib/go-tc/deliveryservice_servers.go +++ b/lib/go-tc/deliveryservice_servers.go @@ -430,6 +430,7 @@ func (server DSServerV4) Upgrade() DSServerV5 { 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 972097252c..592df8d4da 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 @@ func validateServersDeliveryServicesPost(serverID int, expectedDSID []int, expec 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 49e38eb314..594fd52aaf 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 @@ func validateServersDeliveryServicesPost(serverID int, expectedDSID []int, expec 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 2ded463d34..919fdf3b7c 100644 --- a/traffic_ops/testing/api/v5/cachegroupsdeliveryservices_test.go +++ b/traffic_ops/testing/api/v5/cachegroupsdeliveryservices_test.go @@ -42,10 +42,8 @@ func TestCacheGroupsDeliveryServices(t *testing.T) { 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 307dda3870..1ab7e5b840 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 @@ func validateServersDeliveryServicesPost(serverID int, expectedDSID []int, expec 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 b0695cb7a6..f9b0e7d24a 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 @@ func (dss *TODSSDeliveryService) Read(h http.Header, useIMS bool) ([]interface{} } 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 @@ ds.id in ( 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 73d45328e8..da9c9e7c5f 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 @@ func ValidateDSCapabilities(dsIDs []int, serverName string, tx *sql.Tx) (error, 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 ce6bbbf7b6..20ffd2401d 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 @@ var TableAssignDSServersController = function(deliveryService, servers, assigned }, { headerName: "Cache Group", - field: "cachegroup", + field: "cacheGroup", }, { headerName: "Profile(s)",