Knife bootstrap_environment should use Explicit config before Implicit
#3864
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
👍 |
|
please rebase onto master and see if that fixes the test failures. |
| @@ -40,7 +40,7 @@ def initialize(config, run_list, chef_config, secret = nil) | |||
| end | |||
|
|
|||
| def bootstrap_environment | |||
| @chef_config[:environment] || '_default' | |||
| @config[:environment] || @chef_config[:environment] || '_default' | |||
There was a problem hiding this comment.
pretty certain that you want to drop @chef_config entirely here.
|
created a #3946 epic for issues like this all over the knife codebase. |
b56cdef
to
291bc78
Compare
…ontext Setting these values in Chef::Config is being deprecated in favor of using the knife config directly. chef#3946
291bc78
to
b258f11
Compare
|
In light of #3946, I have dropped Chef::Config from consideration. Rebased, Squashed. |
|
👍 LGTM |
Merged
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When the knife cli utility is run, it invokes a method named
apply_computed_config, which transposes some of the CLI options onto Chef::Config. Both Chef::Config, as well as the explicitly passed knife options, are given to the Bootstrap Context.Bootstrap Context is responsible for setting values for the bootstrap template... one of which is the Chef environment. Currently, this logic pulls the value directly from Chef::Config, and if there is none there, it defaults to
_default. The problem is that this logic relies onapply_computed_confighaving been run previously. Rather than completely ignoring the Explicit config that we have available to us, we should consider it. If Chef Environment has been explicitly set in this knife invocation, we should use that... else, we can look in Chef::Config... else, we can finally fall back to_default.It is worth noting that chef_node_name is also governed by this same pattern, though the Bootstrap Context does not take the value from Chef::Config, and instead looks only to the explicit config that Knife has received. I would like to see the same behavior for Chef Environment... the underlying issue being that if I programmatically call Knife Bootstrap, I cannot simply pass it a configuration and tell it to run... I must first call
apply_computed_config, otherwise my node will come up with the default environment.