Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Disallow deleting or moving the last server in a Cache Group used by a topology #5155

Merged
rawlinp merged 3 commits into from
Oct 14, 2020
Merged

Disallow deleting or moving the last server in a Cache Group used by a topology #5155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
382e0ac
Query servers by cachegroup name
zrhoffman Oct 14, 2020
8508ded
Topologies empty cachegroup validation:
zrhoffman Oct 14, 2020
a8c3648
- Disallow deleting the last server in a cachegroup used by a topology
zrhoffman Oct 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- Traffic Ops: Added validation to prohibit assigning caches to topology-based delivery services
- Traffic Ops: Added validation to prohibit removing a capability from a server if no other server in the same cachegroup can satisfy the required capabilities of the delivery services assigned to it via topologies.
- Traffic Ops: Added validation to ensure that at least one server per cachegroup in a delivery service's topology has the delivery service's required capabilities.
- Traffic Ops: Added validation to ensure that at least one server exists in each cachegroup that that is used in a Topology.
- Traffic Ops: Added validation to ensure that at least one server exists in each cachegroup that is used in a Topology on the `/api/3.0/topologies` endpoint and the `/api/3.0/servers/{{ID}}` endpoint.
- Traffic Ops: Consider Topologies parentage when queueing or checking server updates
- ORT: Added Topologies to Config Generation.
- Traffic Portal: Added the ability to create, read, update and delete flexible topologies.
Expand All @@ -43,7 +43,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- Added the ability to view Hash ID field (aka xmppID) on Traffic Portals' server summary page
- Added the ability to delete invalidation requests in Traffic Portal
- Added the ability to set TLS config provided here: https://golang.org/pkg/crypto/tls/#Config in Traffic Ops
- Added an indiciator to the Traffic Monitor UI when using a disk backup of Traffic ops.
- Added support for the `cachegroupName` query parameter for `GET /api/3.0/servers` in Traffic Ops
- Added an indiciator to the Traffic Monitor UI when using a disk backup of Traffic Ops.
- Added debugging functionality to CDN-in-a-Box for Traffic Stats.
- Added debugging functionality to the Traffic Router unit tests runner at [`/traffic_router/tests`](https://github.com/apache/trafficcontrol/tree/master/traffic_router/tests)
- Made the Traffic Router unit tests runner at [`/traffic_router/tests`](https://github.com/apache/trafficcontrol/tree/master/traffic_router/tests) run in Alpine Linux
Expand Down
2 changes: 2 additions & 0 deletions docs/source/api/v3/cachegroups.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ Request Structure
+===========+==========+==========================================================================================================================+
| id | no | Return the only :term:`Cache Group` that has this id |
+-----------+----------+--------------------------------------------------------------------------------------------------------------------------+
| name | no | Return only the :term:`Cache Group` identified by this :ref:`cache-group-name` |
+-----------+----------+--------------------------------------------------------------------------------------------------------------------------+
| type | no | Return only :term:`Cache Groups` that are of the :ref:`cache-group-type` identified by this integral, unique identifier |
+-----------+----------+--------------------------------------------------------------------------------------------------------------------------+
| topology | no | Return only :term:`Cache Groups` that are used in the :term:`Topology` identified by this unique identifier |
Expand Down
68 changes: 35 additions & 33 deletions docs/source/api/v3/servers.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,39 +31,41 @@ Request Structure
-----------------
.. table:: Request Query Parameters

+------------+----------+-------------------------------------------------------------------------------------------------------------------+
| Name | Required | Description |
+============+==========+===================================================================================================================+
| cachegroup | no | Return only those servers within the :term:`Cache Group` that has this :ref:`cache-group-id` |
+------------+----------+-------------------------------------------------------------------------------------------------------------------+
| dsId | no | Return only those servers assigned to the :term:`Delivery Service` identified by this integral, unique identifier.|
| | | If the Delivery Service has a :term:`Topology` assigned to it, the :ref:`to-api-servers` endpoint will return |
| | | each server whose :term:`Cache Group` is associated with a :term:`Topology Node` of that Topology and has the |
| | | :term:`Server Capabilities` that are |
| | | :term:`required by the Delivery Service <Delivery Service required capabilities>`. |
+------------+----------+-------------------------------------------------------------------------------------------------------------------+
| hostName | no | Return only those servers that have this (short) hostname |
+------------+----------+-------------------------------------------------------------------------------------------------------------------+
| id | no | Return only the server with this integral, unique identifier |
+------------+----------+-------------------------------------------------------------------------------------------------------------------+
| profileId | no | Return only those servers that are using the :term:`Profile` that has this :ref:`profile-id` |
+------------+----------+-------------------------------------------------------------------------------------------------------------------+
| status | no | Return only those servers with this status - see :ref:`health-proto` |
+------------+----------+-------------------------------------------------------------------------------------------------------------------+
| type | no | Return only servers of this :term:`Type` |
+------------+----------+-------------------------------------------------------------------------------------------------------------------+
| topology | no | Return only servers who belong to cachegroups assigned to the :term:`Topology` identified by this name |
+------------+----------+-------------------------------------------------------------------------------------------------------------------+
| sortOrder | no | Changes the order of sorting. Either ascending (default or "asc") or descending ("desc") |
+------------+----------+-------------------------------------------------------------------------------------------------------------------+
| limit | no | Choose the maximum number of results to return |
+------------+----------+-------------------------------------------------------------------------------------------------------------------+
| offset | no | The number of results to skip before beginning to return results. Must use in conjunction with limit |
+------------+----------+-------------------------------------------------------------------------------------------------------------------+
| page | no | Return the n\ :sup:`th` page of results, where "n" is the value of this parameter, pages are ``limit`` long and |
| | | the first page is 1. If ``offset`` was defined, this query parameter has no effect. ``limit`` must be defined to |
| | | make use of ``page``. |
+------------+----------+-------------------------------------------------------------------------------------------------------------------+
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
| Name | Required | Description |
+================+==========+===================================================================================================================+
| cachegroup | no | Return only those servers within the :term:`Cache Group` that has this :ref:`cache-group-id` |
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
| cachegroupName | no | Return only those servers within the :term:`Cache Group` that has this :ref:`cache-group-name` |
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
| dsId | no | Return only those servers assigned to the :term:`Delivery Service` identified by this integral, unique identifier.|
| | | If the Delivery Service has a :term:`Topology` assigned to it, the :ref:`to-api-servers` endpoint will return |
| | | each server whose :term:`Cache Group` is associated with a :term:`Topology Node` of that Topology and has the |
| | | :term:`Server Capabilities` that are |
| | | :term:`required by the Delivery Service <Delivery Service required capabilities>`. |
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
| hostName | no | Return only those servers that have this (short) hostname |
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
| id | no | Return only the server with this integral, unique identifier |
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
| profileId | no | Return only those servers that are using the :term:`Profile` that has this :ref:`profile-id` |
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
| status | no | Return only those servers with this status - see :ref:`health-proto` |
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
| type | no | Return only servers of this :term:`Type` |
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
| topology | no | Return only servers who belong to cachegroups assigned to the :term:`Topology` identified by this name |
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
| sortOrder | no | Changes the order of sorting. Either ascending (default or "asc") or descending ("desc") |
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
| limit | no | Choose the maximum number of results to return |
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
| offset | no | The number of results to skip before beginning to return results. Must use in conjunction with limit |
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+
| page | no | Return the n\ :sup:`th` page of results, where "n" is the value of this parameter, pages are ``limit`` long and |
| | | the first page is 1. If ``offset`` was defined, this query parameter has no effect. ``limit`` must be defined to |
| | | make use of ``page``. |
+----------------+----------+-------------------------------------------------------------------------------------------------------------------+

.. code-block:: http
:caption: Request Example
Expand Down
44 changes: 44 additions & 0 deletions traffic_ops/testing/api/v3/servers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,53 @@ func TestServers(t *testing.T) {
CreateTestServerWithoutProfileId(t)
UniqueIPProfileTestServers(t)
UpdateTestServerStatus(t)
LastServerInTopologyCacheGroup(t)
})
}

func LastServerInTopologyCacheGroup(t *testing.T) {
const cacheGroupName = "topology-mid-cg-01"
const moveToCacheGroup = "topology-mid-cg-02"
const topologyName = "forked-topology"
const expectedLength = 1
params := url.Values{}
params.Add("cachegroupName", cacheGroupName)
params.Add("topology", topologyName)
servers, _, err := TOSession.GetServersWithHdr(&params, nil)
if err != nil {
t.Fatalf("getting server from cachegroup %s in topology %s: %s", cacheGroupName, topologyName, err.Error())
}
if len(servers.Response) != expectedLength {
t.Fatalf("expected to get %d server from cachegroup %s in topology %s, got %d servers", expectedLength, cacheGroupName, topologyName, len(servers.Response))
}
server := servers.Response[0]
_, reqInf, err := TOSession.DeleteServerByID(*server.ID)
if err == nil {
t.Fatalf("expected an error deleting server with id %d, received no error", *server.ID)
}
if reqInf.StatusCode < http.StatusBadRequest || reqInf.StatusCode >= http.StatusInternalServerError {
t.Fatalf("expected a 400-level error deleting server with id %d, got status code %d: %s", *server.ID, reqInf.StatusCode, err.Error())
}

params = url.Values{}
params.Add("name", moveToCacheGroup)
cgs, _, err := TOSession.GetCacheGroupsByQueryParamsWithHdr(params, nil)
if err != nil {
t.Fatalf("getting cachegroup with hostname %s: %s", moveToCacheGroup, err.Error())
}
if len(cgs) != expectedLength {
t.Fatalf("expected %d cachegroup with hostname %s, received %d cachegroups", expectedLength, moveToCacheGroup, len(cgs))
}
*server.CachegroupID = *cgs[0].ID
_, _, err = TOSession.UpdateServerByID(*server.ID, server)
if err == nil {
t.Fatalf("expected an error moving server with id %d to a different cachegroup, received no error", *server.ID)
}
if reqInf.StatusCode < http.StatusBadRequest || reqInf.StatusCode >= http.StatusInternalServerError {
t.Fatalf("expected a 400-level error moving server with id %d to a different cachegroup, got status code %d: %s", *server.ID, reqInf.StatusCode, err.Error())
}
}

func UpdateTestServerStatus(t *testing.T) {
if len(testData.Servers) < 1 {
t.Fatal("Need at least one server to test updating")
Expand Down
20 changes: 20 additions & 0 deletions traffic_ops/traffic_ops_golang/server/servers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/topology"
"net"
"net/http"
"strconv"
Expand Down Expand Up @@ -779,6 +780,10 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth
"dsId": dbhelpers.WhereColumnInfo{"dss.deliveryservice", nil},
}

if version.Major >= 3 {
queryParamsToSQLCols["cachegroupName"] = dbhelpers.WhereColumnInfo{"cg.name", nil}
}

usesMids := false
queryAddition := ""
dsHasRequiredCapabilities := false
Expand Down Expand Up @@ -1210,6 +1215,13 @@ func Update(w http.ResponseWriter, r *http.Request) {
return
}

cacheGroupIds := []int{*origSer[0].CachegroupID}
serverIds := []int{*origSer[0].ID}
if err := topology.CheckForEmptyCacheGroups(inf.Tx, cacheGroupIds, true, serverIds); err != nil {
api.HandleErr(w, r, tx, http.StatusBadRequest, errors.New("server is the last one in its cachegroup, which is used by a topology, so it cannot be moved to another cachegroup: "+err.Error()), nil)
return
}

server, err = newServer.ToServerV2()
if err != nil {
api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("converting v3 server to v2 for update: %v", err))
Expand Down Expand Up @@ -1591,6 +1603,14 @@ func Delete(w http.ResponseWriter, r *http.Request) {
api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("there are somehow two servers with id %d - cannot delete", id))
return
}
if version.Major >= 3 {
cacheGroupIds := []int{*servers[0].CachegroupID}
serverIds := []int{*servers[0].ID}
if err := topology.CheckForEmptyCacheGroups(inf.Tx, cacheGroupIds, true, serverIds); err != nil {
api.HandleErr(w, r, tx, http.StatusBadRequest, errors.New("server is the last one in its cachegroup, which is used by a topology: "+err.Error()), nil)
return
}
}

userErr, sysErr, errCode = deleteInterfaces(id, tx)
if userErr != nil || sysErr != nil {
Expand Down
Loading