From 75ee979d5fd2edd2e9409724610293d46ccb4d79 Mon Sep 17 00:00:00 2001 From: Evan Zelkowitz <19699200+ezelkow1@users.noreply.github.com> Date: Thu, 6 Dec 2018 10:34:28 -0700 Subject: [PATCH] Add monitoring.json snapshotting These changes are to fix the issue of the monitoring.json and crconfig's being out of sync. Currently it is possible to make a change to a status (or other field that the monitoring.json presents) and have it show up immediately while the changes are not reflected in a crconfig until it has been snapshotted, causing an inconcsistency across the files. This will change the column name in the snapshot table from 'content' to 'crconfig' and then add a new column 'monitoring'. When a snapshot is done we then store both the crconfig and monitoring.json in the table at the same time, maintaining consistency. Monitoring.json is then pulled from the table instead of being generated on the fly like was done previously. --- CHANGELOG.md | 1 + ...0181206000000_create_monitor_snapshots.sql | 28 +++++++++ .../traffic_ops_golang/crconfig/handler.go | 44 ++++++++++++-- .../traffic_ops_golang/crconfig/snapshot.go | 57 ++++++++++++++++--- .../crconfig/snapshot_test.go | 37 ++++++++---- .../monitoring/monitoring.go | 4 +- .../monitoring/monitoring_test.go | 16 +++--- traffic_ops/traffic_ops_golang/routes.go | 6 +- 8 files changed, 154 insertions(+), 39 deletions(-) create mode 100644 traffic_ops/app/db/migrations/20181206000000_create_monitor_snapshots.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index 78eed221ee..5b07ecba41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - To support reusing a single riak cluster connection, an optional parameter is added to riak.conf: "HealthCheckInterval". This options takes a 'Duration' value (ie: 10s, 5m) which affects how often the riak cluster is health checked. Default is currently set to: "HealthCheckInterval": "5s". - Added an API 1.4 endpoint, /api/1.4/cdns/dnsseckeys/refresh, to perform necessary behavior previously served outside the API under `/internal`. - Adds the DS Record text to the cdn dnsseckeys endpoint in 1.4. +- Added monitoring.json snapshotting. This stores the monitoring json in the same table as the crconfig snapshot. Snapshotting is now required in order to push out monitoring changes. ### Changed - Issue 2821: Fixed "Traffic Router may choose wrong certificate when SNI names overlap" diff --git a/traffic_ops/app/db/migrations/20181206000000_create_monitor_snapshots.sql b/traffic_ops/app/db/migrations/20181206000000_create_monitor_snapshots.sql new file mode 100644 index 0000000000..40ae3af8ca --- /dev/null +++ b/traffic_ops/app/db/migrations/20181206000000_create_monitor_snapshots.sql @@ -0,0 +1,28 @@ +/* + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +-- +goose Up +-- SQL in section 'Up' is executed when this migration is applied + +-- snapshots +ALTER TABLE snapshot ADD COLUMN monitoring json; +UPDATE snapshot SET monitoring = '{}'; +ALTER TABLE snapshot ALTER COLUMN monitoring SET NOT NULL; +ALTER TABLE snapshot RENAME content TO crconfig; + +-- +goose Down +-- SQL section 'Down' is executed when this migration is rolled back +ALTER TABLE snapshot DELETE COLUMN monitoring; +ALTER TABLE snapshot RENAME crconfig TO content; diff --git a/traffic_ops/traffic_ops_golang/crconfig/handler.go b/traffic_ops/traffic_ops_golang/crconfig/handler.go index ec980d9ed1..5dd430c94c 100644 --- a/traffic_ops/traffic_ops_golang/crconfig/handler.go +++ b/traffic_ops/traffic_ops_golang/crconfig/handler.go @@ -29,6 +29,7 @@ import ( "github.com/apache/trafficcontrol/lib/go-log" "github.com/apache/trafficcontrol/lib/go-tc" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api" + "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/monitoring" ) // Handler creates and serves the CRConfig from the raw SQL data. @@ -73,6 +74,28 @@ func SnapshotGetHandler(w http.ResponseWriter, r *http.Request) { w.Write([]byte(`{"response":` + snapshot + `}`)) } +// SnapshotGetMonitoringHandler gets and serves the CRConfig from the snapshot table. +func SnapshotGetMonitoringHandler(w http.ResponseWriter, r *http.Request) { + inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"cdn"}, nil) + if userErr != nil || sysErr != nil { + api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr) + return + } + defer inf.Close() + + snapshot, cdnExists, err := GetSnapshotMonitoring(inf.Tx.Tx, inf.Params["cdn"]) + if err != nil { + api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting snapshot: "+err.Error())) + return + } + if !cdnExists { + api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("CDN not found"), nil) + return + } + w.Header().Set(tc.ContentType, tc.ApplicationJson) + w.Write([]byte(`{"response":` + snapshot + `}`)) +} + // SnapshotOldGetHandler gets and serves the CRConfig from the snapshot table, not wrapped in response to match the old non-API CRConfig-Snapshots endpoint func SnapshotOldGetHandler(w http.ResponseWriter, r *http.Request) { inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"cdn"}, nil) @@ -129,11 +152,18 @@ func SnapshotHandler(w http.ResponseWriter, r *http.Request) { return } - if err := Snapshot(inf.Tx.Tx, crConfig); err != nil { - api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New(r.RemoteAddr+" snaphsotting CRConfig: "+err.Error())) + monitoringJSON, err := monitoring.GetMonitoringJSON(inf.Tx.Tx, cdn) + if err != nil { + api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New(r.RemoteAddr+" getting monitoring.json data: "+err.Error())) + return + } + + if err := Snapshot(inf.Tx.Tx, crConfig, monitoringJSON); err != nil { + api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New(r.RemoteAddr+" snaphsotting CRConfig and Monitoring: "+err.Error())) return } - api.CreateChangeLogRawTx(api.ApiChange, "Snapshot of CRConfig performed for "+cdn, inf.User, inf.Tx.Tx) + + api.CreateChangeLogRawTx(api.ApiChange, "Snapshot of CRConfig and Monitor performed for "+cdn, inf.User, inf.Tx.Tx) w.WriteHeader(http.StatusOK) // TODO change to 204 No Content in new version } @@ -152,7 +182,13 @@ func SnapshotOldGUIHandler(w http.ResponseWriter, r *http.Request) { return } - if err := Snapshot(inf.Tx.Tx, crConfig); err != nil { + tm, err := monitoring.GetMonitoringJSON(inf.Tx.Tx, inf.Params["cdn"]) + if err != nil { + api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New(r.RemoteAddr+" getting monitoring.json data: "+err.Error())) + return + } + + if err := Snapshot(inf.Tx.Tx, crConfig, tm); err != nil { writePerlHTMLErr(w, r, inf.Tx.Tx, errors.New(r.RemoteAddr+" making CRConfig: "+err.Error()), err) return } diff --git a/traffic_ops/traffic_ops_golang/crconfig/snapshot.go b/traffic_ops/traffic_ops_golang/crconfig/snapshot.go index dc4ab435d5..3c2e4d512e 100644 --- a/traffic_ops/traffic_ops_golang/crconfig/snapshot.go +++ b/traffic_ops/traffic_ops_golang/crconfig/snapshot.go @@ -27,10 +27,12 @@ import ( "github.com/apache/trafficcontrol/lib/go-log" "github.com/apache/trafficcontrol/lib/go-tc" + "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/monitoring" ) // Snapshot takes the CRConfig JSON-serializable object (which may be generated via crconfig.Make), and writes it to the snapshot table. -func Snapshot(tx *sql.Tx, crc *tc.CRConfig) error { +// It also takes the monitoring config JSON and writes it to the snapshot table. +func Snapshot(tx *sql.Tx, crc *tc.CRConfig, monitoringJSON *monitoring.Monitoring) error { log.Debugln("calling Snapshot") bts, err := json.Marshal(crc) if err != nil { @@ -40,10 +42,16 @@ func Snapshot(tx *sql.Tx, crc *tc.CRConfig) error { if crc.Stats.DateUnixSeconds != nil { date = time.Unix(*crc.Stats.DateUnixSeconds, 0) } + + btstm, err := json.Marshal(monitoringJSON) + if err != nil { + return errors.New("marshalling JSON: " + err.Error()) + } + log.Debugf("calling Snapshot, writing %+v\n", date) - q := `insert into snapshot (cdn, content, last_updated) values ($1, $2, $3) on conflict(cdn) do update set content=$2, last_updated=$3` - if _, err := tx.Exec(q, crc.Stats.CDNName, bts, date); err != nil { - return errors.New("Error inserting the snapshot into database: " + err.Error()) + q := `insert into snapshot (cdn, crconfig, last_updated, monitoring) values ($1, $2, $3, $4) on conflict(cdn) do update set crconfig=$2, last_updated=$3, monitoring=$4` + if _, err := tx.Exec(q, crc.Stats.CDNName, bts, date, btstm); err != nil { + return errors.New("Error inserting the crconfig and monitoring snapshot into database: " + err.Error()) } return nil } @@ -58,17 +66,17 @@ func GetSnapshot(tx *sql.Tx, cdn string) (string, bool, error) { snapshot := sql.NullString{} // cdn left join snapshot, so we get a row with null if the CDN exists but the snapshot doesn't, and no rows if the CDN doesn't exist. q := ` -select s.content as snapshot -from cdn as c -left join snapshot as s on s.cdn = c.name -where c.name = $1 +SELECT s.crconfig AS snapshot +FROM cdn AS c +LEFT JOIN snapshot AS s ON s.cdn = c.name +WHERE c.name = $1 ` if err := tx.QueryRow(q, cdn).Scan(&snapshot); err != nil { if err == sql.ErrNoRows { // CDN doesn't exist return "", false, nil } - return "", false, errors.New("Error querying snapshot: " + err.Error()) + return "", false, errors.New("Error querying crconfig snapshot: " + err.Error()) } if !snapshot.Valid { // CDN exists, but snapshot doesn't @@ -76,3 +84,34 @@ where c.name = $1 } return snapshot.String, true, nil } + +// GetSnapshotMonitoring gets the monitor snapshot for the given CDN. +// If the CDN does not exist, false is returned. +// If the CDN exists, but the snapshot does not, the string for an empty JSON object "{}" is returned. +// An error is only returned on database error, never if the CDN or snapshot does not exist. +// Because all snapshotting is handled by the crconfig endpoints we have to also do the monitoring one +// here as well +func GetSnapshotMonitoring(tx *sql.Tx, cdn string) (string, bool, error) { + log.Debugln("calling GetSnapshotMonitoring") + + monitorSnapshot := sql.NullString{} + // cdn left join snapshot, so we get a row with null if the CDN exists but the snapshot doesn't, and no rows if the CDN doesn't exist. + q := ` +SELECT s.monitoring AS snapshot +FROM cdn AS c +LEFT JOIN snapshot AS s ON s.cdn = c.name +WHERE c.name = $1 +` + if err := tx.QueryRow(q, cdn).Scan(&monitorSnapshot); err != nil { + if err == sql.ErrNoRows { + // CDN doesn't exist + return "", false, nil + } + return "", false, errors.New("Error querying monitor snapshot: " + err.Error()) + } + if !monitorSnapshot.Valid { + // CDN exists, but snapshot doesn't + return `{}`, true, nil + } + return monitorSnapshot.String, true, nil +} diff --git a/traffic_ops/traffic_ops_golang/crconfig/snapshot_test.go b/traffic_ops/traffic_ops_golang/crconfig/snapshot_test.go index 09fc1dbced..535637ab9c 100644 --- a/traffic_ops/traffic_ops_golang/crconfig/snapshot_test.go +++ b/traffic_ops/traffic_ops_golang/crconfig/snapshot_test.go @@ -21,6 +21,7 @@ package crconfig import ( "context" + "database/sql" "database/sql/driver" "encoding/json" "reflect" @@ -28,7 +29,7 @@ import ( "time" "github.com/apache/trafficcontrol/lib/go-tc" - + "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/monitoring" "gopkg.in/DATA-DOG/go-sqlmock.v1" ) @@ -36,10 +37,16 @@ func ExpectedGetSnapshot(crc *tc.CRConfig) ([]byte, error) { return json.Marshal(crc) } +func ExpectedGetMontioringSnapshot(crc *tc.CRConfig, tx *sql.Tx) ([]byte, error) { + tm, _ := monitoring.GetMonitoringJSON(tx, *crc.Stats.CDNName) + return json.Marshal(tm) +} + func MockGetSnapshot(mock sqlmock.Sqlmock, expected []byte, cdn string) { rows := sqlmock.NewRows([]string{"snapshot"}) rows = rows.AddRow(expected) - mock.ExpectQuery("select").WithArgs(cdn).WillReturnRows(rows) + rows = rows.AddRow(expected) + mock.ExpectQuery("SELECT").WithArgs(cdn).WillReturnRows(rows) } func TestGetSnapshot(t *testing.T) { @@ -97,8 +104,8 @@ func (a Any) Match(v driver.Value) bool { return true } -func MockSnapshot(mock sqlmock.Sqlmock, expected []byte, cdn string) { - mock.ExpectExec("insert").WithArgs(cdn, expected, AnyTime{}).WillReturnResult(sqlmock.NewResult(1, 1)) +func MockSnapshot(mock sqlmock.Sqlmock, expected []byte, expectedtm []byte, cdn string) { + mock.ExpectExec("insert").WithArgs(cdn, expected, AnyTime{}, expectedtm).WillReturnResult(sqlmock.NewResult(1, 1)) } func TestSnapshot(t *testing.T) { @@ -112,23 +119,31 @@ func TestSnapshot(t *testing.T) { crc := &tc.CRConfig{} crc.Stats.CDNName = &cdn - mock.ExpectBegin() + + dbCtx, _ := context.WithTimeout(context.TODO(), time.Duration(10)*time.Second) + tx, err := db.BeginTx(dbCtx, nil) + if err != nil { + t.Fatalf("creating transaction: %v", err) + } + expected, err := ExpectedGetSnapshot(crc) if err != nil { t.Fatalf("GetSnapshot creating expected err expected: nil, actual: %v", err) } - MockSnapshot(mock, expected, cdn) - mock.ExpectCommit() - dbCtx, _ := context.WithTimeout(context.TODO(), time.Duration(10)*time.Second) - tx, err := db.BeginTx(dbCtx, nil) + expectedtm, err := ExpectedGetMontioringSnapshot(crc, tx) if err != nil { - t.Fatalf("creating transaction: %v", err) + t.Fatalf("GetSnapshotMonitor creating expected err expected: nil, actual: %v", err) } + + tm, _ := monitoring.GetMonitoringJSON(tx, *crc.Stats.CDNName) + MockSnapshot(mock, expected, expectedtm, cdn) + mock.ExpectCommit() + defer tx.Commit() - if err := Snapshot(tx, crc); err != nil { + if err := Snapshot(tx, crc, tm); err != nil { t.Fatalf("GetSnapshot err expected: nil, actual: %v", err) } } diff --git a/traffic_ops/traffic_ops_golang/monitoring/monitoring.go b/traffic_ops/traffic_ops_golang/monitoring/monitoring.go index c72ebb86cb..16b7681118 100644 --- a/traffic_ops/traffic_ops_golang/monitoring/monitoring.go +++ b/traffic_ops/traffic_ops_golang/monitoring/monitoring.go @@ -109,10 +109,10 @@ func Get(w http.ResponseWriter, r *http.Request) { return } defer inf.Close() - api.RespWriter(w, r, inf.Tx.Tx)(getMonitoringJSON(inf.Tx.Tx, inf.Params["cdn"])) + api.RespWriter(w, r, inf.Tx.Tx)(GetMonitoringJSON(inf.Tx.Tx, inf.Params["cdn"])) } -func getMonitoringJSON(tx *sql.Tx, cdnName string) (*Monitoring, error) { +func GetMonitoringJSON(tx *sql.Tx, cdnName string) (*Monitoring, error) { monitors, caches, routers, err := getMonitoringServers(tx, cdnName) if err != nil { return nil, fmt.Errorf("error getting servers: %v", err) diff --git a/traffic_ops/traffic_ops_golang/monitoring/monitoring_test.go b/traffic_ops/traffic_ops_golang/monitoring/monitoring_test.go index e025cfe2d4..053a9fdd0e 100644 --- a/traffic_ops/traffic_ops_golang/monitoring/monitoring_test.go +++ b/traffic_ops/traffic_ops_golang/monitoring/monitoring_test.go @@ -659,9 +659,9 @@ func TestGetMonitoringJSON(t *testing.T) { t.Fatalf("creating transaction: %v", err) } - sqlResp, err := getMonitoringJSON(tx, cdn) + sqlResp, err := GetMonitoringJSON(tx, cdn) if err != nil { - t.Errorf("getMonitoringJSON expected: nil error, actual: %v", err) + t.Errorf("GetMonitoringJSON expected: nil error, actual: %v", err) } resp.Response.TrafficServers = sortCaches(resp.Response.TrafficServers) @@ -676,22 +676,22 @@ func TestGetMonitoringJSON(t *testing.T) { sqlResp.DeliveryServices = sortDeliveryServices(sqlResp.DeliveryServices) if !reflect.DeepEqual(sqlResp.TrafficServers, resp.Response.TrafficServers) { - t.Errorf("getMonitoringJSON expected TrafficServers: %+v actual: %+v", resp.Response.TrafficServers, sqlResp.TrafficServers) + t.Errorf("GetMonitoringJSON expected TrafficServers: %+v actual: %+v", resp.Response.TrafficServers, sqlResp.TrafficServers) } if !reflect.DeepEqual(sqlResp.TrafficMonitors, resp.Response.TrafficMonitors) { - t.Errorf("getMonitoringJSON expected TrafficMonitors: %+v actual: %+v", resp.Response.TrafficMonitors, sqlResp.TrafficMonitors) + t.Errorf("GetMonitoringJSON expected TrafficMonitors: %+v actual: %+v", resp.Response.TrafficMonitors, sqlResp.TrafficMonitors) } if !reflect.DeepEqual(sqlResp.Cachegroups, resp.Response.Cachegroups) { - t.Errorf("getMonitoringJSON expected Cachegroups: %+v actual: %+v", resp.Response.Cachegroups, sqlResp.Cachegroups) + t.Errorf("GetMonitoringJSON expected Cachegroups: %+v actual: %+v", resp.Response.Cachegroups, sqlResp.Cachegroups) } if !reflect.DeepEqual(sqlResp.Profiles, resp.Response.Profiles) { - t.Errorf("getMonitoringJSON expected Profiles: %+v actual: %+v", resp.Response.Profiles, sqlResp.Profiles) + t.Errorf("GetMonitoringJSON expected Profiles: %+v actual: %+v", resp.Response.Profiles, sqlResp.Profiles) } if !reflect.DeepEqual(sqlResp.DeliveryServices, resp.Response.DeliveryServices) { - t.Errorf("getMonitoringJSON expected DeliveryServices: %+v actual: %+v", resp.Response.DeliveryServices, sqlResp.DeliveryServices) + t.Errorf("GetMonitoringJSON expected DeliveryServices: %+v actual: %+v", resp.Response.DeliveryServices, sqlResp.DeliveryServices) } if !reflect.DeepEqual(sqlResp.Config, resp.Response.Config) { - t.Errorf("getMonitoringJSON expected Config: %+v actual: %+v", resp.Response.Config, sqlResp.Config) + t.Errorf("GetMonitoringJSON expected Config: %+v actual: %+v", resp.Response.Config, sqlResp.Config) } } diff --git a/traffic_ops/traffic_ops_golang/routes.go b/traffic_ops/traffic_ops_golang/routes.go index 63613043da..a52fdba25f 100644 --- a/traffic_ops/traffic_ops_golang/routes.go +++ b/traffic_ops/traffic_ops_golang/routes.go @@ -53,7 +53,6 @@ import ( "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/federations" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/hwinfo" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/login" - "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/monitoring" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/parameter" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/physlocation" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/ping" @@ -142,7 +141,7 @@ func Routes(d ServerData) ([]Route, []RawRoute, http.Handler, error) { {1.4, http.MethodGet, `cdns/dnsseckeys/refresh/?(\.json)?$`, cdn.RefreshDNSSECKeys, auth.PrivLevelOperations, Authenticated, nil}, //CDN: Monitoring: Traffic Monitor - {1.1, http.MethodGet, `cdns/{cdn}/configs/monitoring(\.json)?$`, monitoring.Get, auth.PrivLevelReadOnly, Authenticated, nil}, + {1.1, http.MethodGet, `cdns/{cdn}/configs/monitoring(\.json)?$`, crconfig.SnapshotGetMonitoringHandler, auth.PrivLevelReadOnly, Authenticated, nil}, //Division: CRUD {1.1, http.MethodGet, `divisions/?(\.json)?$`, api.ReadHandler(division.GetTypeSingleton()), auth.PrivLevelReadOnly, Authenticated, nil}, @@ -276,9 +275,6 @@ func Routes(d ServerData) ([]Route, []RawRoute, http.Handler, error) { {1.2, http.MethodGet, `servers/status$`, handlerToFunc(proxyHandler), 0, NoAuth, []Middleware{}}, {1.2, http.MethodGet, `servers/totals$`, handlerToFunc(proxyHandler), 0, NoAuth, []Middleware{}}, - //Monitoring - {1.2, http.MethodGet, `cdns/{name}/configs/monitoring(\.json)?$`, monitoring.Get, auth.PrivLevelReadOnly, Authenticated, nil}, - //ASNs {1.3, http.MethodGet, `asns/?(\.json)?$`, api.ReadHandler(asn.GetTypeSingleton()), auth.PrivLevelReadOnly, Authenticated, nil}, {1.3, http.MethodPut, `asns/?$`, api.UpdateHandler(asn.GetTypeSingleton()), auth.PrivLevelOperations, Authenticated, nil},