-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
support az aware hashring and multiple sts in one hashring #129
Conversation
@@ -2,4 +2,4 @@ module _ // Auto generated by https://github.com/bwplotka/bingo. DO NOT EDIT | |||
|
|||
go 1.17 | |||
|
|||
require github.com/golangci/golangci-lint v1.45.2 // cmd/golangci-lint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needs to update golangci-lint version and bingo version to work with golang v1.21
@@ -19,6 +19,11 @@ linters: | |||
- varnamelen | |||
- paralleltest | |||
- cyclop | |||
- exhaustruct |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
temp disable some newly added lint checks, feel free to resolve on these lint issues
k8s.io/apimachinery v0.26.1 | ||
k8s.io/client-go v0.26.1 | ||
github.com/prometheus/client_golang v1.17.0 | ||
github.com/thanos-io/thanos v0.33.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
accomodate thanos newer version
return p.listsMetric | ||
} | ||
|
||
func (p prometheusReflectorMetrics) NewListDurationMetric(name string) cache.SummaryMetric { | ||
func (p prometheusReflectorMetrics) NewListDurationMetric(_ string) cache.SummaryMetric { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix lint issues
@@ -283,7 +289,7 @@ func newReflectorMetrics(reg *prometheus.Registry) prometheusReflectorMetrics { | |||
|
|||
const labelParts = 2 | |||
|
|||
func splitLabel(in string) (key, value string) { | |||
func splitLabel(in string) (string, string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix nonamed return values
statefulsets[hashring] = []*appsv1.StatefulSet{} | ||
} | ||
// Append the new value to the slice associated with the hashring key | ||
statefulsets[hashring] = append(statefulsets[hashring], sts.DeepCopy()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
support multiple sts placed in a single hashring in our setup
@@ -679,6 +686,40 @@ func (c *controller) populate(ctx context.Context, hashrings []receive.HashringC | |||
} | |||
} | |||
|
|||
func (c *controller) populateEndpoint(sts *appsv1.StatefulSet, podIndex int, err error, pod *corev1.Pod) *receive.Endpoint { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extracted this function to reduce cognitive complexity required by lint
## What changes are proposed in this pull request? 1. Support az aware hashring in Thanos v0.32+ https://thanos.io/tip/components/receive.md/#az-aware-ketama-hashring-experimental, 2. also supported multiple sts in one hashring config 3. fix lint and go dependency issues OSS PR: observatorium#129 ## How is this tested? main_test.go, Make thanos-receive-controller pending test in deployment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm thanks for this
…rium#129) * support az aware hashring * Update receive-controller.json * support multiple statefulsets in 1 hashring * add more logs * style * fix lint issue * debug * return when encountering error * remove whitespace
* support az aware hashring and multiple sts in one hashring (observatorium#129) * support az aware hashring * Update receive-controller.json * support multiple statefulsets in 1 hashring * add more logs * style * fix lint issue * debug * return when encountering error * remove whitespace * Fix k8s permissions (observatorium#133) * Fix k8s permissions * fix ci * fix ci * sync * add pantheon migration state * Revert "Fix k8s permissions (observatorium#133)" This reverts commit e545b83. --------- Co-authored-by: Alec Rajeev <[email protected]>
* support az aware hashring and multiple sts in one hashring (observatorium#129) * support az aware hashring * Update receive-controller.json * support multiple statefulsets in 1 hashring * add more logs * style * fix lint issue * debug * return when encountering error * remove whitespace * Fix k8s permissions (observatorium#133) * Fix k8s permissions * fix ci * fix ci * sync * add pantheon migration state * Revert "Fix k8s permissions (observatorium#133)" This reverts commit e545b83. --------- Co-authored-by: Alec Rajeev <[email protected]> Signed-off-by: Yi Jin <[email protected]>
* support az aware hashring and multiple sts in one hashring (observatorium#129) * support az aware hashring * Update receive-controller.json * support multiple statefulsets in 1 hashring * add more logs * style * fix lint issue * debug * return when encountering error * remove whitespace * Fix k8s permissions (observatorium#133) * Fix k8s permissions * fix ci * fix ci * sync * add pantheon migration state * Revert "Fix k8s permissions (observatorium#133)" This reverts commit e545b83. --------- Co-authored-by: Alec Rajeev <[email protected]> Signed-off-by: Yi Jin <[email protected]>
* support az aware hashring and multiple sts in one hashring (observatorium#129) * support az aware hashring * Update receive-controller.json * support multiple statefulsets in 1 hashring * add more logs * style * fix lint issue * debug * return when encountering error * remove whitespace * Fix k8s permissions (observatorium#133) * Fix k8s permissions * fix ci * fix ci * sync * add pantheon migration state * Revert "Fix k8s permissions (observatorium#133)" This reverts commit e545b83. --------- Co-authored-by: Alec Rajeev <[email protected]> Signed-off-by: Yi Jin <[email protected]>
* support az aware hashring and multiple sts in one hashring (observatorium#129) * support az aware hashring * Update receive-controller.json * support multiple statefulsets in 1 hashring * add more logs * style * fix lint issue * debug * return when encountering error * remove whitespace * Fix k8s permissions (observatorium#133) * Fix k8s permissions * fix ci * fix ci * sync * add pantheon migration state * Revert "Fix k8s permissions (observatorium#133)" This reverts commit e545b83. --------- Co-authored-by: Alec Rajeev <[email protected]> Signed-off-by: Yi Jin <[email protected]>
* support az aware hashring and multiple sts in one hashring (observatorium#129) * support az aware hashring * Update receive-controller.json * support multiple statefulsets in 1 hashring * add more logs * style * fix lint issue * debug * return when encountering error * remove whitespace * Fix k8s permissions (observatorium#133) * Fix k8s permissions * fix ci * fix ci * sync * add pantheon migration state * Revert "Fix k8s permissions (observatorium#133)" This reverts commit e545b83. --------- Co-authored-by: Alec Rajeev <[email protected]> Signed-off-by: Yi Jin <[email protected]>
What changes are proposed in this pull request?
https://thanos.io/tip/components/receive.md/#az-aware-ketama-hashring-experimental
How is this tested?
main_test.go,
Make thanos-receive-controller
#127