-
Notifications
You must be signed in to change notification settings - Fork 83
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
configure HA webhook according to replicas in the helm chart #918
base: master
Are you sure you want to change the base?
Conversation
@kon-angelo You need rebase this pull request with latest master branch. Please check. |
252ec35
to
ae1d0e6
Compare
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
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.
Looking into the code of the resource manager webhook, it will not work as expected.
See inline comment.
high-availability-config.resources.gardener.cloud/replicas: {{ .Values.replicaCount }} | ||
labels: | ||
{{ include "labels" . | indent 4 }} | ||
high-availability-config.resources.gardener.cloud/type: server |
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.
If you look into the code of the resourcemanager webhook, you see that it would overwrite the value calculated by the presence of the label high-availability-config.resources.gardener.cloud/type: server
. Instead of 2
, we would have 3
according to the current values we are using in the ControllerDeployment
Instead I suggest to use the controller settings in this case.
See: https://github.com/gardener/gardener/blob/145d6c7a55b928c9c9df7049ccb340f777d53803/pkg/utils/kubernetes/highavailability_test.go#L30
high-availability-config.resources.gardener.cloud/replicas: {{ .Values.replicaCount }} | |
labels: | |
{{ include "labels" . | indent 4 }} | |
high-availability-config.resources.gardener.cloud/type: server | |
{{- if eq Values.replicaCount 1 }} | |
high-availability-config.resources.gardener.cloud/failure-tolerance-type: "" | |
{{- end }} | |
labels: | |
{{ include "labels" . | indent 4 }} | |
{{- if ne Values.replicaCount 1 }} | |
high-availability-config.resources.gardener.cloud/type: server | |
{{- else }} | |
high-availability-config.resources.gardener.cloud/type: controller | |
{{- end }} |
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.
As discussed off-line, this alternative relies on some detailed behaviour of the HA webhook, which makes it hard to understand.
Maybe an boolean flag to set the annotation high-availability-config.resources.gardener.cloud/replicas: "1"
is a better solution for the moment.
How to categorize this PR?
/area control-plane
/kind enhancement
/platform openstack
What this PR does / why we need it:
Allows you to run the extension with a single replica. For 2+ replicas it should be no-op.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Release note: