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

Post events for cluster-scoped objects to current namespace #165

Merged
merged 2 commits into from
Aug 7, 2019

Conversation

nolar
Copy link
Contributor

@nolar nolar commented Aug 5, 2019

K8s-events cannot be posted cluster-scoped (?), and attached to cluster-scoped resources via API. However, we post them to the current namespace — so that they are not lost completely.

Issue : #164

Description

See #164 for details.

Brief summary: K8s events are namespaced, there are no cluster-scoped events. Also, k8s events can refer to an object via spec.involvedObject of type ObjectReference. This structure contains namespace field, to refer to the involved object's namespace (also, name, uid, etc).

I could not find a way to post namespaced events for cluster-scoped resources. It always fails with namespace mismatch (regardless of which library is used). Event via curl — see the issue comments.

So, we post the k8s-events to the current namespace, so that they are available via kubectl get events, despite they are not seen in kubectl describe on the involved objects.

It is not a full problem solution, but it is a bit better than just losing them completely.

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)

@nolar nolar added the bug Something isn't working label Aug 5, 2019
@nolar nolar requested a review from samurang87 as a code owner August 5, 2019 18:06
@zincr
Copy link

zincr bot commented Aug 5, 2019

🤖 zincr found 0 problems , 0 warnings

✅ Large Commits
✅ Approvals
✅ Specification
✅ Dependency Licensing

@zincr
Copy link

zincr bot commented Aug 5, 2019

🤖 zincr found 1 problem , 0 warnings

❌ Approvals
✅ Large Commits
✅ Specification
✅ Dependency Licensing

Details on how to resolve are provided below


Approvals

All proposed changes must be reviewed by project maintainers before they can be merged

Not enough people have approved this pull request - please ensure that 1 additional user, who have not contributed to this pull request approve the changes.

  • ✅ Approved by PR author @nolar
  • ❌ 1 additional approval needed
     

@@ -44,7 +52,7 @@ def append_owner_reference(objs, owner):
owner = build_owner_reference(owner)
for obj in dicts.walk(objs):
refs = obj.setdefault('metadata', {}).setdefault('ownerReferences', [])
matching = [ref for ref in refs if ref['uid'] == owner['uid']]
matching = [ref for ref in refs if ref.get('uid') == owner.get('uid')]
Copy link
Contributor

Choose a reason for hiding this comment

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

is this safe to keep refs with id None if the owner's uid is None?
not sure if both owner's uid and ref's uid can be None is the same time though!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps, ref['uid'] is mandatory. I was replacing these things in almost automated way, didn't pay attention (previously, it would fail with a KeyError in such cases).

The only case, when this can happen so that ref['uid'] is None/absent, is when we create both the owner & the child, and keep them in-memory, not yet posted to the API, therefore not yet having the uids — and then we try to connect them. Adding an additional 2nd owner would hit this mach None == None, despite the 1st & 2nd owners can have different apiVersion/kind/name.

This is quite an artificial case. I'm not even sure if it is possible to add owner references without a uid, just by a name. We can solve it in advance, but maybe as a separate PR — not related to the event-posting. And I'm not even sure how to properly reproduce this scenario.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants