Release 1.0.0-beta4

[burningalchemist]

Hi @amimof ,

If you have time, could you please add 1.0.0-beta4 tag, as we have hostname in metrics now and also a bugfix for a certificate version representation since 1.0.0-beta3.

Kind regards,
Sergei

[amimof]

Hi @burningalchemist

I've been doing some thinking and I realised that the hostname label which you helped introduce in #25 will only return the hostname of the Pod (if run in a container). So my plan is to also add a Kubernetes specific label which will return the name of the node. I believe that was the desired behaviour from the beginning.

[burningalchemist]

Hi @amimof,

Yeah, exactly today I bumped into the same issue. It seems to only work as expected if deployed as a native service and not as a container. I was able to get desired behaviour with Prometheus relabel_config (by adding __meta_kubernetes_node_name). It was a relatively minor configuration change. But your plan also makes sense! 👍

Please, let me know if I can assist in testing, implementation or something. 😄

[amimof]

@burningalchemist Yes we have also been using a relabel_config for our clusters. I believe it should be possible to retrieve the node name from an env var from within a pod. For example os.Getenv("NODE_NAME").

[alexdotsh]

@amimof
@burningalchemist

I've created a PR implementing the expected behaviour for retrieving the Nodename. Please let me know if looks alright. :)

[burningalchemist]

@alexmirkhaydarov it looks fine, but it also makes us unable to use it outside of Kubernetes, thus making node_cert_exporter less generic.

Currently, Kubernetes nodeName label can be added with Prometheus, or correctly retrieved if hostNetwork: true is set, but to run it natively with nodeName only, we would need to have NODE_NAME env set on a host manually, which doesn't make sense.

It's up to @amimof, but I would not consider removing hostname completely, but rather having it as a separate label, or actually implementing a fallback to os.Hostname if NODE_NAME can't be found. I'd try the latter.

[amimof]

@alexmirkhaydarov Yes correct. I don't believe that the hostname label is to be replaced. It has its uses as well as the node name. I haven't had this confirmed, but doesn't Kubernetes add default environment variables to pods such as the node name?

[burningalchemist]

@amimof Unfortunately not. All the environment variables get injected based upon data available in manifests. Some of the fields are mandatory for manifests, so it makes them look default.

[alexdotsh]

@burningalchemist Yes you're right. I've picked up at some-point it won't be useable outside of Kubernetes and so I was working on a fallback fix. But I think having as a separate label makes total sense :)

@amimof
Should we agree to use Nodename as a separate label or use os.Hostname as a fallback?

[amimof]

@burningalchemist
@alexmirkhaydarov
Yes node name as a separate label. So it would be up to the admin to set env var using downward api or relabel config in Prometheus since there are no other reasonable solution.

[alexdotsh]

@amimof I've updated the original PR to include the additional Nodename label.
Removed change based on your comment about it being set by the Admin. 😄

[amimof]

@alexmirkhaydarov Thanks. I didn't meant for you to remove the Downward API config in the deploy manifest. What I meant was that the admin is in control of their config. But may use the provided one from this repo which contains necessary config to include node name as an env var. Sorry for the misunderstanding.

[alexdotsh]

The removal of the downward API back has been reverted on the deploy manifest. This issue can be closed now as the PR has been merged with the necessary changes.

[amimof]

Just released v1.0.0-beta.4. Thank you so much for your time and effort 👏