[GEP-20] Zone pinning for shoots on multi-zonal seeds 📌 #6579

timuthy · 2022-08-25T15:24:06Z

How to categorize this PR?

/area control-plane
/area high-availability
/kind enhancement

What this PR does / why we need it:
This PR adds the zone pinning requirement discussed by GEP-20, ref.

In essence it does the following:

Gardenlet labels namespaces which belong to non-HA or single-zonal shoots if it's responsible for a multi-zonal seed (seed which has worker across availability zones).
A pod-zone-affinity webhook in GRM adds a zone-aware affinity term for such control-plane pods so that those will be scheduled into a single zone only.
After etcd was deployed successfully, it's assumed that at least one stateful pod was scheduled. Gardenlet then extracts the zone information from that pod/node.
From that moment on, pods' affinity configurations is enhanced by a nodeAffinity to "pin" pods to a specific zone. It is especially important to schedule pods to the original zone when shoots are being woken up from hibernation.

Which issue(s) this PR fixes:
Fixes partially #6529

Release note:

Gardener has been being prepared for more shoot HA use-cases and thus some assumption about currently running landscapes are required: If you use a `multi-zonal` labelled seed and scheduled non-HA shoots onto it, this release of Gardener will potentially cause scheduling conflicts to the control-plane pods as it will try to locate all pods into a single zone only. Pods that can't be re-scheduled (mainly because of volume dependencies) will remain in `Pending` state.

Gardener is prepared to run non-HA and single-zonal shoots on multi-zonal seeds. In such a setup, control-plane pods of the mentioned shoots are scheduled into a single availability zone only to avoid any extra cross zonal traffic that would usually involve higher latency and cost. **PLEASE NOTE**: The `StorageClass` in seeds used for control-plane components must have `volumeBindingMode: WaitForFirstConsumer` to let the zone-pinning work properly.

timuthy · 2022-08-26T06:35:56Z

/hold
I plan to improve the affinity terms after having a second look. With the current approach we rely on the fact, that any pod with a volume is created and scheduled first. This becomes especially important when a shoot is woken up from hibernation. If a stateless pod is scheduled first to any zone, then the remaining stateful pods are supposed to be scheduled to exactly this zone. Unfortunately, there is no guarantee that this stateless pod will run in very same zone that was used before the cluster was hibernated.

This PR will be transferred into draft mode as long as I'm working on the improvement.

timuthy · 2022-08-30T12:46:56Z

Update: The PR is ready for review now.

oliver-goetz · 2022-09-01T08:34:03Z

/assign

rfranzke · 2022-09-02T07:39:59Z

/assign

oliver-goetz

I added some small comments and have two general questions where I don't know enough about the context to answer them by myself 😄

When I see a shoot starting up etcd-main and etcd-events are starting at the same time on different nodes.
a) When there is no zone pinning annotation yet, couldn't it happen, that these etcd nodes would be in located in different zones? If this could happen, their volumes would be in different zones and couldn't be pinned to the same zone anymore right?
b) How about the migration scenario when introducing zone pinning. There are other control-plane components like loki and prometheus which use PVCs. Could it be possible that their volumes are located in different zones and could not be attached anymore when zone pinning was activated?

pkg/operation/botanist/namespaces.go

pkg/gardenlet/controller/shoot/shoot_control_reconcile.go

pkg/resourcemanager/webhook/podzoneaffinity/handler.go

pkg/apis/core/v1beta1/constants/types_constants.go

pkg/operation/botanist/namespaces.go

pkg/gardenlet/controller/shoot/shoot_control_reconcile.go

pkg/operation/botanist/namespaces.go

docs/concepts/resource-manager.md

pkg/resourcemanager/webhook/podschedulername/podschedulername_suite_test.go

pkg/resourcemanager/webhook/podzoneaffinity/handler.go

After etcd is ready we assume that at least one pod with a volume has been scheduled. This is the earliest point in time when we can "pin" the zone, i.e. all remaining volumes also reside in that zone and for the future it needs to be assured that all pods (esp. the ones with volumes) are scheduled there.

For non-HA or single-zonal HA shoot clusters, control-plane pods should be scheduled into a single zone only to avoid any cross zonal traffic.

timuthy · 2022-09-05T15:07:49Z

Thanks for your feedback @rfranzke and @oliver-goetz. I addressed all your comments, open is still #6579 (comment). PTAL.

rfranzke · 2022-09-06T03:42:43Z

Gardener has been being prepared for more shoot HA use-cases and thus some assumption about currently running landscapes are required: If you use a `multi-zonal` labelled seed and scheduled non-HA shoots onto it, this release of Gardener will potentially cause scheduling conflicts to the control-plane pods as it will try to locate all pods into a single zone only. Pods that can't be re-scheduled (mainly because of volume dependencies) will remain in `Pending` state.

Just to double-check: This is likely no problem because the "seed.gardener.cloud/multi-zonal" label must be added manually to the Seeds, right? I can imagine that there are folks in the wild that have seeds with multiple AZs, however I believe that nobody has added this label yet given that it's quite new and the whole HA story is still alpha/in development. In this case, I'm fine with it.

rfranzke

Thanks for addressing my comments @timuthy!

pkg/operation/botanist/namespaces.go

test/integration/resourcemanager/podzoneaffinity/podzoneaffinity_test.go

timuthy · 2022-09-06T06:02:50Z

@rfranzke

Just to double-check: This is likely no problem because the "seed.gardener.cloud/multi-zonal" label must be added manually to the Seeds, right?

Yes, that's mostly right. We planned to enable automatic labeling for ManagedSeeds because there we have information about workers.

timuthy · 2022-09-06T19:24:29Z

Thanks @rfranzke. I incorporated your feedback and in addition found some spots where I could improve the test handling regarding your suggestions: 56a86cf, b4a4869

rfranzke

Cool @timuthy, thanks for also cleaning up the other webhook test suites!

rfranzke · 2022-09-07T04:12:42Z

/approve

@oliver-goetz Do you also want to have another look?

oliver-goetz

There is one small thing left.

oliver-goetz · 2022-09-07T10:41:53Z

pkg/operation/botanist/namespaces.go

+		delete(namespace.Labels, v1beta1constants.ShootControlPlaneEnforceZone)
+		if zonePinningRequired(b.Shoot.GetInfo(), b.Seed.GetInfo()) && !metav1.HasLabel(namespace.ObjectMeta, v1beta1constants.ShootControlPlaneEnforceZone) {
+			metav1.SetMetaDataLabel(&namespace.ObjectMeta, v1beta1constants.ShootControlPlaneEnforceZone, "")
 		}


Suggested change

delete(namespace.Labels, v1beta1constants.ShootControlPlaneEnforceZone)

if zonePinningRequired(b.Shoot.GetInfo(), b.Seed.GetInfo()) && !metav1.HasLabel(namespace.ObjectMeta, v1beta1constants.ShootControlPlaneEnforceZone) {

metav1.SetMetaDataLabel(&namespace.ObjectMeta, v1beta1constants.ShootControlPlaneEnforceZone, "")

}

if !zonePinningRequired(b.Shoot.GetInfo(), b.Seed.GetInfo()) {

delete(namespace.Labels, v1beta1constants.ShootControlPlaneEnforceZone)

} else if !metav1.HasLabel(namespace.ObjectMeta, v1beta1constants.ShootControlPlaneEnforceZone) {

metav1.SetMetaDataLabel(&namespace.ObjectMeta, v1beta1constants.ShootControlPlaneEnforceZone, "")

}

Otherwise, the label would be changed back and forth during reconciliation, if enforce zone is activated.
This function would set its value to "", but AddZoneInformationToSeedNamespace would add a zone value again later.

Nice catch @oliver-goetz. I fixed it, PTAL.

oliver-goetz

/lgtm

🚀

gardener-prow · 2022-09-07T15:57:44Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: oliver-goetz, rfranzke

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [oliver-goetz,rfranzke]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

gardener-prow bot added area/control-plane Control plane related area/high-availability High availability related kind/enhancement Enhancement, improvement, extension cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. labels Aug 25, 2022

gardener-prow bot requested review from acumino and ary1992 August 25, 2022 15:24

gardener-prow bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Aug 25, 2022

timuthy force-pushed the feature.cp-zone-pinning branch from c0f146a to e70fe64 Compare August 26, 2022 04:49

gardener-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 26, 2022

timuthy marked this pull request as draft August 26, 2022 06:36

gardener-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 26, 2022

timuthy force-pushed the feature.cp-zone-pinning branch from e70fe64 to 793fd20 Compare August 30, 2022 10:44

gardener-prow bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Aug 30, 2022

timuthy force-pushed the feature.cp-zone-pinning branch from 793fd20 to 9fd0d28 Compare August 30, 2022 11:08

timuthy marked this pull request as ready for review August 30, 2022 12:40

gardener-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 30, 2022

gardener-prow bot requested review from oliver-goetz and plkokanov August 30, 2022 12:40

timuthy changed the title ~~[GEP-20] Zone pinning for shoots on multi-zonal seeds~~ [GEP-20] Zone pinning for shoots on multi-zonal seeds 📌 Aug 30, 2022

timuthy mentioned this pull request Aug 30, 2022

☂️ [GEP-20] Highly Available Seed and Shoot Clusters #6529

Closed

56 tasks

timuthy removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 30, 2022

gardener-prow bot assigned oliver-goetz Sep 1, 2022

gardener-prow bot assigned rfranzke Sep 2, 2022

oliver-goetz reviewed Sep 2, 2022

View reviewed changes

rfranzke requested changes Sep 2, 2022

View reviewed changes

timuthy added 3 commits September 5, 2022 17:05

Add zone pinning label to seed namespaces

f411d44

Add zone pinning for shoots on multi-zonal seeds

76ba07a

For non-HA or single-zonal HA shoot clusters, control-plane pods should be scheduled into a single zone only to avoid any cross zonal traffic.

timuthy force-pushed the feature.cp-zone-pinning branch from cdda796 to 2e63c6a Compare September 5, 2022 15:05

Address review comments

1254ae5

timuthy force-pushed the feature.cp-zone-pinning branch from 2e63c6a to 1254ae5 Compare September 5, 2022 15:06

rfranzke reviewed Sep 6, 2022

View reviewed changes

rfranzke requested a review from oliver-goetz September 6, 2022 03:57

timuthy added 3 commits September 6, 2022 20:31

Address review comments II

aadd165

Remove unneeded async assertions in integration tests

56a86cf

Pass Gomega object to assertion func

b4a4869

rfranzke reviewed Sep 7, 2022

View reviewed changes

gardener-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 7, 2022

oliver-goetz requested changes Sep 7, 2022

View reviewed changes

Fix losing zone information on reconciliation

84569c2

timuthy force-pushed the feature.cp-zone-pinning branch from 8d6ba8e to 84569c2 Compare September 7, 2022 15:52

oliver-goetz approved these changes Sep 7, 2022

View reviewed changes

gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Sep 7, 2022

gardener-prow bot merged commit 1127acd into gardener:master Sep 7, 2022

rfranzke mentioned this pull request Sep 8, 2022

[Flaky Test] PodZoneAffinity Integration Test Suite #6648

Closed

This was referenced Oct 26, 2022

[GEP-20] Potential race condition for zone-pinning #6905

Closed

[GEP-20] Remove zone pinning #6934

Merged

[release-v1.58][GEP-20] Remove zone pinning #6938

Merged

timuthy deleted the feature.cp-zone-pinning branch December 23, 2022 07:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GEP-20] Zone pinning for shoots on multi-zonal seeds 📌 #6579

[GEP-20] Zone pinning for shoots on multi-zonal seeds 📌 #6579

timuthy commented Aug 25, 2022 •

edited

timuthy commented Aug 26, 2022

timuthy commented Aug 30, 2022

oliver-goetz commented Sep 1, 2022

rfranzke commented Sep 2, 2022

oliver-goetz left a comment

timuthy commented Sep 5, 2022

rfranzke commented Sep 6, 2022

rfranzke left a comment

timuthy commented Sep 6, 2022

timuthy commented Sep 6, 2022

rfranzke left a comment

rfranzke commented Sep 7, 2022

oliver-goetz left a comment

oliver-goetz Sep 7, 2022

timuthy Sep 7, 2022

oliver-goetz left a comment

gardener-prow bot commented Sep 7, 2022

[GEP-20] Zone pinning for shoots on multi-zonal seeds 📌 #6579

[GEP-20] Zone pinning for shoots on multi-zonal seeds 📌 #6579

Conversation

timuthy commented Aug 25, 2022 • edited

timuthy commented Aug 26, 2022

timuthy commented Aug 30, 2022

oliver-goetz commented Sep 1, 2022

rfranzke commented Sep 2, 2022

oliver-goetz left a comment

Choose a reason for hiding this comment

timuthy commented Sep 5, 2022

rfranzke commented Sep 6, 2022

rfranzke left a comment

Choose a reason for hiding this comment

timuthy commented Sep 6, 2022

timuthy commented Sep 6, 2022

rfranzke left a comment

Choose a reason for hiding this comment

rfranzke commented Sep 7, 2022

oliver-goetz left a comment

Choose a reason for hiding this comment

oliver-goetz Sep 7, 2022

Choose a reason for hiding this comment

timuthy Sep 7, 2022

Choose a reason for hiding this comment

oliver-goetz left a comment

Choose a reason for hiding this comment

gardener-prow bot commented Sep 7, 2022

timuthy commented Aug 25, 2022 •

edited