Add TargetAllocator CRD #2516
Add TargetAllocator CRD #2516
Conversation
There was a problem hiding this comment.
I think this LGTM. It's a solid simple start that we can add to later. Are we going to add the collector selector in this PR or in a later one? Also, can we adjust the v1alpha2 collector's existing TA config to be a minimal version of this, or do we want to do that in a follow?
swiatekm
force-pushed
the
feat/add-ta-crd
branch
from
79f0d97 to
6f165f0
Compare
I'd like to add it alongside the conversion from the embedded TA to the v1alpha2 CRD.
I'm not completely convinced we want to do this at all tbh, it will make the conversion a lot more complicated. |
| } | ||
|
|
||
| // TargetAllocatorStatus defines the observed state of Target Allocator. | ||
| type TargetAllocatorStatus struct { |
There was a problem hiding this comment.
note for the future, it would be REALLY cool if we could add some info here like number of jobs, collectors discovered, etc.
There was a problem hiding this comment.
It would be cool to book a ticket for it :)
There was a problem hiding this comment.
I wonder how difficult it is to actually do this. We'd have to get the target allocator metrics, which while not exactly difficult, seems not very robust to me.
There was a problem hiding this comment.
certainly something to figure out in the future. #2549
| // Enabled indicates whether to use a PrometheusOperator custom resources as targets or not. | ||
| // +optional | ||
| Enabled bool `json:"enabled,omitempty"` | ||
| // Interval between consecutive scrapes. Equivalent to the same setting on the Prometheus CRD. |
There was a problem hiding this comment.
is there any precedence order for the interval defined here and on the service/pod monitors?
There was a problem hiding this comment.
It's exactly the same as the respective setting on the Prometheus CR - a default that can be overridden in individual CRs.
There was a problem hiding this comment.
is it smth that should be documented here?
There was a problem hiding this comment.
The comment already says it's equivalent to the Prometheus CR attribute, but I can also mention it's an overridable default.
| } | ||
|
|
||
| // TargetAllocatorStatus defines the observed state of Target Allocator. | ||
| type TargetAllocatorStatus struct { |
swiatekm
force-pushed
the
feat/add-ta-crd
branch
from
6f165f0 to
d4e88ba
Compare
|
I've also added a separate type for filter strategies and set the right default in kubebuilder tags. |
swiatekm
force-pushed
the
feat/add-ta-crd
branch
from
d4e88ba to
b7722a5
Compare
Description:
Add type definitions for the new TargetAllocator CRD. See the linked issue for the plan on how this will be rolled out.
Link to tracking Issue: #2422