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

Resolve conflicting sources.list file #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alexander-bauer
Copy link

The current version puts the Kibana repository in elasticsearch.list, which conflicts with the elasticsearch-formula. This PR moves this repository listing into kibana.list, where it will still be read by apt, but does not conflict with the Elasticsearch formula.

@aboe76 aboe76 requested a review from blbradley May 20, 2017 19:00
@alexander-bauer
Copy link
Author

alexander-bauer commented May 20, 2017

It looks like the version installed by the tests was 5.4 (the latest, I think), and the test checks that the version starts with 5.0. I think this may be an outdated test case? Sorry, I'm not familiar with the testing infrastructure.


Edit: These tests failing is resolved in #9.

@alexander-bauer
Copy link
Author

@blbradley With #9 merged, I think tests will pass on this branch. I'm not sure how to get them to run again, though.

@blbradley
Copy link

@alexander-bauer Merge master into this, and then push.

- The current version puts the Kibana repository in `elasticsearch.list`, which conflicts with the [elasticsearch-formula](https://github.com/saltstack-formulas/elasticsearch-formula)
- This change lists the source in `kibana.list`
@blbradley
Copy link

Can you show the error that using this formula and elasticsearch-formula at the same time gives you? I have previously used them together as-is with no problems.

@alexander-bauer
Copy link
Author

It wasn't strictly an error, but an ordering problem. If both are installed simultaneously, then every state.apply causes changes. Both this and elasticsearch-formula manage /etc/apt/sources.list.d/elasticsearch.list (kibana-formula,
elasticsearch-formula)

This would be fine if they were able to put two repositories in the same file, but both formulas do so using pkgrepo.managed, which is only able to maintain one repository in a file at once. (There's a note about it right under the state header.)

Although both get installed and configured properly without this change, only one repository is listed at once, so doing upgrades later outside of Salt will fail to retrieve one of them. Also, both states are always listed as changed, which goes against Salt's ideas about being idempotent.

@blbradley
Copy link

I'd still need to see the idempotency issue or be able to reproduce it.

I chose the same sources file as elasticsearch-formula because I'm not sure that Salt responds well with duplicate Debian source entries. Duplicate entries are a misconfiguration and should be avoided.

@blbradley
Copy link

FWIW, I appreciate you bringing up the issue. This is one of those tasks where I made a choice and had to deferred potential issues until they came up. This also brings up another issue: does this formula depend on elasticsearch-formula?

@alexander-bauer Please open up a separate issue about the idempotency issue. We can discuss the overall idempotency issue there.

@alexander-bauer
Copy link
Author

It looks like I'm accidentally running an ancient version of Elasticsearch (whoops), so this issue can be solved for me by upgrading to ES 5. However, the trouble I was seeing stems from the fact that pre-ES 5, Kibana and ES lived in different repositories. elasticsearch-formula switches between them based on major version, as here.

Perhaps it would be worthwhile to incorporate that logic in this formula, to install separate repository if the other is not present, but that would couple this formula rather tightly to the other.

Quoting from my last comment on #11.

What I could do on this PR is to check whether the major Kibana version is 5, and if so, install to the elasticsearch.list file, and otherwise, to the kibana.list file. However, if I go this way, then attempting to install an older Elasticsearch via the formula, with a modern Kibana will cause the same idempotency problem.

I would call that a non-issue, but it seems like the default for elasticsearch-formula is to install 2.x, so including both this and that formula with default options will cause the idempotency problem. (And possibly also version incompatibility, which might by why I've been having trouble.)

I think the right course of action is then as follows:

  1. Either document the major version setting more clearly in elasticsearch-formula, or change the default
  2. Change this PR to install kibana.list if the desired major version is less than 5, or elasticsearch.list otherwise.

Another point to discuss might be to rename the list file in all of these formulas to elastic.list for major versions 5 and up, because the joint repository seems to cover all the elastic products.

@blbradley Please let me know your thoughts, now that I'm a bit more educated on this!

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.

2 participants