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

Add configuration flag allowing the use of a custom b2d ISO source URL #267

Merged
merged 1 commit into from
Oct 9, 2014

Conversation

bjaglin
Copy link
Contributor

@bjaglin bjaglin commented Oct 6, 2014

Until now we have been using a heavily patched fork of the now-retired bash-based boot2docker script, but all we need on the script-side (apart from the ability to use a custom ISO) is now in the go port, so I'd like to move on and benefit from the all-in-one DMG installer on OS X.

I think there will always be a need for custom ISOs. We am using one currently in order to a company proxy for the docker engine and auto-create a larger swap partition at the first boot. These are of course things that could be made configurable, but is it really worth it? There will be always specific/exotic needs adding up over time, so this allows advanced users to do their own thing.

@bjaglin
Copy link
Contributor Author

bjaglin commented Oct 7, 2014

@tianon @SvenDowideit any chance to get that in before the 1.3.0 package is released?

@SchumacherFM
Copy link
Contributor

+1 as we also have a custom ISO

@SvenDowideit
Copy link
Contributor

if you type boot2docker --help you should see the --iso= flag. Is this what you're asking? (I'm not sure)

@zeeyang
Copy link
Contributor

zeeyang commented Oct 7, 2014

It would be helpful to have a default ISO binded to a particular driver. I'm working on a VMware Fusion driver that would require ISO with open-vm-tools.

@SvenDowideit
Copy link
Contributor

@zeeyang yay :) made an exploratory PR to allow the drivers to change the cfg defaults - iirc, that part worked ok, so 'should-be-doable' :)

basically, I added a driverConfigInit() which was called foreach driver during the flag setup code - just after the basic defaults get set.

@bjaglin
Copy link
Contributor Author

bjaglin commented Oct 8, 2014

@SvenDowideit I know about the custom iso path locally, but this is about automating the initial retrieval or upgrade of a remote custom image. Is the documentation I added unclear?

@SvenDowideit
Copy link
Contributor

ah, so add a --iso-url - thankyou, i've been on holidays, and that slipped my mind (but there was this niggling feeling :) )

sorry, i did completely missread this as an issue, not a PR - reviewing!

@SvenDowideit SvenDowideit changed the title Add configuration flag allowing the use of a custom b2d ISO Add configuration flag allowing the use of a custom b2d ISO source URL Oct 8, 2014
@bjaglin
Copy link
Contributor Author

bjaglin commented Oct 8, 2014

That's actually a PR, not an issue, so I did add a flag with similar semantics (just a different name). Welcome back ;)

fmt.Printf("Latest release is %s\n", tag)
url := ""
if B2D.URLCustomISO == "" {
url = "https://api.github.com/repos/boot2docker/boot2docker/releases"
Copy link
Contributor

Choose a reason for hiding this comment

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

this defaulting should be done in the config() code.

perhaps this 'get latest release name' could be auto-invoked when the url starts with github.com and ends with /releases ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The thing is that the official download is a 2-step process: retrieve the latest version, then the ISO based on a path containing the latest version. I wanted the new flag to be more of an 1-step override, bypassing the official release scheme, since it can take time to align a custom ISO to upstream or one might want to pin to a specific version anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

y, and that would work fine still - the idea is to allow other github distributed 2 step processes and when the url is not a github.com..../release url, to treat it as an iso file as in your added step.

That way, I can specify github.com/boot2docker/boot2docker/release/v0.5.4/boot2docker.iso and use your one-step code to get an old release, or github.com/unclejack/debian2docker/releases to do the 2 step to get the latest release of that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

copy that, I'll rework the PR

@SvenDowideit
Copy link
Contributor

yeah, i remember being against this a while ago, but things have changed a little :)

oh, and @tianon can you review please?

@bjaglin
Copy link
Contributor Author

bjaglin commented Oct 8, 2014

Second iteration is up.

The new flag, now called ISOURL/--iso-url, does not have override semantics anymore. It has the default value of the official boot2docker org/repo /releases API endpoint, as the syntax allows both for latest-github-release provisioning (when the URL matches a certain pattern) or direct download.

@bjaglin
Copy link
Contributor Author

bjaglin commented Oct 8, 2014

I have tested successfully the retrieval of the latest release of a boot2docker fork (@YungSang's)

mini:boot2docker-cli brice$ ./boot2docker-cli --iso-url=https://api.github.com/repos/YungSang/boot2docker/releases download
Latest release for YungSang/boot2docker is yungsang/v1.2.1
Downloading boot2docker ISO image...
Success: downloaded https://github.com/YungSang/boot2docker/releases/download/yungsang/v1.2.1/boot2docker.iso
    to /Users/brice/.boot2docker/boot2docker.iso

@SvenDowideit
Copy link
Contributor

many 👍 LGTM from me :) @tianon @bfirsh please review?

@zeeyang it won't bee to painful to add the driver over-ride code when we work further on that :)

return fmt.Errorf("Failed to get latest release: %s", err)
url := B2D.ISOURL

re := regexp.MustCompile("https://api.github.com/repos/([^/]+)/([^/]+)/releases")
Copy link
Contributor

Choose a reason for hiding this comment

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

For regexps that MustCompile, it is typical to make a var outside the function so that the failure will happen immediately at startup... but probably not a big deal in this case.
Otherwise, LGTM.

@tianon
Copy link
Contributor

tianon commented Oct 9, 2014

Nice, seems reasonable.

LGTM

@tianon
Copy link
Contributor

tianon commented Oct 9, 2014

Merging even with @gmlewis's nit because I totally agree, but I think there are several more in the codebase that would be worth factoring out too, so I think those can wait for another PR.

tianon added a commit that referenced this pull request Oct 9, 2014
Add configuration flag allowing the use of a custom b2d ISO source URL
@tianon tianon merged commit fbdc38f into boot2docker:master Oct 9, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants