Testnet: refactor testnet script and Dockerfiles #925

coneiric · 2018-06-27T21:26:10Z

By submitting this pull-request, I confirm the following:

I have read and understood the developer guide in kovri-docs.
I have checked that another pull-request for this purpose does not exist.
I have considered and confirmed that this submission will be valuable to others.
I accept that this submission may not be used and that this pull-request may be closed by the will of the maintainer.
I give this submission freely under the BSD 3-clause license.

Number of small refactors to make creating and using the testnet easier:

Fixes issues with user permissions between Docker host and containers
Enables dynamic builds for testnets based on the Arch Dockerfile
Fixes Python syntax bug if Docker host is running Python3
Enables monitoring by default
Detects presence of monitoring containers

anonimal

In commit 0d42c37

Arch doesn't support building OpenSSL statically (needed for Crypto++).

It's only for cpp-netlib. crypto++ is not reliant on openssl. Can you fix the message in that commit?

anonimal · 2018-06-29T05:59:18Z

contrib/testnet/Dockerfiles/Dockerfile.arch

@@ -28,12 +28,16 @@

 FROM greyltc/archlinux:latest

+ARG userid
+
+ENV userid=$userid


Build Web Docker image? [httpd:2.4] [Y/n] y Sending build context to Docker daemon 741MB Step 1/1 : FROM httpd:2.4 ---> fb2f3851a971 [Warning] One or more build-args [userid] were not consumed

I'm imagining this is expected because the apache dockerfile wasn't updated (and it doesn't need to be)?

No, I missed this when refactoring. Will remove the build-arg from the web Docker image.

anonimal · 2018-06-29T06:04:35Z

I don't think this PR was intended to fix the grafana portion of the testnet, but just FYI: I'm greeted with this screen after logging in to 127.0.0.1:3030 (i.e., it's still not working).

anonimal · 2018-06-29T06:05:09Z

Or am I assuming that the plugins were to be installed via docker?

coneiric · 2018-06-29T19:05:17Z

It's only for cpp-netlib. crypto++ is not reliant on openssl. Can you fix the message in that commit?

Absolutely, the compiler error message made me think it was for crypto++. Will update the message.

I don't think this PR was intended to fix the grafana portion of the testnet

No, it wasn't, just to make actually building the testnet easier. I can keep working on the Grafana in another PR.

Or am I assuming that the plugins were to be installed via docker?

If their API supports installing plugins, we can do it during Docker setup. Otherwise, maybe we just include instructions for manually installing the plugins?

anonimal · 2018-06-29T20:19:12Z

Otherwise, maybe we just include instructions for manually installing the plugins?

As automated as possible.

MoroccanMalinois · 2018-06-29T21:59:25Z

(i.e., it's still not working)
Or am I assuming that the plugins were to be installed via docker?

I thinks it's working (had same screen after login), it's not required to add more users or apps&plugins. If you click on "Home", you should see a list of dashboards that you can click on (hopefully 😅 )

coneiric · 2018-06-29T22:46:49Z

If you click on "Home", you should see a list of dashboards that you can click on

Can confirm, it's working.

I added a commit to install the InfluxDB admin panel, which you can add to any dashboard. It lets you view and kill InfluxDB queries for that dashboard. If this isn't useful/wanted, I can drop the commit.

anonimal · 2018-06-30T00:54:51Z

Can confirm, it's working.

Ok, but the actual monitoring isn't working for netdb.

anonimal · 2018-06-30T04:06:42Z

Does anyone want to try fixing this now or should we merge?

MoroccanMalinois · 2018-06-30T11:52:35Z

i'll take a look tonight

coneiric · 2018-07-01T20:09:23Z

I can look as well.

MoroccanMalinois

Nice changes 👍

Since this enables monitoring by default, it should add
"--i2pcontroladdress 0.0.0.0 --i2pcontrolport 7650" to "KOVRI_BIN_ARGS" and "KOVRI_FW_BIN_ARGS"

Also (unrelated to this PR), if you don't mind adding this:

--- a/contrib/testnet/monitoring/collect.sh
+++ b/contrib/testnet/monitoring/collect.sh
@@ -37,7 +37,7 @@ docker_base_name=$5
 while true; do
    sleep 15
    data=""
 -  for _seq in $($sequence); do
 +  for _seq in $(eval $sequence); do
       _host="${network_octets}.$((10#${_seq}))"

There is one last error (unrelated to this PR), don't know yet how to solve it : after the changes to boost log sinks management, variable "stats" in contrib/testnet/monitoring/collect.sh is not populated. Will take a look but if someone has an idea 😄

MoroccanMalinois · 2018-07-01T00:55:36Z

contrib/testnet/testnet.sh

@@ -271,7 +272,14 @@ set_bins()
    mount_repo_bins="-v ${KOVRI_REPO}/build/kovri:/usr/bin/kovri \
      -v ${KOVRI_REPO}/build/kovri-util:/usr/bin/kovri-util"

-    read_bool_input "Build repo binaries from within the container?" KOVRI_BUILD_REPO_BINS "Exec make release-static"
+    build_cmd="make clean"


make clean

make clean util

I'm not sure this is necessary. make release and make release-static include rules for making util, but I can add this in case those recipes change.

coneiric · 2018-07-01T21:54:59Z

Since this enables monitoring by default, it should add
"--i2pcontroladdress 0.0.0.0 --i2pcontrolport 7650" to "KOVRI_BIN_ARGS" and "KOVRI_FW_BIN_ARGS"

Will do, maybe this is what's causing kovri-util control stats to fail as well. Will also add your recommendations for the loop in collect.sh.

coneiric · 2018-07-01T22:29:20Z

I added your recommended changes @MoroccanMalinois, and will see if that solves the problem with $stats not being populated in collect.sh. If not, I will keep looking for a solution.

Thank you @anonimal and @MoroccanMalinois for your help on this PR.

I will rebase against master, and add the PR reference, once #935 is merged.

MoroccanMalinois · 2018-07-01T23:27:34Z

Thank YOU @coneiric (not just for this PR 😄 )

Got this last work-around

--- a/contrib/testnet/monitoring/collect.sh
+++ b/contrib/testnet/monitoring/collect.sh
@@ -43,11 +43,12 @@ while true; do
     IFS=$'\n'
 
     # Get statistics from kovri instances
-    stats=$(/usr/bin/kovri-util control stats --host $_host --disable-console-log)
+    stats=$(/usr/bin/kovri-util --disable-file-log --disable-color-log --enable-auto-flush-log control stats --host $_host 2>&1)

And btw, it can take ~2min after testnets.sh start to see data in graphs

coneiric · 2018-07-02T00:28:55Z

Got this last work-around

And with this, it looks like everything is working.

I removed util from make clean util from testnet creation, since it caused binaries to be rebuilt with the final build command being make clean util release or make clean util release-static.

If you had another reason for including util in the build command, I'll add it back.

anonimal

Thanks for working on this you two. @coneiric can you please reference this PR number (and optionally/preferably give credit to @MoroccanMalinois for the applicable changes)?

coneiric · 2018-07-02T20:35:05Z

Thanks for working on this you two. @coneiric can you please reference this PR number (and optionally/preferably give credit to @MoroccanMalinois for the applicable changes)?

Absolutely, will do both, updating now.

Ensure container user has same UID as Docker host user. Referencing monero-project#925

Arch doesn't support building OpenSSL statically (needed for cpp-netlib). Credit to anonimal for correction about OpenSSL dependency. Referencing monero-project#925

Print with parentheses is valid for Python2/3, without parentheses is broken in Python3. Referencing monero-project#925

Enable graphical monitoring by default, and add I2PControl to testnet instances. Credit to MoroccanMalinois for enabling I2PControl. Referencing monero-project#925

Helper function to detect if monitoring containers exist. Referencing monero-project#925

Install panel to view and kill currently running queries. Referencing monero-project#925

Credit to MoroccanMalinois. Referencing monero-project#925

Disable file logs, color console logging, and redirect stderr to stdout. Credit to MoroccanMalinois. Referencing monero-project#925

MoroccanMalinois

If you had another reason for including util in the build command, I'll add it back.

Nah sorry, must have been a pb with my local copy

fb4ac97 Testnet: workaround for stats collector logs (oneiric) 11979f3 Testnet: add eval for sequence bash variable (oneiric) 771c141 Testnet: install InfluxDB admin panel (oneiric) 62704d9 Testnet: detect monitoring containers (oneiric) 76fa304 Testnet: enable monitoring by default (oneiric) e71f276 Testnet: use print with parens for Grafana API key (oneiric) 2125eea Testnet: support Arch container dynamic builds (oneiric) 26ce47b Testnet: set container uid to Docker host user (oneiric)

anonimal suggested changes Jun 29, 2018

View reviewed changes

coneiric force-pushed the testnet-ref branch from 8c4a9c9 to c1fdc8e Compare June 29, 2018 21:23

coneiric force-pushed the testnet-ref branch from c1fdc8e to bd64396 Compare June 29, 2018 22:43

MoroccanMalinois reviewed Jul 1, 2018

View reviewed changes

coneiric force-pushed the testnet-ref branch 2 times, most recently from 5c6d09e to e9774ee Compare July 1, 2018 22:26

coneiric force-pushed the testnet-ref branch from e465ae0 to 0f3c33c Compare July 2, 2018 00:23

anonimal suggested changes Jul 2, 2018

View reviewed changes

coneiric added 7 commits July 2, 2018 20:36

Testnet: set container uid to Docker host user

26ce47b

Ensure container user has same UID as Docker host user. Referencing monero-project#925

Testnet: support Arch container dynamic builds

2125eea

Arch doesn't support building OpenSSL statically (needed for cpp-netlib). Credit to anonimal for correction about OpenSSL dependency. Referencing monero-project#925

Testnet: use print with parens for Grafana API key

e71f276

Print with parentheses is valid for Python2/3, without parentheses is broken in Python3. Referencing monero-project#925

Testnet: enable monitoring by default

76fa304

Enable graphical monitoring by default, and add I2PControl to testnet instances. Credit to MoroccanMalinois for enabling I2PControl. Referencing monero-project#925

Testnet: detect monitoring containers

62704d9

Helper function to detect if monitoring containers exist. Referencing monero-project#925

Testnet: install InfluxDB admin panel

771c141

Install panel to view and kill currently running queries. Referencing monero-project#925

Testnet: add eval for sequence bash variable

11979f3

Credit to MoroccanMalinois. Referencing monero-project#925

Testnet: workaround for stats collector logs

fb4ac97

Disable file logs, color console logging, and redirect stderr to stdout. Credit to MoroccanMalinois. Referencing monero-project#925

coneiric force-pushed the testnet-ref branch from 0f3c33c to fb4ac97 Compare July 2, 2018 20:41

MoroccanMalinois approved these changes Jul 2, 2018

View reviewed changes

anonimal approved these changes Jul 2, 2018

View reviewed changes

anonimal merged commit fb4ac97 into monero-project:master Jul 2, 2018

coneiric deleted the testnet-ref branch July 13, 2018 03:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Testnet: refactor testnet script and Dockerfiles #925

Testnet: refactor testnet script and Dockerfiles #925

coneiric commented Jun 27, 2018

anonimal left a comment

anonimal Jun 29, 2018

coneiric Jun 29, 2018

anonimal commented Jun 29, 2018

anonimal commented Jun 29, 2018

coneiric commented Jun 29, 2018

anonimal commented Jun 29, 2018

MoroccanMalinois commented Jun 29, 2018

coneiric commented Jun 29, 2018 •

edited

anonimal commented Jun 30, 2018

anonimal commented Jun 30, 2018

MoroccanMalinois commented Jun 30, 2018

coneiric commented Jul 1, 2018

MoroccanMalinois left a comment

MoroccanMalinois Jul 1, 2018

coneiric Jul 1, 2018

coneiric commented Jul 1, 2018

coneiric commented Jul 1, 2018 •

edited

MoroccanMalinois commented Jul 1, 2018

coneiric commented Jul 2, 2018

anonimal left a comment

coneiric commented Jul 2, 2018

MoroccanMalinois left a comment

Testnet: refactor testnet script and Dockerfiles #925

Testnet: refactor testnet script and Dockerfiles #925

Conversation

coneiric commented Jun 27, 2018

anonimal left a comment

Choose a reason for hiding this comment

anonimal Jun 29, 2018

Choose a reason for hiding this comment

coneiric Jun 29, 2018

Choose a reason for hiding this comment

anonimal commented Jun 29, 2018

anonimal commented Jun 29, 2018

coneiric commented Jun 29, 2018

anonimal commented Jun 29, 2018

MoroccanMalinois commented Jun 29, 2018

coneiric commented Jun 29, 2018 • edited

anonimal commented Jun 30, 2018

anonimal commented Jun 30, 2018

MoroccanMalinois commented Jun 30, 2018

coneiric commented Jul 1, 2018

MoroccanMalinois left a comment

Choose a reason for hiding this comment

MoroccanMalinois Jul 1, 2018

Choose a reason for hiding this comment

coneiric Jul 1, 2018

Choose a reason for hiding this comment

coneiric commented Jul 1, 2018

coneiric commented Jul 1, 2018 • edited

MoroccanMalinois commented Jul 1, 2018

coneiric commented Jul 2, 2018

anonimal left a comment

Choose a reason for hiding this comment

coneiric commented Jul 2, 2018

MoroccanMalinois left a comment

Choose a reason for hiding this comment

coneiric commented Jun 29, 2018 •

edited

coneiric commented Jul 1, 2018 •

edited