Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Commit

Permalink
review feedback
Browse files Browse the repository at this point in the history 
use Path.join
some formatting changes
  • Loading branch information
@KarateCowboy
KarateCowboy committed May 5, 2017
1 parent b4c8b85 commit 434c6c4
Show file tree
Hide file tree
Showing 8 changed files with 158 additions and 172 deletions.
49 changes: 23 additions & 26 deletions commands/login.js
View file
Original file line number Diff line number Diff line change
@@ -1,49 +1,46 @@
'use strict';
const cli = require('heroku-cli-util')
const child = require('child_process')
const log = require('../lib/log')

const cli = require('heroku-cli-util');
const co = require('co');
const child = require('child_process');
const log = require('../lib/log');

module.exports = function(topic) {
module.exports = function (topic) {
return {
topic: topic,
command: 'login',
flags: [{ name: 'verbose', char: 'v', hasValue: false }],
flags: [{name: 'verbose', char: 'v', hasValue: false}],
description: 'Logs in to the Heroku Docker registry',
needsApp: false,
needsAuth: true,
run: cli.command(co.wrap(login))
};
};
run: cli.command(login)
}
}

function* login(context, heroku) {
let herokuHost = process.env.HEROKU_HOST || 'heroku.com';
let registry = `registry.${ herokuHost }`;
let password = context.auth.password;
async function login (context, heroku) {
let herokuHost = process.env.HEROKU_HOST || 'heroku.com'
let registry = `registry.${ herokuHost }`
let password = context.auth.password

try {
let user = yield dockerLogin(registry, password, context.flags.verbose);
let user = await dockerLogin(registry, password, context.flags.verbose)
}
catch (err) {
cli.error(`Error: docker login exited with ${ err }`);
cli.hush(err.stack || err);
cli.error(`Error: docker login exited with ${ err }`)
cli.hush(err.stack || err)
}
}

function dockerLogin(registry, password, verbose) {
function dockerLogin (registry, password, verbose) {
return new Promise((resolve, reject) => {
let args = [
'login',
'--username=_',
`--password=${ password }`,
registry
];
log(verbose, args);
child.spawn('docker', args, { stdio: 'inherit' })
]
log(verbose, args)
child.spawn('docker', args, {stdio: 'inherit'})
.on('exit', (code, signal) => {
if (signal || code) reject(signal || code);
else resolve();
});
});
if (signal || code) reject(signal || code)
else resolve()
})
})
}
9 changes: 3 additions & 6 deletions commands/logout.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
'use strict';

const cli = require('heroku-cli-util');
const co = require('co');
const child = require('child_process');
const log = require('../lib/log');

Expand All @@ -13,16 +10,16 @@ module.exports = function(topic) {
description: 'Logs out from the Heroku Docker registry',
needsApp: false,
needsAuth: false,
run: cli.command(co.wrap(logout))
run: cli.command(logout)
};
};

function* logout(context, heroku) {
async function logout(context, heroku) {
let herokuHost = process.env.HEROKU_HOST || 'heroku.com';
let registry = `registry.${ herokuHost }`;

try {
let user = yield dockerLogout(registry, context.flags.verbose);
let user = await dockerLogout(registry, context.flags.verbose);
}
catch (err) {
cli.error(`Error: docker logout exited with ${ err }`);
Expand Down
32 changes: 19 additions & 13 deletions commands/push.js
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
const cli = require('heroku-cli-util')
const co = require('co')
let Sanbashi = require('../lib/sanbashi')
const Sanbashi = require('../lib/sanbashi')
const Chalk = require('chalk')

module.exports = function (topic) {
return {
topic: topic,
command: 'push',
description: 'Builds, then pushes a Docker image to deploy your Heroku app',
description: 'builds, then pushes Docker images to deploy your Heroku app',
needsApp: true,
needsAuth: true,
variableArgs: true,
Expand All @@ -22,30 +22,36 @@ module.exports = function (topic) {
hasValue: false
}
],
run: cli.command(co.wrap(push))
run: cli.command(push)
}
}

function* push (context, heroku) {
let push = async function (context, heroku) {
console.log('in push')
const recurse = !!context.flags.recursive
if(context.args.length === 0 && !recurse){
cli.error('Error: Requires either --recursive or one or more process types')
if (context.args.length === 0 && !recurse) {
cli.error(`Error: Requires either --recursive or one or more process types
${Chalk.bold.underline.magenta('Usage:')}
${ Chalk.white(' heroku container:push web')} # Pushes Dockerfile in current directory
${ Chalk.white(' heroku container:push web worker')} # Pushes Dockerfile.web and Dockerfile.worker found in the current directory
${ Chalk.white(' heroku container:push web worker --recursive')} # Pushes Dockerfile.web and Dockerfile.worker found in the current directory or subdirectories
${ Chalk.white(' heroku container:push --recursive')} # Pushes Dockerfile.* found in current directory or subdirectories`)
process.exit(1)
}
let herokuHost = process.env.HEROKU_HOST || 'heroku.com'
let registry = `registry.${ herokuHost }`
let dockerfiles = Sanbashi.getDockerfiles(process.cwd(), recurse)
let possibleJobs = Sanbashi.getJobs(`${ registry }/${ context.app }`, context.args, dockerfiles)
let jobs = yield Sanbashi.chooseJobs(possibleJobs)
let jobs = await Sanbashi.chooseJobs(possibleJobs)
if (!jobs.length) {
cli.warn('No images to push')
process.exit()
process.exit(1)
}

try {
for (let job of jobs) {
cli.log(`\n=== Building ${ job.name } (${ job.dockerfile })`)
yield Sanbashi.buildImage(job.dockerfile, job.resource, context.flags.verbose)
cli.log(Chalk.bold.white.bgMagenta(`\n=== Building ${job.name} (${job.dockerfile})`))
await Sanbashi.buildImage(job.dockerfile, job.resource, context.flags.verbose)
}
}
catch (err) {
Expand All @@ -56,8 +62,8 @@ function* push (context, heroku) {

try {
for (let job of jobs) {
cli.log(`\n=== Pushing ${ job.name } (${ job.dockerfile })`)
yield Sanbashi.pushImage(job.resource, context.flags.verbose)
cli.log(Chalk.bold.white.bgMagenta(`\n=== Pushing ${job.name} (${job.dockerfile })`))
await Sanbashi.pushImage(job.resource, context.flags.verbose)
}
}
catch (err) {
Expand Down
2 changes: 0 additions & 2 deletions lib/log.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
'use strict';

module.exports = (visible, args) => {
if (!visible) return;
console.log(`> docker ${ args.join(' ') }`);
Expand Down
33 changes: 15 additions & 18 deletions lib/sanbashi.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,23 @@ let Inquirer = require('inquirer')
const Child = require('child_process')
const log = require('./log')

const DOCKERFILE_REGEX = /\/Dockerfile(.\w*)?$/
class Sanbashi {
constructor () {}
const DOCKERFILE_REGEX = /\bDockerfile(.\w*)?$/
let Sanbashi = function(){}

static getDockerfiles (rootdir, recursive) {
Sanbashi.getDockerfiles = function(rootdir, recursive) {
let match = recursive ? './**/Dockerfile?(.)*' : 'Dockerfile*'
let dockerfiles = Glob.sync(match, {
cwd: rootdir,
nonull: false,
nodir: true
})
if(recursive){
if (recursive) {
dockerfiles = dockerfiles.filter(df => df.match(/Dockerfile\.[\w]+/))
}
return dockerfiles.map(file => Path.join(rootdir, file))
}

static getJobs (resourceRoot, procs, dockerfiles) {
Sanbashi.getJobs = function(resourceRoot, procs, dockerfiles) {
return dockerfiles
// convert all Dockerfiles into job Objects
.map((dockerfile) => {
Expand Down Expand Up @@ -54,8 +53,9 @@ class Sanbashi {
}, {})
}

static chooseJobs (jobs) {
return Object.keys(jobs).map(processType => {
Sanbashi.chooseJobs = async function(jobs) {
let chosenJobs = []
for(let processType in jobs){
let group = jobs[processType]
if (group.length > 1) {
let prompt = {
Expand All @@ -64,18 +64,16 @@ class Sanbashi {
choices: group.map(j => j.dockerfile),
message: `Found multiple Dockerfiles with process type ${processType}. Please choose one to build and push `
}
return Inquirer.prompt(prompt)
.then((answer) => {
return group.find(o => o.dockerfile === answer.web)
}
)
let answer = await Inquirer.prompt(prompt)
chosenJobs.push(group.find(o => o.dockerfile === answer[processType]))
} else {
return group[0]
chosenJobs.push(group[0])
}
})
}
return chosenJobs
}

static buildImage (dockerfile, resource, verbose) {
Sanbashi.buildImage = function(dockerfile, resource, verbose) {
return new Promise((resolve, reject) => {
let cwd = Path.dirname(dockerfile)
let args = ['build', '-f', dockerfile, '-t', resource, cwd]
Expand All @@ -90,7 +88,7 @@ class Sanbashi {
})
}

static pushImage (resource, verbose) {
Sanbashi.pushImage = function(resource, verbose) {
return new Promise((resolve, reject) => {
let args = ['push', resource]
log(verbose, args)
Expand All @@ -103,6 +101,5 @@ class Sanbashi {
})
})
}
}

module.exports = Sanbashi
14 changes: 7 additions & 7 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,28 +19,28 @@
"license": "ISC",
"dependencies": {
"camelcase": "1.0.2",
"co": "4.6.0",
"chalk": "1.1.3",
"dotenv": "^1.1.0",
"glob": "7.1.0",
"heroku-cli-util": "^6.1.17",
"heroku-client": "^1.9.1",
"inquirer": "^3.0.6",
"heroku-cli-util": "6.1.17",
"heroku-client": "1.9.1",
"inquirer": "3.0.6",
"is-there": "4.0.0",
"lodash": "3.3.1",
"mkdirp": "^0.5.0",
"node-uuid": "^1.4.3",
"node-uuid": "1.4.3",
"progress": "1.1.8",
"request": "2.53.0",
"superagent": "^0.21.0",
"superagent": "0.21.0",
"yamljs": "0.2.4"
},
"devDependencies": {
"chai": "^3.2.0",
"sinon": "^2.1.0",
"depcheck": "^0.4.7",
"dir-compare": "^0.0.2",
"fs-extra": "^0.23.1",
"mocha": "^2.2.4",
"sinon": "^2.1.0",
"wrench": "^1.5.8"
}
}
48 changes: 33 additions & 15 deletions test/sanbashi.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,51 +24,69 @@ describe('Sanbashi', () => {
})
describe('.getJobs', () => {
it('returns objects representing jobs per Dockerfile', () => {
const dockerfiles = ['./Dockerfile.web', './Nested/Dockerfile.web']
const dockerfiles = [
Path.join('.', 'Dockerfile.web'),
Path.join('.', 'Nested', 'Dockerfile.web')
]
const resourceRoot = 'rootfulroot'
const results = Sanbashi.getJobs(resourceRoot, ['web'], dockerfiles)
expect(results.web).to.have.property('length', 2)
expect(results.web[0]).to.have.property('depth', 1, 'dockerfile', './Dockerfile.web', 'postfix', 1)
expect(results.web[1]).to.have.property('depth', 2, 'dockerfile', './Nested/Dockerfile.web', 'postfix', 1)
})
it('filters out by process type', () => {
const dockerfiles = ['./Dockerfile.web', './Nested/Dockerfile.worker']
const dockerfiles = [
Path.join('.', 'Dockerfile.web'),
Path.join('.', 'Nested', 'Dockerfile.worker')
]
const resourceRoot = 'rootfulroot'
const results = Sanbashi.getJobs(resourceRoot, ['web'], dockerfiles)
expect(results.web).to.have.property('length', 1)
expect(results.web[0]).to.have.property('name', 'web')
expect(results).to.not.have.property('worker')
})
it('sorts dockerfiles by directory depth, then proc type', () => {
const dockerfiles = ['./Nested/Dockerfile.worker', './Dockerfile.web', './Nested/Dockerfile']
const dockerfiles = [
Path.join('.', 'Nested', 'Dockerfile.worker'),
Path.join('.', 'Dockerfile.web'),
Path.join('.', 'Nested', 'Dockerfile')
]
const resourceRoot = 'rootfulroot'
const results = Sanbashi.getJobs(resourceRoot, [], dockerfiles)
expect(results.web).to.have.property('length', 2)
expect(results.web[0]).to.have.property('dockerfile', './Dockerfile.web')
expect(results.web[1]).to.have.property('dockerfile', './Nested/Dockerfile')
expect(results.worker[0]).to.have.property('dockerfile', './Nested/Dockerfile.worker')
expect(results.web[0]).to.have.property('dockerfile', 'Dockerfile.web')
expect(results.web[1]).to.have.property('dockerfile', 'Nested/Dockerfile')
expect(results.worker[0]).to.have.property('dockerfile', 'Nested/Dockerfile.worker')
})
it('groups the jobs by process type', () => {
const dockerfiles = ['./Nested/Dockerfile.worker', './Dockerfile.web', './Nested/Dockerfile']
const dockerfiles = [
Path.join('.', 'Nested', 'Dockerfile.worker'),
Path.join('.', 'Dockerfile.web'),
Path.join('.', 'Nested', 'Dockerfile')
]
const resourceRoot = 'rootfulroot'
const results = Sanbashi.getJobs(resourceRoot, [], dockerfiles)
expect(results).to.have.keys('worker', 'web')
expect(results['worker'].map(j => j.dockerfile)).to.have.members(['./Nested/Dockerfile.worker'])
expect(results['web'].map(j => j.dockerfile)).to.have.members(['./Dockerfile.web', './Nested/Dockerfile'])
expect(results['worker'].map(j => j.dockerfile)).to.have.members([Path.join('.', 'Nested', 'Dockerfile.worker')])
expect(results['web'].map(j => j.dockerfile)).to.have.members([Path.join('.', 'Dockerfile.web'), Path.join('.', 'Nested', 'Dockerfile')])
})
})
describe('.chooseJobs', () => {
it('returns the entry when ony one exist', () => {
const dockerfiles = ['./Nested/Dockerfile.web']
it('returns the entry when only one exists', async () => {
const dockerfiles = [Path.join('.', 'Nested', 'Dockerfile.web')]
const jobs = Sanbashi.getJobs('rootfulroot', [], dockerfiles)
let chosenJob = Sanbashi.chooseJobs(jobs)
let chosenJob = await Sanbashi.chooseJobs(jobs)
expect(chosenJob[0]).to.have.property('dockerfile', dockerfiles[0])
expect(chosenJob).to.have.property('length',1)
expect(chosenJob).to.have.property('length', 1)
})
it.skip('queries the user when more than one job of a type exists', () => {
//really no way to simulate user input for Inquirer
Sinon.stub(Inquirer, 'prompt').resolves('something')
const dockerfiles = ['./Nested/Dockerfile.web', './OtherNested/Dockerfile.web','./AnotherNested/Dockerfile.web', './Dockerfile.web']
const dockerfiles = [
Path.join('.', 'Nested', 'Dockerfile.web'),
Path.join('.', 'OtherNested', 'Dockerfile.web'),
Path.join('.', 'AnotherNested', 'Dockerfile.web', './Dockerfile.web')
]
let jobs = Sanbashi.getJobs('rootfulroot', ['web'], dockerfiles)
let res = Sanbashi.chooseJobs(jobs)
// .then((chosen) => {
Expand All @@ -80,7 +98,7 @@ describe('Sanbashi', () => {
console.dir(res)
})
afterEach(() => {
if(Inquirer.prompt.restore)
if (Inquirer.prompt.restore)
Inquirer.prompt.restore()
})
})
Expand Down
Loading

0 comments on commit 434c6c4

Please sign in to comment.