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/07/21 16:57:02 UTC

[GitHub] [trafficcontrol] zrhoffman opened a new pull request #4901: Consider Topologies parentage when queueing or checking server updates

zrhoffman opened a new pull request #4901:
URL: https://github.com/apache/trafficcontrol/pull/4901


   <!--
   ************ 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.
   -->
   
   - [x] This PR fixes #4847 by adding Topologies support on the following endpoints:
       - `PUT /api/3.0/servers/{id}/status`
       - `GET /api/3.0/servers/{host_name}/update_status`
   <!-- 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. -->
   
   - Documentation
   - Traffic Control Client - Go
   - Traffic Ops
   
   ## 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. -->
   Run the API tests
   ```shell script
   docker-compose -f docker-compose.yml up --build --force-recreate -d
   docker-compose -f docker-compose.yml -f docker-compose.traffic-ops-test.yml up --build --force-recreate integration
   ```
   
   ## 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!
   
   -->
   
   - [x] This PR includes tests
   - [x] This PR includes documentation
   - [x] This PR includes an update to CHANGELOG.md
   - [x] This PR includes any and all required license headers
   - [x] This PR does not include a database migration
   - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://www.apache.org/security/) for details)
   
   <!--
   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 pull request #4901: Consider Topologies parentage when queueing or checking server updates

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on pull request #4901:
URL: https://github.com/apache/trafficcontrol/pull/4901#issuecomment-666654680


   retest this please


----------------------------------------------------------------
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] rob05c commented on pull request #4901: Consider Topologies parentage when queueing or checking server updates

Posted by GitBox <gi...@apache.org>.
rob05c commented on pull request #4901:
URL: https://github.com/apache/trafficcontrol/pull/4901#issuecomment-668276499


   retest this please


----------------------------------------------------------------
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 #4901: Consider Topologies parentage when queueing or checking server updates

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #4901:
URL: https://github.com/apache/trafficcontrol/pull/4901#discussion_r460169600



##########
File path: traffic_ops/traffic_ops_golang/routing/routes.go
##########
@@ -1077,7 +1077,7 @@ func Routes(d ServerData) ([]Route, []RawRoute, http.Handler, error) {
 		//Server status
 		{api.Version{1, 1}, http.MethodPut, `servers/{id}/status$`, server.UpdateStatusHandler, auth.PrivLevelOperations, Authenticated, nil, 776663851, perlBypass},
 		{api.Version{1, 1}, http.MethodPost, `servers/{id}/queue_update$`, server.QueueUpdateHandler, auth.PrivLevelOperations, Authenticated, nil, 9189471, perlBypass},
-		{api.Version{1, 3}, http.MethodGet, `servers/{host_name}/update_status$`, server.GetServerUpdateStatusHandler, auth.PrivLevelReadOnly, Authenticated, nil, 438451599, noPerlBypass},
+		{api.Version{1, 3}, http.MethodGet, `servers/{host_name}/update_status$`, server.GetServerUpdateStatusHandlerV2, auth.PrivLevelReadOnly, Authenticated, nil, 438451599, noPerlBypass},

Review comment:
       The behavior is the same for v1 and v2, but I see how that can be confusing. Using an explicit v1 handler in 6407610e29.




----------------------------------------------------------------
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 #4901: Consider Topologies parentage when queueing or checking server updates

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #4901:
URL: https://github.com/apache/trafficcontrol/pull/4901#discussion_r463158681



##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error
+
+	selectQuery := `
+WITH RECURSIVE topology_ancestors AS (
+	SELECT tcp.child parent, tc.cachegroup
+	FROM "server" s
+	JOIN cachegroup c ON s.cachegroup = c.id
+	JOIN topology_cachegroup tc ON c."name" = tc.cachegroup
+	JOIN topology_cachegroup_parents tcp ON tc.id = tcp.child
+	WHERE s.host_name = $1
+UNION ALL
+	SELECT tcp.parent, tc.cachegroup
+	FROM topology_ancestors ta, topology_cachegroup_parents tcp
+	JOIN topology_cachegroup tc ON tcp.parent = tc.id
+	WHERE ta.parent = tcp.child
+), server_topology_ancestors AS (
+SELECT s.id, s.cachegroup, s.cdn_id, s.upd_pending, s.reval_pending, s.status
+FROM server s
+JOIN cachegroup c ON s.cachegroup = c.id
+JOIN topology_ancestors ta ON c."name" = ta.cachegroup
+WHERE s.host_name != $1
+), parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending, ps.status
+		FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending),
+		TRUE IN (
+			SELECT sta.upd_pending FROM  server_topology_ancestors sta)
+		) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending),
+		TRUE IN (
+			SELECT sta.reval_pending FROM  server_topology_ancestors sta)
+		) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'
+WHERE s.host_name = $1
+GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name
+ORDER BY s.id
+` // remove the type.name = 'EDGE' condition if other server types should have their parents processed
+
+	rows, err = tx.Query(selectQuery, hostName)
+	if err != nil {
+		log.Error.Printf("could not execute query: %s\n", err)
+		return nil, tc.DBError
+	}
+	defer log.Close(rows, "unable to close db connection")
+
+	for rows.Next() {
+		var serverUpdateStatus tc.ServerUpdateStatus
+		var serverType string
+		if err := rows.Scan(&serverUpdateStatus.HostId, &serverUpdateStatus.HostName, &serverType, &serverUpdateStatus.RevalPending, &serverUpdateStatus.UseRevalPending, &serverUpdateStatus.UpdatePending, &serverUpdateStatus.Status, &serverUpdateStatus.ParentPending, &serverUpdateStatus.ParentRevalPending); err != nil {
+			log.Error.Printf("could not scan server update status: %s\n", err)
+			return nil, tc.DBError
+		}
+		updateStatuses = append(updateStatuses, serverUpdateStatus)
+	}
+	return updateStatuses, nil
+}
+
+func GetServerUpdateStatusHandlerV2(w http.ResponseWriter, r *http.Request) {
+	inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"host_name"}, nil)
+	if userErr != nil || sysErr != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+		return
+	}
+	defer inf.Close()
+
+	serverUpdateStatus, err := getServerUpdateStatusV2(inf.Tx.Tx, inf.Config, inf.Params["host_name"])
+	if err != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, err)
+		return
+	}
+	api.WriteRespRaw(w, r, serverUpdateStatus)
+}
+
+// getServerUpdateStatusV2 supports /servers/all/update_status in addition to /servers/{host_name}/update_status
+// This special case is believed to be used nowhere.
+func getServerUpdateStatusV2(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
+	// language=SQL
+	baseSelectStatement := `
+WITH parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending
+	FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending), FALSE) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending), FALSE) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'

Review comment:
       It is for API version 2. IMO we should only let mids be child cachegroups in API version 3 and up and preserve the behavior of deprecated handlers.

##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error
+
+	selectQuery := `
+WITH RECURSIVE topology_ancestors AS (
+	SELECT tcp.child parent, tc.cachegroup
+	FROM "server" s
+	JOIN cachegroup c ON s.cachegroup = c.id
+	JOIN topology_cachegroup tc ON c."name" = tc.cachegroup
+	JOIN topology_cachegroup_parents tcp ON tc.id = tcp.child
+	WHERE s.host_name = $1
+UNION ALL
+	SELECT tcp.parent, tc.cachegroup
+	FROM topology_ancestors ta, topology_cachegroup_parents tcp
+	JOIN topology_cachegroup tc ON tcp.parent = tc.id
+	WHERE ta.parent = tcp.child
+), server_topology_ancestors AS (
+SELECT s.id, s.cachegroup, s.cdn_id, s.upd_pending, s.reval_pending, s.status
+FROM server s
+JOIN cachegroup c ON s.cachegroup = c.id
+JOIN topology_ancestors ta ON c."name" = ta.cachegroup
+WHERE s.host_name != $1
+), parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending, ps.status
+		FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending),
+		TRUE IN (
+			SELECT sta.upd_pending FROM  server_topology_ancestors sta)
+		) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending),
+		TRUE IN (
+			SELECT sta.reval_pending FROM  server_topology_ancestors sta)
+		) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'
+WHERE s.host_name = $1
+GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name
+ORDER BY s.id
+` // remove the type.name = 'EDGE' condition if other server types should have their parents processed
+
+	rows, err = tx.Query(selectQuery, hostName)
+	if err != nil {
+		log.Error.Printf("could not execute query: %s\n", err)
+		return nil, tc.DBError
+	}
+	defer log.Close(rows, "unable to close db connection")
+
+	for rows.Next() {
+		var serverUpdateStatus tc.ServerUpdateStatus
+		var serverType string
+		if err := rows.Scan(&serverUpdateStatus.HostId, &serverUpdateStatus.HostName, &serverType, &serverUpdateStatus.RevalPending, &serverUpdateStatus.UseRevalPending, &serverUpdateStatus.UpdatePending, &serverUpdateStatus.Status, &serverUpdateStatus.ParentPending, &serverUpdateStatus.ParentRevalPending); err != nil {
+			log.Error.Printf("could not scan server update status: %s\n", err)
+			return nil, tc.DBError
+		}
+		updateStatuses = append(updateStatuses, serverUpdateStatus)
+	}
+	return updateStatuses, nil
+}
+
+func GetServerUpdateStatusHandlerV2(w http.ResponseWriter, r *http.Request) {
+	inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"host_name"}, nil)
+	if userErr != nil || sysErr != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+		return
+	}
+	defer inf.Close()
+
+	serverUpdateStatus, err := getServerUpdateStatusV2(inf.Tx.Tx, inf.Config, inf.Params["host_name"])
+	if err != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, err)
+		return
+	}
+	api.WriteRespRaw(w, r, serverUpdateStatus)
+}
+
+// getServerUpdateStatusV2 supports /servers/all/update_status in addition to /servers/{host_name}/update_status
+// This special case is believed to be used nowhere.
+func getServerUpdateStatusV2(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
+	// language=SQL
+	baseSelectStatement := `

Review comment:
       >Complex SQL could use comments
   
   * Commented query additions in f97f121267, this one is the legacy query.
   
   > magic strings would be better as constants (OFFLINE, use_reval_pending, global)
   
   * Using `tc.CacheStatusOffline` instead of 'OFFLINE' in 5ef79b21cb
   
   * Using `tc.UseRevalPendingParameterName` instead of `'use_reval_pending'` ccb41948c2
   
   * Use `tc.GlobalConfigFileName` instead of hard-coded `'global'` in b1e912424d

##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error

Review comment:
       Removed predeclaration in b8ddc95d5a




----------------------------------------------------------------
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] rob05c commented on a change in pull request #4901: Consider Topologies parentage when queueing or checking server updates

Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #4901:
URL: https://github.com/apache/trafficcontrol/pull/4901#discussion_r460165620



##########
File path: traffic_ops/traffic_ops_golang/routing/routes.go
##########
@@ -1077,7 +1077,7 @@ func Routes(d ServerData) ([]Route, []RawRoute, http.Handler, error) {
 		//Server status
 		{api.Version{1, 1}, http.MethodPut, `servers/{id}/status$`, server.UpdateStatusHandler, auth.PrivLevelOperations, Authenticated, nil, 776663851, perlBypass},
 		{api.Version{1, 1}, http.MethodPost, `servers/{id}/queue_update$`, server.QueueUpdateHandler, auth.PrivLevelOperations, Authenticated, nil, 9189471, perlBypass},
-		{api.Version{1, 3}, http.MethodGet, `servers/{host_name}/update_status$`, server.GetServerUpdateStatusHandler, auth.PrivLevelReadOnly, Authenticated, nil, 438451599, noPerlBypass},
+		{api.Version{1, 3}, http.MethodGet, `servers/{host_name}/update_status$`, server.GetServerUpdateStatusHandlerV2, auth.PrivLevelReadOnly, Authenticated, nil, 438451599, noPerlBypass},

Review comment:
       This is API 1.3, but serving `HandlerV2`? Is that correct?




----------------------------------------------------------------
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 #4901: Consider Topologies parentage when queueing or checking server updates

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #4901:
URL: https://github.com/apache/trafficcontrol/pull/4901#discussion_r463158241



##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error
+
+	selectQuery := `
+WITH RECURSIVE topology_ancestors AS (

Review comment:
       Commented in f97f121267

##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error
+
+	selectQuery := `
+WITH RECURSIVE topology_ancestors AS (
+	SELECT tcp.child parent, tc.cachegroup
+	FROM "server" s
+	JOIN cachegroup c ON s.cachegroup = c.id
+	JOIN topology_cachegroup tc ON c."name" = tc.cachegroup
+	JOIN topology_cachegroup_parents tcp ON tc.id = tcp.child
+	WHERE s.host_name = $1
+UNION ALL
+	SELECT tcp.parent, tc.cachegroup
+	FROM topology_ancestors ta, topology_cachegroup_parents tcp
+	JOIN topology_cachegroup tc ON tcp.parent = tc.id
+	WHERE ta.parent = tcp.child
+), server_topology_ancestors AS (
+SELECT s.id, s.cachegroup, s.cdn_id, s.upd_pending, s.reval_pending, s.status
+FROM server s
+JOIN cachegroup c ON s.cachegroup = c.id
+JOIN topology_ancestors ta ON c."name" = ta.cachegroup
+WHERE s.host_name != $1
+), parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending, ps.status
+		FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending),
+		TRUE IN (
+			SELECT sta.upd_pending FROM  server_topology_ancestors sta)
+		) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending),
+		TRUE IN (
+			SELECT sta.reval_pending FROM  server_topology_ancestors sta)
+		) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'

Review comment:
       > Is this still correct?
   
   Nope!
   
   > Are there cases it wouldn't be?
   
   Yep!
   
   Removed in 94d4d987b6.

##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error
+
+	selectQuery := `
+WITH RECURSIVE topology_ancestors AS (
+	SELECT tcp.child parent, tc.cachegroup
+	FROM "server" s
+	JOIN cachegroup c ON s.cachegroup = c.id
+	JOIN topology_cachegroup tc ON c."name" = tc.cachegroup
+	JOIN topology_cachegroup_parents tcp ON tc.id = tcp.child
+	WHERE s.host_name = $1
+UNION ALL
+	SELECT tcp.parent, tc.cachegroup
+	FROM topology_ancestors ta, topology_cachegroup_parents tcp
+	JOIN topology_cachegroup tc ON tcp.parent = tc.id
+	WHERE ta.parent = tcp.child
+), server_topology_ancestors AS (
+SELECT s.id, s.cachegroup, s.cdn_id, s.upd_pending, s.reval_pending, s.status
+FROM server s
+JOIN cachegroup c ON s.cachegroup = c.id
+JOIN topology_ancestors ta ON c."name" = ta.cachegroup
+WHERE s.host_name != $1
+), parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending, ps.status
+		FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending),
+		TRUE IN (
+			SELECT sta.upd_pending FROM  server_topology_ancestors sta)
+		) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending),
+		TRUE IN (
+			SELECT sta.reval_pending FROM  server_topology_ancestors sta)
+		) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'
+WHERE s.host_name = $1
+GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name
+ORDER BY s.id
+` // remove the type.name = 'EDGE' condition if other server types should have their parents processed
+
+	rows, err = tx.Query(selectQuery, hostName)
+	if err != nil {
+		log.Error.Printf("could not execute query: %s\n", err)
+		return nil, tc.DBError
+	}
+	defer log.Close(rows, "unable to close db connection")

Review comment:
       Included function name in 73ff6a16bc.

##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error
+
+	selectQuery := `
+WITH RECURSIVE topology_ancestors AS (
+	SELECT tcp.child parent, tc.cachegroup
+	FROM "server" s
+	JOIN cachegroup c ON s.cachegroup = c.id
+	JOIN topology_cachegroup tc ON c."name" = tc.cachegroup
+	JOIN topology_cachegroup_parents tcp ON tc.id = tcp.child
+	WHERE s.host_name = $1
+UNION ALL
+	SELECT tcp.parent, tc.cachegroup
+	FROM topology_ancestors ta, topology_cachegroup_parents tcp
+	JOIN topology_cachegroup tc ON tcp.parent = tc.id
+	WHERE ta.parent = tcp.child
+), server_topology_ancestors AS (
+SELECT s.id, s.cachegroup, s.cdn_id, s.upd_pending, s.reval_pending, s.status
+FROM server s
+JOIN cachegroup c ON s.cachegroup = c.id
+JOIN topology_ancestors ta ON c."name" = ta.cachegroup
+WHERE s.host_name != $1
+), parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending, ps.status
+		FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending),
+		TRUE IN (
+			SELECT sta.upd_pending FROM  server_topology_ancestors sta)
+		) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending),
+		TRUE IN (
+			SELECT sta.reval_pending FROM  server_topology_ancestors sta)
+		) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'
+WHERE s.host_name = $1
+GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name
+ORDER BY s.id
+` // remove the type.name = 'EDGE' condition if other server types should have their parents processed
+
+	rows, err = tx.Query(selectQuery, hostName)
+	if err != nil {
+		log.Error.Printf("could not execute query: %s\n", err)

Review comment:
       Changed all occurrences of `log.Error.Printf()` in `server` package (and the project) to `log.Errorf()` in 1c6326a5e1.

##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error
+
+	selectQuery := `
+WITH RECURSIVE topology_ancestors AS (
+	SELECT tcp.child parent, tc.cachegroup
+	FROM "server" s
+	JOIN cachegroup c ON s.cachegroup = c.id
+	JOIN topology_cachegroup tc ON c."name" = tc.cachegroup
+	JOIN topology_cachegroup_parents tcp ON tc.id = tcp.child
+	WHERE s.host_name = $1
+UNION ALL
+	SELECT tcp.parent, tc.cachegroup
+	FROM topology_ancestors ta, topology_cachegroup_parents tcp
+	JOIN topology_cachegroup tc ON tcp.parent = tc.id
+	WHERE ta.parent = tcp.child
+), server_topology_ancestors AS (
+SELECT s.id, s.cachegroup, s.cdn_id, s.upd_pending, s.reval_pending, s.status
+FROM server s
+JOIN cachegroup c ON s.cachegroup = c.id
+JOIN topology_ancestors ta ON c."name" = ta.cachegroup
+WHERE s.host_name != $1
+), parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending, ps.status
+		FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending),
+		TRUE IN (
+			SELECT sta.upd_pending FROM  server_topology_ancestors sta)
+		) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending),
+		TRUE IN (
+			SELECT sta.reval_pending FROM  server_topology_ancestors sta)
+		) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'
+WHERE s.host_name = $1
+GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name
+ORDER BY s.id
+` // remove the type.name = 'EDGE' condition if other server types should have their parents processed
+
+	rows, err = tx.Query(selectQuery, hostName)
+	if err != nil {
+		log.Error.Printf("could not execute query: %s\n", err)
+		return nil, tc.DBError
+	}
+	defer log.Close(rows, "unable to close db connection")
+
+	for rows.Next() {
+		var serverUpdateStatus tc.ServerUpdateStatus
+		var serverType string
+		if err := rows.Scan(&serverUpdateStatus.HostId, &serverUpdateStatus.HostName, &serverType, &serverUpdateStatus.RevalPending, &serverUpdateStatus.UseRevalPending, &serverUpdateStatus.UpdatePending, &serverUpdateStatus.Status, &serverUpdateStatus.ParentPending, &serverUpdateStatus.ParentRevalPending); err != nil {

Review comment:
       Shortened `serverUpdateStatus` to `us` in 29c188bd74

##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error
+
+	selectQuery := `
+WITH RECURSIVE topology_ancestors AS (
+	SELECT tcp.child parent, tc.cachegroup
+	FROM "server" s
+	JOIN cachegroup c ON s.cachegroup = c.id
+	JOIN topology_cachegroup tc ON c."name" = tc.cachegroup
+	JOIN topology_cachegroup_parents tcp ON tc.id = tcp.child
+	WHERE s.host_name = $1
+UNION ALL
+	SELECT tcp.parent, tc.cachegroup
+	FROM topology_ancestors ta, topology_cachegroup_parents tcp
+	JOIN topology_cachegroup tc ON tcp.parent = tc.id
+	WHERE ta.parent = tcp.child
+), server_topology_ancestors AS (
+SELECT s.id, s.cachegroup, s.cdn_id, s.upd_pending, s.reval_pending, s.status
+FROM server s
+JOIN cachegroup c ON s.cachegroup = c.id
+JOIN topology_ancestors ta ON c."name" = ta.cachegroup
+WHERE s.host_name != $1
+), parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending, ps.status
+		FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending),
+		TRUE IN (
+			SELECT sta.upd_pending FROM  server_topology_ancestors sta)
+		) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending),
+		TRUE IN (
+			SELECT sta.reval_pending FROM  server_topology_ancestors sta)
+		) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'
+WHERE s.host_name = $1
+GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name
+ORDER BY s.id
+` // remove the type.name = 'EDGE' condition if other server types should have their parents processed
+
+	rows, err = tx.Query(selectQuery, hostName)
+	if err != nil {
+		log.Error.Printf("could not execute query: %s\n", err)
+		return nil, tc.DBError
+	}
+	defer log.Close(rows, "unable to close db connection")
+
+	for rows.Next() {
+		var serverUpdateStatus tc.ServerUpdateStatus
+		var serverType string
+		if err := rows.Scan(&serverUpdateStatus.HostId, &serverUpdateStatus.HostName, &serverType, &serverUpdateStatus.RevalPending, &serverUpdateStatus.UseRevalPending, &serverUpdateStatus.UpdatePending, &serverUpdateStatus.Status, &serverUpdateStatus.ParentPending, &serverUpdateStatus.ParentRevalPending); err != nil {
+			log.Error.Printf("could not scan server update status: %s\n", err)

Review comment:
       Changed all occurrences of `log.Error.Printf()` in `server` package (and the project) to `log.Errorf()` in 1c6326a5e1.

##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error
+
+	selectQuery := `
+WITH RECURSIVE topology_ancestors AS (
+	SELECT tcp.child parent, tc.cachegroup
+	FROM "server" s
+	JOIN cachegroup c ON s.cachegroup = c.id
+	JOIN topology_cachegroup tc ON c."name" = tc.cachegroup
+	JOIN topology_cachegroup_parents tcp ON tc.id = tcp.child
+	WHERE s.host_name = $1
+UNION ALL
+	SELECT tcp.parent, tc.cachegroup
+	FROM topology_ancestors ta, topology_cachegroup_parents tcp
+	JOIN topology_cachegroup tc ON tcp.parent = tc.id
+	WHERE ta.parent = tcp.child
+), server_topology_ancestors AS (
+SELECT s.id, s.cachegroup, s.cdn_id, s.upd_pending, s.reval_pending, s.status
+FROM server s
+JOIN cachegroup c ON s.cachegroup = c.id
+JOIN topology_ancestors ta ON c."name" = ta.cachegroup
+WHERE s.host_name != $1
+), parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending, ps.status
+		FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending),
+		TRUE IN (
+			SELECT sta.upd_pending FROM  server_topology_ancestors sta)
+		) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending),
+		TRUE IN (
+			SELECT sta.reval_pending FROM  server_topology_ancestors sta)
+		) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'
+WHERE s.host_name = $1
+GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name
+ORDER BY s.id
+` // remove the type.name = 'EDGE' condition if other server types should have their parents processed
+
+	rows, err = tx.Query(selectQuery, hostName)
+	if err != nil {
+		log.Error.Printf("could not execute query: %s\n", err)
+		return nil, tc.DBError
+	}
+	defer log.Close(rows, "unable to close db connection")
+
+	for rows.Next() {
+		var serverUpdateStatus tc.ServerUpdateStatus
+		var serverType string
+		if err := rows.Scan(&serverUpdateStatus.HostId, &serverUpdateStatus.HostName, &serverType, &serverUpdateStatus.RevalPending, &serverUpdateStatus.UseRevalPending, &serverUpdateStatus.UpdatePending, &serverUpdateStatus.Status, &serverUpdateStatus.ParentPending, &serverUpdateStatus.ParentRevalPending); err != nil {
+			log.Error.Printf("could not scan server update status: %s\n", err)
+			return nil, tc.DBError
+		}
+		updateStatuses = append(updateStatuses, serverUpdateStatus)
+	}
+	return updateStatuses, nil
+}
+
+func GetServerUpdateStatusHandlerV2(w http.ResponseWriter, r *http.Request) {
+	inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"host_name"}, nil)
+	if userErr != nil || sysErr != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+		return
+	}
+	defer inf.Close()
+
+	serverUpdateStatus, err := getServerUpdateStatusV2(inf.Tx.Tx, inf.Config, inf.Params["host_name"])
+	if err != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, err)
+		return
+	}
+	api.WriteRespRaw(w, r, serverUpdateStatus)
+}
+
+// getServerUpdateStatusV2 supports /servers/all/update_status in addition to /servers/{host_name}/update_status

Review comment:
       Finished GoDoc sentence in 2ea1793d4a, complete with a trailing period.

##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error
+
+	selectQuery := `
+WITH RECURSIVE topology_ancestors AS (
+	SELECT tcp.child parent, tc.cachegroup
+	FROM "server" s
+	JOIN cachegroup c ON s.cachegroup = c.id
+	JOIN topology_cachegroup tc ON c."name" = tc.cachegroup
+	JOIN topology_cachegroup_parents tcp ON tc.id = tcp.child
+	WHERE s.host_name = $1
+UNION ALL
+	SELECT tcp.parent, tc.cachegroup
+	FROM topology_ancestors ta, topology_cachegroup_parents tcp
+	JOIN topology_cachegroup tc ON tcp.parent = tc.id
+	WHERE ta.parent = tcp.child
+), server_topology_ancestors AS (
+SELECT s.id, s.cachegroup, s.cdn_id, s.upd_pending, s.reval_pending, s.status
+FROM server s
+JOIN cachegroup c ON s.cachegroup = c.id
+JOIN topology_ancestors ta ON c."name" = ta.cachegroup
+WHERE s.host_name != $1
+), parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending, ps.status
+		FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending),
+		TRUE IN (
+			SELECT sta.upd_pending FROM  server_topology_ancestors sta)
+		) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending),
+		TRUE IN (
+			SELECT sta.reval_pending FROM  server_topology_ancestors sta)
+		) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'
+WHERE s.host_name = $1
+GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name
+ORDER BY s.id
+` // remove the type.name = 'EDGE' condition if other server types should have their parents processed
+
+	rows, err = tx.Query(selectQuery, hostName)
+	if err != nil {
+		log.Error.Printf("could not execute query: %s\n", err)
+		return nil, tc.DBError
+	}
+	defer log.Close(rows, "unable to close db connection")
+
+	for rows.Next() {
+		var serverUpdateStatus tc.ServerUpdateStatus
+		var serverType string
+		if err := rows.Scan(&serverUpdateStatus.HostId, &serverUpdateStatus.HostName, &serverType, &serverUpdateStatus.RevalPending, &serverUpdateStatus.UseRevalPending, &serverUpdateStatus.UpdatePending, &serverUpdateStatus.Status, &serverUpdateStatus.ParentPending, &serverUpdateStatus.ParentRevalPending); err != nil {
+			log.Error.Printf("could not scan server update status: %s\n", err)
+			return nil, tc.DBError
+		}
+		updateStatuses = append(updateStatuses, serverUpdateStatus)
+	}
+	return updateStatuses, nil
+}
+
+func GetServerUpdateStatusHandlerV2(w http.ResponseWriter, r *http.Request) {
+	inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"host_name"}, nil)
+	if userErr != nil || sysErr != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+		return
+	}
+	defer inf.Close()
+
+	serverUpdateStatus, err := getServerUpdateStatusV2(inf.Tx.Tx, inf.Config, inf.Params["host_name"])
+	if err != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, err)
+		return
+	}
+	api.WriteRespRaw(w, r, serverUpdateStatus)
+}
+
+// getServerUpdateStatusV2 supports /servers/all/update_status in addition to /servers/{host_name}/update_status
+// This special case is believed to be used nowhere.

Review comment:
       Rephrased GoDoc in 2ea1793d4a




----------------------------------------------------------------
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] rob05c commented on a change in pull request #4901: Consider Topologies parentage when queueing or checking server updates

Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #4901:
URL: https://github.com/apache/trafficcontrol/pull/4901#discussion_r460169388



##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error
+
+	selectQuery := `
+WITH RECURSIVE topology_ancestors AS (

Review comment:
       This SQL is pretty complex, it could definitely use some comments

##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error
+
+	selectQuery := `
+WITH RECURSIVE topology_ancestors AS (
+	SELECT tcp.child parent, tc.cachegroup
+	FROM "server" s
+	JOIN cachegroup c ON s.cachegroup = c.id
+	JOIN topology_cachegroup tc ON c."name" = tc.cachegroup
+	JOIN topology_cachegroup_parents tcp ON tc.id = tcp.child
+	WHERE s.host_name = $1
+UNION ALL
+	SELECT tcp.parent, tc.cachegroup
+	FROM topology_ancestors ta, topology_cachegroup_parents tcp
+	JOIN topology_cachegroup tc ON tcp.parent = tc.id
+	WHERE ta.parent = tcp.child
+), server_topology_ancestors AS (
+SELECT s.id, s.cachegroup, s.cdn_id, s.upd_pending, s.reval_pending, s.status
+FROM server s
+JOIN cachegroup c ON s.cachegroup = c.id
+JOIN topology_ancestors ta ON c."name" = ta.cachegroup
+WHERE s.host_name != $1
+), parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending, ps.status
+		FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending),
+		TRUE IN (
+			SELECT sta.upd_pending FROM  server_topology_ancestors sta)
+		) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending),
+		TRUE IN (
+			SELECT sta.reval_pending FROM  server_topology_ancestors sta)
+		) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'

Review comment:
       Is this still correct? Are there cases it wouldn't be? E.g. if a Topology has servers EDGE->MID->MID ?

##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error
+
+	selectQuery := `
+WITH RECURSIVE topology_ancestors AS (
+	SELECT tcp.child parent, tc.cachegroup
+	FROM "server" s
+	JOIN cachegroup c ON s.cachegroup = c.id
+	JOIN topology_cachegroup tc ON c."name" = tc.cachegroup
+	JOIN topology_cachegroup_parents tcp ON tc.id = tcp.child
+	WHERE s.host_name = $1
+UNION ALL
+	SELECT tcp.parent, tc.cachegroup
+	FROM topology_ancestors ta, topology_cachegroup_parents tcp
+	JOIN topology_cachegroup tc ON tcp.parent = tc.id
+	WHERE ta.parent = tcp.child
+), server_topology_ancestors AS (
+SELECT s.id, s.cachegroup, s.cdn_id, s.upd_pending, s.reval_pending, s.status
+FROM server s
+JOIN cachegroup c ON s.cachegroup = c.id
+JOIN topology_ancestors ta ON c."name" = ta.cachegroup
+WHERE s.host_name != $1
+), parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending, ps.status
+		FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending),
+		TRUE IN (
+			SELECT sta.upd_pending FROM  server_topology_ancestors sta)
+		) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending),
+		TRUE IN (
+			SELECT sta.reval_pending FROM  server_topology_ancestors sta)
+		) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'
+WHERE s.host_name = $1
+GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name
+ORDER BY s.id
+` // remove the type.name = 'EDGE' condition if other server types should have their parents processed
+
+	rows, err = tx.Query(selectQuery, hostName)
+	if err != nil {
+		log.Error.Printf("could not execute query: %s\n", err)
+		return nil, tc.DBError
+	}
+	defer log.Close(rows, "unable to close db connection")
+
+	for rows.Next() {
+		var serverUpdateStatus tc.ServerUpdateStatus
+		var serverType string
+		if err := rows.Scan(&serverUpdateStatus.HostId, &serverUpdateStatus.HostName, &serverType, &serverUpdateStatus.RevalPending, &serverUpdateStatus.UseRevalPending, &serverUpdateStatus.UpdatePending, &serverUpdateStatus.Status, &serverUpdateStatus.ParentPending, &serverUpdateStatus.ParentRevalPending); err != nil {
+			log.Error.Printf("could not scan server update status: %s\n", err)

Review comment:
       panic, `log.Errorf`

##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error
+
+	selectQuery := `
+WITH RECURSIVE topology_ancestors AS (
+	SELECT tcp.child parent, tc.cachegroup
+	FROM "server" s
+	JOIN cachegroup c ON s.cachegroup = c.id
+	JOIN topology_cachegroup tc ON c."name" = tc.cachegroup
+	JOIN topology_cachegroup_parents tcp ON tc.id = tcp.child
+	WHERE s.host_name = $1
+UNION ALL
+	SELECT tcp.parent, tc.cachegroup
+	FROM topology_ancestors ta, topology_cachegroup_parents tcp
+	JOIN topology_cachegroup tc ON tcp.parent = tc.id
+	WHERE ta.parent = tcp.child
+), server_topology_ancestors AS (
+SELECT s.id, s.cachegroup, s.cdn_id, s.upd_pending, s.reval_pending, s.status
+FROM server s
+JOIN cachegroup c ON s.cachegroup = c.id
+JOIN topology_ancestors ta ON c."name" = ta.cachegroup
+WHERE s.host_name != $1
+), parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending, ps.status
+		FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending),
+		TRUE IN (
+			SELECT sta.upd_pending FROM  server_topology_ancestors sta)
+		) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending),
+		TRUE IN (
+			SELECT sta.reval_pending FROM  server_topology_ancestors sta)
+		) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'
+WHERE s.host_name = $1
+GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name
+ORDER BY s.id
+` // remove the type.name = 'EDGE' condition if other server types should have their parents processed
+
+	rows, err = tx.Query(selectQuery, hostName)
+	if err != nil {
+		log.Error.Printf("could not execute query: %s\n", err)
+		return nil, tc.DBError
+	}
+	defer log.Close(rows, "unable to close db connection")
+
+	for rows.Next() {
+		var serverUpdateStatus tc.ServerUpdateStatus
+		var serverType string
+		if err := rows.Scan(&serverUpdateStatus.HostId, &serverUpdateStatus.HostName, &serverType, &serverUpdateStatus.RevalPending, &serverUpdateStatus.UseRevalPending, &serverUpdateStatus.UpdatePending, &serverUpdateStatus.Status, &serverUpdateStatus.ParentPending, &serverUpdateStatus.ParentRevalPending); err != nil {
+			log.Error.Printf("could not scan server update status: %s\n", err)
+			return nil, tc.DBError
+		}
+		updateStatuses = append(updateStatuses, serverUpdateStatus)
+	}
+	return updateStatuses, nil
+}
+
+func GetServerUpdateStatusHandlerV2(w http.ResponseWriter, r *http.Request) {
+	inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"host_name"}, nil)
+	if userErr != nil || sysErr != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+		return
+	}
+	defer inf.Close()
+
+	serverUpdateStatus, err := getServerUpdateStatusV2(inf.Tx.Tx, inf.Config, inf.Params["host_name"])
+	if err != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, err)
+		return
+	}
+	api.WriteRespRaw(w, r, serverUpdateStatus)
+}
+
+// getServerUpdateStatusV2 supports /servers/all/update_status in addition to /servers/{host_name}/update_status
+// This special case is believed to be used nowhere.
+func getServerUpdateStatusV2(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
+	// language=SQL
+	baseSelectStatement := `

Review comment:
       Nitpicks: Complex SQL could use comments, and magic strings would be better as constants (OFFLINE, use_reval_pending, global)

##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error
+
+	selectQuery := `
+WITH RECURSIVE topology_ancestors AS (
+	SELECT tcp.child parent, tc.cachegroup
+	FROM "server" s
+	JOIN cachegroup c ON s.cachegroup = c.id
+	JOIN topology_cachegroup tc ON c."name" = tc.cachegroup
+	JOIN topology_cachegroup_parents tcp ON tc.id = tcp.child
+	WHERE s.host_name = $1
+UNION ALL
+	SELECT tcp.parent, tc.cachegroup
+	FROM topology_ancestors ta, topology_cachegroup_parents tcp
+	JOIN topology_cachegroup tc ON tcp.parent = tc.id
+	WHERE ta.parent = tcp.child
+), server_topology_ancestors AS (
+SELECT s.id, s.cachegroup, s.cdn_id, s.upd_pending, s.reval_pending, s.status
+FROM server s
+JOIN cachegroup c ON s.cachegroup = c.id
+JOIN topology_ancestors ta ON c."name" = ta.cachegroup
+WHERE s.host_name != $1
+), parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending, ps.status
+		FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending),
+		TRUE IN (
+			SELECT sta.upd_pending FROM  server_topology_ancestors sta)
+		) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending),
+		TRUE IN (
+			SELECT sta.reval_pending FROM  server_topology_ancestors sta)
+		) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'
+WHERE s.host_name = $1
+GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name
+ORDER BY s.id
+` // remove the type.name = 'EDGE' condition if other server types should have their parents processed
+
+	rows, err = tx.Query(selectQuery, hostName)
+	if err != nil {
+		log.Error.Printf("could not execute query: %s\n", err)
+		return nil, tc.DBError
+	}
+	defer log.Close(rows, "unable to close db connection")
+
+	for rows.Next() {
+		var serverUpdateStatus tc.ServerUpdateStatus
+		var serverType string
+		if err := rows.Scan(&serverUpdateStatus.HostId, &serverUpdateStatus.HostName, &serverType, &serverUpdateStatus.RevalPending, &serverUpdateStatus.UseRevalPending, &serverUpdateStatus.UpdatePending, &serverUpdateStatus.Status, &serverUpdateStatus.ParentPending, &serverUpdateStatus.ParentRevalPending); err != nil {
+			log.Error.Printf("could not scan server update status: %s\n", err)
+			return nil, tc.DBError
+		}
+		updateStatuses = append(updateStatuses, serverUpdateStatus)
+	}
+	return updateStatuses, nil
+}
+
+func GetServerUpdateStatusHandlerV2(w http.ResponseWriter, r *http.Request) {
+	inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"host_name"}, nil)
+	if userErr != nil || sysErr != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+		return
+	}
+	defer inf.Close()
+
+	serverUpdateStatus, err := getServerUpdateStatusV2(inf.Tx.Tx, inf.Config, inf.Params["host_name"])
+	if err != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, err)
+		return
+	}
+	api.WriteRespRaw(w, r, serverUpdateStatus)
+}
+
+// getServerUpdateStatusV2 supports /servers/all/update_status in addition to /servers/{host_name}/update_status

Review comment:
       GoDoc error, needs to be a complete sentence, trailing period.

##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error
+
+	selectQuery := `
+WITH RECURSIVE topology_ancestors AS (
+	SELECT tcp.child parent, tc.cachegroup
+	FROM "server" s
+	JOIN cachegroup c ON s.cachegroup = c.id
+	JOIN topology_cachegroup tc ON c."name" = tc.cachegroup
+	JOIN topology_cachegroup_parents tcp ON tc.id = tcp.child
+	WHERE s.host_name = $1
+UNION ALL
+	SELECT tcp.parent, tc.cachegroup
+	FROM topology_ancestors ta, topology_cachegroup_parents tcp
+	JOIN topology_cachegroup tc ON tcp.parent = tc.id
+	WHERE ta.parent = tcp.child
+), server_topology_ancestors AS (
+SELECT s.id, s.cachegroup, s.cdn_id, s.upd_pending, s.reval_pending, s.status
+FROM server s
+JOIN cachegroup c ON s.cachegroup = c.id
+JOIN topology_ancestors ta ON c."name" = ta.cachegroup
+WHERE s.host_name != $1
+), parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending, ps.status
+		FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending),
+		TRUE IN (
+			SELECT sta.upd_pending FROM  server_topology_ancestors sta)
+		) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending),
+		TRUE IN (
+			SELECT sta.reval_pending FROM  server_topology_ancestors sta)
+		) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'
+WHERE s.host_name = $1
+GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name
+ORDER BY s.id
+` // remove the type.name = 'EDGE' condition if other server types should have their parents processed
+
+	rows, err = tx.Query(selectQuery, hostName)
+	if err != nil {
+		log.Error.Printf("could not execute query: %s\n", err)
+		return nil, tc.DBError
+	}
+	defer log.Close(rows, "unable to close db connection")
+
+	for rows.Next() {
+		var serverUpdateStatus tc.ServerUpdateStatus
+		var serverType string
+		if err := rows.Scan(&serverUpdateStatus.HostId, &serverUpdateStatus.HostName, &serverType, &serverUpdateStatus.RevalPending, &serverUpdateStatus.UseRevalPending, &serverUpdateStatus.UpdatePending, &serverUpdateStatus.Status, &serverUpdateStatus.ParentPending, &serverUpdateStatus.ParentRevalPending); err != nil {
+			log.Error.Printf("could not scan server update status: %s\n", err)
+			return nil, tc.DBError
+		}
+		updateStatuses = append(updateStatuses, serverUpdateStatus)
+	}
+	return updateStatuses, nil
+}
+
+func GetServerUpdateStatusHandlerV2(w http.ResponseWriter, r *http.Request) {
+	inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"host_name"}, nil)
+	if userErr != nil || sysErr != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+		return
+	}
+	defer inf.Close()
+
+	serverUpdateStatus, err := getServerUpdateStatusV2(inf.Tx.Tx, inf.Config, inf.Params["host_name"])
+	if err != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, err)
+		return
+	}
+	api.WriteRespRaw(w, r, serverUpdateStatus)
+}
+
+// getServerUpdateStatusV2 supports /servers/all/update_status in addition to /servers/{host_name}/update_status
+// This special case is believed to be used nowhere.

Review comment:
       GoDoc, `getServerUpdateStatusV2 is a special case believed to be used nowhere.`

##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error
+
+	selectQuery := `
+WITH RECURSIVE topology_ancestors AS (
+	SELECT tcp.child parent, tc.cachegroup
+	FROM "server" s
+	JOIN cachegroup c ON s.cachegroup = c.id
+	JOIN topology_cachegroup tc ON c."name" = tc.cachegroup
+	JOIN topology_cachegroup_parents tcp ON tc.id = tcp.child
+	WHERE s.host_name = $1
+UNION ALL
+	SELECT tcp.parent, tc.cachegroup
+	FROM topology_ancestors ta, topology_cachegroup_parents tcp
+	JOIN topology_cachegroup tc ON tcp.parent = tc.id
+	WHERE ta.parent = tcp.child
+), server_topology_ancestors AS (
+SELECT s.id, s.cachegroup, s.cdn_id, s.upd_pending, s.reval_pending, s.status
+FROM server s
+JOIN cachegroup c ON s.cachegroup = c.id
+JOIN topology_ancestors ta ON c."name" = ta.cachegroup
+WHERE s.host_name != $1
+), parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending, ps.status
+		FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending),
+		TRUE IN (
+			SELECT sta.upd_pending FROM  server_topology_ancestors sta)
+		) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending),
+		TRUE IN (
+			SELECT sta.reval_pending FROM  server_topology_ancestors sta)
+		) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'
+WHERE s.host_name = $1
+GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name
+ORDER BY s.id
+` // remove the type.name = 'EDGE' condition if other server types should have their parents processed
+
+	rows, err = tx.Query(selectQuery, hostName)
+	if err != nil {
+		log.Error.Printf("could not execute query: %s\n", err)
+		return nil, tc.DBError
+	}
+	defer log.Close(rows, "unable to close db connection")
+
+	for rows.Next() {
+		var serverUpdateStatus tc.ServerUpdateStatus
+		var serverType string
+		if err := rows.Scan(&serverUpdateStatus.HostId, &serverUpdateStatus.HostName, &serverType, &serverUpdateStatus.RevalPending, &serverUpdateStatus.UseRevalPending, &serverUpdateStatus.UpdatePending, &serverUpdateStatus.Status, &serverUpdateStatus.ParentPending, &serverUpdateStatus.ParentRevalPending); err != nil {
+			log.Error.Printf("could not scan server update status: %s\n", err)
+			return nil, tc.DBError
+		}
+		updateStatuses = append(updateStatuses, serverUpdateStatus)
+	}
+	return updateStatuses, nil
+}
+
+func GetServerUpdateStatusHandlerV2(w http.ResponseWriter, r *http.Request) {
+	inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"host_name"}, nil)
+	if userErr != nil || sysErr != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+		return
+	}
+	defer inf.Close()
+
+	serverUpdateStatus, err := getServerUpdateStatusV2(inf.Tx.Tx, inf.Config, inf.Params["host_name"])
+	if err != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, err)
+		return
+	}
+	api.WriteRespRaw(w, r, serverUpdateStatus)
+}
+
+// getServerUpdateStatusV2 supports /servers/all/update_status in addition to /servers/{host_name}/update_status
+// This special case is believed to be used nowhere.
+func getServerUpdateStatusV2(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
+	// language=SQL
+	baseSelectStatement := `
+WITH parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending
+	FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending), FALSE) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending), FALSE) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'

Review comment:
       As above, is this correct?

##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error
+
+	selectQuery := `
+WITH RECURSIVE topology_ancestors AS (
+	SELECT tcp.child parent, tc.cachegroup
+	FROM "server" s
+	JOIN cachegroup c ON s.cachegroup = c.id
+	JOIN topology_cachegroup tc ON c."name" = tc.cachegroup
+	JOIN topology_cachegroup_parents tcp ON tc.id = tcp.child
+	WHERE s.host_name = $1
+UNION ALL
+	SELECT tcp.parent, tc.cachegroup
+	FROM topology_ancestors ta, topology_cachegroup_parents tcp
+	JOIN topology_cachegroup tc ON tcp.parent = tc.id
+	WHERE ta.parent = tcp.child
+), server_topology_ancestors AS (
+SELECT s.id, s.cachegroup, s.cdn_id, s.upd_pending, s.reval_pending, s.status
+FROM server s
+JOIN cachegroup c ON s.cachegroup = c.id
+JOIN topology_ancestors ta ON c."name" = ta.cachegroup
+WHERE s.host_name != $1
+), parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending, ps.status
+		FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending),
+		TRUE IN (
+			SELECT sta.upd_pending FROM  server_topology_ancestors sta)
+		) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending),
+		TRUE IN (
+			SELECT sta.reval_pending FROM  server_topology_ancestors sta)
+		) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'
+WHERE s.host_name = $1
+GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name
+ORDER BY s.id
+` // remove the type.name = 'EDGE' condition if other server types should have their parents processed
+
+	rows, err = tx.Query(selectQuery, hostName)
+	if err != nil {
+		log.Error.Printf("could not execute query: %s\n", err)
+		return nil, tc.DBError
+	}
+	defer log.Close(rows, "unable to close db connection")
+
+	for rows.Next() {
+		var serverUpdateStatus tc.ServerUpdateStatus
+		var serverType string
+		if err := rows.Scan(&serverUpdateStatus.HostId, &serverUpdateStatus.HostName, &serverType, &serverUpdateStatus.RevalPending, &serverUpdateStatus.UseRevalPending, &serverUpdateStatus.UpdatePending, &serverUpdateStatus.Status, &serverUpdateStatus.ParentPending, &serverUpdateStatus.ParentRevalPending); err != nil {

Review comment:
       Nitpick: Go encourages short variable names in small scopes. Something like `us := tc.ServerUpdateStatus{}` would make this much easier to read IMO

##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error
+
+	selectQuery := `
+WITH RECURSIVE topology_ancestors AS (
+	SELECT tcp.child parent, tc.cachegroup
+	FROM "server" s
+	JOIN cachegroup c ON s.cachegroup = c.id
+	JOIN topology_cachegroup tc ON c."name" = tc.cachegroup
+	JOIN topology_cachegroup_parents tcp ON tc.id = tcp.child
+	WHERE s.host_name = $1
+UNION ALL
+	SELECT tcp.parent, tc.cachegroup
+	FROM topology_ancestors ta, topology_cachegroup_parents tcp
+	JOIN topology_cachegroup tc ON tcp.parent = tc.id
+	WHERE ta.parent = tcp.child
+), server_topology_ancestors AS (
+SELECT s.id, s.cachegroup, s.cdn_id, s.upd_pending, s.reval_pending, s.status
+FROM server s
+JOIN cachegroup c ON s.cachegroup = c.id
+JOIN topology_ancestors ta ON c."name" = ta.cachegroup
+WHERE s.host_name != $1
+), parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending, ps.status
+		FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending),
+		TRUE IN (
+			SELECT sta.upd_pending FROM  server_topology_ancestors sta)
+		) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending),
+		TRUE IN (
+			SELECT sta.reval_pending FROM  server_topology_ancestors sta)
+		) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'
+WHERE s.host_name = $1
+GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name
+ORDER BY s.id
+` // remove the type.name = 'EDGE' condition if other server types should have their parents processed
+
+	rows, err = tx.Query(selectQuery, hostName)
+	if err != nil {
+		log.Error.Printf("could not execute query: %s\n", err)
+		return nil, tc.DBError
+	}
+	defer log.Close(rows, "unable to close db connection")

Review comment:
       Should this have context like the function name?

##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error
+
+	selectQuery := `
+WITH RECURSIVE topology_ancestors AS (
+	SELECT tcp.child parent, tc.cachegroup
+	FROM "server" s
+	JOIN cachegroup c ON s.cachegroup = c.id
+	JOIN topology_cachegroup tc ON c."name" = tc.cachegroup
+	JOIN topology_cachegroup_parents tcp ON tc.id = tcp.child
+	WHERE s.host_name = $1
+UNION ALL
+	SELECT tcp.parent, tc.cachegroup
+	FROM topology_ancestors ta, topology_cachegroup_parents tcp
+	JOIN topology_cachegroup tc ON tcp.parent = tc.id
+	WHERE ta.parent = tcp.child
+), server_topology_ancestors AS (
+SELECT s.id, s.cachegroup, s.cdn_id, s.upd_pending, s.reval_pending, s.status
+FROM server s
+JOIN cachegroup c ON s.cachegroup = c.id
+JOIN topology_ancestors ta ON c."name" = ta.cachegroup
+WHERE s.host_name != $1
+), parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending, ps.status
+		FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending),
+		TRUE IN (
+			SELECT sta.upd_pending FROM  server_topology_ancestors sta)
+		) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending),
+		TRUE IN (
+			SELECT sta.reval_pending FROM  server_topology_ancestors sta)
+		) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'
+WHERE s.host_name = $1
+GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name
+ORDER BY s.id
+` // remove the type.name = 'EDGE' condition if other server types should have their parents processed
+
+	rows, err = tx.Query(selectQuery, hostName)
+	if err != nil {
+		log.Error.Printf("could not execute query: %s\n", err)

Review comment:
       Should be `log.Errorf`. The underlying `log.Error.Printf` should never be used, because it will panic if `log.Error` is nil.

##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error

Review comment:
       Nitpick: no reason to predeclare these, can be `rows, err := tx.Query` below. Safer to never have them be invalid values




----------------------------------------------------------------
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] rob05c merged pull request #4901: Consider Topologies parentage when queueing or checking server updates

Posted by GitBox <gi...@apache.org>.
rob05c merged pull request #4901:
URL: https://github.com/apache/trafficcontrol/pull/4901


   


----------------------------------------------------------------
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] rob05c commented on a change in pull request #4901: Consider Topologies parentage when queueing or checking server updates

Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #4901:
URL: https://github.com/apache/trafficcontrol/pull/4901#discussion_r463166421



##########
File path: traffic_ops/traffic_ops_golang/server/servers_update_status.go
##########
@@ -46,18 +46,148 @@ func GetServerUpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
-	baseSelectStatement :=
-		`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
-         LEFT JOIN status AS pstatus ON pstatus.id = ps.status
-         WHERE pstatus.name != 'OFFLINE' ),
-         use_reval_pending AS (SELECT value::boolean FROM parameter WHERE name = 'use_reval_pending' AND config_file = 'global' UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY)
-         SELECT s.id, s.host_name, type.name AS type, (s.reval_pending::boolean) as server_reval_pending, use_reval_pending.value, s.upd_pending, status.name AS status, COALESCE(bool_or(ps.upd_pending), FALSE) AS parent_upd_pending, COALESCE(bool_or(ps.reval_pending), FALSE) AS parent_reval_pending FROM use_reval_pending, server s
-         LEFT JOIN status ON s.status = status.id
-         LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
-         LEFT JOIN type ON type.id = s.type
-         LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id AND ps.cdn_id = s.cdn_id AND type.name = 'EDGE'` //remove the EDGE reference if other server types should have their parents processed
-
-	groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+
+	updateStatuses := []tc.ServerUpdateStatus{}
+	var rows *sql.Rows
+	var err error
+
+	selectQuery := `
+WITH RECURSIVE topology_ancestors AS (
+	SELECT tcp.child parent, tc.cachegroup
+	FROM "server" s
+	JOIN cachegroup c ON s.cachegroup = c.id
+	JOIN topology_cachegroup tc ON c."name" = tc.cachegroup
+	JOIN topology_cachegroup_parents tcp ON tc.id = tcp.child
+	WHERE s.host_name = $1
+UNION ALL
+	SELECT tcp.parent, tc.cachegroup
+	FROM topology_ancestors ta, topology_cachegroup_parents tcp
+	JOIN topology_cachegroup tc ON tcp.parent = tc.id
+	WHERE ta.parent = tcp.child
+), server_topology_ancestors AS (
+SELECT s.id, s.cachegroup, s.cdn_id, s.upd_pending, s.reval_pending, s.status
+FROM server s
+JOIN cachegroup c ON s.cachegroup = c.id
+JOIN topology_ancestors ta ON c."name" = ta.cachegroup
+WHERE s.host_name != $1
+), parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending, ps.status
+		FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending),
+		TRUE IN (
+			SELECT sta.upd_pending FROM  server_topology_ancestors sta)
+		) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending),
+		TRUE IN (
+			SELECT sta.reval_pending FROM  server_topology_ancestors sta)
+		) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'
+WHERE s.host_name = $1
+GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name
+ORDER BY s.id
+` // remove the type.name = 'EDGE' condition if other server types should have their parents processed
+
+	rows, err = tx.Query(selectQuery, hostName)
+	if err != nil {
+		log.Error.Printf("could not execute query: %s\n", err)
+		return nil, tc.DBError
+	}
+	defer log.Close(rows, "unable to close db connection")
+
+	for rows.Next() {
+		var serverUpdateStatus tc.ServerUpdateStatus
+		var serverType string
+		if err := rows.Scan(&serverUpdateStatus.HostId, &serverUpdateStatus.HostName, &serverType, &serverUpdateStatus.RevalPending, &serverUpdateStatus.UseRevalPending, &serverUpdateStatus.UpdatePending, &serverUpdateStatus.Status, &serverUpdateStatus.ParentPending, &serverUpdateStatus.ParentRevalPending); err != nil {
+			log.Error.Printf("could not scan server update status: %s\n", err)
+			return nil, tc.DBError
+		}
+		updateStatuses = append(updateStatuses, serverUpdateStatus)
+	}
+	return updateStatuses, nil
+}
+
+func GetServerUpdateStatusHandlerV2(w http.ResponseWriter, r *http.Request) {
+	inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"host_name"}, nil)
+	if userErr != nil || sysErr != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+		return
+	}
+	defer inf.Close()
+
+	serverUpdateStatus, err := getServerUpdateStatusV2(inf.Tx.Tx, inf.Config, inf.Params["host_name"])
+	if err != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, err)
+		return
+	}
+	api.WriteRespRaw(w, r, serverUpdateStatus)
+}
+
+// getServerUpdateStatusV2 supports /servers/all/update_status in addition to /servers/{host_name}/update_status
+// This special case is believed to be used nowhere.
+func getServerUpdateStatusV2(tx *sql.Tx, cfg *config.Config, hostName string) ([]tc.ServerUpdateStatus, error) {
+	// language=SQL
+	baseSelectStatement := `
+WITH parentservers AS (
+	SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending
+	FROM server ps
+	LEFT JOIN status AS pstatus ON pstatus.id = ps.status
+	WHERE pstatus.name != 'OFFLINE'
+), use_reval_pending AS (
+	SELECT value::BOOLEAN
+	FROM parameter
+	WHERE name = 'use_reval_pending'
+	AND config_file = 'global'
+	UNION ALL SELECT FALSE FETCH FIRST 1 ROW ONLY
+)
+SELECT
+	s.id,
+	s.host_name,
+	type.name AS type,
+	(s.reval_pending::BOOLEAN) AS server_reval_pending,
+	use_reval_pending.value,
+	s.upd_pending,
+	status.name AS status,
+	COALESCE(BOOL_OR(ps.upd_pending), FALSE) AS parent_upd_pending,
+	COALESCE(BOOL_OR(ps.reval_pending), FALSE) AS parent_reval_pending
+	FROM use_reval_pending,
+		 server s
+LEFT JOIN status ON s.status = status.id
+LEFT JOIN cachegroup cg ON s.cachegroup = cg.id
+LEFT JOIN type ON type.id = s.type
+LEFT JOIN parentservers ps ON ps.cachegroup = cg.parent_cachegroup_id
+	AND ps.cdn_id = s.cdn_id
+	AND type.name = 'EDGE'

Review comment:
       At some point, I'm hoping we can remove 'EDGE' vs 'MID' types altogether, and just have a 'CACHE' type




----------------------------------------------------------------
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