Specifying client count causes error & program exit

[Dreikasehoch]

Expected Behavior

When starting pykms_Server.py with the client count flag (either -c or --client-count), the provided client count value should be honored by the application and everything should run as usual.

Actual Behavior

No matter what value is given for client-count, the application writes an error message to console then exits.
e.g. for the case where -c 30 is given, the application will print argument `-c/--client-count`: invalid with: '30'. Exiting...

This is a tricky problem for Docker users since there's no option to skip the -c flag with the current Dockerfile.

Steps to Reproduce the Problem

1. Create a new python environment with all the needed prerequisites.
2. Clone the repo
3. Start py-kms using /usr/bin/python3 pykms_Server.py :: 1688 -c 30

Specifications

• Version: Cloned the repo, up to date as of commit [9d9a363]
• Platform: Ubuntu 20.04 LTS
• Subsystem: Python 3.9.0

[lepoarro]

Same problem here with latest docker (pykmsorg/py-kms:latest) version running on Unraid argument '-c/--client-count': invalid with: '26'. Exiting.... Previous version worked flawlessly!

[libots]

As a temporary fix for Docker users, you can specify the entrypoint manually (removing the -c flag). I added the following to my docker-compose.yaml: entrypoint: ["/bin/sh","-c","/usr/bin/python3 pykms_Server.py :: 1688 -l 1033 -a 120 -r 10080 -w RANDOM -V INFO -F /var/log/py3-kms.log"], which has it working now.

[lepoarro]

Under Unraid OS you can't really manipulate Docker environment the way @libots proposed. What you can do is edit/change environment variables and that's just about it. Right now I got it working in small Debian VM by directly cloning from github and running the python script without the -c flag. I really hope they fix the Dockerfile.

[cfredericksen]

I added the following to my k8s pykms config to override the ENTRYPOINT.

    command: ["/bin/sh"]
    args: ["-c", "/usr/bin/python3 pykms_Server.py :: 1688 -l 1033 -a 120 -r 10080 -w RANDOM -V INFO -F /var/log/py3-kms.log"]

[Dreikasehoch]

So the issue seems to come down to this sanity check on the input params where it iterates over a concatenated array of input argument values for clientcount, timeoutidle, etc; doing a quick check for null values or data of a wrong type.

py-kms/py-kms/pykms_Server.py

Lines 397 to 399 in 9d9a363

	if (value is not None) and (not isinstance(value, int)):
	pretty_printer(log_obj = loggersrv.error, to_exit = True,
	put_text = "{reverse}{red}{bold}argument `%s`: invalid with: '%s'. Exiting...{end}" %(opt, value))

Sadly, the statement will always evaluate to True and panic out since the datatype of clientcount is a string as specified below instead of an int as expected in the check.

py-kms/py-kms/pykms_Server.py

Lines 196 to 197 in 9d9a363

	server_parser.add_argument("-c", "--client-count", action = "store", dest = srv_options['count']['des'] , default = srv_options['count']['def'],
	help = srv_options['count']['help'], type = str)

Has a conversion from string to int for input arguments been lost recently? These segments of code are multiple months old and I can't think of why they'd work until just recently.

[simonmicro]

Hmm, we recently moved to only support Python3 - maybe the old Python2 instance does not had that problem, but since we changed the python version all people are updating their clients and therefore may have found a undiscovered bug. Maybe the used libs changed. But the possibility for that is not very high.
Fell free to fix it - I currently have no time for further investigation (sadly)...

[srepac]

Old version of py-kms used type=int

parser.add_argument("-c", "--client-count", dest = srv_options['count']['des'] , default = srv_options['count']['def' ], help = srv_options['count']['help'], type = int)

really that should be the only change that needs to be done to fix this. I made the edit but someone needs to approve it. We need this fixed asap for docker users.

[spitfire]

Seems to still be an issue with docker - See #80

[simonmicro]

@Dreikasehoch @spitfire Fixed (both in repo and in docker). Please close this issue now.

[Dreikasehoch]

The fix looks good, thanks!