From 42a6c03efda3d3b3f9fc0ddaf5ee3fad4de8548c Mon Sep 17 00:00:00 2001 From: Rawlin Peters Date: Fri, 3 Apr 2020 17:31:23 -0600 Subject: [PATCH 1/3] Update cachegroup APIs for topologies - for GET /cachegroups add support for a topology query param for returning all cachegroups that are used in a given topology - for DELETE /cachegroups/:id return an informative error message if trying to delete a cachegroup that is currently used by a topology --- docs/source/api/v2/cachegroups.rst | 2 ++ .../traffic_ops_golang/cachegroup/cachegroups.go | 12 ++++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/docs/source/api/v2/cachegroups.rst b/docs/source/api/v2/cachegroups.rst index c84e4ba676..5b1f69d6ae 100644 --- a/docs/source/api/v2/cachegroups.rst +++ b/docs/source/api/v2/cachegroups.rst @@ -38,6 +38,8 @@ Request Structure +-----------+----------+--------------------------------------------------------------------------------------------------------------------------+ | 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 Topology identified by this unique identifier | + +-----------+----------+--------------------------------------------------------------------------------------------------------------------------+ | orderby | no | Choose the ordering of the results - must be the name of one of the fields of the objects in the ``response`` array | +-----------+----------+--------------------------------------------------------------------------------------------------------------------------+ | sortOrder | no | Changes the order of sorting. Either ascending (default or "asc") or descending ("desc") | diff --git a/traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go b/traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go index 102560ed70..0d29ea8702 100644 --- a/traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go +++ b/traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go @@ -88,23 +88,28 @@ func (cg *TOCacheGroup) SetID(i int) { // Is the cachegroup being used? func isUsed(tx *sqlx.Tx, ID int) (bool, error) { + var usedByTopology bool var usedByServer bool var usedByParent bool var usedBySecondaryParent bool var usedByASN bool query := `SELECT + (SELECT id FROM topology_cachegroup WHERE topology_cachegroup.cachegroup = (SELECT name FROM cachegroup WHERE id = $1) LIMIT 1) IS NOT NULL, (SELECT id FROM server WHERE server.cachegroup = $1 LIMIT 1) IS NOT NULL, (SELECT id FROM cachegroup WHERE cachegroup.parent_cachegroup_id = $1 LIMIT 1) IS NOT NULL, (SELECT id FROM cachegroup WHERE cachegroup.secondary_parent_cachegroup_id = $1 LIMIT 1) IS NOT NULL, (SELECT id FROM asn WHERE cachegroup = $1 LIMIT 1) IS NOT NULL;` - err := tx.QueryRow(query, ID).Scan(&usedByServer, &usedByParent, &usedBySecondaryParent, &usedByASN) + err := tx.QueryRow(query, ID).Scan(&usedByTopology, &usedByServer, &usedByParent, &usedBySecondaryParent, &usedByASN) if err != nil { log.Errorf("received error: %++v from query execution", err) return false, err } //Only return the immediate error + if usedByTopology { + return true, errors.New("cachegroup is in use by one or more topologies") + } if usedByServer { return true, errors.New("cachegroup is in use by one or more servers") } @@ -427,6 +432,7 @@ func (cg *TOCacheGroup) Read() ([]interface{}, error, error, int) { "name": dbhelpers.WhereColumnInfo{"cachegroup.name", nil}, "shortName": dbhelpers.WhereColumnInfo{"cachegroup.short_name", nil}, "type": dbhelpers.WhereColumnInfo{"cachegroup.type", nil}, + "topology": dbhelpers.WhereColumnInfo{"topology_cachegroup.topology", nil}, } where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(cg.ReqInfo.Params, queryParamsToQueryCols) if len(errs) > 0 { @@ -658,7 +664,9 @@ FROM cachegroup LEFT JOIN coordinate ON coordinate.id = cachegroup.coordinate INNER JOIN type ON cachegroup.type = type.id LEFT JOIN cachegroup AS cgp ON cachegroup.parent_cachegroup_id = cgp.id -LEFT JOIN cachegroup AS cgs ON cachegroup.secondary_parent_cachegroup_id = cgs.id` +LEFT JOIN cachegroup AS cgs ON cachegroup.secondary_parent_cachegroup_id = cgs.id +LEFT JOIN topology_cachegroup ON cachegroup.name = topology_cachegroup.cachegroup +` } func multipleCacheGroupsWhere() string { From d03bd2c776489e5659f6610f8ba651e878999ff4 Mon Sep 17 00:00:00 2001 From: Rawlin Peters Date: Fri, 15 May 2020 11:07:12 -0600 Subject: [PATCH 2/3] Add tests, changelog, and move v2 docs changes to v3 --- CHANGELOG.md | 1 + docs/source/api/v2/cachegroups.rst | 2 - docs/source/api/v3/cachegroups.rst | 2 + traffic_ops/client/cachegroup.go | 23 +++++++++++- .../testing/api/v3/cachegroups_test.go | 37 ++++++++++++++++++- 5 files changed, 60 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 00ebd023e5..7876ccd612 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Added an optional readiness check service to cdn-in-a-box that exits successfully when it is able to get a `200 OK` from all delivery services - [Flexible Topologies (in progress)](https://github.com/apache/trafficcontrol/blob/master/blueprints/flexible-topologies.md) - Traffic Ops: Added an API 3.0 endpoint, /api/3.0/topologies, to create, read, update and delete flexible topologies. + - Traffic Ops: Added support for `topology` query parameter to `GET /api/3.0/cachegroups` to return all cachegroups used in the given topology. - Traffic Portal: Added the ability to create, read, update and delete flexible topologies. ### Fixed diff --git a/docs/source/api/v2/cachegroups.rst b/docs/source/api/v2/cachegroups.rst index 5b1f69d6ae..c84e4ba676 100644 --- a/docs/source/api/v2/cachegroups.rst +++ b/docs/source/api/v2/cachegroups.rst @@ -38,8 +38,6 @@ Request Structure +-----------+----------+--------------------------------------------------------------------------------------------------------------------------+ | 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 Topology identified by this unique identifier | - +-----------+----------+--------------------------------------------------------------------------------------------------------------------------+ | orderby | no | Choose the ordering of the results - must be the name of one of the fields of the objects in the ``response`` array | +-----------+----------+--------------------------------------------------------------------------------------------------------------------------+ | sortOrder | no | Changes the order of sorting. Either ascending (default or "asc") or descending ("desc") | diff --git a/docs/source/api/v3/cachegroups.rst b/docs/source/api/v3/cachegroups.rst index 51a6b02133..725ae96470 100644 --- a/docs/source/api/v3/cachegroups.rst +++ b/docs/source/api/v3/cachegroups.rst @@ -38,6 +38,8 @@ Request Structure +-----------+----------+--------------------------------------------------------------------------------------------------------------------------+ | 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 | + +-----------+----------+--------------------------------------------------------------------------------------------------------------------------+ | orderby | no | Choose the ordering of the results - must be the name of one of the fields of the objects in the ``response`` array | +-----------+----------+--------------------------------------------------------------------------------------------------------------------------+ | sortOrder | no | Changes the order of sorting. Either ascending (default or "asc") or descending ("desc") | diff --git a/traffic_ops/client/cachegroup.go b/traffic_ops/client/cachegroup.go index 30f495f152..6ee8de2c28 100644 --- a/traffic_ops/client/cachegroup.go +++ b/traffic_ops/client/cachegroup.go @@ -156,7 +156,7 @@ func (to *Session) GetCacheGroupNullableByName(name string) ([]tc.CacheGroupNull return data.Response, reqInf, nil } -// GET a CacheGroup by the CacheGroup name. +// GET a CacheGroup by the CacheGroup short name. func (to *Session) GetCacheGroupNullableByShortName(shortName string) ([]tc.CacheGroupNullable, ReqInf, error) { route := fmt.Sprintf("%s?shortName=%s", API_CACHEGROUPS, url.QueryEscape(shortName)) resp, remoteAddr, err := to.request(http.MethodGet, route, nil) @@ -189,3 +189,24 @@ func (to *Session) DeleteCacheGroupByID(id int) (tc.Alerts, ReqInf, error) { } return alerts, reqInf, nil } + +func (to *Session) GetCacheGroupsByQueryParams(qparams url.Values) ([]tc.CacheGroupNullable, ReqInf, error) { + route := API_CACHEGROUPS + if len(qparams) > 0 { + route += "?" + qparams.Encode() + } + + resp, remoteAddr, err := to.request(http.MethodGet, route, nil) + reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: remoteAddr} + if err != nil { + return nil, reqInf, err + } + defer resp.Body.Close() + + var data tc.CacheGroupsNullableResponse + if err := json.NewDecoder(resp.Body).Decode(&data); err != nil { + return nil, reqInf, err + } + + return data.Response, reqInf, nil +} diff --git a/traffic_ops/testing/api/v3/cachegroups_test.go b/traffic_ops/testing/api/v3/cachegroups_test.go index 605f735d86..733fcf03ad 100644 --- a/traffic_ops/testing/api/v3/cachegroups_test.go +++ b/traffic_ops/testing/api/v3/cachegroups_test.go @@ -17,6 +17,7 @@ package v3 import ( "fmt" + "net/url" "reflect" "testing" @@ -24,9 +25,10 @@ import ( ) func TestCacheGroups(t *testing.T) { - WithObjs(t, []TCObj{Types, Parameters, CacheGroups}, func() { + WithObjs(t, []TCObj{Types, Parameters, CacheGroups, Topologies}, func() { GetTestCacheGroupsByName(t) GetTestCacheGroupsByShortName(t) + GetTestCacheGroupsByTopology(t) CheckCacheGroupsAuthentication(t) UpdateTestCacheGroups(t) }) @@ -70,16 +72,47 @@ func GetTestCacheGroupsByName(t *testing.T) { if err != nil { t.Errorf("cannot GET CacheGroup by name: %v - %v", err, resp) } + if *resp[0].Name != *cg.Name { + t.Errorf("name expected: %s, actual: %s", *cg.Name, *resp[0].Name) + } } } func GetTestCacheGroupsByShortName(t *testing.T) { for _, cg := range testData.CacheGroups { - resp, _, err := TOSession.GetCacheGroupNullableByName(*cg.ShortName) + resp, _, err := TOSession.GetCacheGroupNullableByShortName(*cg.ShortName) if err != nil { t.Errorf("cannot GET CacheGroup by shortName: %v - %v", err, resp) } + if *resp[0].ShortName != *cg.ShortName { + t.Errorf("short name expected: %s, actual: %s", *cg.ShortName, *resp[0].ShortName) + } + } +} + +func GetTestCacheGroupsByTopology(t *testing.T) { + for _, top := range testData.Topologies { + qparams := url.Values{} + qparams.Set("topology", top.Name) + resp, _, err := TOSession.GetCacheGroupsByQueryParams(qparams) + if err != nil { + t.Errorf("cannot GET CacheGroups by topology: %v - %v", err, resp) + } + expectedCGs := topologyCachegroups(top) + for _, cg := range resp { + if _, exists := expectedCGs[*cg.Name]; !exists { + t.Errorf("GET cachegroups by topology - expected one of: %v, actual: %s", expectedCGs, *cg.Name) + } + } + } +} + +func topologyCachegroups(top tc.Topology) map[string]struct{} { + res := make(map[string]struct{}) + for _, node := range top.Nodes { + res[node.Cachegroup] = struct{}{} } + return res } func UpdateTestCacheGroups(t *testing.T) { From 59d6fb1005576218c08246171707843ba05e3e6f Mon Sep 17 00:00:00 2001 From: Rawlin Peters Date: Mon, 18 May 2020 15:34:56 -0600 Subject: [PATCH 3/3] Add godoc comment, use log.Close(), remove redundant type info --- traffic_ops/client/cachegroup.go | 4 +++- .../traffic_ops_golang/cachegroup/cachegroups.go | 10 +++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/traffic_ops/client/cachegroup.go b/traffic_ops/client/cachegroup.go index 6ee8de2c28..a0d66b6802 100644 --- a/traffic_ops/client/cachegroup.go +++ b/traffic_ops/client/cachegroup.go @@ -23,6 +23,7 @@ import ( "net/http" "net/url" + "github.com/apache/trafficcontrol/lib/go-log" "github.com/apache/trafficcontrol/lib/go-tc" ) @@ -190,6 +191,7 @@ func (to *Session) DeleteCacheGroupByID(id int) (tc.Alerts, ReqInf, error) { return alerts, reqInf, nil } +// GetCacheGroupsByQueryParams gets cache groups by the given query parameters. func (to *Session) GetCacheGroupsByQueryParams(qparams url.Values) ([]tc.CacheGroupNullable, ReqInf, error) { route := API_CACHEGROUPS if len(qparams) > 0 { @@ -201,7 +203,7 @@ func (to *Session) GetCacheGroupsByQueryParams(qparams url.Values) ([]tc.CacheGr if err != nil { return nil, reqInf, err } - defer resp.Body.Close() + defer log.Close(resp.Body, "unable to close cachegroups response body") var data tc.CacheGroupsNullableResponse if err := json.NewDecoder(resp.Body).Decode(&data); err != nil { diff --git a/traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go b/traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go index 0d29ea8702..f0c7698506 100644 --- a/traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go +++ b/traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go @@ -428,11 +428,11 @@ func (cg *TOCacheGroup) Read() ([]interface{}, error, error, int) { // Query Parameters to Database Query column mappings // see the fields mapped in the SQL query queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{ - "id": dbhelpers.WhereColumnInfo{"cachegroup.id", api.IsInt}, - "name": dbhelpers.WhereColumnInfo{"cachegroup.name", nil}, - "shortName": dbhelpers.WhereColumnInfo{"cachegroup.short_name", nil}, - "type": dbhelpers.WhereColumnInfo{"cachegroup.type", nil}, - "topology": dbhelpers.WhereColumnInfo{"topology_cachegroup.topology", nil}, + "id": {"cachegroup.id", api.IsInt}, + "name": {"cachegroup.name", nil}, + "shortName": {"cachegroup.short_name", nil}, + "type": {"cachegroup.type", nil}, + "topology": {"topology_cachegroup.topology", nil}, } where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(cg.ReqInfo.Params, queryParamsToQueryCols) if len(errs) > 0 {