Remove install command, replace with a script #2101
Conversation
@@ -28,6 +29,7 @@ | |||
|
|||
- The `--stage1-image` flag was removed. Scripts using it should be updated to use one of `--stage1-url`, `--stage1-path`, `--stage1-name`, `--stage1-hash` or `--stage1-from-dir` | |||
- All uses of the deprecated `--insecure-skip-verify` flag should be replaced with the `--insecure-options` flag which allows user to selectively disable security features. | |||
- The `rkt install` command was removed in favor of `scripts/setup-data-dir.sh` script. |
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.
of the
else | ||
mkdir --mode="${mode}" "${dir}" | ||
fi | ||
chgrp rkt "${dir}" |
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.
Do we need to test that the rkt group exists? I guess it is not necessary since we have set -e
above. Does set -e
persist across function calls?
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 don't think so, the whole point is to prepare the directory structure for rkt group, otherwise rkt will happily create required directories on first run.
set -e
is global here.
9af22a0
to
caad3e2
Compare
Updated. |
#!/usr/bin/env bash | ||
|
||
# The script can take one parameter being a path to the rkt data | ||
# directory. If none is passed, /var/lib/rkt is assumed. |
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.
keep in sync with dist/init/systemd/tmpfiles.d/rkt.conf
?
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.
Added.
The group package looks up given group name. It is moved to a separate package, so we don't have to include the github.com/coreos/rkt/common package it in github.com/coreos/rkt/tests/testutils. Importing it there ended up in errors when building the inspect binary, which is importing testutils. common package was importing indirectly some other package, which requires cgo, but cgo was disabled when building inspect.
caad3e2
to
0d30539
Compare
Updated, moving the group handling to separate package, so we won't error out because of trying to build a package that uses cgo with CGO_ENABLED=0. |
LGTM |
Remove install command, replace with a script
@@ -0,0 +1,75 @@ | |||
#!/usr/bin/env bash |
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 this be in dist/ ?
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.
Right.
Move setup-data-dir.sh in dist/scripts/ so it will be in the release tarball. It was introduced by rkt#2101
No description provided.