[dcgm-exporter] Support exposing metrics on hostNetwork

[nikhaild]

Add support to toggle hostNetwork field of pod spec for dcgm-exporter daemonset pod spec,
allowing dcgm-exporter pods to be scraped by say prometheus-server that runs outside of the
bounds of the k8s cluster overlay network (and still be able to reach dcgm-exporter pods,
i.e. scraping each daemonset pod's port on the node - this is easier to reason and deal with
compared to say scraping a NodePort service ports on each node).

Fixes #1086

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[rajathagasthya]

/ok-to-test d858d24

[tariq1890]

@nikhaild, thanks for your contribution. In general, we like to ensure that our generated YAML manifests are aligned with the upstream helm chart of dcgm-exporter. It would be good if you can open a PR/issue in NVIDIA/dcgm-exporter and get the maintainers of dcgm-exporter to weigh in on this first.

[nikhaild]

In general, we like to ensure that our generated YAML manifests are aligned with the upstream helm chart of dcgm-exporter. It would be good if you can open a PR/issue in NVIDIA/dcgm-exporter and get the maintainers of dcgm-exporter to weigh in on this first.

Thanks @tariq1890 for taking a look!

Upstream dcgm-exporter helm chart already supports "templatizing" this hostNetwork field (was done a while ago, tracker PR#64), code ref

      {{- if .Values.hostNetwork }}
      hostNetwork: {{ .Values.hostNetwork }}

It's just not set explicitly to a default value in dcgm-exporter helm chart values.yaml, but looks like some folks use it already (ref issue#495).

Mind clarifying what exactly should I ask from dcgm-exporter maintainer folks? Would appreciate if you could elaborate a bit.

[Bixlid]

Hi,
This feature would be very usefull for us too. Have you any idea of what's missing to get it merged ?
Thanks.

[tariq1890]

Please add a unit test as well

[nikhaild]

Thanks @tariq1890 for the review!

Please add a unit test as well

I've updated the unit tests in TestTransformDCGMExporter() (controllers/transforms_test.go) to include check for spec.DNSPolicy too.

I also saw there was no unit test for "case for DCGM running on the host itself(DGX BaseOS)" existing scenario/code-path:

gpu-operator/controllers/object_controls.go

Lines 1694 to 1698 in 1de9839

	// case for DCGM running on the host itself(DGX BaseOS)
	remoteEngine := getContainerEnv(&(obj.Spec.Template.Spec.Containers[0]), DCGMRemoteEngineEnvName)
	if remoteEngine != "" && strings.HasPrefix(remoteEngine, "localhost") {
	// enable hostNetwork for communication with external DCGM using localhost
	obj.Spec.Template.Spec.HostNetwork = true

have added a unit test case for it too here: https://github.com/NVIDIA/gpu-operator/pull/1962/changes#diff-97f4959bc802c8178bd5dd8118463f0f937130ec0cb9dbfb74d3ec01f2793f9aR1372-R1406

Let me know please if these tests look sufficient!

[tariq1890]

Thanks @nikhaild the test cases look sufficient to me !

Can you squash your commit history and enable signoff to satisfy the DCO check?

[nikhaild]

Thanks @nikhaild the test cases look sufficient to me !

Can you squash your commit history and enable signoff to satisfy the DCO check?

Squashed, also rebased on upstream/main!

[tariq1890]

/ok to test 95255ef

[rahulait]

LGTM