(BKR-1058) Implement a simpler monolithic install method #57
Conversation
|
Can one of the admins verify this patch? |
lib/beaker-pe/install/pe_utils.rb
Outdated
|
|
||
| step "Sign agent certificates" do | ||
| # This will sign all cert requests | ||
| sign_certificate_for() |
There was a problem hiding this comment.
There appears to be a potential race condition here where the agents haven't necessarily submitted their CSRs by the time we get here. Will amend...
There was a problem hiding this comment.
Oh, looks like sign_certificate_for(hosts) will still try to sign them all at once, and will repeat until they're all signed. Will update it to just do that then!
|
Refer to this link for build results (access rights to CI server needed): |
nicklewis
force-pushed
the
improved-monolithic-install
branch
from
926570d to
d395b5c
Compare
|
Refer to this link for build results (access rights to CI server needed): |
|
👍 looks good to me, but I'd definitely like to get input from @puppetlabs/integration before merging this. |
There was a problem hiding this comment.
Really excited to see this go live and start using it.
We need to eventually consolidate all the various install / upgrade code that exists between this repo, https://github.com/puppetlabs/beaker-pe-large-environments and https://github.com/puppetlabs/pe_acceptance_tests/blob/2017.1.x/lib/puppet_enterprise_acceptance/setup_helpers.rb
lib/beaker-pe/install/pe_utils.rb
Outdated
| roles.all? {|role| host['roles'].include?(role)} | ||
| end | ||
|
|
||
| def determine_install_type(hosts, opts) |
There was a problem hiding this comment.
Can you add some rdoc/yard style comment header here detailing input and output.
specifically looking for what generic, simple_monolithic and simple_split mean and what determines them.
lib/beaker-pe/install/pe_utils.rb
Outdated
| end | ||
| end | ||
|
|
||
| def simple_monolithic_install(master, agents, opts={}) |
There was a problem hiding this comment.
Can you add some rdoc/yard style comment header here detailing input and output.
| install_type = determine_install_type(hosts, opts) | ||
| case install_type | ||
| when :simple_monolithic | ||
| simple_monolithic_install(hosts.first, hosts.drop(1), opts) |
There was a problem hiding this comment.
Is it safe to assume that the first host will always be the master? It looks like calls to this method are already coming from somewhere else that has already performed a sorted_hosts call, but not sure if I want to rely on that assumption.
There was a problem hiding this comment.
I think it's safe to assume that because it's currently assumed throughout the whole file. If that were to stop being true, many other things would break.
lib/beaker-pe/install/pe_utils.rb
Outdated
| dispatcher.create_new_node_group_model(node_group) | ||
| on master, puppet("agent -t"), :acceptable_exit_codes => [0,2] | ||
| dispatcher.create_new_node_group_model(node_group) | ||
| on master, puppet("agent -t"), :acceptable_exit_codes => [0,2] |
There was a problem hiding this comment.
Since this should now only happen if we need to add the pe_repo class, should we restrict the puppet agent run to only accept 2?
There was a problem hiding this comment.
I'm not sure I want to assume that work will always be performed. If it should do some work and doesn't, then agent install should fail later. If nothing fails later, then it must have worked alright.
nicklewis
force-pushed
the
improved-monolithic-install
branch
from
d395b5c to
39c9d63
Compare
|
I squashed in some method docs. |
|
Refer to this link for build results (access rights to CI server needed): |
|
I have no idea what the problem is with that build, but I'm confident it's at least not due to my documentation. |
|
@nicklewis those failures are related to the S3 outage; let's rekick this when Amazon resolves that. |
|
How do I make this retest? |
|
@puppetlabs-jenkins retest this please |
|
Refer to this link for build results (access rights to CI server needed): |
|
@ericwilliamson Any other input on this? |
|
@ericwilliamson or @puppetlabs/integration? Do you have anymore input for this PR? |
|
@nicklewis @kevpl Nope, 👍 |
Currently, the `do_install` method is a mega-method responsible for installing and upgrading every version, layout, and platform of PE. It knows how to handle masterless installs, legacy agent installs, and extremely old versions of PE. Because of that legacy, it has some inefficiencies and is inflexible. Since the most common case by far is a simple monolithic or split PE install with a set of frictionless agents, we can dramatically simplify and therefore speed up the basic case. This commit implements a determine_install_type method that will detect whether the task being asked of it is one of those simple cases. The `do_install` method now branches based on the type of install, calling `simple_monolithic_install` for a basic monolithic + agents install, or `generic_install` for everything else. Simple split installs are currently detected, but not handled specially (they still fall back to the generic install). Doing away with some of the legacy concerns allows this method to be much simpler and more efficient. In particular, because the method has a defined task (mono master + agents, rather than a generic list of hosts), it can be optimized around that task. This is accomplished now by first installing the master, and then installing all the agents in parallel. This method also removes an extra agent run that hasn't been necessary since PE 3.3. For a monolithic master with four frictionless agents, this new method takes ~6 min 30 sec, compared to ~11 minutes before.
nicklewis
force-pushed
the
improved-monolithic-install
branch
from
39c9d63 to
691a101
Compare
|
Rebased this to not bring in an older version of the changes from #56. |
|
Refer to this link for build results (access rights to CI server needed): |
Currently, the
do_installmethod is a mega-method responsible forinstalling and upgrading every version, layout, and platform of PE. It
knows how to handle masterless installs, legacy agent installs, and
extremely old versions of PE. Because of that legacy, it has some
inefficiencies and is inflexible.
Since the most common case by far is a simple monolithic or split PE
install with a set of frictionless agents, we can dramatically simplify and
therefore speed up the basic case. This commit implements a
determine_install_type method that will detect whether the task being asked
of it is one of those simple cases. The
do_installmethod now branchesbased on the type of install, calling
simple_monolithic_installfor a basic monolithic + agents install, orgeneric_installfor everything else. Simple split installs are currentlydetected, but not handled specially (they still fall back to the generic
install).
Doing away with some of the legacy concerns allows this method to be much
simpler and more efficient. In particular, because the method has a defined
task (mono master + agents, rather than a generic list of hosts), it can be
optimized around that task. This is accomplished now by first installing
the master, and then installing all the agents in parallel. This method
also removes an extra agent run that hasn't been necessary since PE 3.3.
For a monolithic master with four frictionless agents, this new method
takes ~6 min 30 sec, compared to ~11 minutes before.
Because this new method is selected by detecting what's being asked, it should
be automatically picked up by any projects that are currently testing against
this layout, with no project-specific changes required.