[kong] add controller to service monitor

[rainest]

What this PR does / why we need it:

Adds the controller metrics port to the controller container template and Prometheus ServiceMonitor template. The port only appears if the controller's major version is >= 2.

Which issue this PR fixes

Part of Kong/kubernetes-ingress-controller#1497

Special notes for your reviewer:

This change imposes new requirements on values. The controller image tag must be semver-compliant. The template will completely fail to render if it is not.

This does not pose issues for our stock image tags, but may affect users who use custom controller images. If they do not currently use semver tags, they must make them semver compliant. IMO this is a reasonable ask: they can use the base image version and include any additional info relevant to their build using semver metadata.

The helper output is annoyingly considered a string, with no available function to convert it to a boolean. Go templates are cumbersome.

Checklist

• PR is based off the current tip of the next branch and targets next, not main
• New or modified sections of values.yaml are documented in the README.md
• Title of the PR and commit headers start with chart name (e.g. [kong])

[mflendrich]

This change imposes new requirements on values. The controller image tag must be semver-compliant. The template will completely fail to render if it is not.

Hm, this will not work nicely with custom builds and with our tags such as next.

What do you think about something like this:

• add a toggle to values.yaml that will control whether the controller is included in the service monitor, with the following semantics:
  • if unset, defaults to checking the semver version from the tag - if inconclusive, throwing a human-readable error suggesting that the user set the setting
  • if enabled/disabled: enabling/disabling the controller port in the service monitor
• control rendering of the controller endpoint in the service monitor based solely on the value of that toggle

[rainest]

Bah, yeah, main and next won't work with this.

For custom builds, the ask is that you use semver metadata. If you want to distinguish your custom build in the tag, and not just by using your registry, you'd have something like 2.0.0+foobar to denote that it's a 2.0.0 build that also includes your foobar modification.

We could use something compatible for our bleeding edge tags also (2.1.0-next or whatever), but it doesn't look like there's any simple way to do that with the docker-meta action.

I don't want to add toggles for individual version-dependent features that we want to have on by default on compatible versions. You shouldn't have to cross-reference the chart feature set against controller release notes, and shouldn't have to walk through all the values.yaml settings to toggle on things that you'd expect to be on by default.

I can add an optional user-set value for the controller semver version. If you provide it, we use that for the semver input instead of the tag. You'd only set it if you are using something like next or main. Those should be the minority of deployments by far, and probably mostly only at our instruction (we want someone to test an unreleased feature and ask them to use next in an issue). Not ideal, but it'd allow us to still use version gates without individual feature toggles.

[rainest]

After 98e50e6:

$ helm template ex /tmp/symkong --set serviceMonitor.enabled=true --set ingressController.image.tag=v2.2 --set ingressController.image.effectiveSemver=v2.5.0 | grep -A2 cmetrics 
        - name: cmetrics
          containerPort: 10255
          protocol: TCP

$ helm template ex /tmp/symkong --set serviceMonitor.enabled=true --set ingressController.image.tag=vdfgfdgdfg1.2 --set ingressController.image.effectiveSemver=v2.5.0 | grep -A2 cmetrics                                                                                             
        - name: cmetrics
          containerPort: 10255
          protocol: TCP

[mflendrich]

some of the attached comments may show my helm templating language ignorance 🤷‍♂️

[mflendrich]

could we make this template parametric in the following way, for reusability:

• accept the major version as parameter
• accept the image object as a parameter

so that it's reusable if we, say, need to check elsewhere if the gateway has major version >= 3?

[rainest]

Looked at this briefly and I think I'd like to defer it for now. This function is intentionally a bit weird because we want this to work for the beta, and for that the major version comparison is necessary over a more conventional semver.Compare: 2.0.0-beta.1 is not greater than 2.0.0 (and IIRC it's not even greater than 1.3, ignoring that using that would cause issues if we eventually released 1.4). Realistically our more immediate need, if any, will be to check if the controller version is >= 2.1, not >= 3.

We could work around that by converting to semver first, extracting the major, and passing that in, but IMO more work than its worth at this point--makes more sense to release a simpler version for now and then refactor into something more reusable after 2.0 GA.

Making a "function" in template land is a bit cumbersome, since parameters aren't really a thing--you have only a single parameter, so you have to construct a dict on the fly and don't have the benefit of signature checking. It's clunky but doable.

Whether we truly want to do that is an open question--my general experience with developing entirely in templates is that it's an exercise in masochism. This is a good example, where we have odd workarounds for not having proper functions and types and lack access to the full feature set of the underlying semver library. I think a reasonable next major project is to begin porting chart functionality into a full Go operator that handles complex decisions in Go and only uses templates for a final "fill in the exact given values" step to create a manifest.