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

Multiple dockerfiles #6

Merged
merged 27 commits into from
Jul 5, 2017
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
4d70559
4.1.1
hunterloftis Sep 29, 2016
d7cf202
push multiple, recursive, choose nearest, warn multiple, fail ambiguo…
hunterloftis Sep 30, 2016
e66a79b
Update push.js
jbyrum Oct 5, 2016
a66925e
more readable formatting
hunterloftis Oct 5, 2016
502e3f1
multiple dockerfiles
KarateCowboy May 3, 2017
b4c8b85
some changes for excluding certain files
KarateCowboy May 3, 2017
9522087
review feedback
KarateCowboy May 4, 2017
c00e780
minor fixes and updates
KarateCowboy May 11, 2017
99f90f8
remove heroku-client dependency
KarateCowboy May 12, 2017
c940247
do not treat 'Dockerfile' as 'Dockerfile.web'
KarateCowboy May 19, 2017
be01673
use regular dockerfile when recurse arg omitted
KarateCowboy May 22, 2017
be7651f
Remove docker text
jbyrum May 22, 2017
5b148f9
Remove Docker text
jbyrum May 22, 2017
fc6fc7b
Update to usage text
jbyrum May 22, 2017
a3b4a22
Merge pull request #17 from heroku/jbyrum-text-updates
KarateCowboy May 22, 2017
632a3c8
change headers to use cli.styledHeader
KarateCowboy May 22, 2017
832be4e
review feedback
KarateCowboy Jun 5, 2017
db8f6db
fixed merge conflicts
KarateCowboy Jun 5, 2017
0773479
fix pushing standard dockerfiles
KarateCowboy Jun 9, 2017
4f8e25e
more regression fixing
KarateCowboy Jun 9, 2017
7377b6c
fix recursive regression
KarateCowboy Jun 12, 2017
cbcf60e
add handling for directories w/o dockerfiles
KarateCowboy Jun 15, 2017
f18dae3
Made text updates to push
jbyrum Jun 19, 2017
df8ce5f
logout/in command fix
jbyrum Jun 19, 2017
d744280
login text change
jbyrum Jun 19, 2017
fb84194
Merge pull request #18 from heroku/new-push-text
KarateCowboy Jun 20, 2017
ec54401
consolidate spawing methods into helper
KarateCowboy Jun 30, 2017
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion commands/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ module.exports = function index(pkg) {
return {
topic: pkg.topic,
description: pkg.description,
help: pkg.description,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you put the help back in with an output example?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To which command are you referring? the basic heroku containers just prints the version. What would you like added -- the sum of all the help for the other commands?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an example of it printing the version then, we're supposed to have output examples for every command. I realize that isn't the most useful thing in the world, but it clearly demonstrates what a command is for.

this is the (soon-to-be enforced) rule for core plugins anyways, but we should do it on any plugin we can

run: showVersion
};

Expand Down
53 changes: 26 additions & 27 deletions commands/login.js
Original file line number Diff line number Diff line change
@@ -1,49 +1,48 @@
'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');

module.exports = function(topic) {
module.exports = function (topic) {
return {
topic: topic,
command: 'login',
flags: [{ name: 'verbose', char: 'v', hasValue: false }],
description: 'Logs in to the Heroku Docker registry',
flags: [{name: 'verbose', char: 'v', hasValue: false}],
description: 'logs in to the Heroku Container Registry',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"logs into"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"logs in to" is actually correct. "in" is part of the verb phase

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so I recently ran through our entire help and audited everything: heroku/cli-engine#196

What I found was there is a mix of using present tense and another type that I think might be "imperative mood" but I'm not sure. Anyways, this "imperative mood" (if that is the correct term) is used much more commonly and I think makes more sense.

So in this case it would be "log in to the Heroku Container Registry". Not "logs in to the Heroku Container Registry". I plan to add this to the style guide.

Also, I'm not sure what language we use elsewhere in documentation, but I'm pretty sure in the CLI we never use "log in" as 2 words, it's always "login" and "logout". So I think my first comment here was wrong for a few reasons.

help: `Usage:
heroku container:login`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing the $ sign and this example shold show some output

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.error(`Error: docker login exited with ${ err }`)
cli.hush(err.stack || err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should exit with a nonzero code, it looks like it's exiting with 0

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

}
}

function dockerLogin(registry, password, verbose) {
function dockerLogin (registry, password, verbose) {
return new Promise((resolve, reject) => {
let args = [
'login',
'--username=_',
`--password=${ password }`,
registry
];
if (verbose) {
console.log(['> docker'].concat(args).join(' '));
}
child.spawn('docker', args, { stdio: 'inherit' })
]
log(verbose, args)
child.spawn('docker', args, {stdio: 'inherit'})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should abstract this into 1 spawn function that returns a promise

.on('exit', (code, signal) => {
if (signal || code) reject(signal || code);
else resolve();
});
});
if (signal || code) reject(signal || code)
else resolve()
})
})
}
18 changes: 8 additions & 10 deletions commands/logout.js
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
'use strict';

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

module.exports = function(topic) {
return {
topic: topic,
command: 'logout',
flags: [{ name: 'verbose', char: 'v', hasValue: false }],
description: 'Logs out from the Heroku Docker registry',
description: 'logs out from the Heroku Container Registry',
help: `Usage:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the CLI already automatically adds a "usage" help bit. Just take the help attribute out here since it's not adding anything useful

heroku container:logout`,
needsApp: false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add help with an output example?

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 All @@ -34,9 +34,7 @@ function dockerLogout(registry, verbose) {
'logout',
registry
];
if (verbose) {
console.log(['> docker'].concat(args).join(' '));
}
log(verbose, args);
child.spawn('docker', args, { stdio: 'inherit' })
.on('exit', (code, signal) => {
if (signal || code) reject(signal || code);
Expand Down
121 changes: 63 additions & 58 deletions commands/push.js
Original file line number Diff line number Diff line change
@@ -1,77 +1,82 @@
'use strict';
const cli = require('heroku-cli-util')
const Sanbashi = require('../lib/sanbashi')

const cli = require('heroku-cli-util');
const co = require('co');
const child = require('child_process');
let usage = `
${cli.color.bold.underline.magenta('Usage:')}
${ cli.color.white('heroku container:push web')} # Pushes Dockerfile in current directory to web process type
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use cli.color.cmd

${ cli.color.white('heroku container:push web worker --recursive')} # Pushes Dockerfile.web and Dockerfile.worker found in current & subdirectories
${ cli.color.white('heroku container:push --recursive')} # Pushes Dockerfile.* found in current & subdirectories`

module.exports = function(topic) {
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,
args: [{ name: 'process', optional: true }],
flags: [{ name: 'verbose', char: 'v', hasValue: false }],
run: cli.command(co.wrap(push))
};
};
variableArgs: true,
help: usage,
flags: [
{
name: 'verbose',
char: 'v',
hasValue: false
},
{
name: 'recursive',
char: 'R',
hasValue: false
}
],
run: cli.command(push)
}
}

function* push(context, heroku) {
let herokuHost = process.env.HEROKU_HOST || 'heroku.com';
let registry = `registry.${ herokuHost }`;
let proc = context.args.process || 'web';
let resource = `${ registry }/${ context.app }/${ proc }`;
let push = async function (context, heroku) {
const recurse = !!context.flags.recursive
if (context.args.length === 0 && !recurse) {
cli.error(`Error: Requires either --recursive or one or more process types\n ${usage} `)
process.exit(1)
}
if(context.args.length > 1 && !recurse){
cli.error(`Error: Please specify one target process type\n ${usage} `)
process.exit(1)
}
let herokuHost = process.env.HEROKU_HOST || 'heroku.com'
let registry = `registry.${ herokuHost }`
let dockerfiles = Sanbashi.getDockerfiles(process.cwd(), recurse)
let processTypes = ['standard']
if(recurse){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you run standard on this file at least to fix the formatting?

processTypes = context.args
}
let possibleJobs = Sanbashi.getJobs(`${ registry }/${ context.app }`, processTypes, dockerfiles)
let jobs = await Sanbashi.chooseJobs(possibleJobs)
if (!jobs.length) {
cli.warn('No images to push')
process.exit(1)
}

try {
let build = yield buildImage(resource, context.cwd, context.flags.verbose);
for (let job of jobs) {
cli.styledHeader(`Building ${job.name} (${job.dockerfile})`)
await Sanbashi.buildImage(job.dockerfile, job.resource, context.flags.verbose)
}
}
catch (err) {
cli.error(`Error: docker build exited with ${ err }`);
process.exit(1);
cli.error(`Error: docker build exited with ${ err }`)
cli.hush(err.stack || err)
process.exit(1)
}

try {
let push = yield pushImage(resource, context.flags.verbose);
for (let job of jobs) {
cli.styledHeader(`Pushing ${job.name} (${job.dockerfile })`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are a lot of little style issues like the indentation on this line. It's probably too much work to put it in for the whole project, but it would be good to run it at least on the new code

await Sanbashi.pushImage(job.resource, context.flags.verbose)
}
}
catch (err) {
cli.error(`Error: docker push exited with ${ err }`);
process.exit(1);
cli.error(`Error: docker push exited with ${ err }`)
cli.hush(err.stack || err)
process.exit(1)
}
}

function buildImage(resource, cwd, verbose) {
return new Promise((resolve, reject) => {
let args = [
'build',
'-t',
resource,
cwd
];
if (verbose) {
console.log(['> docker'].concat(args).join(' '));
}
child.spawn('docker', args, { stdio: 'inherit' })
.on('exit', (code, signal) => {
if (signal || code) reject(signal || code);
else resolve();
});
});
}

function pushImage(resource, verbose) {
return new Promise((resolve, reject) => {
let args = [
'push',
resource
];
if (verbose) {
console.log(['> docker'].concat(args).join(' '));
}
child.spawn('docker', args, { stdio: 'inherit' })
.on('exit', (code, signal) => {
if (signal || code) reject(signal || code);
else resolve();
});
});
}
4 changes: 4 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
var pkg = require('./package.json');

module.exports = {
topic: {
description: 'Use containers to build and deploy Heroku apps',
name: 'container'
},
commands: [
require('./commands/index')(pkg),
require('./commands/login')(pkg.topic),
Expand Down
4 changes: 4 additions & 0 deletions lib/log.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
module.exports = (visible, args) => {
if (!visible) return;
console.log(`> docker ${ args.join(' ') }`);
};
105 changes: 105 additions & 0 deletions lib/sanbashi.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
// 'sanbashi' is a word in Japanese that refers to a pier or dock, usually very large in size, such as found in Tokyo's O-daiba region

let Glob = require('glob')
let Path = require('path')
let Inquirer = require('inquirer')
const Child = require('child_process')
const log = require('./log')

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

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

Sanbashi.getJobs = function (resourceRoot, procs, dockerfiles) {
return dockerfiles
// convert all Dockerfiles into job Objects
.map((dockerfile) => {
let match = dockerfile.match(DOCKERFILE_REGEX)
if (!match) return
let proc = (match[1] || '.standard').slice(1)
return {
name: proc,
resource: `${ resourceRoot }/${ proc }`,
dockerfile: dockerfile,
postfix: Path.basename(dockerfile) === 'Dockerfile' ? 0 : 1,
depth: Path.normalize(dockerfile).split(Path.sep).length
}
})
// if process types have been specified, filter non matches out
.filter(job => {
return job && (!procs.length || procs.indexOf(job.name) !== -1)
})
// prefer closer Dockerfiles, then prefer Dockerfile over Dockerfile.web
.sort((a, b) => {
return a.depth - b.depth || a.postfix - b.postfix
})
// group all Dockerfiles for the same process type together
.reduce((jobs, job) => {
jobs[job.name] = jobs[job.name] || []
jobs[job.name].push(job)
return jobs
}, {})
}

Sanbashi.chooseJobs = async function (jobs) {
let chosenJobs = []
for (let processType in jobs) {
let group = jobs[processType]
if (group.length > 1) {
let prompt = {
type: 'list',
name: processType,
choices: group.map(j => j.dockerfile),
message: `Found multiple Dockerfiles with process type ${processType}. Please choose one to build and push `
}
let answer = await Inquirer.prompt(prompt)
chosenJobs.push(group.find(o => o.dockerfile === answer[processType]))
} else {
chosenJobs.push(group[0])
}
}
return chosenJobs
}

Sanbashi.buildImage = function (dockerfile, resource, verbose) {
return new Promise((resolve, reject) => {
let cwd = Path.dirname(dockerfile)
let args = ['build', '-f', dockerfile, '-t', resource, cwd]
log(verbose, args)
Child.spawn('docker', args, {
stdio: 'inherit'
})
.on('exit', (code, signal) => {
if (signal || code) reject(signal || code)
else resolve()
})
})
}

Sanbashi.pushImage = function (resource, verbose) {
return new Promise((resolve, reject) => {
let args = ['push', resource]
log(verbose, args)
Child.spawn('docker', args, {
stdio: 'inherit'
})
.on('exit', (code, signal) => {
if (signal || code) reject(signal || code)
else resolve()
})
})
}

module.exports = Sanbashi
Loading