☂️-issue for Server-Side Apply in Gardener

[timebertt]

How to categorize this issue?

/area dev-productivity scalability
/kind impediment

From #4083:

[...] Server-Side Apply. We identified many places, especially in the Gardenlet code, that will benefit from SSA compared to Json-Merge/Strategic-Merge patches that are used today. Less conflicts, less network transfers, less API calls, lower code complexity, etc.. Other components in the Gardener landscape, e.g. Etcd-Druid or provider extensions, will benefit from SSA to at least the same extent.

While investigating how and under which circumstances SSA can be leveraged in Gardener as part of #4027, we got aware of the following major problems that will most probably prevent us from leveraging SSA as originally intended. @timuthy and I agreed on summarizing them in an issue for future reference.

• removing/unsetting fields in custom resources via apply that were set during a previous apply is buggy (ref "null" values are dropped from CustomResources kubernetes/kubernetes#101036, Field deletion doesn't work well in SSA kubernetes-sigs/structured-merge-diff#171). The bug was fixed in v1.21 and backported to v1.{18,19,20}, though we would heavily depend on it once gardener uses SSA to deploy/reconcile extension resources (e.g. Infrastructure) in the seed.
  • We probably don't want to add additional requirements for specific patch versions for the seed's k8s versions (e.g. require at least v1.18.19), so this effectively prevents us from properly using SSA for this use case.
• switching from the typical CreateOrUpdate-logic to SSA in a controller doesn't seem to be straight-forward for resources that were created before SSA was enabled (resources that don't have field ownership tracked in metadata.managedFields yet).
  • The problem is, that the first apply will add the before-first-apply field manager, that owns all fields that were present before the apply. Once the new field manager (e.g. gardenlet) wants to remove a field, it will only give up ownership but the field will still not be removed, as it is still co-owned by the before-first-apply manager.
  • In order to workaround this issue and take over sole ownership of all fields during the migration, we need to take ownership of at least one field before the first apply
  • e.g. take ownership of timestamp annotation:

  $ k patch infra infra --type=merge -p '{"metadata":{"managedFields": [{"apiVersion":"extensions.gardener.cloud/v1alpha1","fieldsType":"FieldsV1","fieldsV1":{"f:metadata":{"f:annotations":{"f:gardener.cloud/timestamp":{}}}},"manager":"gardenlet","operation":"Update","time":"2021-05-27T13:03:49Z"}]}}'
  

  • and then apply the full spec, which will make the new manager owner of all spec fields

There are also other minor problems with SSA, that won't hinder us from using it, but are worth keeping in mind:

• ApplyConfigurations for built-in API resources are only available in client-go@v1.21+ (ref Apply for client-go's typed clients kubernetes/enhancements#2155), so we will need to vendor v1.21 first before using SSA for built-in types, where using go structs is not applicable
• ApplyConfigurations for our own API types need to be generated using applyconfiguration-gen, so same condition for them as above
• several issues cause the resourceVersion to be bumped on every SSA even though it's a no-op (SSA triggers update events on apply to resource when it is no-op kubernetes/kubernetes#100024)
  • one of them regarding the creationTimestamp is solved in v1.22 (ref fieldmanager: Strip managedfields BEFORE we update the timestamp kubernetes/kubernetes#100032)
• status is tracked in managedFields although not set explicitly by an applier (as our extension API types don't specify omitempty for the status field)
  • not problematic as long as the status is protected by the /status subresource and status itself is not updated via SSA
  • fixed in v1.21 (ref Server-Side Apply status wiping kubernetes/kubernetes#99661)
• changes via the /scale subresource are not tracked in managedFields
  • fixed in v1.22 (ref Track ownership of scale subresource kubernetes/kubernetes#98377)
  • not problematic for us right now (I think), as long as we don't use SSA for resources scaled by HPA

[timebertt]

So far there's no decision whether to drop the idea of using SSA in gardener or pursue it further. However, the list above shows that we have to be very conscious about when we can properly leverage it.
We still want to understand the before-first-apply manager and the reasoning behind it, so maybe this provides us with some more arguments for taking a decision here.

[timebertt]

There is an upstream issue about the migration from CSA/CreateOrUpdate to SSA: kubernetes/kubernetes#99003

[gardener-ci-robot]

The Gardener project currently lacks enough active contributors to adequately respond to all issues and PRs.
This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed
  You can:
• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Close this issue or PR with /close

/lifecycle rotten

[gardener-ci-robot]

The Gardener project currently lacks enough active contributors to adequately respond to all issues and PRs.
This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten

/close

[gardener-prow]

@gardener-ci-robot: Closing this issue.

In response to this:

The Gardener project currently lacks enough active contributors to adequately respond to all issues and PRs.
This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.