zatosource / zato

ESB, SOA, REST, APIs and Cloud Integrations in Python
https://zato.io
GNU Affero General Public License v3.0
1.12k stars 240 forks source link

`zato create server` returns success when really server creation failed #326

Closed bkw closed 9 years ago

bkw commented 9 years ago

When referencing a ca-certs file that does not exist, zato create server will display an error message and a traceback, but the cli still exits with code 0.

zato@test-zato: ~$ zato create server --odb_password zato1 --postgresql_schema public --kvdb_password zato /opt/zato/test-zato postgresql localhost 5432 zato1 zatodb1 localhost 6379 /opt/zato/ssl/test-zato.pub /opt/zato/ssl/test-zato.pem /opt/zato/ssl/test-zato.crt /opt/zato/ssl/cacerts.pem test-cluster test-zato
Could not create the server, e:[Traceback (most recent call last):
  File "/opt/zato/1.1/zato-cli/src/zato/cli/create_server.py", line 188, in execute
    self.copy_server_crypto(repo_dir, args)
  File "/opt/zato/1.1/zato-cli/src/zato/cli/__init__.py", line 371, in copy_server_crypto
    self._copy_lb_server_crypto(repo_dir, args, 'server')
  File "/opt/zato/1.1/zato-cli/src/zato/cli/__init__.py", line 365, in _copy_lb_server_crypto
    shutil.copyfile(os.path.abspath(getattr(args, arg_name)), full_path)
  File "/usr/lib/python2.7/shutil.py", line 82, in copyfile
    with open(src, 'rb') as fsrc:
IOError: [Errno 2] No such file or directory: '/opt/zato/ssl/cacerts.pem'
]
OK
zato@test-zato:~$ echo $?
0

I'm currently working on a chef cookbook for zato, but things like that make creating a server very hard.

dsuch commented 9 years ago

Hi @bkw - nice catch, I will be sure to make it return a proper status code in the upcoming Zato 2.0 release.

We already have more than a dozen return codes https://github.com/zatosource/zato/blob/master/code/zato-cli/src/zato/cli/__init__.py#L280

When you say things like that - are you referring to other points you'd like to see refined in order to make your job easier?

Thanks.

bkw commented 9 years ago

Hi @dsuch, thanks for your quick reply. Apologies for being so bit unspecific, I just spent too many hours hunting this one down. There is not really a list of other things, but my cookbook has to jump through some hoops to make configuration idempotent, like connecting to the odb looking for existing cluster configuration, run zdaemon status on the created files to check wether a server has to be started, etc. I'll open separate issues for these eventually, in case I don't find a workaround. Expect a community cookbook for zato soon :-)

dsuch commented 9 years ago

Cool, very nice!

One thing though - have you perhaps considered using zato info rather than dealing with zdaemon directly?

2.0 doesn't use zdaemon anymore because it turned out to be too problematic and if you need information on whether a component is running, a thing like zato info /path/to/a/component | grep component_running will do the trick. The command can also output to JSON.

bkw commented 9 years ago

Thanks for the hint, @dsuch , I'll look into that.

Meanwhile, here's another faulty exitcode:

zato@test-zato:~$ zato create web_admin --postgresql_schema public --odb_password zato1 --tech_account_password tech-account-secret /opt/zato/test-zato-webadmin postgresql localhost 5432 zato1 zatodb1 /opt/zato/ssl/test-zato.pub /opt/zato/ssl/test-zato.pem /opt/zato/ssl/test-zato.crt /opt/zato/ssl/cacerts.pem zato
Superuser created successfully.
OK
zato@test-zato:~$ echo $?
1
dsuch commented 9 years ago

Hi @bkw - since commit d83554b5ca6f7d8dd7e60998436ab9874fba0b4f all unexpected exceptions are always caught and a status code of 16 is assigned to such situations.

I don't envision that status code ever changing but still, it's best to compare with non-0 rather than 16 directly.

I will close this ticket soon if you don't mind it and I'll open another one to deal with zato create web_admin returning a non-0 exit code. Tentatively I believe this is because you ran the command twice and the other time it discovered the ODB schema already existed. But it shouldn't be an error naturally. We'll see. But in another ticket :-)

bkw commented 9 years ago

Great, thanks!