This is an automated email from the ASF dual-hosted git repository. zrhoffman pushed a commit to branch 6.0.x in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
commit 8f5d0bc27527373a4002bba7f44a1b6018025de3 Author: mattjackson220 <[email protected]> AuthorDate: Fri Aug 20 09:19:53 2021 -0600 Fixed Federations IMS (#6122) * Fixed Federations IMS * updates per comments (cherry picked from commit bfd9f91241c6024233d3af1db22163ce7e085be7) --- CHANGELOG.md | 1 + traffic_ops/testing/api/v4/cdnfederations_test.go | 33 +++++++++++ traffic_ops/testing/api/v4/federations_test.go | 21 ++++++- .../cdnfederation/cdnfederations.go | 4 ++ .../federations/allfederations.go | 54 ++++++++--------- .../traffic_ops_golang/federations/federations.go | 67 ++++++++-------------- 6 files changed, 109 insertions(+), 71 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 41251a4..010ddd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -100,6 +100,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - [#6091](https://github.com/apache/trafficcontrol/issues/6091) - Fixed cache config of internal cache communication for https origins - [#6066](https://github.com/apache/trafficcontrol/issues/6066) - Fixed missing/incorrect indices on some tables - [#5576](https://github.com/apache/trafficcontrol/issues/5576) - Inconsistent Profile Name restrictions +- Fixed Federations IMS so TR federations watcher will get updates. ### Changed - Updated the Traffic Ops Python client to 3.0 diff --git a/traffic_ops/testing/api/v4/cdnfederations_test.go b/traffic_ops/testing/api/v4/cdnfederations_test.go index 77fc4b7..f66efeb 100644 --- a/traffic_ops/testing/api/v4/cdnfederations_test.go +++ b/traffic_ops/testing/api/v4/cdnfederations_test.go @@ -35,6 +35,7 @@ func TestCDNFederations(t *testing.T) { SortTestCDNFederations(t) UpdateTestCDNFederations(t) GetTestCDNFederations(t) + GetTestCDNFederationsIMS(t) currentTime := time.Now().UTC().Add(-5 * time.Second) time := currentTime.Format(time.RFC1123) var header http.Header @@ -196,6 +197,38 @@ func GetTestCDNFederations(t *testing.T) { } } +func GetTestCDNFederationsIMS(t *testing.T) { + futureTime := time.Now().AddDate(0, 0, 1) + fmtFutureTime := futureTime.Format(time.RFC1123) + opts := client.NewRequestOptions() + opts.Header.Set(rfc.IfModifiedSince, fmtFutureTime) + for _, id := range fedIDs { + opts.QueryParameters.Set("id", strconv.Itoa(id)) + data, reqInf, err := TOSession.GetCDNFederationsByName("foo", opts) + if err != nil { + t.Errorf("could not get federations: %v - alerts: %+v", err, data.Alerts) + } + if reqInf.StatusCode != http.StatusNotModified { + t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode) + } + } + + pastTime := time.Now().AddDate(0, 0, -1) + fmtPastTime := pastTime.Format(time.RFC1123) + opts = client.NewRequestOptions() + opts.Header.Set(rfc.IfModifiedSince, fmtPastTime) + for _, id := range fedIDs { + opts.QueryParameters.Set("id", strconv.Itoa(id)) + data, reqInf, err := TOSession.GetCDNFederationsByName("foo", opts) + if err != nil { + t.Errorf("could not get federations: %v - alerts: %+v", err, data.Alerts) + } + if reqInf.StatusCode != http.StatusOK { + t.Fatalf("Expected 200 status code, got %v", reqInf.StatusCode) + } + } +} + func AssignTestFederationFederationResolvers(t *testing.T) { // Setup if len(fedIDs) < 2 { diff --git a/traffic_ops/testing/api/v4/federations_test.go b/traffic_ops/testing/api/v4/federations_test.go index c1958c6..2e60f7e 100644 --- a/traffic_ops/testing/api/v4/federations_test.go +++ b/traffic_ops/testing/api/v4/federations_test.go @@ -42,10 +42,10 @@ func TestFederations(t *testing.T) { func GetTestFederationsIMS(t *testing.T) { futureTime := time.Now().AddDate(0, 0, 1) - time := futureTime.Format(time.RFC1123) + fmtFutureTime := futureTime.Format(time.RFC1123) opts := client.NewRequestOptions() - opts.Header.Set(rfc.IfModifiedSince, time) + opts.Header.Set(rfc.IfModifiedSince, fmtFutureTime) if len(testData.Federations) == 0 { t.Error("no federations test data") } @@ -57,6 +57,23 @@ func GetTestFederationsIMS(t *testing.T) { if reqInf.StatusCode != http.StatusNotModified { t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode) } + + pastTime := time.Now().AddDate(0, 0, -1) + fmtPastTime := pastTime.Format(time.RFC1123) + + opts = client.NewRequestOptions() + opts.Header.Set(rfc.IfModifiedSince, fmtPastTime) + if len(testData.Federations) == 0 { + t.Error("no federations test data") + } + + resp, reqInf, err = TOSession.AllFederations(opts) + if err != nil { + t.Fatalf("No error expected, but got: %v - alerts: %+v", err, resp.Alerts) + } + if reqInf.StatusCode != http.StatusOK { + t.Fatalf("Expected 200 status code, got %v", reqInf.StatusCode) + } } func GetTestFederations(t *testing.T) { diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go index e34aec9..2607bcf 100644 --- a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go +++ b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go @@ -182,6 +182,10 @@ func (fed *TOCDNFederation) Read(h http.Header, useIMS bool) ([]interface{}, err return nil, userErr, sysErr, errCode, nil } + if errCode == http.StatusNotModified { + return []interface{}{}, nil, nil, http.StatusNotModified, maxTime + } + filteredFederations := []interface{}{} for _, ifederation := range federations { federation := ifederation.(*TOCDNFederation) diff --git a/traffic_ops/traffic_ops_golang/federations/allfederations.go b/traffic_ops/traffic_ops_golang/federations/allfederations.go index 20cfc08..035fa73 100644 --- a/traffic_ops/traffic_ops_golang/federations/allfederations.go +++ b/traffic_ops/traffic_ops_golang/federations/allfederations.go @@ -89,17 +89,7 @@ func GetAll(w http.ResponseWriter, r *http.Request) { } } - fedsResolvers, err, code, maxTime := getFederationResolvers(inf.Tx.Tx, fedInfoIDs(feds), useIMS, r.Header) - if code == http.StatusNotModified { - if maxTime != nil && api.SetLastModifiedHeader(r, useIMS) { - // RFC1123 - date := maxTime.Format("Mon, 02 Jan 2006 15:04:05 MST") - w.Header().Add(rfc.LastModified, date) - } - w.WriteHeader(code) - api.WriteResp(w, r, []tc.IAllFederation{}) - return - } + fedsResolvers, err := getFederationResolvers(inf.Tx.Tx, fedInfoIDs(feds)) if err != nil { api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("federations.Get getting federations resolvers: "+err.Error())) return @@ -125,13 +115,19 @@ FROM ORDER BY ds.xml_id ` - imsQuery := `SELECT max(t) from ( - SELECT max(fds.last_updated) as t FROM - federation_deliveryservice fds - JOIN deliveryservice ds ON ds.id = fds.deliveryservice - JOIN federation fd ON fd.id = fds.federation - UNION ALL - select max(last_updated) as t from last_deleted l where l.table_name='federation_deliveryservice') as res` + imsQuery := `SELECT Max(last_updated) + FROM (SELECT last_updated + FROM federation_deliveryservice fds + UNION ALL + SELECT last_updated + FROM federation_federation_resolver ffr + UNION ALL + SELECT last_updated + FROM federation fd + UNION ALL + SELECT Max(last_updated) AS t + FROM last_deleted l + WHERE l.table_name IN ( 'federation_deliveryservice', 'federation', 'federation_federation_resolver' )) AS res;` if useIMS { runSecond, maxTime = tryIfModifiedSinceQuery(header, tx, "", imsQuery) @@ -181,14 +177,20 @@ ORDER BY ds.xml_id ` - imsQuery := `SELECT max(t) from ( - SELECT max(fds.last_updated) as t from federation_deliveryservice fds - JOIN deliveryservice ds ON ds.id = fds.deliveryservice - JOIN federation fd ON fd.id = fds.federation - JOIN cdn on cdn.id = ds.cdn_id - WHERE cdn.name = $1 - UNION ALL - select max(last_updated) as t from last_deleted l where l.table_name='federation_deliveryservice') as res` + // TODO improve query to be CDN-specific + imsQuery := `SELECT Max(last_updated) + FROM (SELECT last_updated + FROM federation_deliveryservice fds + UNION ALL + SELECT last_updated + FROM federation_federation_resolver ffr + UNION ALL + SELECT last_updated + FROM federation fd + UNION ALL + SELECT Max(last_updated) AS t + FROM last_deleted l + WHERE l.table_name IN ( 'federation_deliveryservice', 'federation', 'federation_federation_resolver' )) AS res;` if useIMS { runSecond, maxTime = tryIfModifiedSinceQuery(header, tx, string(cdn), imsQuery) diff --git a/traffic_ops/traffic_ops_golang/federations/federations.go b/traffic_ops/traffic_ops_golang/federations/federations.go index 7d462c4..02acc73 100644 --- a/traffic_ops/traffic_ops_golang/federations/federations.go +++ b/traffic_ops/traffic_ops_golang/federations/federations.go @@ -103,12 +103,7 @@ func Get(w http.ResponseWriter, r *http.Request) { api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("federations.Get getting federations: "+err.Error())) return } - fedsResolvers, err, code, maxTime := getFederationResolvers(inf.Tx.Tx, fedInfoIDs(feds), useIMS, r.Header) - if code == http.StatusNotModified { - w.WriteHeader(code) - api.WriteResp(w, r, []tc.IAllFederation{}) - return - } + fedsResolvers, err := getFederationResolvers(inf.Tx.Tx, fedInfoIDs(feds)) if err != nil { api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("federations.Get getting federations resolvers: "+err.Error())) return @@ -166,9 +161,7 @@ type FedResolverInfo struct { } // getFederationResolvers takes a slice of federation IDs, and returns a map[federationID]info. -func getFederationResolvers(tx *sql.Tx, fedIDs []int, useIMS bool, header http.Header) (map[int][]FedResolverInfo, error, int, *time.Time) { - var runSecond bool - var maxTime time.Time +func getFederationResolvers(tx *sql.Tx, fedIDs []int) (map[int][]FedResolverInfo, error) { feds := map[int][]FedResolverInfo{} qry := ` SELECT @@ -182,29 +175,9 @@ FROM WHERE ffr.federation = ANY($1) ` - imsQuery := `SELECT max(t) from ( - SELECT max(ffr.last_updated) as t FROM - federation_federation_resolver ffr - JOIN federation_resolver fr ON ffr.federation_resolver = fr.id - JOIN type frt on fr.type = frt.id -WHERE ffr.federation = ANY($1) - UNION ALL - select max(last_updated) as t from last_deleted l where l.table_name='federation_federation_resolver') as res` - - if useIMS { - runSecond, maxTime = tryIfModifiedSinceQueryFederations(header, tx, pq.Array(fedIDs), imsQuery) - if !runSecond { - log.Debugln("IMS HIT") - return feds, nil, http.StatusNotModified, &maxTime - } - log.Debugln("IMS MISS") - } else { - log.Debugln("Non IMS request") - } - rows, err := tx.Query(qry, pq.Array(fedIDs)) if err != nil { - return nil, errors.New("all federations resolvers querying: " + err.Error()), http.StatusInternalServerError, nil + return nil, errors.New("all federations resolvers querying: " + err.Error()) } defer rows.Close() @@ -213,12 +186,12 @@ WHERE ffr.federation = ANY($1) f := FedResolverInfo{} fType := "" if err := rows.Scan(&fedID, &fType, &f.IP); err != nil { - return nil, errors.New("all federations resolvers scanning: " + err.Error()), http.StatusInternalServerError, nil + return nil, errors.New("all federations resolvers scanning: " + err.Error()) } f.Type = tc.FederationResolverTypeFromString(fType) feds[fedID] = append(feds[fedID], f) } - return feds, nil, http.StatusOK, &maxTime + return feds, nil } func tryIfModifiedSinceQueryFederations(header http.Header, tx *sql.Tx, fedID interface{}, query string) (bool, time.Time) { @@ -285,17 +258,25 @@ WHERE ORDER BY ds.xml_id ` - imsQuery := `SELECT max(t) from ( - SELECT max(fds.last_updated) as t FROM - federation_deliveryservice fds - JOIN deliveryservice ds ON ds.id = fds.deliveryservice - JOIN federation fd ON fd.id = fds.federation - JOIN federation_tmuser fu on fu.federation = fd.id - JOIN tm_user u on u.id = fu.tm_user -WHERE - u.username = $1 - UNION ALL - select max(last_updated) as t from last_deleted l where l.table_name='federation_deliveryservice') as res` + imsQuery := `SELECT Max(t) + FROM ((SELECT Greatest(fdsmax, ffrmax, fdmax) AS t + FROM (SELECT Max(fds.last_updated) AS fdsmax, + Max(ffr.last_updated) AS ffrmax, + Max(fd.last_updated) AS fdmax + FROM federation_deliveryservice fds + JOIN federation_federation_resolver ffr + ON ffr.federation = fds.federation + JOIN federation fd + ON fd.id = fds.federation + JOIN federation_tmuser fu + ON fu.federation = fd.id + JOIN tm_user u + ON u.id = fu.tm_user + WHERE u.username = $1) AS t + UNION ALL + SELECT Max(last_updated) AS t + FROM last_deleted l + WHERE l.table_name IN ( 'federation_deliveryservice', 'federation', 'federation_federation_resolver' ))) AS res;` if useIMS { runSecond, maxTime = tryIfModifiedSinceQuery(header, tx, userName, imsQuery)
