Harmonize default value for REANA_HOSTNAME

[Alputer]

This PR is part of harmonizing the treatment of REANA_HOSTNAME accross all REANA components. You can refer to other PRs below.

reanahub/reana-server#717
reanahub/reana-workflow-controller#630

Closes #865

[codecov]

Codecov Report

Attention: Patch coverage is 16.66667% with 10 lines in your changes missing coverage. Please review.

Project coverage is 31.04%. Comparing base (b2074bc) to head (8acfb28).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
reana/reana_dev/cluster.py	11.11%	8 Missing ⚠️
reana/reana_dev/run.py	33.33%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #867      +/-   ##
==========================================
- Coverage   31.07%   31.04%   -0.04%     
==========================================
  Files          26       26              
  Lines        2491     2500       +9     
==========================================
+ Hits          774      776       +2     
- Misses       1717     1724       +7     

Files with missing lines	Coverage Δ
reana/reana_dev/run.py	54.58% <33.33%> (+0.22%)	⬆️
reana/reana_dev/cluster.py	47.19% <11.11%> (-1.64%)	⬇️

[tiborsimko]

Note that if we make reana_hostname to be mandatory and to have a nice default value in the Helm chart, then these conditional default strings would not be needed everywhere.

[tiborsimko]

We may have a problem with the treatment of port numbers.

(1) With the current PR and using:

reana_hostname: localhost

I'm getting missing port numbers in the Dask service URL:

reana=# select * from __reana.service ;
                 id_                  |         name          |                                   uri                                   | status  | owner_id | type_
--------------------------------------+-----------------------+-------------------------------------------------------------------------+---------+----------+-------
 f910b6ef-7fc6-434f-8d56-3bc4c22890fd | dask-service-cb18c6cd | https://localhost/cb18c6cd-2cc2-44ea-8f0f-2addec5722ec/dashboard/status | running |          | dask
(1 row)

(The real URL should be https://localhost:30443/cb18c6cd-2cc2-44ea-8f0f-2addec5722ec/dashboard/status.)

(2) And, if someone would like to customise Helm chart to use hostname including port number:

reana_hostname: localhost:30443

then this would lead to an installation error:

Error: INSTALLATION FAILED: 1 error occurred:
        * Ingress.networking.k8s.io "reana-ingress" is invalid: spec.rules[0].host: Invalid value: "localhost:30443": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')

We may perhaps need to introduce another Helm chart value concerning port information, or subtract the port information from reana_hostname when constructing ingress values.

I guess it may be better to have a new variable about port, and set it by default to 30443, and people could customise it to whatever they wish. (Could be handy if you'd like to run two REANA deployments on the same machine, e.g. for testing purposes.)

(3) Similarly, we may want to beware about HTTP vs HTTPS ports, because some installations terminate SSL early via say HAProxy, and comunicate using HTTP with REANA infrastructure afterwards, as desrcibed in https://docs.reana.io/administration/configuration/configuring-cluster-ingress/#using-reverse-proxy)

[tiborsimko]

For certificate purposes, I think one does not use the port information.

[tiborsimko]

suggestion: ‏Might be good to call the variable --hostport for full compatibility with the Helm chart value naming.

[tiborsimko]

todo: ‏Shouldbe 443 by default.

[tiborsimko]

suggestion: ‏You can rather explain that "... should be set to the port number where the REANA service will be exposed for incoming SSL connections." (Kind of using outward user-oriented language, not inward ingress-oriented language.)

[tiborsimko]

todo: ‏You should revert back to 30443 after testing is done.

[tiborsimko]

todo: ‏I'm note sure port 5000 would work, Kind/K8s may need ports above 30000. You can use a realistic exampe of 30444 for running a second REANA instance in the documentation pages.

[tiborsimko]

question: ‏Before, 30080 and 30443 were automatically added. Now, they would not be, so developers would have to pass `--extra-ports 30080 30443 manually. Perhaps you can check the argument, and if nothing was specified, fall back to 30080 and 30443, so that old behaviour would work out of the box?

[tiborsimko]

todo: ‏Please use realistic examples, i.e. 30443 default value. The help would be more easily copy-pasteable.

[tiborsimko]

suggestion: ‏localhost still stays hard-coded here. If we want to pass the host port, we should probably pass the host name too, for consistency. Alternatively, let's just keep the old content, i.e. not introduce any new argument, since the localhost:30443 part is only printed out as a help. I guess I would prefer the latter, so that we don't change the number of arguments to this script. (It does not seem necessary to know the port number when creating an admin user...)