Add external ports to job automatically when promMetrics are enabled

[busma13]

This PR makees the following changes:

• Creates an addExternalPortsToJobSpec() function that will add the external_ports field to a job if promMetrics are enabled and configure it to use the correct port.
• Adds a call to addExternalPortsToJobSpec() with the submit() and update() function within the JobService class.

ref: #3604

[godber]

see comment about docs

[godber]

You should describe the precedence behavior in the docs here:

https://terascope.github.io/teraslice/docs/configuration/clustering-k8s#prometheus-metrics

Otherwise this is good.

[busma13]

The following properties will override the matching property set within the terafoundation configuration.

I'm guessing this sentence from the docs is not clear enough?

[godber]

Actually, I need to think through that logic more carefully overall.

[godber]

Yeah, I know why this is hard.

Technically, I don't think we have enough properties to control all the conditions an operator/user may want. We have a single prom_metrics_enabled property, set either at the Terafoundation or the job level. But we may want the ability to trigger Prometheus independently on the master or on the jobs. That being the case what does the Terafoundation level prom_metrics_enabled represent? Is it the setting for the metrics on the master or is it the default for the jobs?

I hadn't really considered this until I saw your conditional:

        if (enabledInJob === true || (enabledInJob === undefined && enabledInTF)) {

and wondered what enabledInJob === false would imply.

I think the logic you have here is a great compromise.

Here are the conditions we hypothetically want to support:

• master metrics off, job metrics off
  • no settings anywhere
  • good
• master metrics on, job metrics off
  • enabledInTF: True and enabledInJob: False in all jobs
  • I can see this one potentially being a problem, lets say the metrics are costly somehow but we want them in the master, but not in the jobs, we will be forced to set this for prom_metrics_enabled: False in EVERY job to work around this.
• master metrics on, job metrics on
  • enabledInTF: True
  • good
• master metrics off, job metrics on
  • enabledInTF: False and enabledInJob: True in all jobs
  • this is pretty good, because the odds of not wanting master metrics is low

What I'm getting at is our properties were kind of wrong from the start, for completeness they might need to be:

• Terafoundation
  • prom_metrics_enabled_master
  • prom_metrics_enabled_job_default
• Job
  • prom_metrics_enabled

But really this all seems a little fussy, unless we really need it for that second scenario above.

[godber]

Let's just proceed with this.