WIP: Allow users to choose Endpoints/EndpoinSlice role in Prometheus and Prometheus Agent

[haanhvu]

Description

First task of the remaining tasks to support EndpointSlice, as discussed in #3862 (comment)

Type of change

What type of changes does your code introduce to the Prometheus operator? Put an x in the box that apply.

• CHANGE (fix or feature that would cause existing functionality to not work as expected)
• FEATURE (non-breaking change which adds functionality)
• BUGFIX (non-breaking change which fixes an issue)
• ENHANCEMENT (non-breaking change which improves existing functionality)
• NONE (if none of the other choices apply. Example, tooling, build system, CI, docs, etc.)

Changelog entry

Allow users to choose Endpoints/EndpoinSlice role in Prometheus and Prometheus Agent

[simonpasquier]

I would make it optional without a predefined default so we could maybe toggle the default in the future.

Suggested change

	// +kubebuilder:default="Endpoints"
	// +kubebuilder:validation:Enum=Endpoints;EndpointSlice
	ServiceDiscoveryRole string `json:"serviceDiscoveryRole,omitempty"`
	// ServiceDiscoveryRole defines the service discovery role to use when discovering Kubernetes endpoints from ServiceMonitor objects. By default, the operator uses the `Endpoints` role.
	//
	// +kubebuilder:validation:Enum=Endpoints;EndpointSlice
	// +optional
	ServiceDiscoveryRole *string `json:"serviceDiscoveryRole,omitempty"`

[ArthurSens]

Wouldn't that be a breaking change anyway?

[haanhvu]

I think what Simon meant from the suggested change is:

• If users don't set ServiceDiscoveryRole, or set it as Endpoints, we use Endpoints
• Only if users set it as EndpointSlice, we use EndpointSlice

This way I don't think it would break current users' setup? Since we're already using Endpoints.

[ArthurSens]

Yes, but I also understood that we want to change to EndpoitSlice by default in the future 😬. I was wondering how safe this change would be if we ever happen to make it.

But that's a problem for a future PR, going forward with Endpoints by default sounds the correct approach here :)

[haanhvu]

I think how safe that change would be depends very much on how popular EndpointSlice would be. I researched this a bit (for ServiceMonitor support in GSoC proposal). There're questions around when/whether Kubernetes can deprecate Endpoints, since EndpointSlice has already proved to perform better.

If some far day Endpoints is not used anymore (or very rarely used), I think we can safely make EndpointSlice as the default.

[simonpasquier]

why change from a bool value to a bool pointer?

[haanhvu]

Yeah it's not necessary.

Before I thought we needed to check the value of ServiceDiscoveryRole inside NewConfigGenerator(). If ServiceDiscoveryRole != "EndpointSlice" then we needed to change endpointSliceSupported = false (it may have been true before). That was why I changed the type of endpointSliceSupported to a bool pointer.

But now rereading the code I realized we can also check the value of ServiceDiscoveryRole and finalize the value of endpointSliceSupported inside sync() in Prometheus/Prometheus Agent operator, right before calling NewConfigGenerator(). So no need to make endpointSliceSupported a bool pointer anymore.