[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #4702: Prohibit assigning topology-based delivery services to caches

[GitBox <gi...@apache.org>]

rawlinp commented on a change in pull request #4702:
URL: https://github.com/apache/trafficcontrol/pull/4702#discussion_r432774331



##########
File path: traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
##########
@@ -598,6 +598,66 @@ func GetServerNameFromID(tx *sql.Tx, id int) (string, bool, error) {
 	return name, true, nil
 }
 
+type ServerHostNameAndType struct {
+	HostName string
+	Type     string
+}
+
+func GetServerHostNamesAndTypesFromIDs(tx *sql.Tx, ids []int) ([]ServerHostNameAndType, error) {
+	qry := `
+SELECT
+  s.host_name,
+  t.name
+FROM
+  server s JOIN type t ON s.type = t.id
+WHERE
+  s.id = ANY($1)
+`
+	rows, err := tx.Query(qry, pq.Array(ids))
+	if err != nil {
+		return nil, errors.New("querying server host names and types: " + err.Error())
+	}
+	defer rows.Close()

Review comment:
       fixed in bab0027b5999970f74cf85e4abe15e7398f3392f 

##########
File path: traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
##########
@@ -677,6 +737,34 @@ func GetCacheGroupNameFromID(tx *sql.Tx, id int64) (tc.CacheGroupName, bool, err
 	return tc.CacheGroupName(name), true, nil
 }
 
+// GetDeliveryServicesWithTopologies returns a list containing the delivery services in the given dsIDs
+// list that have a topology assigned. An error indicates unexpected errors that occurred when querying.
+func GetDeliveryServicesWithTopologies(tx *sql.Tx, dsIDs []int) ([]int, error) {
+	q := `
+SELECT
+  id
+FROM
+  deliveryservice
+WHERE
+  id = ANY($1::bigint[])
+  AND topology IS NOT NULL
+`
+	rows, err := tx.Query(q, pq.Array(dsIDs))
+	if err != nil {
+		return nil, errors.New("querying deliveryservice topologies: " + err.Error())
+	}
+	defer rows.Close()

Review comment:
       fixed in bab0027b5999970f74cf85e4abe15e7398f3392f 

##########
File path: traffic_ops/testing/api/v3/deliveryserviceservers_test.go
##########
@@ -37,6 +39,52 @@ func TestDeliveryServiceServersWithRequiredCapabilities(t *testing.T) {
 	})
 }
 
+func AssignServersToTopologyBasedDeliveryService(t *testing.T) {
+	ds, _, err := TOSession.GetDeliveryServiceByXMLIDNullable("ds-top")
+	if err != nil {
+		t.Fatalf("cannot GET delivery service 'ds-top': %s", err.Error())
+	}
+	if len(ds) != 1 {
+		t.Fatalf("expected one delivery service: 'ds-top', actual: %v", ds)
+	}
+	if ds[0].Topology == nil {
+		t.Fatal("expected delivery service: 'ds-top' to have a non-nil Topology, actual: nil")
+	}
+	serversResp, _, err := TOSession.GetServers()
+	servers := []tc.Server{}
+	for _, s := range serversResp {
+		if s.CDNID == *ds[0].CDNID && s.Type == tc.CacheTypeEdge.String() {
+			servers = append(servers, s)
+		}
+	}
+	if len(servers) < 1 {
+		t.Fatalf("expected: at least one EDGE in cdn %s, actual: %v", *ds[0].CDNName, servers)
+	}
+	serverNames := []string{}
+	for _, s := range servers {
+		if s.CDNID == *ds[0].CDNID && s.Type == tc.CacheTypeEdge.String() {
+			serverNames = append(serverNames, s.HostName)
+		} else {
+			t.Fatalf("expected only EDGE servers in cdn '%s', actual: %v", *ds[0].CDNName, servers)
+		}
+	}
+	_, reqInf, err := TOSession.AssignServersToDeliveryService(serverNames, "ds-top")
+	if err == nil {
+		t.Fatal("assigning servers to topology-based delivery service - expected: error, actual: nil error")
+	}
+	if reqInf.StatusCode < http.StatusBadRequest || reqInf.StatusCode >= http.StatusInternalServerError {
+		t.Fatalf("assigning servers to topology-based delivery service - expected: 400-level status code, actual: %d", reqInf.StatusCode)
+	}
+
+	_, reqInf, err = TOSession.CreateDeliveryServiceServers(*ds[0].ID, []int{servers[0].ID}, false)
+	if err == nil {
+		t.Fatal("creating delliveryserviceserver assignment for topology-based delivery service - expected: error, actual: nil error")

Review comment:
       fixed in bab0027b5999970f74cf85e4abe15e7398f3392f 

##########
File path: traffic_ops/testing/api/v3/deliveryserviceservers_test.go
##########
@@ -37,6 +39,52 @@ func TestDeliveryServiceServersWithRequiredCapabilities(t *testing.T) {
 	})
 }
 
+func AssignServersToTopologyBasedDeliveryService(t *testing.T) {
+	ds, _, err := TOSession.GetDeliveryServiceByXMLIDNullable("ds-top")
+	if err != nil {
+		t.Fatalf("cannot GET delivery service 'ds-top': %s", err.Error())
+	}
+	if len(ds) != 1 {
+		t.Fatalf("expected one delivery service: 'ds-top', actual: %v", ds)
+	}
+	if ds[0].Topology == nil {
+		t.Fatal("expected delivery service: 'ds-top' to have a non-nil Topology, actual: nil")
+	}
+	serversResp, _, err := TOSession.GetServers()
+	servers := []tc.Server{}
+	for _, s := range serversResp {
+		if s.CDNID == *ds[0].CDNID && s.Type == tc.CacheTypeEdge.String() {
+			servers = append(servers, s)
+		}
+	}
+	if len(servers) < 1 {
+		t.Fatalf("expected: at least one EDGE in cdn %s, actual: %v", *ds[0].CDNName, servers)
+	}
+	serverNames := []string{}
+	for _, s := range servers {
+		if s.CDNID == *ds[0].CDNID && s.Type == tc.CacheTypeEdge.String() {
+			serverNames = append(serverNames, s.HostName)
+		} else {
+			t.Fatalf("expected only EDGE servers in cdn '%s', actual: %v", *ds[0].CDNName, servers)
+		}
+	}
+	_, reqInf, err := TOSession.AssignServersToDeliveryService(serverNames, "ds-top")
+	if err == nil {
+		t.Fatal("assigning servers to topology-based delivery service - expected: error, actual: nil error")
+	}
+	if reqInf.StatusCode < http.StatusBadRequest || reqInf.StatusCode >= http.StatusInternalServerError {
+		t.Fatalf("assigning servers to topology-based delivery service - expected: 400-level status code, actual: %d", reqInf.StatusCode)
+	}
+
+	_, reqInf, err = TOSession.CreateDeliveryServiceServers(*ds[0].ID, []int{servers[0].ID}, false)
+	if err == nil {
+		t.Fatal("creating delliveryserviceserver assignment for topology-based delivery service - expected: error, actual: nil error")
+	}
+	if reqInf.StatusCode < http.StatusBadRequest || reqInf.StatusCode >= http.StatusInternalServerError {
+		t.Fatalf("creating delliveryserviceserver assignment for topology-based delivery service - expected: 400-level status code, actual: %d", reqInf.StatusCode)

Review comment:
       fixed in bab0027b5999970f74cf85e4abe15e7398f3392f 

##########
File path: traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
##########
@@ -598,6 +598,66 @@ func GetServerNameFromID(tx *sql.Tx, id int) (string, bool, error) {
 	return name, true, nil
 }
 
+type ServerHostNameAndType struct {
+	HostName string
+	Type     string
+}
+
+func GetServerHostNamesAndTypesFromIDs(tx *sql.Tx, ids []int) ([]ServerHostNameAndType, error) {
+	qry := `
+SELECT
+  s.host_name,
+  t.name
+FROM
+  server s JOIN type t ON s.type = t.id
+WHERE
+  s.id = ANY($1)
+`
+	rows, err := tx.Query(qry, pq.Array(ids))
+	if err != nil {
+		return nil, errors.New("querying server host names and types: " + err.Error())
+	}
+	defer rows.Close()
+
+	servers := []ServerHostNameAndType{}
+	for rows.Next() {
+		s := ServerHostNameAndType{}
+		if err := rows.Scan(&s.HostName, &s.Type); err != nil {
+			return nil, errors.New("scanning server host name and type: " + err.Error())
+		}
+		servers = append(servers, s)
+	}
+	return servers, nil
+}
+
+// GetServerTypesFromHostNames returns the host names and types of the given server host names or an error if any occur.
+func GetServerTypesFromHostNames(tx *sql.Tx, hostNames []string) ([]ServerHostNameAndType, error) {
+	qry := `
+SELECT
+  s.host_name,
+  t.name
+FROM
+  server s JOIN type t ON s.type = t.id
+WHERE
+  s.host_name = ANY($1)
+`
+	rows, err := tx.Query(qry, pq.Array(hostNames))
+	if err != nil {
+		return nil, errors.New("querying server host names and types: " + err.Error())
+	}
+	defer rows.Close()

Review comment:
       fixed in bab0027b5999970f74cf85e4abe15e7398f3392f 




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