Fix Cassandra, improve config and tests #59179

JohnAZoidberg · 2019-04-08T12:47:36Z

Motivation for this change

It's more intuitive and discoverable when you can configure set some options using Nix attributes - plus we can check it using some assertions.
Make it possible to set the variables in cassandra-env.sh.
Allow client connections by default because that's what users expect.
Fix tests and test more things
Make binaries work again (properly wrap them)

The test doesn't work - it doesn't work without this PR. I'll figure out why, when I have time. Seems like a race condition of the VMs starting, it's not a problem with the module itself, rather the tests.
[EDIT]: I think I fixed the tests, please have ofborg run them.

I tested manually on my system that the different configuration options actually have the desired effect.

Things done

JohnAZoidberg · 2019-04-09T02:17:05Z

This PR now also fixes the tests for Cassandra.
They take 5 minutes to run (on my laptop) but that's fine for booting and shutting down 3 VMs and rebalancing a Cassandra cluster.

jpyen · 2019-04-10T12:21:50Z

This is very necessary. Thanks Daniel.

JohnAZoidberg · 2019-04-12T20:55:47Z

I broke some of the binaries in #59090...
I misinterpreted the error when testing some of the binaries. I thought I just used it wrong parameters but turns out I broke some of them while making others work.
This PR (5041afc9690894f1a809ad6fc) fixes it.

JohnAZoidberg · 2019-04-22T03:39:04Z

Please somebody have a look - Cassandra is currently broken on master.

aanderse · 2019-04-22T10:43:42Z

Hi @JohnAZoidberg,

One delay here is that there is no clear indication of who the cassandra expert is for NixOS. Looking at the git history for both the package and service is a bit sparse. If no one is an expert on cassandra it will take a fair bit of work to review and validate these changes. So the question is... who do we need to get involved for review of this PR before someone with the appropriate access will feel comfortable merging?

aanderse · 2019-04-22T11:01:57Z

@GrahamcOfBorg build cassandra
@GrahamcOfBorg test cassandra

aanderse · 2019-04-22T11:21:41Z

@JohnAZoidberg cassandra is missing from all-tests.nix.

JohnAZoidberg · 2019-04-22T16:31:59Z

Actually yeah, that makes me think I should add my new config options to the tests so that non-Cassandra people can trust that they do the right thing. (And also they're automatically checked.)

JohnAZoidberg · 2019-04-22T21:40:04Z

Does anybody know whether I can rely on the IPs staying like they are? Or can I force them to be like I assume?

aanderse · 2019-04-22T21:53:41Z

Does anybody know whether I can rely on the IPs staying like they are? Or can I force them to be like I assume?

Looking through nixos/tests I'm inclined to believe setting a static IP might be a good solution.

JohnAZoidberg · 2019-04-23T03:48:58Z

I added some more tests that test almost all config options.
And also made the IPs static.

nixos/tests/all-tests.nix

JohnAZoidberg · 2019-04-24T21:29:24Z

@fuzzy-id You rewrote the module - do you have any thoughts?

aanderse · 2019-04-25T16:24:08Z

Looks like openjdk on arch is broken in master ☹️

hash mismatch in fixed-output derivation '/nix/store/pc2krcggyflqbgpz6zlpzxr7hjyfdlif-OpenJDK11U-jre_aarch64_linux_hotspot_11.0.3_7.tar.gz':
  wanted: sha256:0hisjzgv29x2af0wvpy1c9j027qvv08cd9r9f0hg40wdczbbgxaa
  got:    sha256:0r45f358ih5xmygqm91g6nkq9d6nnjl8zz75kl4xbij00svzlcfy

Unrelated to this change, so I'm going to ignore those aarch64-linux errors.

fuzzy-id · 2019-04-25T20:01:30Z

On first sight this looks good and promising. @JohnAZoidberg did you run the tests with all versions of cassandra we currently have in nixpkgs?

Allow for more intuitive specifying of seed node addresses with Nix syntax.

Seems like you can't have a node as its own seed when it's listening on an interface instead of an IP. At least the way it was done in the test doesn't work and I can't figure out any other way than to just listen on the IP address instead.

Would previously overwrite the binary with the wrapper and thus wrap itself (resulting in an infinite recursion on execution) for the binaries in /bin.t

If we have the ability to enable remote JMX we should also support setting credentials for that because they become required if you turn it on.

If you're on a multi user system you don't want to have the password in the nix-store. With the new jmxRolesFile option you can specify your own protected file instead.

Below that it works but only when supplying a custom password file with restricted permissions (i.e. outside the nix-store). We can't do that using an absolute path in the tests.

If the `seedAddresses` is not set, don't force `SimpleSeedProvider` to be in `seed_provider`. This could cause problems in a multi-datacenter deployment when a different seed provider is preferred.

JohnAZoidberg · 2019-06-13T02:47:37Z

Oh, I was responsible for those conflicts 😆

aanderse · 2019-06-14T00:36:43Z

Thanks @JohnAZoidberg

ivan · 2019-06-14T03:29:40Z

The NixOS manual doesn't seem to build with this merged:

# nixos-rebuild switch --builders ''
building Nix...
building the system configuration...
trace: warning: Option `programs.captive-browser.package' has no description.
these derivations will be built:
  /nix/store/d9ihpqcd0d6x5dswjfkwrsiwrv1jd2gk-options-db.xml.drv
  /nix/store/33p1av88g1y974qgjbmxz3a53nsv61vp-generated-docbook.drv
  /nix/store/w7zbd8q8akfdcy47vwplpz8n63kwi61b-nixos-manual-combined.drv
  /nix/store/y8z3qqwv7ivskf4m8rzqiz6jpqrgmk1k-manual-olinkdb.drv
  /nix/store/aslfagf5y3c267g41aj2kr3c9j2z8zvm-nixos-manual-html.drv
  /nix/store/dfsssp62vf9963yqdcq9wwc5axljxki2-nixos-help.drv
  /nix/store/rjpb8pnpk4m30dwbi245fpz4bycqb9x0-nixos-manual.desktop.drv
  /nix/store/yfkra26ay0lc76j06cbypj9dmk4c677l-nixos-manpages.drv
  /nix/store/y3qm2sy4y3czpisxdfn67g8vnpn5ccjl-system-path.drv
  /nix/store/n2k8cpgsmzs61v3jhfq4g1v694q1kyjp-dbus-1.drv
  /nix/store/sr7ywq644wvzcdaqm82mha8kf6nf0rw0-unit-dbus.service.drv
  /nix/store/h9dnqlg1pbkhnwx4040cff2z3bx4ab24-user-units.drv
  /nix/store/lija5f5blhlm19clnvlr8l98zpwh4clj-unit-polkit.service.drv
  /nix/store/p7xypymw1czr1rmkiy65rd8ki2xgy4bi-unit-dbus.service.drv
  /nix/store/wqzj1ycaw5va2iy3ddsjdbxckgb01hxj-unit-systemd-fsck-.service.drv
  /nix/store/wrig2rm34mbwq3sfyz5zpzii5xq04hda-system-units.drv
  /nix/store/w31lswqzbb21hgn4hy5inyq697kd7kw8-etc.drv
  /nix/store/0n3zi29zf8k7p1l46qkxy3qn0yh69qh0-nixos-system-ra-19.09.git.412d110.drv
building '/nix/store/d9ihpqcd0d6x5dswjfkwrsiwrv1jd2gk-options-db.xml.drv'...
intermediate.xml:12476: parser error : Opening and ending tag mismatch: literal line 12471 and para
</para></nixos:option-description><para><emphasis>Type:</emphasis> null or strin
       ^
intermediate.xml:12476: parser error : Opening and ending tag mismatch: literal line 12471 and nixos:option-description
</para></nixos:option-description><para><emphasis>Type:</emphasis> null or strin
                                  ^
intermediate.xml:12480: parser error : Opening and ending tag mismatch: para line 12470 and listitem
            </filename></member></simplelist></listitem></varlistentry><varliste
                                                        ^
intermediate.xml:12480: parser error : Opening and ending tag mismatch: option-description line 12470 and varlistentry
            </filename></member></simplelist></listitem></varlistentry><varliste
                                                                       ^
intermediate.xml:60118: parser error : Opening and ending tag mismatch: listitem line 12470 and variablelist
      </filename></member></simplelist></listitem></varlistentry></variablelist>
                                                                               ^
intermediate.xml:60118: parser error : Opening and ending tag mismatch: varlistentry line 12470 and appendix
lename></member></simplelist></listitem></varlistentry></variablelist></appendix
                                                                               ^
intermediate.xml:60120: parser error : Premature end of data in tag variablelist line 3

^
intermediate.xml:60120: parser error : Premature end of data in tag appendix line 3

^
unable to parse intermediate.xml
builder for '/nix/store/d9ihpqcd0d6x5dswjfkwrsiwrv1jd2gk-options-db.xml.drv' failed with exit code 6
cannot build derivation '/nix/store/33p1av88g1y974qgjbmxz3a53nsv61vp-generated-docbook.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/w7zbd8q8akfdcy47vwplpz8n63kwi61b-nixos-manual-combined.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/yfkra26ay0lc76j06cbypj9dmk4c677l-nixos-manpages.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/aslfagf5y3c267g41aj2kr3c9j2z8zvm-nixos-manual-html.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/y3qm2sy4y3czpisxdfn67g8vnpn5ccjl-system-path.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/0n3zi29zf8k7p1l46qkxy3qn0yh69qh0-nixos-system-ra-19.09.git.412d110.drv': 1 dependencies couldn't be built
error: build of '/nix/store/0n3zi29zf8k7p1l46qkxy3qn0yh69qh0-nixos-system-ra-19.09.git.412d110.drv' failed

@aanderse

Manual build broken by 79f7f89, which is part of pull request NixOS#59179 (Fix Cassandra, improve config and tests). The issue was just a small error because of an unbalanced <literal/> tag, so only a "/" was missing :-) Signed-off-by: aszlig <aszlig@nix.build> Cc: @aanderse

turion

Just some minor improvement comments, but I can't say I understand nix well enough to judge completely.

turion · 2019-06-16T15:03:40Z

nixos/modules/services/databases/cassandra.nix

+        cp "$cassandraEnvPkg" "$out/cassandra-env.sh"
+
+        # Delete default JMX Port, otherwise we can't set it using env variable
+        sed -i '/JMX_PORT="7199"/d' "$out/cassandra-env.sh"


If the source line ever changes, this will just fail silently?

If they remove or change this line, sed will not remove it and not issue a warning.
substituteInPlace would warn, but it doesn't have an option to delete an entire line.

I think it'll be fine if I implement the test for jmxPort like you suggested below.

turion · 2019-06-16T15:08:12Z

nixos/tests/cassandra.nix

-      networking.firewall.enable = false;
-      services.cassandra = cassandraCfg // extra;
+      networking = {
+        firewall.allowedTCPPorts = [ 7000 7199 9042 ];


Instead of 7199, don't you want jmxPort?

Good idea, see #65616

The change to "NixOS Test Cluster" in NixOS#59179 broke startup of existing clusters that used the previously-default cluster name "Test Cluster": ERROR 23:00:47 Fatal exception during initialization org.apache.cassandra.exceptions.ConfigurationException: Saved cluster name Test Cluster != configured name NixOS Test Cluster Fixes NixOS#63388.

JohnAZoidberg requested a review from infinisil as a code owner April 8, 2019 12:47

GrahamcOfBorg added 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Apr 8, 2019

JohnAZoidberg mentioned this pull request Apr 9, 2019

cassandra 3.11.3 -> 3.11.4 #58025

Merged

10 tasks

JohnAZoidberg force-pushed the cassandra-module branch 2 times, most recently from e2ed494 to 5041afc Compare April 9, 2019 06:02

GrahamcOfBorg added 10.rebuild-darwin: 1-10 and removed 10.rebuild-darwin: 0 labels Apr 9, 2019

JohnAZoidberg changed the title ~~More and better configuration for Apache Cassandra module~~ Fix Cassandra and improve config Apr 12, 2019

JohnAZoidberg mentioned this pull request Apr 12, 2019

cassandra: Wrap new binaries #59090

Merged

10 tasks

JohnAZoidberg changed the title ~~Fix Cassandra and improve config~~ Fix Cassandra, improve config and tests Apr 23, 2019

aanderse requested changes Apr 24, 2019

View reviewed changes

nixos/tests/all-tests.nix Outdated Show resolved Hide resolved

JohnAZoidberg force-pushed the cassandra-module branch from a7dd426 to d71bfa7 Compare April 24, 2019 20:25

JohnAZoidberg mentioned this pull request Apr 25, 2019

openjdk11: Update source hashes for aarch64 #60224

Merged

10 tasks

JohnAZoidberg added 17 commits June 13, 2019 04:34

nixos/cassandra: Allow setting of seed addresses

746b82b

Allow for more intuitive specifying of seed node addresses with Nix syntax.

nixos/cassandra: Add nixos conf for Java env

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

f003143

nixos/cassandra: Add clusterName

c1991fb

nixos/cassandra: Fix test by listening on IP

6778ee1

Seems like you can't have a node as its own seed when it's listening on an interface instead of an IP. At least the way it was done in the test doesn't work and I can't figure out any other way than to just listen on the IP address instead.

cassandra: Properly wrap all binaries

d8ad5ad

Would previously overwrite the binary with the wrapper and thus wrap itself (resulting in an infinite recursion on execution) for the binaries in /bin.t

nixos/cassandra: Reenable tests

2bcca92

nixos/cassandra: Allow setting JMX credentials

35531f4

If we have the ability to enable remote JMX we should also support setting credentials for that because they become required if you turn it on.

nixos/cassandra: Use ipAddress and capital letters

4c880fd

nixos/cassandra: Test clusterName

2d014f4

nixos/cassandra: Test seedAddresses

545ac18

nixos/cassandra: Set test VM IPs statically

2368345

nixos/cassandra: Test maxHeapSize

3162f45

nixos/cassandra: Test jmxRoles

7d646f2

nixos/cassandra: Add option for password file path

9ecd584

If you're on a multi user system you don't want to have the password in the nix-store. With the new jmxRolesFile option you can specify your own protected file instead.

nixos/cassandra: Use docbook instead of markdown style

79f7f89

nixos/cassandra: Test remote auth only on v3.11+

8e5ba87

Below that it works but only when supplying a custom password file with restricted permissions (i.e. outside the nix-store). We can't do that using an absolute path in the tests.

nixos/cassandra: Don't force SimpleSeedProvider

0350312

If the `seedAddresses` is not set, don't force `SimpleSeedProvider` to be in `seed_provider`. This could cause problems in a multi-datacenter deployment when a different seed provider is preferred.

JohnAZoidberg force-pushed the cassandra-module branch from 0a15f75 to 0350312 Compare June 13, 2019 02:43

aanderse approved these changes Jun 14, 2019

View reviewed changes

aanderse merged commit fadceeb into NixOS:master Jun 14, 2019

JohnAZoidberg deleted the cassandra-module branch June 14, 2019 07:54

turion reviewed Jun 16, 2019

View reviewed changes

ivan mentioned this pull request Jun 17, 2019

cassandra does not start: Saved cluster name Test Cluster != configured name NixOS Test Cluster #63388

Closed

ivan mentioned this pull request Jun 18, 2019

nixos/cassandra: use cassandra's default cluster name "Test Cluster" #63392

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Cassandra, improve config and tests #59179

Fix Cassandra, improve config and tests #59179

JohnAZoidberg commented Apr 8, 2019 •

edited

Loading

JohnAZoidberg commented Apr 9, 2019

jpyen commented Apr 10, 2019

JohnAZoidberg commented Apr 12, 2019 •

edited

Loading

JohnAZoidberg commented Apr 22, 2019

aanderse commented Apr 22, 2019 •

edited

Loading

aanderse commented Apr 22, 2019 •

edited

Loading

aanderse commented Apr 22, 2019

JohnAZoidberg commented Apr 22, 2019

JohnAZoidberg commented Apr 22, 2019

aanderse commented Apr 22, 2019

JohnAZoidberg commented Apr 23, 2019 •

edited

Loading

JohnAZoidberg commented Apr 24, 2019

aanderse commented Apr 25, 2019

fuzzy-id commented Apr 25, 2019

JohnAZoidberg commented Jun 13, 2019

aanderse commented Jun 14, 2019

ivan commented Jun 14, 2019

turion left a comment

turion Jun 16, 2019

JohnAZoidberg Jul 30, 2019

turion Jun 16, 2019

JohnAZoidberg Jul 30, 2019

Fix Cassandra, improve config and tests #59179

Fix Cassandra, improve config and tests #59179

Conversation

JohnAZoidberg commented Apr 8, 2019 • edited Loading

Motivation for this change

Things done

JohnAZoidberg commented Apr 9, 2019

jpyen commented Apr 10, 2019

JohnAZoidberg commented Apr 12, 2019 • edited Loading

JohnAZoidberg commented Apr 22, 2019

aanderse commented Apr 22, 2019 • edited Loading

aanderse commented Apr 22, 2019 • edited Loading

aanderse commented Apr 22, 2019

JohnAZoidberg commented Apr 22, 2019

JohnAZoidberg commented Apr 22, 2019

aanderse commented Apr 22, 2019

JohnAZoidberg commented Apr 23, 2019 • edited Loading

JohnAZoidberg commented Apr 24, 2019

aanderse commented Apr 25, 2019

fuzzy-id commented Apr 25, 2019

JohnAZoidberg commented Jun 13, 2019

aanderse commented Jun 14, 2019

ivan commented Jun 14, 2019

turion left a comment

Choose a reason for hiding this comment

turion Jun 16, 2019

Choose a reason for hiding this comment

JohnAZoidberg Jul 30, 2019

Choose a reason for hiding this comment

turion Jun 16, 2019

Choose a reason for hiding this comment

JohnAZoidberg Jul 30, 2019

Choose a reason for hiding this comment

JohnAZoidberg commented Apr 8, 2019 •

edited

Loading

JohnAZoidberg commented Apr 12, 2019 •

edited

Loading

aanderse commented Apr 22, 2019 •

edited

Loading

aanderse commented Apr 22, 2019 •

edited

Loading

JohnAZoidberg commented Apr 23, 2019 •

edited

Loading