Refactor components to use default labels and owner references from etcd
#559
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gardener-robot
added
area/robustness
gardener-robot-ci-1
added
reviewed/ok-to-test
aaronfern
requested changes
Mar 18, 2023
| ) | ||
|
|
||
| // 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.
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. |
seshachalam-yv
force-pushed
the
refactor/pkg/components
branch
from
2011f2e
to
ea18c4b
Compare
gardener-robot-ci-3
added
the
reviewed/ok-to-test
gardener-robot-ci-1
removed
the
reviewed/ok-to-test
|
/invite @abdasgupta |
|
@abdasgupta You have pull request review open invite, please check |
|
/hold Until I address this review comment #539 (comment) |
gardener-robot
added
the
reviewed/do-not-merge
seshachalam-yv
force-pushed
the
refactor/pkg/components
branch
from
ea18c4b
to
72e0b04
Compare
gardener-robot-ci-2
added
reviewed/ok-to-test
|
/unhold PR is ready for review @unmarshall @shreyas-s-rao @aaronfern @abdasgupta |
gardener-robot
removed
the
reviewed/do-not-merge
shreyas-s-rao
requested changes
Jun 6, 2023
There was a problem hiding this comment.
@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>
gardener-robot-ci-1
added
reviewed/ok-to-test
- Renamed field `PodAdditionalLabels` to `AdditionalPodLabels`.
Co-authored-by: Shreyas Rao <42259948+shreyas-s-rao@users.noreply.github.com>
gardener-robot-ci-3
added
reviewed/ok-to-test
gardener-robot-ci-1
added
the
reviewed/ok-to-test
gardener-robot-ci-3
removed
the
reviewed/ok-to-test
shreyas-s-rao
approved these changes
Jun 9, 2023
There was a problem hiding this comment.
@seshachalam-yv Thanks for making all the requested changes!
/lgtm
gardener-robot
added
reviewed/lgtm
gardener-robot-ci-1
added
the
reviewed/ok-to-test
gardener-robot
added
the
status/closed
istvanballok
added a commit
to istvanballok/gardener
that referenced
this pull request
Aug 18, 2023
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
istvanballok
added a commit
to istvanballok/gardener
that referenced
this pull request
Aug 18, 2023
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>
gardener-prow bot
pushed a commit
to gardener/gardener
that referenced
this pull request
Aug 18, 2023
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>
gardener-ci-robot
pushed a commit
to gardener-ci-robot/gardener
that referenced
this pull request
Aug 18, 2023
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>
gardener-prow bot
pushed a commit
to gardener/gardener
that referenced
this pull request
Aug 18, 2023
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>
briantopping
pushed a commit
to briantopping/gardener
that referenced
this pull request
Aug 22, 2023
) 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>
nickytd
pushed a commit
to nickytd/gardener
that referenced
this pull request
Sep 11, 2023
) 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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.statefulsetservicepoddisruptionbudgetleaseconfigmapTo get default labels and ownerreferences for etcd, introduced two functions
GetDefaultLabelsreturns the default labels foretcd.GetEtcdAsOwnerReferencereturns an OwnerReference object that represents theetcdIn addition, the
configmapcomponent has been further improved with the addition of a new function calledgetEtcdConfigYaml. This function helps to formulate theconfigmapin a more efficient way.RoleHelm charts into component #538 (review) from Shreyas about improving serviceaccount test - 72e0b04druidv1alpha1.GroupVersion.String()instead of the static stringdruid.gardener.cloud/v1alpha1Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
cc: @unmarshall
Release note: