Versionable Properties #310

donkers · 2015-08-29T09:07:44Z

This will allow some server properties to be stored in the version files. This is for feature request #309, more detail on implementation can be found in the issue discussion.

To test the new code I did the following on a Debian Jessie install, and a CentOS 7 install.

Started, stopped, and opened the console of, server instances running versions; 1.2.5, 1.6.4, 1.7.10, and 1.8.8.

Ran the commands below to confirmed that all variables/setting get dealt with and set correctly. When performing this test the only value that differed between the two branches was LOG_PATH, as expected.

#Ensure the UPDATE_URL is set to the master branch
msm update
msm config > master.txt
msm 1-2-5 config >> master.txt
msm 1-7-10 config >> master.txt
msm 1-8-8 config >> master.txt

#Ensure the UPDATE_URL is set to the versionable_properties branch
msm update
msm config > versionable_properties.txt
msm 1-2-5 config >> versionable_properties.txt
msm 1-7-10 config >> versionable_properties.txt
msm 1-8-8 config >> versionable_properties.txt

diff -u master.txt versionable_properties.txt

I also snuck in commit 66c6097 to addresses issue #208. msm [SERVER] console now works when run as root as well as the owner of the server process (defaults to minecraft).

…cases.

…rty() function.

…as CONSOLE_* variables.

… to the server_set_property() function.

…r_conf().

… files. This allows overrides of version properties in the server.properties.

…case statement and check if path is already absolute.

…mmand_server_console() function.

renderorange · 2015-08-30T15:29:40Z

Hey donkers, this pull request seems well covered from the outset. I'm going to run through some testing with a full installation and process, per normal :)
I'll let you know if I find anything.

renderorange · 2015-08-31T02:56:24Z

One issue I found, the msm help command is returning the versioning banner typically displayed when starting the server. This is new behavior.

/opt/msm/servers # msm help
[MSM Info: Assuming 'minecraft/1.7.0' for this server. You should override this value by adding 'msm-version=minecraft/x.x.x' to '/opt/msm/servers/legacy/server.properties' to make this message go away]
Usage: /usr/local/bin/msm command:
(...)

this is happening in server_property, https://github.com/donkers/msm/blob/master/init/msm#L1778

adding a few debug blocks to the server_property function

/opt/msm/servers # msm help
-
# setting VERSION_CONF #
# server_property 0 VERSION #
# get_closest_version minecraft/1.7.0
# version == minecraft/1.7.0
-
# setting VERSION_CONF #
# server_property 1 VERSION #
# get_closest_version unknown
# version == unknown
[MSM Info: Assuming 'minecraft/1.7.0' for this server. You should override this value by adding 'msm-version=minecraft/x.x.x' to '/opt/msm/servers/legacy/server.properties' to make this message go away]
Usage: /usr/local/bin/msm command:
(...)

it would appear msm is setting the version to unknown if the version isn't set in server.properties for a server, which is triggering the message in the if block following. The above scenario, I have 2 servers set up, with one hard-coded to 1.7 in server.properties, one without a setting in the file.

/opt/msm/servers # ll
total 8.0K
drwxrwxr-x 3 minecraft minecraft 4.0K Aug 30 15:43 legacy
drwxrwxr-x 4 minecraft minecraft 4.0K Aug 30 15:36 stable
/opt/msm/servers # grep 'msm-version' */server.properties
stable/server.properties:msm-version=minecraft/1.7.0

if I remove the version setting in stable, the banner will trigger twice

/opt/msm/servers # grep 'msm-version' */server.properties
/opt/msm/servers # msm help
-
# setting VERSION_CONF #
# server_property 0 VERSION #
# get_closest_version unknown
# version == unknown
[MSM Info: Assuming 'minecraft/1.7.0' for this server. You should override this value by adding 'msm-version=minecraft/x.x.x' to '/opt/msm/servers/stable/server.properties' to make this message go away]
-
# setting VERSION_CONF #
# server_property 1 VERSION #
# get_closest_version unknown
# version == unknown
[MSM Info: Assuming 'minecraft/1.7.0' for this server. You should override this value by adding 'msm-version=minecraft/x.x.x' to '/opt/msm/servers/legacy/server.properties' to make this message go away]
Usage: /usr/local/bin/msm command:
(...)

Given you create a new server, and don't set the msm-version in server.properties, when 'msm help' is run, then it should only display the help menu.

I hope my explanation is clear and makes sense.

renderorange · 2015-08-31T03:20:19Z

of note, this behavior is also happening for msm update

/opt/msm/servers # msm update
[MSM Info: Assuming 'minecraft/1.7.0' for this server. You should override this value by adding 'msm-version=minecraft/x.x.x' to '/opt/msm/servers/stable/server.properties' to make this message go away]
[MSM Info: Assuming 'minecraft/1.7.0' for this server. You should override this value by adding 'msm-version=minecraft/x.x.x' to '/opt/msm/servers/legacy/server.properties' to make this message go away]
Checking for updates to version 0.9.3..

…was not strict enough.

donkers · 2015-09-06T03:05:08Z

I have now defined a list of properties that can be put in the versioning files.

LOG_PATH;WHITELIST_PATH;BANNED_PLAYERS_PATH;BANNED_IPS_PATH;OPS_PATH OPS_LIST;

I was previously using a regex filter that was causing the versioning files to processed when it wasn't needed.

renderorange · 2015-09-07T03:51:46Z

edit: sorry, I did realize there was one more issue I found, not counting the "list" command issue (which is not due to your code changes). The one I did find has to do with the log roll command.

when running the log roll command which would run via cron, the path is being created within the user directory (relative, not absolute), instead of the archive dir:

# /etc/init.d/msm all logroll

then, inside of my 'stable' server's directory, notice the opt dir

/opt/msm/servers/stable # ll
total 44K
-rw-rw-r-- 1 minecraft minecraft    0 Sep  7 04:34 active
-rw-rw-r-- 1 minecraft minecraft    2 Sep  7 02:39 banned-ips.json
-rw-rw-r-- 1 minecraft minecraft    2 Sep  7 03:24 banned-players.json
-rw-rw-r-- 1 minecraft minecraft  180 Sep  7 02:37 eula.txt
drwxrwxr-x 2 minecraft minecraft 4.0K Sep  7 04:34 logs
-rw-rw-r-- 1 minecraft minecraft  143 Sep  7 02:42 ops.json
drwxrwxr-x 3 minecraft minecraft 4.0K Sep  7 04:35 opt
lrwxrwxrwx 1 minecraft minecraft   70 Sep  7 02:36 server.jar -> /opt/msm/jars/minecraft/2015-09-07-02-36-30-minecraft_server.1.8.8.jar
-rw-rw-r-- 1 minecraft minecraft  802 Sep  7 04:34 server.properties
-rw-rw-r-- 1 minecraft minecraft  114 Sep  7 04:34 usercache.json
-rw-rw-r-- 1 minecraft minecraft   93 Sep  7 02:42 whitelist.json
lrwxrwxrwx 1 minecraft minecraft   42 Sep  7 02:39 world -> /opt/msm/servers/stable/worldstorage/world
drwxrwxr-x 3 minecraft minecraft 4.0K Sep  7 02:39 worldstorage

/opt/msm/servers/stable # ll opt/
total 4.0K
drwxrwxr-x 3 minecraft minecraft 4.0K Sep  7 04:35 msm
/opt/msm/servers/stable # ll opt/msm/archives/logs/stable/stable-2015-09-07-04-35-32.log.gz 
-rw-rw-r-- 1 minecraft minecraft 350 Sep  7 04:35 opt/msm/archives/logs/stable/stable-2015-09-07-04-35-32.log.gz

…$*_PATH variables unnecessarily.

donkers · 2015-09-12T21:37:26Z

What I found was happening was the SERVER_PATH was being pretended to all *_PATH variables that didn’t already begin with it. i.e. /opt/msm/archives/logs/1-2-5 would become /opt/msm/servers/1-2-5/opt/msm/archives/logs/1-2-5. 556733d should fix this problem.

I have repeated my original testing method again, comparing the output of the following commands using msmhq:master and donkers:master versions of the script. (on Debian only this time)

msm server list
msm all worlds list 
msm jargroup list
msm config
msm all config
msm 1-2-5 start
msm 1-2-5 stop now
msm 1-3-2 start
msm 1-3-2 stop now
msm 1-6-4 start
msm 1-6-4 stop now
msm 1-7-10 start
msm 1-7-10 stop now
msm 1-8-8 start
msm 1-8-8 stop now
msm all worlds backup
msm all logroll

With the exception of timestamps and such the output almost matches. It seems the MSM Info: Assuming 'minecraft/1.7.0'... is still printed during a msm <server> logroll or msm <server> config when the given server has unknown version, where it did not before.

Upon further inspection of the code I was unable to figure out an easy way to rectify this and think it may have to be a necessary compromise to support #309, at least for the time being.

P.S. : When doing your testing are you using any automation or are you just doing it manually?

renderorange · 2015-09-13T22:09:10Z

I'm generally okay with the message printing on those commands, since they are less likely to happen by the admin, but I'd like to get @marcuswhybrow's opinion on that one.

As for testing, I am doing it manually. The testing path, in my mind, is generally so: read through the commits, test anything the code change will affect. Then run through high risk items, and if time allows, test anything else within medium risk. Follow down a rabbit hole if I see something amiss.

We do have a test suite (https://github.com/msmhq/msm/blob/master/test.sh), which runs to verify all pull requests. It hasn't been updated in a while, and could use some update lovin. @zachlatta was pushing for more code coverage when he was working a lot on reviewing and accepting pulls, but it seemed like people kept resisting updating the tests.

I'm all about automation :) I would love to take the time to update the test suite for a more thorough code coverage, to allow for a more thorough continuous integration process, but trying to grab issues and pull requests comes from the last of my freetime. If I had more time to do opensource things, yeah.

Anywho.

…or config command

donkers · 2015-09-22T09:33:23Z

That should bring the behaviour more in line with the current msmhq:master. No unknown version warning for logroll or config commands.

renderorange · 2015-09-30T04:52:59Z

Hey, sorry my focus has been elsewhere in the last while, and haven't been able to test things out. You should have the ability to merge your own pull request, since you have had successful merges before. I believe the purpose of opening up commit rights was to get the bottleneck of waiting on people to accept pulls out of the way, and to keep development moving with the community.

If you have any questions, please feel free to ask.

Versionable Properties

donkers added 12 commits August 23, 2015 09:56

Add set_property() function to load properties from versioning files.

a0850d7

Use the server_set_property() function to set property values in all …

4f85edb

…cases.

Move the if block for making paths absolute into the server_set_prope…

3e9e72e

…rty() function.

Change if conditions so versioning files are read for *_PATH as well …

9b60033

…as CONSOLE_* variables.

Only return 0 if this is a CONSOLE_ variable.

64e14e5

Add LOG_PATH to version files.

7f11ec6

Code re-factoring: Move case statment applying post-changes to values…

ef2a6c5

… to the server_set_property() function.

Moved the server.properties checking to it's own function, read_serve…

f4d0c14

…r_conf().

Call read_server_conf() when reading set_property lines in versioning…

0a637b8

… files. This allows overrides of version properties in the server.properties.

refactor of server_set_property(), move content of if block into the …

af13191

…case statement and check if path is already absolute.

Applied the 'script /dev/null' fix to the screen -r command in the co…

66c6097

…mmand_server_console() function.

Merge branch 'versionable_properties'

d38a328

donkers added 3 commits September 6, 2015 12:37

Defined a list of properties that can be versioned, the regex filter …

c41ba62

…was not strict enough.

Change case of local versionable_properties to match precedence.

911ba13

Missed a ; in the versionable_properties list. Damn it.

7646dfa

Fix bug where in some situations $SERVER_PATH was being prepended to …

556733d

…$*_PATH variables unnecessarily.

Prevent info about unset server version being printed during logroll …

299d2b0

…or config command

donkers added a commit that referenced this pull request Oct 7, 2015

Merge pull request #310 from donkers/master

1c5ada2

Versionable Properties

donkers merged commit 1c5ada2 into msmhq:master Oct 7, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Versionable Properties #310

Versionable Properties #310

donkers commented Aug 29, 2015

renderorange commented Aug 30, 2015

renderorange commented Aug 31, 2015

renderorange commented Aug 31, 2015

donkers commented Sep 6, 2015

renderorange commented Sep 7, 2015

donkers commented Sep 12, 2015

renderorange commented Sep 13, 2015

donkers commented Sep 22, 2015

renderorange commented Sep 30, 2015

Versionable Properties #310

Versionable Properties #310

Conversation

donkers commented Aug 29, 2015

renderorange commented Aug 30, 2015

renderorange commented Aug 31, 2015

renderorange commented Aug 31, 2015

donkers commented Sep 6, 2015

renderorange commented Sep 7, 2015

donkers commented Sep 12, 2015

renderorange commented Sep 13, 2015

donkers commented Sep 22, 2015

renderorange commented Sep 30, 2015