Skip to content

Commit

Permalink
Handle dyno name parsing for Fir apps (#3075)
Browse files Browse the repository at this point in the history
* Handle dyno name parsing for Fir apps

* Fix missing accept header for tests.

* Remove dangling .only in test

* Improve process type and dyno number split logic
  • Loading branch information
sbosio authored Nov 12, 2024
1 parent 4f7472f commit c0a3519
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 30 deletions.
30 changes: 21 additions & 9 deletions packages/cli/src/commands/ps/index.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
import color from '@heroku-cli/color'
import {Command, flags} from '@heroku-cli/command'
import {ux} from '@oclif/core'
import * as Heroku from '@heroku-cli/schema'
import {ago} from '../../lib/time'
import {APIClient} from '@heroku-cli/command'
import {AccountQuota} from '../../lib/types/account_quota'
import {AppProcessTier} from '../../lib/types/app_process_tier'
import {DynoExtended} from '../../lib/types/dyno_extended'
import heredoc from 'tsheredoc'
import {Account} from '../../lib/types/fir'

function getProcessNumber(s: string) : number {
return Number.parseInt(s.split('.', 2)[1], 10)
const [processType, dynoNumber] = (s.match(/^([^.]+)\.(.*)$/) || []).slice(1, 3)

if (!processType || !dynoNumber?.match(/^\d+$/))
return 0

return Number.parseInt(dynoNumber, 10)
}

function uniqueValues(value: string, index: number, self: string[]) : boolean {
Expand Down Expand Up @@ -51,7 +57,7 @@ function truncate(s: string) {
function printExtended(dynos: DynoExtended[]) {
const sortedDynos = dynos.sort(byProcessTypeAndNumber)

ux.table(
ux.table<DynoExtended>(
sortedDynos,
{
ID: {get: (dyno: DynoExtended) => dyno.id},
Expand All @@ -75,7 +81,7 @@ function printExtended(dynos: DynoExtended[]) {
)
}

async function printAccountQuota(heroku: APIClient, app: Required<Heroku.App>, account: Required<Heroku.Account>) {
async function printAccountQuota(heroku: APIClient, app: AppProcessTier, account: Account) {
if (app.process_tier !== 'free' && app.process_tier !== 'eco') {
return
}
Expand Down Expand Up @@ -188,18 +194,24 @@ export default class Index extends Command {
app: flags.app({required: true}),
remote: flags.remote(),
json: flags.boolean({description: 'display as json'}),
extended: flags.boolean({char: 'x', hidden: true}), // should be removed? Platform API doesn't serialize extended attributes even if the query param `extended=true` is sent.
extended: flags.boolean({char: 'x', hidden: true}), // only works with sudo privileges
}

public async run(): Promise<void> {
const {flags, ...restParse} = await this.parse(Index)
const {app, json, extended} = flags
const types = restParse.argv as string[]
const suffix = extended ? '?extended=true' : '' // read previous comment, including this on the request doesn't make any difference.
const suffix = extended ? '?extended=true' : ''
const promises = {
dynos: this.heroku.request<DynoExtended[]>(`/apps/${app}/dynos${suffix}`),
appInfo: this.heroku.request<Required<Heroku.App>>(`/apps/${app}`, {headers: {Accept: 'application/vnd.heroku+json; version=3.process-tier'}}),
accountInfo: this.heroku.request<Required<Heroku.Account>>('/account'),
dynos: this.heroku.request<DynoExtended[]>(`/apps/${app}/dynos${suffix}`, {
headers: {Accept: 'application/vnd.heroku+json; version=3.sdk'},
}),
appInfo: this.heroku.request<AppProcessTier>(`/apps/${app}`, {
headers: {Accept: 'application/vnd.heroku+json; version=3.sdk'},
}),
accountInfo: this.heroku.request<Account>('/account', {
headers: {Accept: 'application/vnd.heroku+json; version=3.sdk'},
}),
}
const [{body: dynos}, {body: appInfo}, {body: accountInfo}] = await Promise.all([promises.dynos, promises.appInfo, promises.accountInfo])
const shielded = appInfo.space && appInfo.space.shield
Expand Down
4 changes: 4 additions & 0 deletions packages/cli/src/lib/types/app_process_tier.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import {App} from './fir'
export interface AppProcessTier extends App {
process_tier: string
}
5 changes: 3 additions & 2 deletions packages/cli/src/lib/types/dyno_extended.d.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {Dyno} from '@heroku-cli/schema'
import {Dyno} from './fir'

export interface DynoExtended extends Required<Dyno> {
export interface DynoExtended extends Dyno {
/**
* Extended information.
*/
Expand All @@ -14,4 +14,5 @@ export interface DynoExtended extends Required<Dyno> {
region: string | null,
route: string | null,
}
[name: string]: unknown
}
69 changes: 50 additions & 19 deletions packages/cli/test/unit/commands/ps/index.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ const hourAgo = new Date(Date.now() - (60 * 60 * 1000))
const hourAgoStr = strftime('%Y/%m/%d %H:%M:%S %z', hourAgo)

function stubAccountQuota(code: number, body: Record<string, unknown>) {
nock('https://api.heroku.com', {reqheaders: {accept: 'application/vnd.heroku+json; version=3.process-tier'}})
nock('https://api.heroku.com', {reqheaders: {accept: 'application/vnd.heroku+json; version=3.sdk'}})
.get('/apps/myapp')
.reply(200, {process_tier: 'eco', owner: {id: '1234'}, id: '6789'})
nock('https://api.heroku.com')
nock('https://api.heroku.com', {reqheaders: {accept: 'application/vnd.heroku+json; version=3.sdk'}})
.get('/apps/myapp/dynos')
.reply(200, [{command: 'bash', size: 'Eco', name: 'run.1', type: 'run', updated_at: hourAgo, state: 'up'}])
nock('https://api.heroku.com')
nock('https://api.heroku.com', {reqheaders: {accept: 'application/vnd.heroku+json; version=3.sdk'}})
.get('/account')
.reply(200, {id: '1234'})
nock('https://api.heroku.com', {reqheaders: {Accept: 'application/vnd.heroku+json; version=3.account-quotas'}})
Expand All @@ -26,10 +26,10 @@ function stubAccountQuota(code: number, body: Record<string, unknown>) {
}

function stubAppAndAccount() {
nock('https://api.heroku.com', {reqheaders: {Accept: 'application/vnd.heroku+json; version=3.process-tier'}})
nock('https://api.heroku.com', {reqheaders: {Accept: 'application/vnd.heroku+json; version=3.sdk'}})
.get('/apps/myapp')
.reply(200, {process_tier: 'basic', owner: {id: '1234'}, id: '6789'})
nock('https://api.heroku.com')
nock('https://api.heroku.com', {reqheaders: {accept: 'application/vnd.heroku+json; version=3.sdk'}})
.get('/account')
.reply(200, {id: '1234'})
}
Expand All @@ -40,7 +40,7 @@ describe('ps', function () {
})

it('shows dyno list', async function () {
const api = nock('https://api.heroku.com')
const api = nock('https://api.heroku.com', {reqheaders: {accept: 'application/vnd.heroku+json; version=3.sdk'}})
.get('/apps/myapp/dynos')
.reply(200, [
{command: 'npm start', size: 'Eco', name: 'web.1', type: 'web', updated_at: hourAgo, state: 'up'},
Expand Down Expand Up @@ -68,8 +68,39 @@ describe('ps', function () {
expect(stderr.output).to.equal('')
})

it('shows dyno list for Fir apps', async function () {
const api = nock('https://api.heroku.com', {reqheaders: {accept: 'application/vnd.heroku+json; version=3.sdk'}})
.get('/apps/myapp/dynos')
.reply(200, [
{command: 'npm start', size: '1X-Classic', name: 'web.4ed720fa31-ur8z1', type: 'web', updated_at: hourAgo, state: 'up'},
{command: 'npm start', size: '1X-Classic', name: 'web.4ed720fa31-5om2v', type: 'web', updated_at: hourAgo, state: 'up'},
{command: 'npm start ./worker.js', size: '2X-Compute', name: 'node-worker.4ed720fa31-w4llb', type: 'node-worker', updated_at: hourAgo, state: 'up'},
])
stubAppAndAccount()

await runCommand(Cmd, [
'--app',
'myapp',
])

api.done()

expect(stdout.output).to.equal(heredoc`
=== node-worker (2X-Compute): npm start ./worker.js (1)
node-worker.4ed720fa31-w4llb: up ${hourAgoStr} (~ 1h ago)
=== web (1X-Classic): npm start (2)
web.4ed720fa31-5om2v: up ${hourAgoStr} (~ 1h ago)
web.4ed720fa31-ur8z1: up ${hourAgoStr} (~ 1h ago)
`)
expect(stderr.output).to.equal('')
})

it('shows shield dynos in dyno list for apps in a shielded private space', async function () {
const api = nock('https://api.heroku.com')
const api = nock('https://api.heroku.com', {reqheaders: {accept: 'application/vnd.heroku+json; version=3.sdk'}})
.get('/apps/myapp')
.reply(200, {space: {shield: true}})
.get('/apps/myapp/dynos')
Expand Down Expand Up @@ -101,7 +132,7 @@ describe('ps', function () {
})

it('errors when no dynos found', async function () {
const api = nock('https://api.heroku.com')
const api = nock('https://api.heroku.com', {reqheaders: {accept: 'application/vnd.heroku+json; version=3.sdk'}})
.get('/apps/myapp/dynos')
.reply(200, [
{command: 'npm start', size: 'Eco', name: 'web.1', type: 'web', updated_at: hourAgo, state: 'up'},
Expand All @@ -126,7 +157,7 @@ describe('ps', function () {
})

it('shows dyno list as json', async function () {
const api = nock('https://api.heroku.com')
const api = nock('https://api.heroku.com', {reqheaders: {accept: 'application/vnd.heroku+json; version=3.sdk'}})
.get('/account')
.reply(200, {id: '1234'})
.get('/apps/myapp')
Expand All @@ -148,7 +179,7 @@ describe('ps', function () {
})

it('shows extended info', async function () {
const api = nock('https://api.heroku.com')
const api = nock('https://api.heroku.com', {reqheaders: {accept: 'application/vnd.heroku+json; version=3.sdk'}})
.get('/account')
.reply(200, {id: '1234'})
.get('/apps/myapp')
Expand Down Expand Up @@ -194,7 +225,7 @@ describe('ps', function () {
})

it('shows extended info for Private Space app', async function () {
const api = nock('https://api.heroku.com')
const api = nock('https://api.heroku.com', {reqheaders: {accept: 'application/vnd.heroku+json; version=3.sdk'}})
.get('/account')
.reply(200, {id: '1234'})
.get('/apps/myapp')
Expand Down Expand Up @@ -258,7 +289,7 @@ describe('ps', function () {
})

it('shows shield dynos in extended info if app is in a shielded private space', async function () {
const api = nock('https://api.heroku.com')
const api = nock('https://api.heroku.com', {reqheaders: {accept: 'application/vnd.heroku+json; version=3.sdk'}})
.get('/account')
.reply(200, {id: '1234'})
.get('/apps/myapp')
Expand Down Expand Up @@ -432,18 +463,18 @@ describe('ps', function () {
})

it('does not print out for apps that are not owned', async function () {
nock('https://api.heroku.com')
nock('https://api.heroku.com', {reqheaders: {accept: 'application/vnd.heroku+json; version=3.sdk'}})
.get('/account')
.reply(200, {id: '1234'})
nock('https://api.heroku.com', {reqheaders: {Accept: 'application/vnd.heroku+json; version=3.process-tier'}})
nock('https://api.heroku.com', {reqheaders: {Accept: 'application/vnd.heroku+json; version=3.sdk'}})
.get('/apps/myapp')
.reply(200, {
process_tier: 'eco', owner: {id: '5678'},
})
nock('https://api.heroku.com', {reqheaders: {Accept: 'application/vnd.heroku+json; version=3.account-quotas'}})
.get('/accounts/1234/actions/get-quota')
.reply(200, {account_quota: 1000, quota_used: 1, apps: []})
const dynos = nock('https://api.heroku.com')
const dynos = nock('https://api.heroku.com', {reqheaders: {accept: 'application/vnd.heroku+json; version=3.sdk'}})
.get('/apps/myapp/dynos')
.reply(200, [{command: 'bash', size: 'Eco', name: 'run.1', type: 'run', updated_at: hourAgo, state: 'up'}])
const ecoExpression = heredoc`
Expand All @@ -465,13 +496,13 @@ describe('ps', function () {
})

it('does not print out for non-eco apps', async function () {
nock('https://api.heroku.com')
nock('https://api.heroku.com', {reqheaders: {accept: 'application/vnd.heroku+json; version=3.sdk'}})
.get('/account')
.reply(200, {id: '1234'})
nock('https://api.heroku.com', {reqheaders: {Accept: 'application/vnd.heroku+json; version=3.process-tier'}})
nock('https://api.heroku.com', {reqheaders: {Accept: 'application/vnd.heroku+json; version=3.sdk'}})
.get('/apps/myapp')
.reply(200, {process_tier: 'eco', owner: {id: 1234}})
const dynos = nock('https://api.heroku.com')
const dynos = nock('https://api.heroku.com', {reqheaders: {accept: 'application/vnd.heroku+json; version=3.sdk'}})
.get('/apps/myapp/dynos')
.reply(200, [{command: 'bash', size: 'Eco', name: 'run.1', type: 'run', updated_at: hourAgo, state: 'up'}])
const ecoExpression = heredoc`
Expand Down Expand Up @@ -511,7 +542,7 @@ describe('ps', function () {
})

it('logs to stdout and exits zero when no dynos', async function () {
const dynos = nock('https://api.heroku.com')
const dynos = nock('https://api.heroku.com', {reqheaders: {accept: 'application/vnd.heroku+json; version=3.sdk'}})
.get('/apps/myapp/dynos')
.reply(200, [])
stubAppAndAccount()
Expand Down

0 comments on commit c0a3519

Please sign in to comment.