You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by mi...@apache.org on 2020/08/06 22:04:11 UTC

[trafficcontrol] branch master updated: Update status: Support more than 1 server for a given hostname (#4936)

This is an automated email from the ASF dual-hosted git repository.

mitchell852 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git


The following commit(s) were added to refs/heads/master by this push:
     new d04111c  Update status: Support more than 1 server for a given hostname (#4936)
d04111c is described below

commit d04111c107485f663a21ffaa1d262e633235b5ba
Author: Zach Hoffman <za...@zrhoffman.net>
AuthorDate: Thu Aug 6 22:03:56 2020 +0000

    Update status: Support more than 1 server for a given hostname (#4936)
    
    * Use null cachegroup names in recursive CTE base queries to avoid the
    need to filter out those cachegroups later
    
    * Show status pending statuses of ancestors based on the original server's
    ID, which supports accurate results when servers share a hostname
---
 docs/source/api/v3/servers_hostname_update_status.rst  |  2 +-
 traffic_ops/testing/api/v3/serverupdatestatus_test.go  | 18 ++++++++++++++++++
 traffic_ops/traffic_ops_golang/server/put_status.go    |  4 +---
 .../traffic_ops_golang/server/servers_update_status.go | 14 ++++++--------
 4 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/docs/source/api/v3/servers_hostname_update_status.rst b/docs/source/api/v3/servers_hostname_update_status.rst
index 767d3f7..022ad2c 100644
--- a/docs/source/api/v3/servers_hostname_update_status.rst
+++ b/docs/source/api/v3/servers_hostname_update_status.rst
@@ -92,4 +92,4 @@ Each object in the returned array\ [1]_ will contain the following fields:
 		"parent_reval_pending": false
 	}]
 
-.. [1] Despite that the returned object is an array, exactly one server's information is requested and thus returned. That is to say, the array should always have a length of exactly one.
+.. [1] The returned object is an array, and there is no guarantee that one server exists for a given hostname. However, for each server in the array, that server's update status will be accurate for the server with that particular server ID.
diff --git a/traffic_ops/testing/api/v3/serverupdatestatus_test.go b/traffic_ops/testing/api/v3/serverupdatestatus_test.go
index 8c00de9..6238c98 100644
--- a/traffic_ops/testing/api/v3/serverupdatestatus_test.go
+++ b/traffic_ops/testing/api/v3/serverupdatestatus_test.go
@@ -482,5 +482,23 @@ func TestSetTopologiesServerUpdateStatuses(t *testing.T) {
 		if updateStatusByCacheGroup[otherEdgeCacheGroup].ParentPending {
 			t.Fatalf("expected UpdPending: %t, actual: %t", false, updateStatusByCacheGroup[otherEdgeCacheGroup].ParentPending)
 		}
+
+		edgeHostName := *cachesByCacheGroup[edgeCacheGroup].HostName
+		*cachesByCacheGroup[edgeCacheGroup].HostName = *cachesByCacheGroup[midCacheGroup].HostName
+		_, _, err = TOSession.UpdateServerByID(*cachesByCacheGroup[edgeCacheGroup].ID, cachesByCacheGroup[edgeCacheGroup])
+		if err != nil {
+			t.Fatalf("unable to update %s's hostname to %s: %s", edgeHostName, *cachesByCacheGroup[midCacheGroup].HostName, err)
+		}
+
+		_, _, err = TOSession.GetServerUpdateStatus(*cachesByCacheGroup[midCacheGroup].HostName, nil)
+		if err != nil {
+			t.Fatalf("expected no error getting server updates for a non-unique hostname %s, got %s", *cachesByCacheGroup[midCacheGroup].HostName, err)
+		}
+
+		*cachesByCacheGroup[edgeCacheGroup].HostName = edgeHostName
+		_, _, err = TOSession.UpdateServerByID(*cachesByCacheGroup[edgeCacheGroup].ID, cachesByCacheGroup[edgeCacheGroup])
+		if err != nil {
+			t.Fatalf("unable to revert %s's hostname back to %s: %s", edgeHostName, edgeHostName, err)
+		}
 	})
 }
diff --git a/traffic_ops/traffic_ops_golang/server/put_status.go b/traffic_ops/traffic_ops_golang/server/put_status.go
index fa8f2a2..14c7c00 100644
--- a/traffic_ops/traffic_ops_golang/server/put_status.go
+++ b/traffic_ops/traffic_ops_golang/server/put_status.go
@@ -115,7 +115,7 @@ WITH RECURSIVE topology_descendants AS (
 /* This is the base case of the recursive CTE, the topology node for the
  * cachegroup containing cachegroup $2.
  */
-	SELECT tcp.parent child, tc.cachegroup
+	SELECT tcp.parent child, NULL cachegroup
 	FROM cachegroup c
 	JOIN topology_cachegroup tc ON c."name" = tc.cachegroup
 	JOIN topology_cachegroup_parents tcp ON tc.id = tcp.parent
@@ -133,8 +133,6 @@ UNION ALL
 SELECT c.id
 FROM cachegroup c
 JOIN topology_descendants td ON c."name" = td.cachegroup
-/* Filter out cachegroup id $2 */
-WHERE c.id != $2
 )
 UPDATE server
 SET upd_pending = TRUE
diff --git a/traffic_ops/traffic_ops_golang/server/servers_update_status.go b/traffic_ops/traffic_ops_golang/server/servers_update_status.go
index 6372d51..392d997 100644
--- a/traffic_ops/traffic_ops_golang/server/servers_update_status.go
+++ b/traffic_ops/traffic_ops_golang/server/servers_update_status.go
@@ -57,7 +57,7 @@ WITH RECURSIVE topology_ancestors AS (
 /* This is the base case of the recursive CTE, the topology node for the
  * cachegroup containing server $4.
  */
-	SELECT tcp.child parent, tc.cachegroup
+	SELECT tcp.child parent, NULL cachegroup, s.id base_server_id
 	FROM "server" s
 	JOIN cachegroup c ON s.cachegroup = c.id
 	JOIN topology_cachegroup tc ON c."name" = tc.cachegroup
@@ -65,7 +65,7 @@ WITH RECURSIVE topology_ancestors AS (
 	WHERE s.host_name = $4
 UNION ALL
 /* Find all direct topology parent nodes tc of a given topology ancestor ta. */
-	SELECT tcp.parent, tc.cachegroup
+	SELECT tcp.parent, tc.cachegroup, ta.base_server_id
 	FROM topology_ancestors ta, topology_cachegroup_parents tcp
 	JOIN topology_cachegroup tc ON tcp.parent = tc.id
 	WHERE ta.parent = tcp.child
@@ -73,12 +73,10 @@ UNION ALL
  * ancestor topology node found by topology_ancestors.
  */
 ), server_topology_ancestors AS (
-SELECT s.id, s.cachegroup, s.cdn_id, s.upd_pending, s.reval_pending, s.status
+SELECT s.id, s.cachegroup, s.cdn_id, s.upd_pending, s.reval_pending, s.status, ta.base_server_id
 FROM server s
 JOIN cachegroup c ON s.cachegroup = c.id
 JOIN topology_ancestors ta ON c."name" = ta.cachegroup
-/* Filter out cachegroup of host_name $4 */
-WHERE s.cachegroup != (SELECT s.cachegroup FROM server s WHERE s.host_name = $4)
 ), parentservers AS (
 	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending, ps.status
 		FROM server ps
@@ -101,12 +99,12 @@ SELECT
 	status.name AS status,
 		/* True if the cachegroup parent or any ancestor topology node has pending updates. */
 		TRUE IN (
-			SELECT sta.upd_pending FROM server_topology_ancestors sta
+			SELECT sta.upd_pending FROM server_topology_ancestors sta WHERE sta.base_server_id = s.id
 			UNION SELECT COALESCE(BOOL_OR(ps.upd_pending), FALSE)
 		) AS parent_upd_pending,
 		/* True if the cachegroup parent or any ancestor topology node has pending revalidation. */
 		TRUE IN (
-			SELECT sta.reval_pending FROM server_topology_ancestors sta
+			SELECT sta.reval_pending FROM server_topology_ancestors sta WHERE sta.base_server_id = s.id
 			UNION SELECT COALESCE(BOOL_OR(ps.reval_pending), FALSE)
 		) AS parent_reval_pending
 	FROM use_reval_pending,
@@ -206,7 +204,7 @@ ORDER BY s.id
 	var rows *sql.Rows
 	var err error
 	if hostName == "all" {
-		rows, err = tx.Query(baseSelectStatement + groupBy, tc.CacheStatusOffline, tc.UseRevalPendingParameterName, tc.GlobalConfigFileName)
+		rows, err = tx.Query(baseSelectStatement+groupBy, tc.CacheStatusOffline, tc.UseRevalPendingParameterName, tc.GlobalConfigFileName)
 		if err != nil {
 			log.Errorf("could not execute select server update status query: %s\n", err)
 			return nil, tc.DBError