Skip to content
This repository was archived by the owner on Sep 14, 2020. It is now read-only.

Conversation

nolar
Copy link
Contributor

@nolar nolar commented Feb 13, 2020

What do these changes do?

Patch custom/built-in resources with status declared as a sub-resource via a separate URL — instead of being a field of a resource itself.

Description

Previously, Kopf always assumed that custom resources and built-in resources store status as part of the object (which was the case in early Kubernetes versions).

But this has changed since sub-resources were introduced. While it was not a big problem for custom resources, where the developers could define the sub-resources and skip "status" there, it became a problem with some built-in resources like "storage.k8s.io/volumeattachments", which have the status defined as a sub-resource and operator developers cannot control it.

As a result, when Kopf was patching the object itself with {"status": ...} payload, the status was lost, and the reaction cycle did not continue, so the handlers were skipped.

With this PR, Kopf discovers and respects the status-as-a-subresource both in custom and built-in resources, and patches them accordingly: either as a whole object with a status field (if not a sub-resource), or separately for object & status endpoints (depending on the patch's content).

The latter aspect can cause for 2 PATCH requests to be made when both the body (e.g. metadata) and the status has to be updated — when previously it was only 1 (but did not work with this kind of resources at all).

From the user perspective, a wider set of built-in resources can be handled now, and their custom resources can also have status declared as a sub-resource (if they wish so).

Issues/PRs

Issues: #308 closes #119

Type of changes

  • New feature (non-breaking change which adds functionality)

Checklist

  • The code addresses only the mentioned problem, and this problem only
  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt

@zincr
Copy link

zincr bot commented Feb 13, 2020

🤖 zincr found 0 problems , 0 warnings

✅ Large Commits
✅ Approvals
✅ Specification
✅ Dependency Licensing

@lgtm-com
Copy link

lgtm-com bot commented Feb 13, 2020

This pull request introduces 1 alert when merging c7679a3 into 405ff50 - view on LGTM.com

new alerts:

  • 1 for Unused import

@nolar nolar added the enhancement New feature or request label Feb 13, 2020
@nolar nolar merged commit 8e31871 into zalando-incubator:master Feb 20, 2020
@nolar nolar deleted the discover-substatus branch February 20, 2020 11:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Status handling
3 participants