Add deploy logic for Thanos receive ingest component #13
Conversation
philipgough
force-pushed
the
receive-dev
branch
7 times, most recently
from
d76e930 to
b55c4a1
Compare
philipgough
force-pushed
the
receive-dev
branch
2 times, most recently
from
6bf64fb to
820fe4c
Compare
| @@ -29,13 +29,13 @@ type RouterSpec struct { | |||
| // Replicas is the number of router replicas. | |||
| // +kubebuilder:validation:Minimum=1 | |||
| // +kubebuilder:default=1 | |||
| // +kubebuilder:validation:Optional | |||
| Replicas *int32 `json:"replicas,omitempty"` | |||
| // +kubebuilder:validation:Required | |||
There was a problem hiding this comment.
Do we need both default and required?
There was a problem hiding this comment.
i think it works in terms of the openapi spec tbh
| } | ||
|
|
||
| // buildController sets up the controller with the Manager. | ||
| func (r *ThanosReceiveReconciler) buildController(bld builder.Builder) error { |
There was a problem hiding this comment.
Does this need to be a separate method? I don't think we can reuse/test this.
There was a problem hiding this comment.
as of now no, but will be adding watch logic etc to endpointslices here so the function will grow.
|
|
||
| objs = append(objs, r.buildHashrings(receive)...) | ||
|
|
||
| var errCount int32 |
There was a problem hiding this comment.
Might be better to wrap errors here instead of just count? Or use merrors?
There was a problem hiding this comment.
i think it might become unwieldy - we already log on each of the failed requests but we will log the count at the top level. if anything we could drop it entirely but i think its a useful summary of the reconciliation loop?
| } | ||
|
|
||
| func (r *ThanosReceiveReconciler) handleDeletionTimestamp(log logr.Logger, receiveHashring *monitoringthanosiov1alpha1.ThanosReceive) (ctrl.Result, error) { | ||
| if controllerutil.ContainsFinalizer(receiveHashring, receiveFinalizer) { |
There was a problem hiding this comment.
When do we add the finalizer? I don't think this would fire without adding as per https://book.kubebuilder.io/reference/using-finalizers
There was a problem hiding this comment.
we don't add it yet so this is a no-op right now indeed - will need to add logic to clean up orphaned resources etc
| // Endpoint represents a single logical member of a hashring. | ||
| type Endpoint struct { | ||
| // Address is the address of the endpoint. | ||
| Address string `json:"address"` |
There was a problem hiding this comment.
Wonder if we can add some validation to these address fields
There was a problem hiding this comment.
yes, will take care of that in the follow up when we do the hashring work. just wanted to port over the thanos types here for visibility
philipgough
changed the title
The following PR adds code for the ingester components only in the split router/ingester model.
To complete the Thanos receive work, I will split the work into two separate PRs after this one merges to make things easier to review.