Skip to content

Conversation

joyent-automation
Copy link

DATASET-1318 cleanup zoneinit and make reboot optional

This PR was migrated-from-gerrit, https://cr.joyent.us/#/c/2970/.
The raw archive of this CR is here.
See MANTA-4594 for info on Joyent Eng's migration from Gerrit.

CR discussion

@bahamas10 commented at 2017-11-20T20:24:14

Patch Set 1:

New commits:
commit c957f870bd73803cf3709bb5f15f5fe10f19bbe7
initial commit

@joshwilsdon commented at 2018-02-05T17:26:33

Patch Set 1:

(6 comments)

What testing has been done on this?

Is it worth adding something to the README.md indicating what should be done in the image before reboot can be set to false? We need at least to have removed the utmp file and logs, right?

When the system comes up, some logs will have a different hostname too initially, right since things log before we have set the hostname?

Maybe it'd be worth booting up a zone using the same image with this zoneinit and the existing zoneinit, and then do a diff between all the files after it completes provisioning?

Patch Set 1 code comments
includes/00-mdata.sh#5 @joshwilsdon

What is the min-platform that has filewait? Might be worth adding a comment here that all platforms after XXX have this?

includes/11-files.sh#15 @joshwilsdon

Again curious why we changed which egrep we might be using? Do we know that any egrep will work? Or have we set the PATH more restrictively here? If not: maybe we should?

zoneinit#1 @joshwilsdon

Why are we changing the shebang here? This seems like it might cause problems since we might end up using the zone's /usr/local/bin/bash for which we don't control the version (e.g. in the case of a custom image). Wouldn't we be better always using the version from the platform?

zoneinit#15 @joshwilsdon

How does this end up getting set?

zoneinit#43 @joshwilsdon

where does ${INC-zoneinit} get set?

zoneinit#44 @joshwilsdon

If you're changing the style from ${VAR} to $VAR, you should do this one too to be consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants