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

feature: refactor pod enhancer so all workload resource types are used #74

Merged
merged 5 commits into from
Dec 19, 2023

Conversation

Jiahui-Zhang-20
Copy link
Contributor

Previously:

The pod enhancer does not consider workload resource types such as ReplicaSet and Deployment. There were several k8s API calls for retrieving throughout the project. This makes it hard to refactor and use the indexer for caching. This is because each API call has to be specific to the object type (e.g. Pod, Job, Deployment...)

Now:

The query logic is refactored with a function named findObject that generically returns metav1.Object. The cached object is also no longer justinterface{} but instead metav1.Object.

In order to associate a pod state change to any owning objects, we use DFS to find all owning objects (usually one) of type metav1.Object. Then the pod enhancer calls specific owning object enhancers to add metadata to the sentry event.

enhancers_pod.go Outdated Show resolved Hide resolved
enhancers_pod.go Outdated Show resolved Hide resolved
Copy link
Contributor

@tonyo tonyo left a comment

Choose a reason for hiding this comment

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

A few minor comments, otherwise good stuff 👍

watcher_pods.go Outdated Show resolved Hide resolved
enhancers.go Show resolved Hide resolved
enhancers.go Outdated Show resolved Hide resolved
sentry_dsn_data.go Outdated Show resolved Hide resolved
watcher_events_test.go Outdated Show resolved Hide resolved
watcher_pods.go Outdated Show resolved Hide resolved
Copy link
Contributor

@tonyo tonyo left a comment

Choose a reason for hiding this comment

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

Thanks for updating, just one non-blocking suggestion.

enhancers.go Show resolved Hide resolved
enhancers_test.go Outdated Show resolved Hide resolved
@Jiahui-Zhang-20 Jiahui-Zhang-20 merged commit 5b0689b into master Dec 19, 2023
3 checks passed
@Jiahui-Zhang-20 Jiahui-Zhang-20 deleted the jz-extend-pod-enhancer branch December 19, 2023 18:02
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