Skip to content
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

refactor(core): prevent not found error from kubectl #364

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

diafour
Copy link
Member

@diafour diafour commented Sep 12, 2024

Description

  • Add subresources status field.
  • GET /apis/v1/.../resourcetype/NAME/status give error NotFound if no subresources field in CRD.

Why do we need it, and what problem does it solve?

Prevent errors from kubectl:

# kubectl -n NSNAME patch internalvirtualizationvirtualmachineinstance VMNAME --subresource status --type='json' --patch '[{"op": "remove","path": "/status/migrationState"}]'
Error from server (NotFound): internalvirtualizationvirtualmachineinstances.internal.virtualization.deckhouse.io "VMNAME" not found

What is the expected result?

Checklist

  • The code is covered by unit tests.
  • e2e tests passed.
  • Documentation updated according to the changes.
  • Changes were tested in the Kubernetes cluster manually.

- Add subresources status field.
- GET /apis/v1/.../resourcetype/NAME/status give error NotFound if no subresources field in CRD.

Signed-off-by: Ivan Mikheykin <[email protected]>
@diafour diafour requested a review from fl64 September 12, 2024 07:52
@diafour diafour marked this pull request as draft September 12, 2024 09:02
@diafour
Copy link
Member Author

diafour commented Sep 12, 2024

Status field of VMI should not be a subresource. kubevirt updates status field as a part of "main" payload.

Some resources for KubeVirt and CDI store status field as "main" fields, not as a subresource. Set status to subresource will break these resources lifecycles.

Signed-off-by: Ivan Mikheykin <[email protected]>
@diafour diafour marked this pull request as ready for review September 16, 2024 13:22
@@ -1337,4 +1337,6 @@ spec:
type: object
served: true
storage: true
subresources: {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it normal if we delete this fragment?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants