Testnet: add new container for build #703

MoroccanMalinois · 2017-09-06T21:26:41Z

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

I have read and understood the contributor 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.

To avoid rebuilding the docker image for each git revision, the user can now choose to "Use binaries from repo" during testnet creation.

To rebuild the binaries:

testnet.sh exec make

Or get a shell first:

testnet.sh exec sh

and then

 make

MoroccanMalinois · 2017-09-06T21:42:30Z

In addition, in the new dockerfile, maybe we could also add gdb (i took the liberty to add iptables as my next PR will require it).

Just an fyi, whatever the size of the image, it doesn't change the size of instances (as it uses AUFS EDIT: or OverlayFS now ), so the impact of the docker image size on testnet is quite limited.

anonimal

Referencing our #kovri-dev chat for this feature. Thank you @MoroccanMalinois.

On a related sidenote, could you please add the license to the testnet file for this PR (I should've added it to #702)?

anonimal · 2017-09-06T21:42:30Z

contrib/docker/testnet.sh

+     esac
+  fi
+
+  mount_repo_bins=""


mount_repo_bins=""

I don't think this is necessary. Did you find this necessary?

anonimal · 2017-09-06T21:50:38Z

contrib/docker/testnet.sh

+  mount_repo_bins=""
+  if [[ "$KOVRI_USE_REPO_BINS" = true ]]; then
+    mount_repo_bins="-v ${KOVRI_REPO}/build/kovri:/usr/bin/kovri -v ${KOVRI_REPO}/build/kovri-util:/usr/bin/kovri-util"
+


If we're going to start bool'ing, how about we create bool vars for building? This [bool'ing] can then be applied elsewhere so we never have to enter keyboard input once they are all set.

What do you think? If we apply elsewhere, I think it should be in a separate commit.

anonimal · 2017-09-06T21:53:40Z

contrib/docker/testnet.sh

+    -w /home/kovri/kovri \
+    $KOVRI_IMAGE \
+    $@
+  catch "Failed to run a shell"


"Failed to run a shell"

But this could be a failed docker run too?

anonimal · 2017-09-06T22:03:41Z

Hi @MoroccanMalinois, I just saw these:

In addition, in the new dockerfile, maybe we could also add gdb (i took the liberty to add iptables as my next PR will require it).

Ok, sounds good. Since it's dev-only, let's do what we need.

Just an fyi, whatever the size of the image, it doesn't change the size of instances (as it uses AUFS EDIT: or OverlayFS now ), so the impact of the docker image size on testnet is quite limited.

I'm more concerned about how we manage logging. Even without debug levels, there's a lot of writing. I have some ideas on potential resolutions, will talk more in #kovri-dev.

…build

MoroccanMalinois · 2017-09-07T04:36:04Z

Thanks @anonimal. Ready for review

anonimal · 2017-09-07T20:54:19Z

contrib/docker/testnet.sh

@@ -101,7 +140,16 @@ set_image()
    KOVRI_IMAGE=${_default_image}
  fi

-  read_input "Build Kovri Docker image? [$KOVRI_IMAGE]" NULL "docker build -t $KOVRI_IMAGE $KOVRI_REPO"
+  # Select Dockerfile
+  local _default_dockerfile="Dockerfile"


May as well change to Dockerfile_dev since this is testnet. Yes/no?

anonimal · 2017-09-07T22:14:55Z

contrib/docker/testnet.sh

+    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?" KOVRI_BUILD_REPO_BINS "Exec make"


Re: our debugging in #kovri-dev, any objection to making this a release-static build?

anonimal · 2017-09-07T22:17:31Z

contrib/docker/testnet.sh

@@ -74,6 +100,19 @@ set_repo()
  fi
 }

+set_bins()
+{
+  read_bool_input "Use binaries from repo?" KOVRI_USE_REPO_BINS ""


Can you add a note to the question (or somewhere) about ensuring that the binaries are built statically if not built within the container?

…build

anonimal

Thanks @MoroccanMalinois

446d43d Testnet: (#703) use boolean vars for cleanup and image build (MoroccanMalinois) 2e9341b Testnet: (#703) add container for rebuild (MoroccanMalinois) b360ec5 Testnet: (#703) add license (MoroccanMalinois)

anonimal suggested changes Sep 6, 2017

View reviewed changes

Testnet: (monero-project#703) add license

b360ec5

MoroccanMalinois force-pushed the dock_builder branch from ce66310 to 5ccea95 Compare September 7, 2017 02:51

MoroccanMalinois added a commit to MoroccanMalinois/kovri that referenced this pull request Sep 7, 2017

Testnet: (monero-project#703) add container for rebuild

adca3ad

MoroccanMalinois added a commit to MoroccanMalinois/kovri that referenced this pull request Sep 7, 2017

Testnet: (monero-project#703) use boolean vars for cleanup and image …

5ccea95

…build

MoroccanMalinois force-pushed the dock_builder branch from 5ccea95 to 2e591c4 Compare September 7, 2017 02:55

MoroccanMalinois added a commit to MoroccanMalinois/kovri that referenced this pull request Sep 7, 2017

Testnet: (monero-project#703) add container for rebuild

47db2c1

MoroccanMalinois added a commit to MoroccanMalinois/kovri that referenced this pull request Sep 7, 2017

Testnet: (monero-project#703) use boolean vars for cleanup and image …

2e591c4

…build

anonimal suggested changes Sep 7, 2017

View reviewed changes

MoroccanMalinois force-pushed the dock_builder branch from 2e591c4 to 5e2dd2f Compare September 7, 2017 23:07

MoroccanMalinois added a commit to MoroccanMalinois/kovri that referenced this pull request Sep 7, 2017

Testnet: (monero-project#703) add container for rebuild

84667d8

MoroccanMalinois added a commit to MoroccanMalinois/kovri that referenced this pull request Sep 7, 2017

Testnet: (monero-project#703) use boolean vars for cleanup and image …

5e2dd2f

…build

MoroccanMalinois force-pushed the dock_builder branch from 5e2dd2f to 91a8ac5 Compare September 8, 2017 00:11

MoroccanMalinois added a commit to MoroccanMalinois/kovri that referenced this pull request Sep 8, 2017

Testnet: (monero-project#703) add container for rebuild

da4b25f

MoroccanMalinois added a commit to MoroccanMalinois/kovri that referenced this pull request Sep 8, 2017

Testnet: (monero-project#703) use boolean vars for cleanup and image …

91a8ac5

…build

MoroccanMalinois added 2 commits September 8, 2017 00:13

Testnet: (monero-project#703) add container for rebuild

2e9341b

Testnet: (monero-project#703) use boolean vars for cleanup and image …

446d43d

…build

MoroccanMalinois force-pushed the dock_builder branch from 91a8ac5 to 446d43d Compare September 8, 2017 00:14

monero-project deleted a comment from MoroccanMalinois Sep 8, 2017

anonimal approved these changes Sep 8, 2017

View reviewed changes

anonimal merged commit 446d43d into monero-project:master Sep 8, 2017

MoroccanMalinois deleted the dock_builder branch September 8, 2017 01:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Testnet: add new container for build #703

Testnet: add new container for build #703

MoroccanMalinois commented Sep 6, 2017

MoroccanMalinois commented Sep 6, 2017 •

edited

Loading

anonimal left a comment

anonimal Sep 6, 2017

anonimal Sep 6, 2017

anonimal Sep 6, 2017

anonimal commented Sep 6, 2017

MoroccanMalinois commented Sep 7, 2017

anonimal Sep 7, 2017

anonimal Sep 7, 2017

anonimal Sep 7, 2017

anonimal left a comment

Testnet: add new container for build #703

Testnet: add new container for build #703

Conversation

MoroccanMalinois commented Sep 6, 2017

MoroccanMalinois commented Sep 6, 2017 • edited Loading

anonimal left a comment

Choose a reason for hiding this comment

anonimal Sep 6, 2017

Choose a reason for hiding this comment

anonimal Sep 6, 2017

Choose a reason for hiding this comment

anonimal Sep 6, 2017

Choose a reason for hiding this comment

anonimal commented Sep 6, 2017

MoroccanMalinois commented Sep 7, 2017

anonimal Sep 7, 2017

Choose a reason for hiding this comment

anonimal Sep 7, 2017

Choose a reason for hiding this comment

anonimal Sep 7, 2017

Choose a reason for hiding this comment

anonimal left a comment

Choose a reason for hiding this comment

MoroccanMalinois commented Sep 6, 2017 •

edited

Loading