-
Notifications
You must be signed in to change notification settings - Fork 650
Convert installer to go #1456
Convert installer to go #1456
Conversation
os-config.tpl.yml
Outdated
| - /usr/bin/iptables:/sbin/iptables:ro | ||
| - /media:/media:shared | ||
| - /mnt:/mnt:shared | ||
| - /:/host |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this for debugging? Do we need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its used to get access to the /dev/vda1 partition that we create - udev works outside the container, but not inside
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But isn't being run in the installer container and not the console? /dev in the console container should be the same as /dev on the host anyways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sadly, its not the same - if there are changes to /dev in the host after the container is started, then those changes are not mirrored inside the container. and in this case, its udev making /dev/vda1 which we use inside.
And then when we look at RAID and custom partitions in 0.9, there'll be even more dynamics (same thing if we start to have other devices with loaded modules)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But don't we already bind mount /dev:/host/dev in the console container? Why do we need to bind mount in the entire root filesystem too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh crud. this isn't the host volume I thought it was - removing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that it matters much for just the installer container, but you could probably bind mount /dev:/host/dev and run ros entrypoint as the entrypoint instead of /:/host.
6bfef01 to
e7b0558
Compare
|
e7b0558 to
cd35087
Compare
| } | ||
| } | ||
|
|
||
| useIso := false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Silly question... If the mount and "system-docker load" lines below come after the "system-docker run" lines above, won't that prevent the os/installer image from being made available at the start of the "ros install" command?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the system-docker run commands above are when the user is requesting to install a version that is different to the one on the iso - so it will pull the image from the hub
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, thanks. In my horrible hack, I was blindly mounting and loading the os image since I knew that whatever I packed on my ISO was what I needed to keep the Vagrant users happy and because I was hacking my junk on top of 0.7.1 without all your other nice changes (yet). LGTM.
43608c9 to
aa0c6ad
Compare
| if [ "$INSTALLER" != "0" ]; then | ||
| ./package-installer | ||
| fi | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to make the same change for my temporary workaround as well. Glad you did this too.
|
I didn't notice whether you were copying anything from the ISO into For my own hack, I just ended up adding a couple |
|
What do you think about moving |
cmd/control/install.go
Outdated
| } | ||
| log.Debugf("installRancher done") | ||
|
|
||
| //unused by us? :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is used for the --kexec flag for upgrading (https://github.com/rancher/os/blob/master/cmd/control/os.go#L54). Hopefully available for standard installs at some point too #1292 .
|
Is the image embedded in the ISO going to be the same image used for upgrades? |
|
Oops, wrong button 😄 |
| return nil | ||
| } | ||
|
|
||
| // files is an array of 'sourcefile:destination' - but i've not seen any examples of it being used. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably just historical. I vote to remove it 😄
| gccgo \ | ||
| genisoimage \ | ||
| gettext \ | ||
| git \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👋 may want to check your editor settings; looks like all new lines use tabs, but the existing file uses spaces (here and other files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hate spaces 🗡
tabs forever !!!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you been talking with tianon? ![]()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought tianon preferred spaces 📦
| "errors" | ||
| ) | ||
|
|
||
| // ResolveDevice this isn't reliable - blkid -L LABEL works more often :( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really important if it's true in general and we should investigate it. We use this function to resolve LABEL=RANCHER_STATE during boot. Where did these problems occur? Are you sure you weren't using a /dev that didn't match the host /dev?
I'm assuming that blkid is just a thin CLI around libblkid. Maybe this has to do with the versions of libblkid then. util.ResolveDevice uses libblkid from https://github.com/rancher/os/blob/master/Dockerfile.dapper#L16, whereas the blkid binary will be using libblkid from buildroot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| INCLUDE ../global.cfg | ||
|
|
||
| # each INCLUDEd file has a `DEFAULT mylabel` in it, and the last one wins | ||
| INCLUDE ../linux-previous.cfg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we leave isolinux.cfg alone? rancher.autologin=tty1 rancher.autologin=ttyS0 is getting removed here and we don't really need current/previous when booting from ISO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what I want to do is use the same kernel&initrd cfg file for both iso and installed boot - and I am thinking of using previous on the iso as the recovery option
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We set the autologin parameter on the ISO though, so I'm not sure how we'd share the config. I don't think I understand the recovery part either - why would an ISO have a recovery option?
|
@tylert dare you to make a PR with your preload changes - yes, it'll get re-written in this PR, but it'll save me some time :) |
|
No prob. I will try to get it done this weekend.
|
|
@joshwget yeah, I think I'll defer refactoring until 0.9 though, I'm still not finished, and wanted to have an rc with it in already :/ |
cmd/control/os.go
Outdated
| cli.BoolFlag{ | ||
| Name: "stage, s", | ||
| Usage: "Only stage the new upgrade, don't apply it", | ||
| Usage: "Only stage the new upgrade, don't apply it (ie, create the service, pull the image, then exit - upgrade will happen on next reboot?)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't upgrade on next boot. Can you take out the second half of this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so when does it upgrade?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you run ros os upgrade next. You'd just avoid the image pull by staging.
scripts/run-common
Outdated
| TTYCONS=${ttycons["${ARCH}"]} | ||
|
|
||
| DEFAULT_KERNEL_ARGS="quiet rancher.password=rancher console=${TTYCONS} rancher.autologin=${TTYCONS}" | ||
| DEFAULT_KERNEL_ARGS="rancher.debug=true rancher.password=rancher console=${TTYCONS} rancher.autologin=${TTYCONS}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😞
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah - it helps debug the weirdness - without bothering people at release time
5ead9cb to
b2bc5db
Compare
Signed-off-by: Sven Dowideit <[email protected]>
…ller is easier Signed-off-by: Sven Dowideit <[email protected]>
…some details that will delay it Signed-off-by: Sven Dowideit <[email protected]>
Signed-off-by: Sven Dowideit <[email protected]>
….. the iso only has the smaller os-installer image in it \o/ Signed-off-by: Sven Dowideit <[email protected]>
Signed-off-by: Sven Dowideit <[email protected]>
…alling Signed-off-by: Sven Dowideit <[email protected]>
Signed-off-by: Sven Dowideit <[email protected]>
Signed-off-by: Sven Dowideit <[email protected]>
Signed-off-by: Sven Dowideit <[email protected]>
Signed-off-by: Sven Dowideit <[email protected]>
…r now Signed-off-by: Sven Dowideit <[email protected]>
…ked up latest works \o/ Signed-off-by: Sven Dowideit <[email protected]>
Signed-off-by: Sven Dowideit <[email protected]>
Signed-off-by: Sven Dowideit <[email protected]>
b2bc5db to
e5a7889
Compare
|
the dhcp test failure was due to buildroot 2016.11 - which I've reverted. now working to figure out why the TestUpgrade goes boom |
Signed-off-by: Sven Dowideit <[email protected]>
Signed-off-by: Sven Dowideit <[email protected]>
|
OK, tests pass, I'm going to merge, and then continue work in new PR's, and hopefully mostly after 0.8.0rc3 |
its alive. I'm hoping we can merge a version of this for rc2, and then refine afterwards, but idk - i've come across so many little corner cases already.
linux-current.cfgboot config, andlinux-previous.cfgand a new current cfg addedfor a later PR:
Note, there are commented out bits that will need to be deferred til 0.9 - changes are needed to how we build and package kernels