VPA: match CronJob pods when jobTemplate pod labels are omitted

[isumitsolanki]

What type of PR is this?

/kind bug

What this PR does / why we need it:

When a CronJob omits spec.jobTemplate.spec.template.labels, the field is nil. Building the selector via metav1.SetAsLabelSelector(nil) leads to metav1.LabelSelectorAsSelector(nil), i.e. labels.Nothing(), so no pod matched the VPA despite correct targetRef and parent-controller association. This PR returns labels.Everything() when the job pod template labels are nil or empty, and adds unit tests in pkg/target.

Which issue(s) this PR fixes:

Fixes #9483

Special notes for your reviewer:

Aligns omitted labels with an explicit empty map labels: {} on the job pod template. If multiple VPAs in one namespace only differed by this path, recommender overlap was already possible with {}; this change makes nil behave the same.

Does this PR introduce a user-facing change?

Fixed Vertical Pod Autoscaler not applying recommendations to pods from CronJobs whose job pod template labels were omitted (nil). VPA now matches those pods the same as when an empty label map is set.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: isumitsolanki
Once this PR has been reviewed and has the lgtm label, please assign voelzmo for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• vertical-pod-autoscaler/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[k8s-ci-robot]

Welcome @isumitsolanki!

It looks like this is your first PR to kubernetes/autoscaler 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/autoscaler has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @isumitsolanki. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[adrianmoisey]

/ok-to-test

[adrianmoisey]

I'm not sure that labels.Everything() is the right solution here. I'm pretty sure this will match all pods in that namespace, which may cause performance issues.

[omerap12]

I agree. in large clusters this can be very problematic.

[isumitsolanki]

I never thought in that direction thanks for highlighting this aspect @adrianmoisey @omerap12
what could be the right approach here ?can you please help here ?

[adrianmoisey]

In the original issue where we discussed this, we concluded to document the behaviour, rather than fix it.

[omerap12]

Yup. @adrianmoisey , will it be beneficial if we would print something when user apply VPA with targetRef pointing at CronJob/Job?

[adrianmoisey]

I don't think we can. Returning a warning doesn't feel right for this