Extended LP feature to aggregate params for all profiles. (#7291)

* Extended LP feature to aggregate params for all profiles. * renamed some variables. * reverting changes in unit test. * Resolve unit test * updated CHANGELOG.md * Updated based on review feedback.
apache · Feb 2, 2023 · 9da78ae · 9da78ae
1 parent ff91c6c
commit 9da78ae
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 47 deletions.
diff --git 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
+- [#7291](https://github.com/apache/trafficcontrol/pull/7291) *Traffic Ops* Extended Layered Profile feature to aggregate parameters for all server profiles.
 - [#7314](https://github.com/apache/trafficcontrol/pull/7314) *Traffic Portal* Added capability feature to Delivery Service Form (HTTP, DNS).
 - [#7295](https://github.com/apache/trafficcontrol/pull/7295) *Traffic Portal* Added description and priority order for Layered Profile on server form.
 - [#6234](https://github.com/apache/trafficcontrol/issues/6234) *Traffic Ops, Traffic Portal* Added description field to Server Capabilities

diff --git a/traffic_ops/traffic_ops_golang/monitoring/monitoring.go b/traffic_ops/traffic_ops_golang/monitoring/monitoring.go
@@ -188,7 +188,7 @@ SELECT
 	status.name as status,
 	cachegroup.name as cachegroup,
 	me.tcp_port as port,
-	profile.name as profile,
+	(SELECT STRING_AGG(sp.profile_name, ' ' ORDER by sp.priority ASC) FROM server_profile AS sp where sp.server=me.id group by sp.server) as profile,
 	type.name as type,
 	me.xmpp_id as hashID,
     me.id as serverID
@@ -432,62 +432,33 @@ func getProfiles(tx *sql.Tx, caches []Cache, routers []Router) ([]Profile, error
 	cacheProfileTypes := map[string]string{}
 	profiles := map[string]Profile{}
 	profileNames := []string{}
+	profileTypes := map[string]string{}
 	for _, router := range routers {
-		profiles[router.Profile] = Profile{
-			Name: router.Profile,
-			Type: router.Type,
-		}
+		profileNames = append(profileNames, router.Profile)
+		profileTypes[router.Profile] = router.Type
 	}
 
 	for _, cache := range caches {
 		if _, ok := cacheProfileTypes[cache.Profile]; !ok {
 			cacheProfileTypes[cache.Profile] = cache.Type
-			profiles[cache.Profile] = Profile{
-				Name: cache.Profile,
-				Type: cache.Type,
-			}
 			profileNames = append(profileNames, cache.Profile)
+			profileTypes[cache.Profile] = cache.Type
 		}
 	}
 
-	query := `
-SELECT p.name as profile, pr.name, pr.value
-FROM parameter pr
-JOIN profile p ON p.name = ANY($1)
-JOIN profile_parameter pp ON pp.profile = p.id and pp.parameter = pr.id
-WHERE pr.config_file = $2;
-`
-	rows, err := tx.Query(query, pq.Array(profileNames), CacheMonitorConfigFile)
+	profileParameters, err := aggregateMultipleProfileParameters(tx, profileNames)
 	if err != nil {
 		return nil, err
 	}
-	defer rows.Close()
-
-	for rows.Next() {
-		var profileName sql.NullString
-		var name sql.NullString
-		var value sql.NullString
-		if err := rows.Scan(&profileName, &name, &value); err != nil {
-			return nil, err
-		}
-		if name.String == "" {
-			return nil, fmt.Errorf("null name") // TODO continue and warn?
-		}
-		profile := profiles[profileName.String]
-		if profile.Parameters == nil {
-			profile.Parameters = map[string]interface{}{}
+	for pName, parameters := range profileParameters {
+		profiles[pName] = Profile{
+			Name:       pName,
+			Type:       profileTypes[pName],
+			Parameters: parameters,
 		}
-
-		if valNum, err := strconv.Atoi(value.String); err == nil {
-			profile.Parameters[name.String] = valNum
-		} else {
-			profile.Parameters[name.String] = value.String
-		}
-		profiles[profileName.String] = profile
-
 	}
 
-	profilesArr := []Profile{} // TODO make for efficiency?
+	profilesArr := make([]Profile, len([]Profile{}))
 	for _, profile := range profiles {
 		profilesArr = append(profilesArr, profile)
 	}
@@ -571,3 +542,43 @@ AND c.name = $3
 	}
 	return cfg, nil
 }
+
+func aggregateMultipleProfileParameters(tx *sql.Tx, profileNames []string) (map[string]map[string]interface{}, error) {
+	p := make(map[string]map[string]interface{})
+	query := `
+SELECT p.name, pr.name, pr.value
+FROM parameter pr
+JOIN profile p ON p.name = ANY($1)
+JOIN profile_parameter pp ON pp.profile = p.id and pp.parameter = pr.id
+WHERE pr.config_file = $2
+ORDER BY ARRAY_POSITION($1, p.name), pr.name;`
+
+	for _, profile := range profileNames {
+		profileList := strings.Split(profile, " ")
+		rows, err := tx.Query(query, pq.Array(profileList), CacheMonitorConfigFile)
+		if err != nil {
+			return nil, err
+		}
+		defer rows.Close()
+
+		parameter := make(map[string]interface{})
+		for rows.Next() {
+			var pName, prName, value string
+			if err := rows.Scan(&pName, &prName, &value); err != nil {
+				return nil, err
+			}
+			if prName == "" {
+				return nil, fmt.Errorf("null name") // TODO continue and warn?
+			}
+			if _, ok := parameter[prName]; !ok {
+				if valNum, err := strconv.Atoi(value); err == nil {
+					parameter[prName] = valNum
+				} else {
+					parameter[prName] = value
+				}
+			}
+		}
+		p[profile] = parameter
+	}
+	return p, nil
+}
diff --git a/traffic_ops/traffic_ops_golang/monitoring/monitoring_test.go b/traffic_ops/traffic_ops_golang/monitoring/monitoring_test.go
@@ -271,19 +271,26 @@ func TestGetProfiles(t *testing.T) {
 
 	mock.ExpectBegin()
 	rows := sqlmock.NewRows([]string{"profile", "name", "value"})
+	rows1 := sqlmock.NewRows([]string{"profile", "name", "value"})
 	for _, profile := range profiles {
 		for paramName, paramVal := range profile.Parameters {
-			rows = rows.AddRow(profile.Name, paramName, paramVal)
+			if profile.Name == "routerProfile" {
+				rows = rows.AddRow(profile.Name, paramName, paramVal)
+			} else if profile.Name == "cacheProfile" {
+				rows1 = rows1.AddRow(profile.Name, paramName, paramVal)
+			}
 		}
 	}
 
 	caches := []Cache{cache}
 	routers := []Router{router}
 
-	profileNames := []string{"cacheProfile"}
-
+	profileNames := []string{"routerProfile"}
 	mock.ExpectQuery("SELECT").WithArgs(pq.Array(profileNames), CacheMonitorConfigFile).WillReturnRows(rows)
 
+	profileNames = []string{"cacheProfile"}
+	mock.ExpectQuery("SELECT").WithArgs(pq.Array(profileNames), CacheMonitorConfigFile).WillReturnRows(rows1)
+
 	dbCtx, f := context.WithTimeout(context.TODO(), time.Duration(10)*time.Second)
 	defer f()
 	tx, err := db.BeginTx(dbCtx, nil)
@@ -494,19 +501,26 @@ func TestGetMonitoringJSON(t *testing.T) {
 		}
 
 		rows := sqlmock.NewRows([]string{"profile", "name", "value"})
+		rows1 := sqlmock.NewRows([]string{"profile", "name", "value"})
 		for _, profile := range profiles {
 			for paramName, paramVal := range profile.Parameters {
-				rows = rows.AddRow(profile.Name, paramName, paramVal)
+				if profile.Name == "routerProfile" {
+					rows = rows.AddRow(profile.Name, paramName, paramVal)
+				} else if profile.Name == "cacheProfile" {
+					rows1 = rows1.AddRow(profile.Name, paramName, paramVal)
+				}
 			}
 		}
 
 		// caches := []Cache{cache}
 		// routers := []Router{router}
 
-		profileNames := []string{"cacheProfile"}
-
+		profileNames := []string{"routerProfile"}
 		mock.ExpectQuery("SELECT").WithArgs(pq.Array(profileNames), CacheMonitorConfigFile).WillReturnRows(rows)
 		resp.Response.Profiles = profiles
+
+		profileNames = []string{"cacheProfile"}
+		mock.ExpectQuery("SELECT").WithArgs(pq.Array(profileNames), CacheMonitorConfigFile).WillReturnRows(rows1)
 	}
 	{
 		//