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

adds notifies :reconfigure to supermarket chef_ingredient #32

Conversation

jeremymv2
Copy link
Contributor

This is an attempt to address the observation that when node['supermarket_omnibus']['package_version'] changes, a reconfigure is NOT run, leaving the supermarket instance in a faulted state. This change has been tested as such:

  • package_version does not change -> converge does not change anything
  • package_version changes, -> converge installs new package version and then runs reconfigure

@nellshamrell
Copy link
Contributor

LGTM

@irvingpop
Copy link

hey @jeremymv2 does this adequately cover the upgrade case?

@robbkidd
Copy link
Contributor

We do the reconfigure in our wrapper cookbook.

include_recipe 'supermarket-omnibus-cookbook'

csi = resources('chef_ingredient[supermarket]')
csi.notifies :reconfigure, 'chef_ingredient[supermarket]'
csi.action :upgrade

I am wary of declaring that reconfigure for everyone everywhere. And an install might still get broke because a restart hasn't also happened. Would doing that reconfigure in your wrapper solve the problem?

@irvingpop
Copy link

If everyone needs to put the same code in their wrapper cookbook, then we're doing something wrong. How can we make this workable for everyone?

@jeremymv2
Copy link
Contributor Author

The example above doesn't work because of the way the resource is named in the library of the supermarket-omnibus-cookbook.
https://github.com/chef-cookbooks/supermarket-omnibus-cookbook/blob/master/libraries/supermarket_server.rb#L10

The resource name is actually supermarket_server[supermarket].
https://github.com/chef-cookbooks/supermarket-omnibus-cookbook/blob/master/recipes/default.rb#L11

Referencing it from the recipe by chef_ingredient[supermarket] results in a resource not found error.

I've tested this in my wrapper with success:

include_recipe 'supermarket-omnibus-cookbook'

execute 'reconfig_n_restart' do
  command 'sudo supermarket-ctl reconfigure && sudo supermarket-ctl restart'
  action :nothing
end

super_duper_resource = resources('supermarket_server[supermarket]')
super_duper_resource.notifies :run, 'execute[reconfig_n_restart]'

I agree with @irvingpop. I think we either need to document the commands required for an upgrade or provide a recipe that everyone can consume.

@robbkidd
Copy link
Contributor

Yea. The notifies reconfigure and action upgrade steps in a wrapper broke when we created the supermarket_server resource. I agree that we should certainly provide the steps to perform an upgrade in this cookbook.

Some options:

  • add an upgrade recipe to this cookbook that can be called at the end-user's option
  • add an action—:upgrade?—to the supermarket_server resource provider

Automatically wiring that reconfigure & restart upgrade action up may surprise end-users. Calling it should continue to be something end users explicitly add to their wrapper cookbooks so they can control the timing of the service outage caused by a restart. But I'm open to feedback on doing things automatically from people running private supermarkets.

@nellshamrell
Copy link
Contributor

Closing in favor of #35 (if this is in error, feel free to re-open)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants