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

Fixed service starting in the dep packages #413

Merged
merged 3 commits into from Oct 1, 2018

Conversation

Projects
None yet
3 participants
@soffokl
Copy link
Member

commented Sep 28, 2018

Closes #395

soffokl added some commits Sep 28, 2018

@soffokl soffokl self-assigned this Sep 28, 2018

@soffokl soffokl requested review from Waldz, zolia and vkuznecovas Sep 28, 2018

@soffokl soffokl requested a review from tadovas as a code owner Sep 28, 2018

```

Note: to run server, you will have to accept terms & conditions by adding '--agreed-terms-and-conditions' command line option.
Note 2: it's mandatory to run docker container with --net host to correctly detect VPN service ip which needs to be published to clients, assuming that host on which node is running has external interface with public ip
>**Note:** to run server, you will have to accept terms & conditions by adding '--agreed-terms-and-conditions' command line option.

This comment has been minimized.

Copy link
@Waldz

Waldz Sep 28, 2018

Member

We sync Node WIKI with same running guide. Update it also ;)

This comment has been minimized.

Copy link
@soffokl

soffokl Oct 1, 2018

Author Member

I was planning to do it when this PR will be merged after review.

sudo apt-get install -y --fix-broken
```

### Running service
```bash
sudo service mysterium-node start

This comment has been minimized.

Copy link
@Waldz

Waldz Sep 28, 2018

Member

service myst-node start or service myst start

This comment has been minimized.

Copy link
@Waldz

Waldz Sep 28, 2018

Member

Maybe we can have service aliases and support both names:

  • mysterium_node
  • myst -> mysterium_node

This comment has been minimized.

Copy link
@Waldz

Waldz Sep 28, 2018

Member

But I imagine future like this:

  • myst <- name of binary
  • mysterium_node service which starts myst service .. service
  • mysterium_client service which starts myst connect .. service

This comment has been minimized.

Copy link
@soffokl

soffokl Oct 1, 2018

Author Member
  1. I don't think that we need to add aliases, it will be just extra confusion for the users. We will need to have extra documentation to describe the reasons for aliases. I do not see any advantages having two names for the single service.

  2. In the current implementation, it works exactly the way you described. myst is a bynary, mysterium_node service that starts myst service ...

This comment has been minimized.

Copy link
@Waldz

Waldz Oct 1, 2018

Member

Cool

This comment has been minimized.

Copy link
@Waldz

Waldz Oct 1, 2018

Member

Created #420 for client service


```bash
apt-get update && apt-get install -y curl
curl -s https://swupdate.openvpn.net/repos/repo-public.gpg | apt-key add && echo "deb http://build.openvpn.net/debian/openvpn/stable xenial main" > /etc/apt/sources.list.d/openvpn-aptrepo.list && rm -rf /var/cache/apt/* /var/lib/apt/lists/*

This comment has been minimized.

Copy link
@Waldz

Waldz Sep 28, 2018

Member

Why to remove these instructions?

This comment has been minimized.

Copy link
@soffokl

soffokl Oct 1, 2018

Author Member

Removed section for client download and installation. Since there are no more dedicated client binary and installation process, only instruction for installing single binary left.

```


## Mysterium VPN node and client standalone binaries (.tar.gz)

This comment has been minimized.

Copy link
@Waldz

Waldz Sep 28, 2018

Member

Why to remove this?

This comment has been minimized.

Copy link
@soffokl

soffokl Oct 1, 2018

Author Member

Removed section for client download and installation. Since there are no more dedicated client binary and installation process, only instruction for installing single binary left.

# If you modify this, please make sure to also edit systemd.service

OS_DIR_BIN="/usr/bin"
OS_DIR_CONFIG="/etc/mysterium-node"

This comment has been minimized.

Copy link
@Waldz

Waldz Sep 28, 2018

Member

Where should be config|logs|data directories: /etc/myst or /etc/mysterium-node ?

This comment has been minimized.

Copy link
@soffokl

soffokl Oct 1, 2018

Author Member

Well, my idea was that this service is starting mysterium exit node, so all configs and logs should be in the mysterium-node directories and I left it as it was before.

--config-dir=$OS_DIR_CONFIG \
--data-dir=$OS_DIR_DATA \
--runtime-dir=$OS_DIR_RUN \
service \

This comment has been minimized.

Copy link
@Waldz

Waldz Sep 28, 2018

Member

Currently we have travis job name: "Ubuntu docker image for node" to test .deb installation on Ubuntu.
But it did not identified this error.
Do You have ideas how to do E2E test of Debian service?

This comment has been minimized.

Copy link
@soffokl

soffokl Oct 1, 2018

Author Member

This job just builds a docker image based on Ubuntu. It installs dep package, but in this case installation of dep package was not broken, that is why we didn't find this issue.
Docker uses it's own docker-entrypoint.sh for starting services. But broken was system service.
Not sure how to test it correctly with a systemd service.

This comment has been minimized.

Copy link
@soffokl

soffokl Oct 1, 2018

Author Member

Created issue for the future improvement: #417

@@ -55,7 +55,7 @@ func ensureOrCreateDir(dir string) error {
err := ensureDirExists(dir)
if os.IsNotExist(err) {
log.Info("[Directory config checker] ", "Directory: ", dir, " does not exit. Creating new one")
return os.MkdirAll(dir, 0600)
return os.MkdirAll(dir, 0700)

This comment has been minimized.

Copy link
@Waldz

Waldz Sep 28, 2018

Member

Was is a debugged place??

This comment has been minimized.

Copy link
@soffokl

soffokl Oct 1, 2018

Author Member

Yeah. Actually, every directory should have x attribute to be workable.

@Waldz

Waldz approved these changes Oct 1, 2018

@soffokl soffokl merged commit a74a539 into master Oct 1, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@soffokl soffokl deleted the deb-packages-fix branch Oct 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.