Renaming binary + docker #223

crystalin · 2021-02-02T02:12:04Z

What does it do?

Rename the main binary into "moonbeam" and also provides a public docker image associated with that binary.

What important points reviewers should know?

We need to keep the previous docker image for compatibility with ops for now.

Is there something left for follow-up PRs?

The stagenet PR will need to be merged after also to keep providing the alphanet/stagenet configuration

What value does it bring to the blockchain users?

Having a better naming will avoid confusion for the users.
The new docker image will also make it easier for users to connect to one of our network.

Checklist

Does it require a purge of the network?
You bumped the runtime version if there are breaking changes in the runtime ?
Does it require changes in documentation/tutorials ?

crystalin · 2021-02-02T02:14:13Z

runtime/src/lib.rs

@@ -106,8 +106,8 @@ pub mod opaque {

 /// This runtime version.
 pub const VERSION: RuntimeVersion = RuntimeVersion {
-	spec_name: create_runtime_str!("moonbase-alphanet"),
-	impl_name: create_runtime_str!("moonbase-alphanet"),
+	spec_name: create_runtime_str!("moonbeam"),


@JoshOrndorff
Will that impact the network (preventing to upgrade or ...) ? I'm not sure if the spec_name and impl_name are actually involved or just for description)..

Good question. I don't know. I think we should make this change either way though. Then test it out. If it rules out runtime upgrades, maybe we just purge again?

JoshOrndorff

I'm a big fan of changing the binary name to moonbeam and cleaning the dockerfile. I left a little feedback below.

My biggest question is why we have two dockerfiles now.

For reference, here is Polkadot's dockerfile https://github.com/paritytech/polkadot/blob/master/docker/Dockerfile

JoshOrndorff · 2021-02-02T15:28:24Z

.github/workflows/release.yml

      - name: Prepare
        id: prep
        run: |
-          DOCKER_IMAGE=purestake/moonbase-parachain-testnet
+          DOCKER_IMAGE=purestake/moonbase-parachain


Suggested change

DOCKER_IMAGE=purestake/moonbase-parachain

DOCKER_IMAGE=purestake/moonbeam

Kind of makes sense for the image name to match the binary name.

This is the internal docker image. The public one is the next build step

docker/moonbase-parachain.Dockerfile

JoshOrndorff · 2021-02-02T15:36:26Z

docker/moonbase-parachain.Dockerfile

@@ -4,24 +4,24 @@

 FROM phusion/baseimage:0.11
 LABEL maintainer "[email protected]"
-LABEL description="this is the parachain node running Moonbase Alphanet"
+LABEL description="this is the parachain node running Moonbase"
 ARG PROFILE=release

 RUN mv /usr/share/ca* /tmp && \
 	rm -rf /usr/share/*  && \


Can we just start with a slimmer base image?

What do you suggest ?

I checked and it adds 20MB overhead to the binary. (total 140MB)
Not sure we can do a lot better

I've heard people having success with Alpine. https://hub.docker.com/_/alpine/ It claims to be only 5MB.

I'm not really saying that our image is too big as it is. I just hadn't seen anyone manually removing stuff that comes in the base image before.

JoshOrndorff · 2021-02-02T15:37:13Z

docker/moonbase-parachain.Dockerfile

+	useradd -m -u 1000 -U -s /bin/sh -d /moonbase-parachain moonbeam && \
+	mkdir -p /moonbase-parachain/.local/share/moonbase-parachain && \
+	chown -R moonbeam:moonbeam /moonbase-parachain && \
+	ln -s /moonbase-parachain/.local/share/moonbase-parachain /data && \


Why is the directory still called moonbase-parachain? Why not just call it moonbeam or data.

This docker image is for ops, I kept it as close as our current process

JoshOrndorff · 2021-02-02T15:38:17Z

docker/moonbeam.Dockerfile

@@ -0,0 +1,32 @@
+# Node for Moonbase Alphanet.


Why is there a second dockerfile now?

This is the public docker image file.
The other one (the previous one) is for internal use only

Also I need to change the desc here

JoshOrndorff · 2021-02-02T15:39:34Z

runtime/src/lib.rs

@@ -106,8 +106,8 @@ pub mod opaque {

 /// This runtime version.
 pub const VERSION: RuntimeVersion = RuntimeVersion {
-	spec_name: create_runtime_str!("moonbase-alphanet"),
-	impl_name: create_runtime_str!("moonbase-alphanet"),
+	spec_name: create_runtime_str!("moonbeam"),


Good question. I don't know. I think we should make this change either way though. Then test it out. If it rules out runtime upgrades, maybe we just purge again?

tests/package.json

Co-authored-by: Joshy Orndorff <[email protected]>

crystalin · 2021-02-03T19:00:16Z

@purestakeoskar and @Mitchell-Grimes
So with this PR we are going to have 2 docker images for moonbeam. One is the public (which only contains the binary) and the other one is the internal one (contains the raw specs without the bootnodes).
Should we push the internal one into gcr instead of exposing it to the public ?

crystalin · 2021-02-03T19:03:15Z

@albertov19 this PR will change how the documentation should reference moonbeam through docker.
there will be a new public image: purestake/moonbeam which will have moonbeam as an entrypoint.
Calling docker was done like this before:

 docker run  purestake/moonbase-parachain-testnet /moonbase-alphanet/moonbase-alphanet --chain alphanet

With this new image it will be:

 docker run  purestake/moonbeam --chain alphanet

Crystalin added 2 commits February 2, 2021 02:09

Initial draft for renaming binary + docker

410bc49

format rust code

54da93c

crystalin commented Feb 2, 2021

View reviewed changes

Crystalin added 5 commits February 2, 2021 03:19

Faster tests

75e3e36

Better fix for ts test speed

9d4e7cb

Fix parachain docker image

110cc27

fix parachain docker name

7780fc0

Better entrypoint format in docker

41f18f1

JoshOrndorff reviewed Feb 2, 2021

View reviewed changes

crystalin and others added 3 commits February 3, 2021 12:42

Update docker/moonbase-parachain.Dockerfile

f29d84d

Co-authored-by: Joshy Orndorff <[email protected]>

Merge branch 'master' into crystalin-naming-moonbeam

c49af1e

revert test filename check

4d744eb

crystalin marked this pull request as ready for review February 3, 2021 19:00

crystalin changed the title ~~Initial draft for renaming binary + docker~~ Renaming binary + docker Feb 3, 2021

Crystalin added 3 commits February 9, 2021 16:16

Merge branch 'master' into crystalin-naming-moonbeam

ffb8c48

Testing smaller docker image

69f9590

Merge branch 'master' into crystalin-naming-moonbeam

8c5cb8f

crystalin merged commit 4253817 into master Feb 9, 2021

crystalin deleted the crystalin-naming-moonbeam branch February 9, 2021 23:16

JoshOrndorff mentioned this pull request Feb 11, 2021

update cargo.lock #242

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Renaming binary + docker #223

Renaming binary + docker #223

crystalin commented Feb 2, 2021

crystalin Feb 2, 2021

JoshOrndorff Feb 2, 2021

JoshOrndorff left a comment •

edited

Loading

JoshOrndorff Feb 2, 2021

crystalin Feb 3, 2021

JoshOrndorff Feb 2, 2021

crystalin Feb 3, 2021

crystalin Feb 3, 2021

JoshOrndorff Feb 3, 2021

JoshOrndorff Feb 2, 2021

crystalin Feb 3, 2021

JoshOrndorff Feb 2, 2021

crystalin Feb 3, 2021

crystalin Feb 3, 2021

JoshOrndorff Feb 2, 2021

crystalin commented Feb 3, 2021

crystalin commented Feb 3, 2021

	DOCKER_IMAGE=purestake/moonbase-parachain
	DOCKER_IMAGE=purestake/moonbeam

Renaming binary + docker #223

Renaming binary + docker #223

Conversation

crystalin commented Feb 2, 2021

What does it do?

What important points reviewers should know?

Is there something left for follow-up PRs?

What value does it bring to the blockchain users?

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoshOrndorff left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crystalin commented Feb 3, 2021

crystalin commented Feb 3, 2021

JoshOrndorff left a comment •

edited

Loading