search

junquera / ntopng-docker

Docker for running ntopng (https://github.com/ntop/ntopng)
https://hub.docker.com/r/junquera/ntopng/
GNU General Public License v3.0
10 stars 6 forks source link

add ntopng.conf ? #2

Closed ShagoY closed 5 years ago

ShagoY commented 5 years ago

Hi @junquera :)

First of all, thank you for this ntopng docker version!

I would like to add the file ntopng.conf "out of the box" without making any modifications (build, etc...), is it possible?

Regards.

ShagoY commented 5 years ago

Like https://github.com/junquera/ntopng-docker/commit/54f19b653b70759965c5feefbbdc8b8828e5a2bf

junquera commented 5 years ago

Of course! Sincerely I have ever used any "aditional option", and I never would have thought it.

Thank you!

junquera commented 5 years ago

Would you like to make the pull request?

ShagoY commented 5 years ago

docker-compose.yml : 

version: '3.2'
services:  
    ntopng:
        image: junquera/ntopng
        container_name: ntopng
        restart: unless-stopped
        network_mode: host
        volumes:
        - ./ntopng.conf:/ntop/ntopng.conf:ro
        environment:
        - CONFIG=/ntop/ntopng.conf

Or docker run --net=host -t -p 3000:3000 -e CONFIG=/ntop/ntopng.conf -v ./ntopng.conf:/ntop/ntopng.conf:ro junquera/ntopng

ShagoY commented 5 years ago

ntopng.conf https://www.ntop.org/guides/ntopng/how_to_start/configuration_file.html# And https://www.ntop.org/guides/ntopng/cli_options.html

junquera commented 5 years ago

Hi @ShagoY! I don't really understand what have you done, there are a lot of changes (some in the issue and some others in the pull request), but I've detected that you have worked based in an old commit. Please, pull the latest master changes and integrate your changes there. Then pull request again.

Personally I prefer this way in your issue comments:

# start.sh
 cd /ntop/ntopng && ./ntopng "$@"

than this other in your PR:

 cd /ntop/ntopng && ./ntopng $CONFIG

If you have any doubt or you prefer me to do it, feel free of tell me, but I would like to see this commits with your name: it's your work and your idea!

ShagoY commented 5 years ago

I don't know why I was on an old commit.... btw i made the Pull request again and it's ok with docker cloud.

"$@" uses the tag "cmd" (cmd line and docker-compose) but if you do a "ps aux", you will notice that the cmd is also added to start.sh script.

However, if you use the environment variables ($CONFIG), they are only added to ./ntopng

So it's a design choice, I guess....

junquera commented 5 years ago

Perfect! Do you wanna add the docker-compose file to the PR or is it ok?

ShagoY commented 5 years ago

I have no opinion on that.

I think adding the possibility to use this variable directly in the "readme" may be enough.

In addition, the variable can be used in different ways:

docker run --net=host -e CONFIG=/ntop/ntopng.conf -v./ntopng.conf:/ntop/ntopng.conf:/ntop/ntopng.conf:ro junquera/ntopng

Or

docker run --net=host -e CONFIG="-i=eth1 -w=3000 -n=1" junquera/ntopng

junquera commented 5 years ago

I'll add it then with the two links you told. I think it could be usefull for many people.

Tonight (in Spain, in about five hours) I merge your code! :wink:

ShagoY commented 5 years ago

Excellent! I am a neighbour from France ;-)

junquera commented 5 years ago

Done! Thank you very much! Merci! :smile: