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

[CP-7738] attempt to publish the workload name of a pod #20

Closed
wants to merge 1 commit into from

Conversation

dmepham
Copy link

@dmepham dmepham commented Nov 10, 2023

Description of the issue

Currently, the addPodOwnersAndPodName method attempts to find the "pod name" of from the OwnerReferences attribute of a pod and sets the PodName tag accordingly. If the pod is from a statefulset, the PodName is set as the actual pod name. If the pod is from some other type of workload resource, PodName is set to the name of that workload resource. If the pod is from kube-proxy, that PodName is set to "kube-proxy". if no name can be found from previous conditions, the PodName is set to the Name attribute of the pod.

The issues with this is that setting PodName to the name of the workload resource that controls it is inaccurate. It is also inconsistent, because statefulset pods are set to the name of the statefulset pod.

Description of changes

This PR alters this method to instead set a new tag, WorkloadName, and removes the special case for statefulsets. This means that a WorkloadName and PodName tags will exist. WorkloadName should be the name of the workload resource from the pod, or an empty string if it cannot be determined. podName should be the actual name of the pod.

License

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Tests

Updated existing unit tests to expect the WorkloadName tag instead of PodName. Some existing and unrelated tests are failing

Requirements

Before commit the code, please do the following steps.

  1. Run make fmt and make fmt-sh
  2. Run make linter

Copy link
Collaborator

@bdrennz bdrennz left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@dmepham dmepham closed this Dec 11, 2023
@dmepham dmepham deleted the CP-7738-add-workload-tag branch December 11, 2023 23:08
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