-
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
Refactor components to use default labels and owner references from etcd
#559
Refactor components to use default labels and owner references from etcd
#559
Conversation
) | ||
|
||
// GenerateValues generates `lease.Values` for the lease component with the given parameters. | ||
// GenerateValues generates `lease.Values` for the lease component | ||
func GenerateValues(etcd *druidv1alpha1.Etcd) Values { | ||
return Values{ | ||
BackupEnabled: etcd.Spec.Backup.Store != nil, | ||
EtcdName: etcd.Name, |
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.
I see that in the other components, this is changed to Name
. Can you do this here too?
@seshachalam-yv You need rebase this pull request with latest master branch. Please check. |
2011f2e
to
ea18c4b
Compare
/invite @abdasgupta |
@abdasgupta You have pull request review open invite, please check |
/hold Until I address this review comment #539 (comment) |
ea18c4b
to
72e0b04
Compare
/unhold PR is ready for review @unmarshall @shreyas-s-rao @aaronfern @abdasgupta |
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.
@seshachalam-yv thanks a lot for the well-written PR, and apologies for the late review. I have a few nits to be addressed, but overall LGTM 👍
Co-authored-by: Shreyas Rao <42259948+shreyas-s-rao@users.noreply.github.com>
- Renamed field `PodAdditionalLabels` to `AdditionalPodLabels`.
Co-authored-by: Shreyas Rao <42259948+shreyas-s-rao@users.noreply.github.com>
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.
@seshachalam-yv Thanks for making all the requested changes!
/lgtm
The labels on the etcd service changed with gardener/etcd-druid#559 that was introduced in gardener#8299. The `app` and `role` labels are no longer present on the service, but they are still available on the pods. So by using the pod's labels for the service discovery, we can restore the previous behavior. https://prometheus.io/docs/prometheus/latest/configuration/configuration/#endpoints
The labels on the etcd service changed with gardener/etcd-druid#559 that was introduced in gardener#8299. The `app` and `role` labels are no longer present on the service, but they are still available on the pods. So by using the pod's labels for the service discovery, we can restore the previous behavior. https://prometheus.io/docs/prometheus/latest/configuration/configuration/#endpoints Co-authored-by: Istvan Zoltan Ballok <istvan.zoltan.ballok@sap.com> Co-authored-by: Jeremy Rickards <jeremy.rickards@sap.com>
The labels on the etcd service changed with gardener/etcd-druid#559 that was introduced in #8299. The `app` and `role` labels are no longer present on the service, but they are still available on the pods. So by using the pod's labels for the service discovery, we can restore the previous behavior. https://prometheus.io/docs/prometheus/latest/configuration/configuration/#endpoints Co-authored-by: Jeremy Rickards <jeremy.rickards@sap.com>
The labels on the etcd service changed with gardener/etcd-druid#559 that was introduced in gardener#8299. The `app` and `role` labels are no longer present on the service, but they are still available on the pods. So by using the pod's labels for the service discovery, we can restore the previous behavior. https://prometheus.io/docs/prometheus/latest/configuration/configuration/#endpoints Co-authored-by: Istvan Zoltan Ballok <istvan.zoltan.ballok@sap.com> Co-authored-by: Jeremy Rickards <jeremy.rickards@sap.com>
The labels on the etcd service changed with gardener/etcd-druid#559 that was introduced in #8299. The `app` and `role` labels are no longer present on the service, but they are still available on the pods. So by using the pod's labels for the service discovery, we can restore the previous behavior. https://prometheus.io/docs/prometheus/latest/configuration/configuration/#endpoints Co-authored-by: Istvan Zoltan Ballok <istvan.zoltan.ballok@sap.com> Co-authored-by: Jeremy Rickards <jeremy.rickards@sap.com>
) The labels on the etcd service changed with gardener/etcd-druid#559 that was introduced in gardener#8299. The `app` and `role` labels are no longer present on the service, but they are still available on the pods. So by using the pod's labels for the service discovery, we can restore the previous behavior. https://prometheus.io/docs/prometheus/latest/configuration/configuration/#endpoints Co-authored-by: Jeremy Rickards <jeremy.rickards@sap.com>
) The labels on the etcd service changed with gardener/etcd-druid#559 that was introduced in gardener#8299. The `app` and `role` labels are no longer present on the service, but they are still available on the pods. So by using the pod's labels for the service discovery, we can restore the previous behavior. https://prometheus.io/docs/prometheus/latest/configuration/configuration/#endpoints Co-authored-by: Jeremy Rickards <jeremy.rickards@sap.com>
How to categorize this PR?
/area robustness
/kind enhancement
What this PR does / why we need it:
This PR refactors below components to use default labels and owner references from
etcd
.statefulset
service
poddisruptionbudget
lease
configmap
To get default labels and ownerreferences for etcd, introduced two functions
GetDefaultLabels
returns the default labels foretcd
.GetEtcdAsOwnerReference
returns an OwnerReference object that represents theetcd
In addition, the
configmap
component has been further improved with the addition of a new function calledgetEtcdConfigYaml
. This function helps to formulate theconfigmap
in a more efficient way.Role
Helm charts into component #538 (review) from Shreyas about improving serviceaccount test - 72e0b04druidv1alpha1.GroupVersion.String()
instead of the static stringdruid.gardener.cloud/v1alpha1
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
cc: @unmarshall
Release note: