Merge lp://qastaging/~evarlast/charms/trusty/elasticsearch/add-version-config into lp://qastaging/~charmers/charms/trusty/elasticsearch/trunk

Proposed by Jay R. Wren
Status: Rejected
Rejected by: Marco Ceppi
Proposed branch: lp://qastaging/~evarlast/charms/trusty/elasticsearch/add-version-config
Merge into: lp://qastaging/~charmers/charms/trusty/elasticsearch/trunk
Diff against target: 96 lines (+41/-7)
4 files modified
config.yaml (+13/-0)
tasks/install-elasticsearch.yml (+13/-6)
tests/01-config-changes (+8/-1)
tests/helpers/__init__.py (+7/-0)
To merge this branch: bzr merge lp://qastaging/~evarlast/charms/trusty/elasticsearch/add-version-config
Reviewer Review Type Date Requested Status
Kevin W Monroe Disapprove
Whit Morriss (community) Needs Fixing
Review Queue (community) automated testing Needs Fixing
Michael Nelson (community) Approve
charmers Pending
Review via email: mp+237916@code.qastaging.launchpad.net

Description of the change

Allow specifying the version of the elasticsearch package in charm config.

Use case: as an op, I must control what version of services I roll out to my production and other environments, so that I can test new versions while retaining the ability to roll out new old versions in both existing and new environments.

To post a comment you must log in.
Revision history for this message
Review Queue (review-queue) wrote :

This items has failed automated testing! Results available here http://reports.vapour.ws/charm-tests/charm-bundle-test-1269-results

review: Needs Fixing (automated testing)
Revision history for this message
Michael Nelson (michael.nelson) wrote :

Hi Jay,

Thanks for the update. Just fyi, the reason this hasn't been an issue in our usage is that as an op, we configure our own repository (ie. we control the private repo we're using to install from).

I understand that's not useful for everyone, so another option is to instead provide the deb in the charm (files/elasticsearch.deb) and this will be used instead (ie. under your total control).

That said, +1 to your change.

I need to update the tests so that test deps are installed (nosetests), and the amulet tests run on trusty, not precise (I'll find out about that).

Thanks!

review: Approve
Revision history for this message
Review Queue (review-queue) wrote :

This items has failed automated testing! Results available here http://reports.vapour.ws/charm-tests/charm-bundle-test-10370-results

review: Needs Fixing (automated testing)
Revision history for this message
Michael Nelson (michael.nelson) wrote :
Revision history for this message
Jay R. Wren (evarlast) wrote :

Ah! Thanks for this.

I missed that the official charm is from ~onlineservices-charmers.

On Sun, Nov 23, 2014 at 2:55 PM, Michael Nelson <
<email address hidden>> wrote:

> You probably want to remerge trunk from
>
>
> https://code.launchpad.net/~onlineservices-charmers/charms/trusty/elasticsearch/trunk
> --
>
> https://code.launchpad.net/~evarlast/charms/trusty/elasticsearch/add-version-config/+merge/237916
> You are the owner of
> lp:~evarlast/charms/trusty/elasticsearch/add-version-config.
>

Revision history for this message
Whit Morriss (whitmo) wrote :

Taking a look at this branch. lp:~evarlast/charms/trusty/elasticsearch/add-version-config does not merge cleanly into head of https://code.launchpad.net/~onlineservices-charmers/charms/trusty/elasticsearch/trunk or vice versa.

I'm also curious what the relationship of the charmer/charms version and the online services version (which is more active) is / should be. The online services copy is clearly the canonical one, but whose responsibility is it to keep the charmers copy synced?

Revision history for this message
Whit Morriss (whitmo) wrote :

Marking a need fixing until the merge/upstream resolution gets figured out. Thanks y'all!

review: Needs Fixing
Revision history for this message
Kevin W Monroe (kwmonroe) wrote :

Just spoke to Jay. This change was needed last year to be able to handle a specific elasticsearch minor rev. It hasn't been needed since.

If this version-config feature becomes needed in the future, Jay will refactor his changes to apply to the latest ~onlineservices-charmers charm and open a new MP. Recommend rejecting/closing this MP.

As a side note, if this feature does become active again, please watch out for immutable config. If the version config is changed post-deployment, the charm will need to handle that. Adding config-changed to the yml may be all you need to do this:

...
+ tags:
+ - config-changed
+ - install
+ - upgrade-charm
...

review: Disapprove

Unmerged revisions

36. By Jay R. Wren

Add version config option

Disabled install-recommends.
Disabled extra unneeded runs of apt-get update.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches