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

repo change via major_version #13

Merged
merged 3 commits into from Nov 14, 2016

Conversation

blbradley
Copy link
Contributor

@blbradley blbradley commented Nov 2, 2016

This allows users to install ElasticSearch 2.x or 5.x.

With this, users can't specify the exact version. I'm up creating something more flexible now or later.

Behavior

  • no value or `2': as the formula works now
  • 5: installs 5.x from new repo
  • from 2 to 5: installs 5.x in place of 2.x, data upgraded (i think)
  • from 5 to 2: service tries to start on every highstate (systemd)

@blbradley
Copy link
Contributor Author

@martinhoefling @kpostrup ping!

I updated the top comment to reflect behavior of this one.

@danygielow
Copy link

Version 5.x introduced some more changes that have to be addressed:

The following list has been taken from PRs for the official ansible role elastic/ansible-elasticsearch#178 and elastic/ansible-elasticsearch#182

@blbradley
Copy link
Contributor Author

@danygielow Only bullet item four is directly managed by this formula. It would not be if I can get #12 merged.

@blbradley
Copy link
Contributor Author

Even more importantly, #14 need to be merged before anything so I can show that both 2.x and 5.x work out of the box.

@aboe76
Copy link
Member

aboe76 commented Nov 10, 2016

@blbradley I have merged #14 what can I do to speed this up so the formula is usable with 5.x elastic search?

@blbradley
Copy link
Contributor Author

@aboe76 I have tests for this I need to push, but I'm at a conference today.

@aboe76
Copy link
Member

aboe76 commented Nov 10, 2016

@blbradley, No Problem ping me tomorrow, and I help you merge some stuff in this formula.

@blbradley
Copy link
Contributor Author

@aboe76 If you want to take a look at #12 and mentioned issues inside that would be great. But we need discussion before merging anything else.

@blbradley
Copy link
Contributor Author

#12 is merged! Working on this now.

@blbradley
Copy link
Contributor Author

@aboe76 I'm thinking that should cover it. Thanks for your reviews!

P.S. Don't forget to squash! 😁

@aboe76
Copy link
Member

aboe76 commented Nov 14, 2016

looks good and the test ran OK

@aboe76 aboe76 merged commit 7096fe4 into saltstack-formulas:master Nov 14, 2016
@blbradley blbradley deleted the repo-major-version branch November 14, 2016 21:33
@blbradley
Copy link
Contributor Author

@jcockhren Yo! I know you were interested in this. We are done here!

@aboe76
Copy link
Member

aboe76 commented Nov 14, 2016

@blbradley this leaves #11 only todo, to make everybody happy.

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

Successfully merging this pull request may close these issues.

None yet

3 participants