Skip to content

Commit

Permalink
Add confirmation and warning for DBs with continuous protection (hero…
Browse files Browse the repository at this point in the history
…ku#158)

If a database has continuous protection, then we warn the user that logical backups are not
necessary and that backups of large databases may fail.
  • Loading branch information
coreypurcell authored and jdx committed Jun 19, 2018
1 parent 2a0d0bb commit 433a101
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 10 deletions.
15 changes: 15 additions & 0 deletions packages/heroku-pg/commands/backups/capture.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,21 @@ function * run (context, heroku) {
yield heroku.post(`/postgres/v0/databases/${db.id}/snapshots`, {host: host(db)})
}))
} else {
let dbInfo = yield heroku.request({
host: host(db),
method: 'get',
path: `/client/v11/databases/${db.id}`
}).catch(err => {
if (err.statusCode !== 404) throw err
cli.exit(1, `${cli.color.addon(db.name)} is not yet provisioned.\nRun ${cli.color.cmd('heroku addons:wait')} to wait until the db is provisioned.`)
})
if (dbInfo) {
let dbProtected = /On/.test(dbInfo.info.find(attribute => attribute.name === 'Continuous Protection').values[0])
if (dbProtected) {
cli.warn('Continuous protection is already enabled for this database. Logical backups of large databases are likely to fail.')
cli.warn('See https://devcenter.heroku.com/articles/heroku-postgres-data-safety-and-continuous-protection#physical-backups-on-heroku-postgres.')
}
}
let backup
yield cli.action(`Starting backup of ${cli.color.addon(db.name)}`, co(function * () {
backup = yield heroku.post(`/client/v11/databases/${db.id}/backups`, {host: host(db)})
Expand Down
17 changes: 17 additions & 0 deletions packages/heroku-pg/commands/backups/schedule.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,23 @@ function * run (context, heroku) {

let at = cli.color.cyan(`${schedule.hour}:00 ${schedule.timezone}`)

let dbInfo = yield heroku.request({
host: host(db),
method: 'get',
path: `/client/v11/databases/${db.id}`
}).catch(err => {
if (err.statusCode !== 404) throw err
cli.exit(1, `${cli.color.addon(db.name)} is not yet provisioned.\nRun ${cli.color.cmd('heroku addons:wait')} to wait until the db is provisioned.`)
})

if (dbInfo) {
let dbProtected = /On/.test(dbInfo.info.find(attribute => attribute.name === 'Continuous Protection').values[0])
if (dbProtected) {
cli.warn('Continuous protection is already enabled for this database. Logical backups of large databases are likely to fail.')
cli.warn('See https://devcenter.heroku.com/articles/heroku-postgres-data-safety-and-continuous-protection#physical-backups-on-heroku-postgres.')
}
}

yield cli.action(`Scheduling automatic daily backups of ${cli.color.addon(db.name)} at ${at}`, co(function * () {
schedule.schedule_name = util.getUrl(attachment.config_vars)

Expand Down
30 changes: 25 additions & 5 deletions packages/heroku-pg/test/commands/backups/capture.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ const shouldCapture = function (cmdRun) {
finished_at: '101',
succeeded: true
})

let dbA = {info: [
{name: 'Continuous Protection', values: ['On']}
]}
pg.get('/client/v11/databases/1').reply(200, dbA)

cli.mockConsole()

return cmdRun({app: 'myapp', args: {}, flags: {}})
Expand All @@ -51,7 +57,8 @@ Use heroku pg:backups:info to check progress.
Stop a running backup with heroku pg:backups:cancel.
`))
.then(() => expect(cli.stderr, 'to equal', `Starting backup of postgres-1... done\n${captureText()}`))
.then(() => expect(cli.stderr, 'to match', new RegExp(`Starting backup of postgres-1... done\n${captureText()}`)))
.then(() => expect(cli.stderr, 'to match', new RegExp(`backups of large databases are likely to fail`)))
})

it('captures a db (verbose)', () => {
Expand All @@ -70,6 +77,12 @@ Stop a running backup with heroku pg:backups:cancel.
succeeded: true,
logs: [{created_at: '100', message: 'log message 1'}]
})

let dbA = {info: [
{name: 'Continuous Protection', values: ['Off']}
]}
pg.get('/client/v11/databases/1').reply(200, dbA)

cli.mockConsole()

return cmdRun({app: 'myapp', args: {}, flags: {verbose: true}})
Expand All @@ -81,8 +94,8 @@ Stop a running backup with heroku pg:backups:cancel.
Backing up DATABASE to b005...
100 log message 1
`))
.then(() => expect(cli.stderr, 'to equal', `Starting backup of postgres-1... done
`))
.then(() => expect(cli.stderr, 'to match', new RegExp(`Starting backup of postgres-1... done
`))).then(() => expect(cli.stderr, 'not to match', new RegExp(`backups of large databases are likely to fail`)))
})

it('captures a db (verbose) with non billing app', () => {
Expand All @@ -101,6 +114,12 @@ Backing up DATABASE to b005...
succeeded: true,
logs: [{created_at: '100', message: 'log message 1'}]
})

let dbA = {info: [
{name: 'Continuous Protection', values: ['On']}
]}
pg.get('/client/v11/databases/1').reply(200, dbA)

cli.mockConsole()

return cmdRun({app: 'myapp', args: {}, flags: {verbose: true}})
Expand All @@ -115,8 +134,8 @@ Use heroku pg:backups -a mybillingapp to check the list of backups.
Backing up DATABASE to b005...
100 log message 1
`))
.then(() => expect(cli.stderr, 'to equal', `Starting backup of postgres-1... done
`))
.then(() => expect(cli.stderr, 'to match', new RegExp(`Starting backup of postgres-1... done
`))).then(() => expect(cli.stderr, 'to match', new RegExp(`backups of large databases are likely to fail`)))
})

it('captures a snapshot if called with the --snapshot flag', () => {
Expand All @@ -126,6 +145,7 @@ Backing up DATABASE to b005...

pg = nock('https://postgres-api.heroku.com')
pg.post('/postgres/v0/databases/1/snapshots').reply(200, {})

cli.mockConsole()

return cmdRun({app: 'myapp', args: {}, flags: {snapshot: true}})
Expand Down
35 changes: 30 additions & 5 deletions packages/heroku-pg/test/commands/backups/schedule.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ const shouldSchedule = function (cmdRun) {
}
])
pg = nock('https://postgres-api.heroku.com')
pg.post('/client/v11/databases/1/transfer-schedules', {
'hour': '06', 'timezone': 'America/New_York', 'schedule_name': 'DATABASE_URL'
}).reply(201)

cli.mockConsole()
})

Expand All @@ -34,12 +38,33 @@ const shouldSchedule = function (cmdRun) {
})

it('schedules a backup', () => {
pg.post('/client/v11/databases/1/transfer-schedules', {
'hour': '06', 'timezone': 'America/New_York', 'schedule_name': 'DATABASE_URL'
}).reply(201)
return cmdRun({app: 'myapp', args: {}, flags: {at: '06:00 EDT'}})
let dbA = {info: [
{name: 'Continuous Protection', values: ['On']}
]}
pg.get('/client/v11/databases/1').reply(200, dbA)
return cmdRun({app: 'myapp', args: {}, flags: {at: '06:00 EDT', confirm: 'myapp'}})
.then(() => expect(cli.stdout, 'to equal', ''))
.then(() => expect(cli.stderr, 'to equal', 'Scheduling automatic daily backups of postgres-1 at 06:00 America/New_York... done\n'))
.then(() => expect(cli.stderr, 'to match', /Scheduling automatic daily backups of postgres-1 at 06:00 America\/New_York... done\n/))
})

it('warns user that logical backups are error prone if continuous proctecion is on', () => {
let dbA = {info: [
{name: 'Continuous Protection', values: ['On']}
]}
pg.get('/client/v11/databases/1').reply(200, dbA)

return cmdRun({app: 'myapp', args: {}, flags: {at: '06:00 EDT'}})
.then(() => expect(cli.stderr, 'to match', /backups of large databases are likely to fail/))
})

it('does not warn user that logical backups are error prone if continuous proctecion is off', () => {
let dbA = {info: [
{name: 'Continuous Protection', values: ['Off']}
]}
pg.get('/client/v11/databases/1').reply(200, dbA)

return cmdRun({app: 'myapp', args: {}, flags: {at: '06:00 EDT'}})
.then(() => expect(cli.stderr, 'not to match', /backups of large databases are likely to fail/))
})
}

Expand Down

0 comments on commit 433a101

Please sign in to comment.