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

add support for multiple step converge #564

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
@emiddleton

emiddleton commented Dec 16, 2014

This change allows multiple provisioner runs to be made on a single converge. This can be used to test migration between cookbooks versions #339, to install a new kernel #324 (using cookbook-reboot-handler patched with emiddleton/cookbook-reboot-handler@a94e40e) and to make converge more robust #162

Converge steps are defined as an array of step hashes as shown below. (step hashes support all suite keys except name: and steps:).

platforms:
  - name: centos-6.4

suites:
  - name: mysuite
    steps:
      - run_list:
        - recipe[wakame-vdc::default]
      - run_list:
        - recipe[wakame-vdc::default]

Each step result in a new instance with _step_n attached to the suite name i.e

# kitchen list

Instance                  Driver   Provisioner  Last Action
mysuite-step-1-centos-64  Vagrant  ChefSolo     <Not Created>
mysuite-step-2-centos-64  Vagrant  ChefSolo     <Not Created>

This allows test to be run at each step in the convergence process.

@poelzi

This comment has been minimized.

poelzi commented Dec 16, 2014

it would be nice to have different provisioners in this case.
for example: you need to bootstrap some data into the node before your configuration management is run. in this case some simple bash provisioner would be great.

@emiddleton

This comment has been minimized.

emiddleton commented Dec 17, 2014

@poelzi this patch supports running different provisioners in each run. To run a script in the first step you just need to add the provisioner to the step. i.e.

  - name: default
    steps:
      - run_list:
        - recipe[wakame-vdc::default]
        attributes:
        provisioner:
          name: shell
      - run_list:
        - recipe[wakame-vdc::default]
        attributes:

@emiddleton emiddleton changed the title from add support for multiple steps converge to add support for multiple step converge Jan 9, 2015

@tyler-ball

This comment has been minimized.

Member

tyler-ball commented Feb 18, 2015

My concern with this PR is that I don't know if it solves the referenced issues. Converting the steps config to multiple VM definitions seems like a new way to list different suites. Isn't the idea that I want to perform step 1 on the same VM as I perform step 2?

@emiddleton

This comment has been minimized.

emiddleton commented Feb 20, 2015

@tyler-ball it solves the referenced issues. step2 is step1 followed by step2. Step1 is included so there is a way to test the initial converge worked before testing the second converge. You could optimize this by copying the step1 vm and converging from there for step2 but that looked more involved and this is already a pretty big change.

@tyler-ball

This comment has been minimized.

Member

tyler-ball commented Mar 4, 2015

Hey @emiddleton - thanks for the reply, sorry it took me so long to get back to you. I understand now that instance 2 runs both step 1 and step 2.

I'm going to tag this for 2.0 and move it back into the backlog. For 1.4 we are making some significant changes to the responsibility of Provisioners and Drivers that will affect the code here.

While this does solve some of the linked problems, it is also solving them in a prescribed way. I think of this as 'Deployment Strategy Testing'. This does solve help test 1 deployment strategy but eventually we would like to tackle that issue on its own. There are billions of deployment strategies and this approach only tests 1.

We also have some smaller scale changes we can make inside the provisioners that will allow chef-client to be converged multiple times. That is a specific issue we want to solve with a smaller, targeted patch.

Finally, after we finish the current work to shift around provisioning responsibility, you will have the option of writing your own provisioner that is capable of performing these multi-converge and multi-test execution. This would allow you to release your own Gem containing this multi-capable provisioner.

@tyler-ball tyler-ball added this to the 2.0 milestone Mar 4, 2015

@tyler-ball tyler-ball removed the Developing label Mar 4, 2015

@emiddleton

This comment has been minimized.

emiddleton commented Mar 5, 2015

@tyler-ball The most important use case for multiple convergence is to test state transitions that will effect a production system not deployment strategies which are far less critical. Breakage in deployment causes delays, breakage on production causes system downtime. At the moment the only way to unit test if upgrading a cookbook version or any other cookbook configuration change will break a deployed system is to manually create each possible initial state.

Everyone I have spoken to just creates a production replica and doesn't bother with cookbook testing, which is a shame because it would be much easier to diagnose problems earlier and would result in much more robust cookbooks.

@patcon

This comment has been minimized.

Contributor

patcon commented May 18, 2015

👍

1 similar comment
@manuelmazzuola

This comment has been minimized.

manuelmazzuola commented Jun 15, 2015

+1

@doertedev

This comment has been minimized.

doertedev commented Jul 1, 2015

Cool. I have a cookbook to run on a virgin Debian machine. I connect via root and do $things. The last thing is switching the sshd_config to not allow root anymore and set SSHD on a diff. port.
Would this allow me, if not asserting that the connection actually breaks, to just switch to diff. connection params in the second step?

@beeisabelm

This comment has been minimized.

beeisabelm commented Jul 6, 2015

👍

@CpuID

This comment has been minimized.

CpuID commented Jul 16, 2015

@emiddleton - I am thinking of running this PR via a forked gem until it is merged, do you feel it should be fine under 1.4, as per the comments from @tyler-ball above?

Tested cherry-picking your commit here into my own fork, got a heap of merge conflicts. Looks like the PR itself here claims there's conflicts also.

@CpuID CpuID referenced this pull request Jul 16, 2015

Closed

reconverge with test-kitchen #780

@wjordan

This comment has been minimized.

wjordan commented Oct 25, 2015

For anyone still looking to use this feature in 1.4, I merged this PR (resolving the merge conflicts) along with a couple other open bugfix/feature PRs into a fork at wjordan/test-kitchen#cdo.

@howdoicomputer

This comment has been minimized.

howdoicomputer commented Nov 1, 2015

Hey so @emiddleton and @wjordan thank you so much for your work putting this together. I've been working on a Proxmox cookbook and, because Proxmox includes a heavily patched version of the Linux kernel, I've had a need for double convergence for a single suite. I've forked, merged into master, and tagged this cookbook into an internal repository within my company for use within the one cookbook I've been working on.

The only alternative that I can see at the moment was to build a Vagrant box for a post-kernel-installation recipe and then pull two boxes to test this.

Again, thank you so much.

@howdoicomputer

This comment has been minimized.

howdoicomputer commented Nov 5, 2015

Also, to note: this breaks busser somehow. It won't pass on over my serverspec files. I'm looking into it.

@cheeseplus cheeseplus modified the milestones: 2.0, Accepted Major Jan 14, 2016

@wjordan

This comment has been minimized.

wjordan commented Jan 25, 2016

If anyone (like myself) still depends on this PR and wants to use it with tk v1.5.0, I've made a rebased commit 721244e available in branch wjordan/test-kitchen#multi-step-converge.

Hoping this feature integration gets moved out of the backlog soon. Or, if there's been any progress on the two work items @tyler-ball mentioned in an earlier comment (1. allow chef-client to be converged multiple times; 2. shift around provisioning responsibility, so a separate provisioner gem could be written), I'd be happy to do the remaining work necessary to extract this feature into its own provisioner gem with a small bit of guidance.

@cheeseplus cheeseplus added Accepted and removed Accepted labels Mar 23, 2016

@cheeseplus cheeseplus removed this from the Accepted Major milestone Mar 23, 2016

@tpcwang

This comment has been minimized.

tpcwang commented Apr 5, 2016

+1. I would really like to use a custom provisioner that allows for reboot during converge to help test things like kernel module installation and service starting. Is this issue still targeted for 2.0?

@cheeseplus

This comment has been minimized.

Contributor

cheeseplus commented Apr 5, 2016

The feature has been "Accepted" as a new feature and is targeted for a major release, most likely 2.0 - that said I don't know anyone on TK core not doing any active development toward this goal as we're far more focused on bugs (vs features) currently.

@AReshma

This comment has been minimized.

AReshma commented Jan 25, 2017

This looks fantastic. Waiting for this feature.

@wesm-outreach

This comment has been minimized.

wesm-outreach commented Jul 31, 2017

Any update on this? This has been open for years, and it looks like it's fairly high demand.

@cheeseplus

This comment has been minimized.

Contributor

cheeseplus commented Aug 1, 2017

Lots of people want the feature but designing with the future and multi-node in mind is essentially where this is getting hung up as they are somewhat bound together as we're going to need to restructure some low level parts of kitchen. For reboot support we've already got that in chef-client and test-kitchen supports that so some of the +1s and issues in the thread have been resolved on that front.

The feature as written breaks enough downstream that shoehorning it in at this point is less than desirable to doing a ground up implementation alongside multi-node. These are non-trivial features and need careful consideration before being introduced and unfortunately I've largely been the only person maintaining for the better part of 2 years so large feature implementations have simply been backlogged behind bug fixes and just trying to keep things up-to-date.

Even if this were rebased, I'm not sure we'd take it until we've sorted out how this will fit in with other future plans.

@cheeseplus cheeseplus removed the Accepted label Aug 1, 2017

@cheeseplus

This comment has been minimized.

Contributor

cheeseplus commented Jan 29, 2018

We have https://github.com/chef/chef-rfc/blob/master/rfc084-test-kitchen-multi.md which covers this so closing the PR since it's part of our 2.x roadmap.

@cheeseplus cheeseplus closed this Jan 29, 2018

@lock

This comment has been minimized.

lock bot commented Aug 6, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 6, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.