Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

-b option for telling salt-bootstrap to skip installing dependencies causes it to error out instead. #1168

Closed
arizvisa opened this issue Nov 16, 2017 · 5 comments
Labels
Milestone

Comments

@arizvisa
Copy link
Contributor

Documentation for the -b option specifies the following:

    -b  Assume that dependencies are already installed and software sources are
        set up. If git is selected, git tree is still checked out as dependency
        step.

This implies that the dependency installation step is skipped. However..the following code skips adding any install_*_deps functions to the DEP_FUNC_NAMES var.

bootstrap-salt.sh:6399

# Let's get the dependencies install function
if [ ${_NO_DEPS} -eq $BS_FALSE ]; then
    DEP_FUNC_NAMES="install_${DISTRO_NAME_L}${PREFIXED_DISTRO_MAJOR_VERSION}_${ITYPE}_deps"
    DEP_FUNC_NAMES="$DEP_FUNC_NAMES install_${DISTRO_NAME_L}${PREFIXED_DISTRO_MAJOR_VERSION}${PREFIXED_DISTRO_MINOR_VERSION}_${ITYPE}_deps"
    DEP_FUNC_NAMES="$DEP_FUNC_NAMES install_${DISTRO_NAME_L}${PREFIXED_DISTRO_MAJOR_VERSION}_deps"
    DEP_FUNC_NAMES="$DEP_FUNC_NAMES install_${DISTRO_NAME_L}${PREFIXED_DISTRO_MAJOR_VERSION}${PREFIXED_DISTRO_MINOR_VERSION}_deps"
    DEP_FUNC_NAMES="$DEP_FUNC_NAMES install_${DISTRO_NAME_L}_${ITYPE}_deps"
    DEP_FUNC_NAMES="$DEP_FUNC_NAMES install_${DISTRO_NAME_L}_deps"
elif [ "${ITYPE}" = "git" ]; then
    DEP_FUNC_NAMES="__git_clone_and_checkout"
else
    DEP_FUNC_NAMES=""
fi

At the end of the following code, DEPS_INSTALL_FUNC results in "null" due to NO_DEPS causing DEP_FUNC_NAMES to not include any of the install_*_deps functions.

bootstrap-salt.sh:6413

DEPS_INSTALL_FUNC="null"
for FUNC_NAME in $(__strip_duplicates "$DEP_FUNC_NAMES"); do
    if __function_defined "$FUNC_NAME"; then
        DEPS_INSTALL_FUNC="$FUNC_NAME"
        break
    fi
done
echodebug "DEPS_INSTALL_FUNC=${DEPS_INSTALL_FUNC}"

Which means the following test will always trigger when the -b option is specified.

bootstrap-salt.sh:6545

if [ "$DEPS_INSTALL_FUNC" = "null" ]; then
    echoerror "No dependencies installation function found. Exiting..."
    exit 1
fi

This can be patched by moving this test right before the call to $DEPS_INSTALL_FUNC on line 6559. Or you can patch the code with something like the following:

bootstrap-salt.sh:6545

-if [ "$DEPS_INSTALL_FUNC" = "null" ]; then
+if [ ${_NO_DEPS} -eq $BS_FALSE -a "$DEPS_INSTALL_FUNC" = "null" ]; then
    echoerror "No dependencies installation function found. Exiting..."
    exit 1
fi

bootstrap-salt.sh:6556

-if [ "$_CONFIG_ONLY" -eq $BS_FALSE ]; then
+if [ ${_NO_DEPS} -eq $BS_FALSE -a "$_CONFIG_ONLY" -eq $BS_FALSE ]; then
     # Only execute function is not in config mode only
     echoinfo "Running ${DEPS_INSTALL_FUNC}()"
     $DEPS_INSTALL_FUNC

bootstrap-salt.sh:6572

-    if [ "$_CONFIG_ONLY" -eq $BS_TRUE ]; then
+    if [ ${NO_DEPS} -eq $BS_FALSE -a "$_CONFIG_ONLY" -eq $BS_TRUE ]; then
         # Execute function to satisfy dependencies for configuration step
         echoinfo "Running ${DEPS_INSTALL_FUNC}()"
         $DEPS_INSTALL_FUNC
@arizvisa
Copy link
Contributor Author

Not too important, but this was discovered while trying to workaround issue #1167.

@rallytime
Copy link
Contributor

Ahhh good find @arizvisa. We'd love a PR to fix this if you would like.

@rallytime rallytime added this to the Approved milestone Nov 16, 2017
@arizvisa
Copy link
Contributor Author

Ah awesome! You guys are sooo much better than hashicorp at dealing with your community. I did a quick github edit with the changes that I mentioned. Hopefully it works out.

Thanks for your quick response time!

@rallytime
Copy link
Contributor

@arizvisa Happy to help! That fix looks like a great start.

Can you submit your fix as a pull request to salt-bootstrap so we can review/merge it in? There should be some documentation about that here. Do let me know if you have any questions.

@arizvisa
Copy link
Contributor Author

oh my bad. i totally thought it did. i guess github's editor creates the branch as patch-%d and expects you to submit the pr.

it's ref'd as #1169.

rallytime pushed a commit that referenced this issue Nov 20, 2017
 Fix #1168 : -b option causes error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants