You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by ra...@apache.org on 2021/02/05 17:43:46 UTC
[trafficcontrol] branch master updated: Ability to change the name
of a topology (#5495)
This is an automated email from the ASF dual-hosted git repository.
rawlin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
The following commit(s) were added to refs/heads/master by this push:
new 78871ce Ability to change the name of a topology (#5495)
78871ce is described below
commit 78871cee1ada8c2ee9cba1d103e09fc707def72c
Author: Rima Shah <22...@users.noreply.github.com>
AuthorDate: Fri Feb 5 10:43:31 2021 -0700
Ability to change the name of a topology (#5495)
* Updated TP to pass old topology name in query param and in TO updated the update query
* Removed setting of the primary key to query param, to allow name to be mutable.
* Added test case for api/v4
* Update toplogies to update name after parents and cachegroup have been deleted.
* Removed extra comma from sql query
* Updated query and function name
* Checking for original topology name in Validate()
* Updated Changelog
* Updated based on review comments.
---
CHANGELOG.md | 1 +
traffic_ops/testing/api/v4/topologies_test.go | 38 ++++++++++++++++++++++
.../traffic_ops_golang/topology/topologies.go | 37 ++++++++++++---------
.../app/src/common/api/TopologyService.js | 4 +--
.../topology/edit/FormEditTopologyController.js | 9 ++---
.../modules/form/topology/form.topology.tpl.html | 5 +--
.../app/src/common/service/utils/TopologyUtils.js | 8 ++---
.../test/end_to_end/topologies/topologies-spec.js | 1 -
8 files changed, 74 insertions(+), 29 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 66f18fa..2cd6e29 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
## [unreleased]
### Added
+- Traffic Portal: [#5361](https://github.com/apache/trafficcontrol/issues/5361) - Added the ability to change the name of a topology.
- Traffic Portal: [#5340](https://github.com/apache/trafficcontrol/issues/5340) - Added the ability to resend a user registration from user screen.
- Traffic Portal: [#5394](https://github.com/apache/trafficcontrol/issues/5394) - Converts the tenant table to a tenant tree for usability
- Traffic Portal: upgraded delivery service UI tables to use more powerful/performant ag-grid component
diff --git a/traffic_ops/testing/api/v4/topologies_test.go b/traffic_ops/testing/api/v4/topologies_test.go
index b078e08..a9a9333 100644
--- a/traffic_ops/testing/api/v4/topologies_test.go
+++ b/traffic_ops/testing/api/v4/topologies_test.go
@@ -41,6 +41,7 @@ func TestTopologies(t *testing.T) {
ValidationTestTopologies(t)
UpdateValidateTopologyORGServerCacheGroup(t)
EdgeParentOfEdgeSucceedsWithWarning(t)
+ UpdateTopologyName(t)
})
}
@@ -294,6 +295,43 @@ func UpdateValidateTopologyORGServerCacheGroup(t *testing.T) {
}
}
+func UpdateTopologyName(t *testing.T) {
+ currentTopologyName := "top-used-by-cdn1-and-cdn2"
+
+ // Get details on existing topology
+ resp, _, err := TOSession.GetTopologyWithHdr(currentTopologyName, nil)
+ if err != nil {
+ t.Errorf("unable to get topology with name: %v", currentTopologyName)
+ }
+ newTopologyName := "test-topology"
+ resp.Name = newTopologyName
+
+ // Update topology with new name
+ updateResponse, _, err := TOSession.UpdateTopology(currentTopologyName, *resp)
+ if err != nil {
+ t.Errorf("cannot PUT topology: %v - %v", err, updateResponse)
+ }
+ if updateResponse.Response.Name != newTopologyName {
+ t.Errorf("update topology name failed, expected: %v but got:%v", newTopologyName, updateResponse.Response.Name)
+ }
+
+ //To check whether the primary key change trickled down to DS table
+ resp1, _, err := TOSession.GetDeliveryServiceByXMLIDNullableWithHdr("top-ds-in-cdn2", nil)
+ if err != nil {
+ t.Errorf("failed to get details on DS: %v", err)
+ }
+ if *resp1[0].Topology != newTopologyName {
+ t.Errorf("topology name change failed to trickle to delivery service table, expected: %v but got:%v", newTopologyName, *resp1[0].Topology)
+ }
+
+ // Set everything back as it was for further testing.
+ resp.Name = currentTopologyName
+ r, _, err := TOSession.UpdateTopology(newTopologyName, *resp)
+ if err != nil {
+ t.Errorf("cannot PUT topology: %v - %v", err, r)
+ }
+}
+
func DeleteTestTopologies(t *testing.T) {
for _, top := range testData.Topologies {
delResp, _, err := TOSession.DeleteTopology(top.Name)
diff --git a/traffic_ops/traffic_ops_golang/topology/topologies.go b/traffic_ops/traffic_ops_golang/topology/topologies.go
index 4e0f9f5..08d293c 100644
--- a/traffic_ops/traffic_ops_golang/topology/topologies.go
+++ b/traffic_ops/traffic_ops_golang/topology/topologies.go
@@ -46,6 +46,7 @@ import (
type TOTopology struct {
api.APIInfoImpl `json:"-"`
Alerts tc.Alerts `json:"-"`
+ RequestedName string
tc.Topology
}
@@ -89,6 +90,7 @@ func (topology *TOTopology) GetType() string {
// Validate is a requirement of the api.Validator interface.
func (topology *TOTopology) Validate() error {
+ currentTopoName := topology.APIInfoImpl.ReqInfo.Params["name"]
nameRule := validation.NewStringRule(tovalidate.IsAlphanumericUnderscoreDash, "must consist of only alphanumeric, dash, or underscore characters.")
rules := validation.Errors{}
rules["name"] = validation.Validate(topology.Name, validation.Required, nameRule)
@@ -142,17 +144,17 @@ func (topology *TOTopology) Validate() error {
for index, cacheGroup := range cacheGroups {
cacheGroupIds[index] = *cacheGroup.ID
}
- dsCDNs, err := dbhelpers.GetDeliveryServiceCDNsByTopology(topology.ReqInfo.Tx.Tx, topology.Name)
+ dsCDNs, err := dbhelpers.GetDeliveryServiceCDNsByTopology(topology.ReqInfo.Tx.Tx, currentTopoName)
if err != nil {
log.Errorf("validating topology: %v", err)
return errors.New("unable to validate topology")
}
rules["empty cachegroups"] = topology_validation.CheckForEmptyCacheGroups(topology.ReqInfo.Tx, cacheGroupIds, dsCDNs, false, nil)
- rules["required capabilities"] = topology.validateDSRequiredCapabilities()
+ rules["required capabilities"] = topology.validateDSRequiredCapabilities(currentTopoName)
//Get current Topology-CG for the requested change.
topoCachegroupNames := topology.getCachegroupNames()
- userErr, sysErr, _ = dbhelpers.CheckTopologyOrgServerCGInDSCG(topology.ReqInfo.Tx.Tx, dsCDNs, topology.Name, topoCachegroupNames)
+ userErr, sysErr, _ = dbhelpers.CheckTopologyOrgServerCGInDSCG(topology.ReqInfo.Tx.Tx, dsCDNs, currentTopoName, topoCachegroupNames)
if userErr != nil {
return userErr
}
@@ -176,6 +178,7 @@ func (topology *TOTopology) Validate() error {
}
func (topology *TOTopology) nodesInOtherTopologies() ([]tc.TopologyNode, map[string][]string, error) {
+ currentTopoName := topology.APIInfoImpl.ReqInfo.Params["name"]
baseError := errors.New("unable to verify that there are no cycles across all topologies")
where := `WHERE name != :topology_name`
query := selectQueryWithParentNames() + where + `
@@ -183,7 +186,7 @@ func (topology *TOTopology) nodesInOtherTopologies() ([]tc.TopologyNode, map[str
UNION ` + selectNonTopologyParentCacheGroupsQuery()
parameters := map[string]interface{}{
- "topology_name": topology.Name,
+ "topology_name": currentTopoName,
"edge_type_prefix": strings.ToLower(tc.EdgeTypePrefix) + "%",
"mid_type_prefix": strings.ToLower(tc.MidTypePrefix) + "%",
}
@@ -268,12 +271,12 @@ func (topology *TOTopology) nodesInOtherTopologies() ([]tc.TopologyNode, map[str
return nodes, topologiesByCacheGroup, nil
}
-func (topology TOTopology) validateDSRequiredCapabilities() error {
+func (topology TOTopology) validateDSRequiredCapabilities(currentTopoName string) error {
baseError := errors.New("unable to verify that delivery service required capabilities are satisfied")
tx := topology.APIInfo().Tx.Tx
- dsRequiredCapabilities, dsCDNs, err := getDSRequiredCapabilitiesByTopology(topology.Name, tx)
+ dsRequiredCapabilities, dsCDNs, err := getDSRequiredCapabilitiesByTopology(currentTopoName, tx)
if err != nil {
- log.Errorf("validating delivery service required capabilities for topology %s: %v", topology.Name, err)
+ log.Errorf("validating delivery service required capabilities for topology %s: %v", currentTopoName, err)
return baseError
}
if len(dsRequiredCapabilities) == 0 {
@@ -310,7 +313,7 @@ GROUP BY s.id, s.cdn_id, c.name
}
cachegroupServers, serverCapabilities, serverCDNs, err := dbhelpers.ScanCachegroupsServerCapabilities(rows)
if err != nil {
- log.Errorf("validating delivery service required capabilities for topology %s: %v", topology.Name, err)
+ log.Errorf("validating delivery service required capabilities for topology %s: %v", currentTopoName, err)
return baseError
}
@@ -395,6 +398,7 @@ func (topology TOTopology) GetKeys() (map[string]interface{}, bool) {
// SetKeys is a requirement of the api.Updater interface and is called by
// api.UpdateHandler().
func (topology *TOTopology) SetKeys(keys map[string]interface{}) {
+ topology.RequestedName = topology.Name
topology.Name, _ = keys["name"].(string)
}
@@ -572,10 +576,10 @@ func (topology *TOTopology) addParents() (error, error, int) {
return nil, nil, http.StatusOK
}
-func (topology *TOTopology) setDescription() (error, error, int) {
- rows, err := topology.ReqInfo.Tx.Query(updateQuery(), topology.Description, topology.Name)
+func (topology *TOTopology) setTopologyDetails() (error, error, int) {
+ rows, err := topology.ReqInfo.Tx.Query(updateQuery(), topology.RequestedName, topology.Description, topology.Name)
if err != nil {
- return nil, fmt.Errorf("topology update: error setting the description for topology %v: %v", topology.Name, err.Error()), http.StatusInternalServerError
+ return nil, fmt.Errorf("topology update: error setting the name and/or description for topology %v: %v", topology.Name, err.Error()), http.StatusInternalServerError
}
defer log.Close(rows, "unable to close DB connection")
for rows.Next() {
@@ -597,9 +601,6 @@ func (topology *TOTopology) Update(h http.Header) (error, error, int) {
return fmt.Errorf("cannot find exactly 1 topology with the query string provided"), nil, http.StatusBadRequest
}
oldTopology := TOTopology{APIInfoImpl: topology.APIInfoImpl, Topology: topologies[0].(tc.Topology)}
- if userErr, sysErr, errCode := topology.setDescription(); userErr != nil || sysErr != nil {
- return userErr, sysErr, errCode
- }
if err := oldTopology.removeParents(); err != nil {
return nil, err, http.StatusInternalServerError
@@ -624,6 +625,9 @@ func (topology *TOTopology) Update(h http.Header) (error, error, int) {
return nil, err, http.StatusInternalServerError
}
}
+ if userErr, sysErr, errCode := topology.setTopologyDetails(); userErr != nil || sysErr != nil {
+ return userErr, sysErr, errCode
+ }
if userErr, sysErr, errCode := topology.addNodes(); userErr != nil || sysErr != nil {
return userErr, sysErr, errCode
}
@@ -781,8 +785,9 @@ WHERE tcp.child IN
func updateQuery() string {
query := `
UPDATE topology t SET
-description = $1
-WHERE t.name = $2
+name = $1,
+description = $2
+WHERE t.name = $3
RETURNING t.name, t.description, t.last_updated
`
return query
diff --git a/traffic_portal/app/src/common/api/TopologyService.js b/traffic_portal/app/src/common/api/TopologyService.js
index 59d1fe0..7deddba 100644
--- a/traffic_portal/app/src/common/api/TopologyService.js
+++ b/traffic_portal/app/src/common/api/TopologyService.js
@@ -42,8 +42,8 @@ var TopologyService = function($http, ENV, locationUtils, messageModel) {
);
};
- this.updateTopology = function(topology) {
- return $http.put(ENV.api['root'] + 'topologies', topology, { params: { name: topology.name } }).then(
+ this.updateTopology = function(topology, currentName) {
+ return $http.put(ENV.api['root'] + 'topologies', topology, { params: { name: currentName } }).then(
function(result) {
return result;
},
diff --git a/traffic_portal/app/src/common/modules/form/topology/edit/FormEditTopologyController.js b/traffic_portal/app/src/common/modules/form/topology/edit/FormEditTopologyController.js
index c0fcc75..ff8e772 100644
--- a/traffic_portal/app/src/common/modules/form/topology/edit/FormEditTopologyController.js
+++ b/traffic_portal/app/src/common/modules/form/topology/edit/FormEditTopologyController.js
@@ -37,11 +37,12 @@ var FormEditTopologyController = function(topologies, cacheGroups, $scope, $cont
saveLabel: 'Update'
};
- $scope.save = function(name, description, topologyTree) {
- let normalizedTopology = topologyUtils.getNormalizedTopology(name, description, topologyTree);
- topologyService.updateTopology(normalizedTopology).
+ $scope.save = function(currentName, newName, description, topologyTree) {
+ let normalizedTopology = topologyUtils.getNormalizedTopology(newName, description, topologyTree);
+ topologyService.updateTopology(normalizedTopology, currentName).
then(function(result) {
- messageModel.setMessages(result.data.alerts, false);
+ messageModel.setMessages(result.data.alerts, true);
+ locationUtils.navigateToPath('/topologies/edit?name=' + result.data.response.name);
});
};
diff --git a/traffic_portal/app/src/common/modules/form/topology/form.topology.tpl.html b/traffic_portal/app/src/common/modules/form/topology/form.topology.tpl.html
index da35785..98ed62e 100644
--- a/traffic_portal/app/src/common/modules/form/topology/form.topology.tpl.html
+++ b/traffic_portal/app/src/common/modules/form/topology/form.topology.tpl.html
@@ -62,7 +62,7 @@ under the License.
<div class="form-group" ng-class="{'has-error': hasError(topologyForm.name), 'has-feedback': hasError(topologyForm.name)}">
<label class="control-label col-md-2 col-sm-2 col-xs-12">Name *</label>
<div class="col-md-10 col-sm-10 col-xs-12">
- <input name="name" type="text" class="form-control" ng-model="topology.name" ng-disabled="!settings.isNew" pattern="^[a-zA-Z0-9\-_]+$" required autofocus>
+ <input name="name" type="text" class="form-control" ng-model="topology.name" pattern="^[a-zA-Z0-9\-_]+$" required autofocus>
<small class="input-error" ng-show="hasPropertyError(topologyForm.name, 'required')">Required</small>
<small class="input-error" ng-show="hasPropertyError(topologyForm.name, 'pattern')">Alphanumeric, dash, or underscore characters only</small>
<span ng-show="hasError(topologyForm.name)" class="form-control-feedback"><i class="fa fa-times"></i></span>
@@ -87,7 +87,8 @@ under the License.
</div>
<div class="modal-footer">
<button type="button" class="btn btn-danger" ng-show="!settings.isNew" ng-click="confirmDelete(topology)">Delete</button>
- <button type="button" class="btn btn-success" ng-disabled="topologyForm.$pristine || topologyForm.$invalid" ng-click="save(topology.name, topology.description, topologyTree)">{{settings.saveLabel}}</button>
+ <button ng-if="settings.isNew" type="button" class="btn btn-success" ng-disabled="topologyForm.$pristine || topologyForm.$invalid" ng-click="save(topology.name, topology.description, topologyTree)">{{settings.saveLabel}}</button>
+ <button ng-if="!settings.isNew" type="button" class="btn btn-success" ng-disabled="topologyForm.$pristine || topologyForm.$invalid" ng-click="save(topologyName, topology.name, topology.description, topologyTree)">{{settings.saveLabel}}</button>
</div>
</form>
</div>
diff --git a/traffic_portal/app/src/common/service/utils/TopologyUtils.js b/traffic_portal/app/src/common/service/utils/TopologyUtils.js
index 086bea2..09e6288 100644
--- a/traffic_portal/app/src/common/service/utils/TopologyUtils.js
+++ b/traffic_portal/app/src/common/service/utils/TopologyUtils.js
@@ -76,16 +76,16 @@ var TopologyUtils = function() {
Object.keys(all).forEach(function (guid) {
let item = all[guid];
if (!('children' in item)) {
- item.children = []
+ item.children = [];
}
if (item.parents.length === 0) {
item.parent = { name: '', type: '' };
item.secParent = { name: '', type: '' };
- roots.push(item)
+ roots.push(item);
} else if (item.parents[0] in all) {
- let p = all[item.parents[0]]
+ let p = all[item.parents[0]];
if (!('children' in p)) {
- p.children = []
+ p.children = [];
}
p.children.push(item);
// add parent to each node
diff --git a/traffic_portal/test/end_to_end/topologies/topologies-spec.js b/traffic_portal/test/end_to_end/topologies/topologies-spec.js
index 548e47d..1fb0bf6 100644
--- a/traffic_portal/test/end_to_end/topologies/topologies-spec.js
+++ b/traffic_portal/test/end_to_end/topologies/topologies-spec.js
@@ -77,7 +77,6 @@ describe('Traffic Portal Topologies Test Suite', function() {
});
}).get(0).click();
expect(pageData.updateButton.isEnabled()).toBe(false);
- expect(pageData.name.getAttribute('disabled')).toBe('true');
pageData.description.clear().then(function() {
pageData.description.sendKeys("Updated description");
});