Skip to content

test: code coverage increase for kubelet_client #132484

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 26, 2025
Merged

test: code coverage increase for kubelet_client #132484

merged 1 commit into from
Jun 26, 2025
+343 −34

Conversation

ylink-lfs
Copy link
Contributor

@ylink-lfs ylink-lfs commented Jun 24, 2025 •
edited
Loading

What type of PR is this?

/kind cleanup
/sig node

What this PR does / why we need it:

Test coverage increase for kubelet package

Which issue(s) this PR is related to:

Part of #109717

Special notes for your reviewer:

Before the commits are made, the coverage of kubelet-client is 28.2%:

➜  kubernetes git:(master) ✗ make test WHAT=./pkg/kubelet/client KUBE_COVER=y
+++ [0624 19:43:09] Set GOMAXPROCS automatically to 8
+++ [0624 19:43:09] Normalizing Go targets
+++ [0624 19:43:10] Saving coverage output in '/tmp/k8s_coverage/20250624-194310'
+++ [0624 19:43:10] Running tests with code coverage and with -race
✓  pkg/kubelet/client (3.063s) (coverage: 28.2% of statements)

DONE 3 tests in 6.343s
+++ [0624 19:43:16] Combined coverage report: /tmp/k8s_coverage/20250624-194310/combined-coverage.html

This commit increases the coverage to 92.3%:

➜  kubernetes git:(test/kubelet_client_ut) ✗ make test WHAT=./pkg/kubelet/client KUBE_COVER=y
+++ [0624 19:40:46] Set GOMAXPROCS automatically to 8
+++ [0624 19:40:46] Normalizing Go targets
+++ [0624 19:40:46] Saving coverage output in '/tmp/k8s_coverage/20250624-194046'
+++ [0624 19:40:46] Running tests with code coverage and with -race
✓  pkg/kubelet/client (2.961s) (coverage: 92.3% of statements)

DONE 16 tests in 6.333s
+++ [0624 19:40:51] Combined coverage report: /tmp/k8s_coverage/20250624-194046/combined-coverage.html

Does this PR introduce a user-facing change?

None

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

None

@Copilot Copilot AI review requested due to automatic review settings June 24, 2025 11:50
@k8s-ci-robot k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. sig/node Categorizes an issue or PR as relevant to SIG Node. labels Jun 24, 2025
@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 24, 2025
@k8s-ci-robot
Copy link
Contributor

Hi @ylink-lfs. 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.

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.

Copilot

This comment was marked as outdated.

Sign in to view

bart0sh
bart0sh reviewed Jun 24, 2025
View reviewed changes
bart0sh
bart0sh reviewed Jun 24, 2025
View reviewed changes
bart0sh
bart0sh reviewed Jun 24, 2025
View reviewed changes
@bart0sh
Copy link
Contributor

bart0sh commented Jun 24, 2025

/triage accepted
/ok-to-test
/release-note-none
/priority important-longterm

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jun 24, 2025
@bart0sh bart0sh moved this from Triage to Needs Reviewer in SIG Node: code and documentation PRs Jun 24, 2025
@ylink-lfs ylink-lfs force-pushed the test/kubelet_client_ut branch from 84aa5e6 to 651eeac Compare June 24, 2025 15:24
@ylink-lfs ylink-lfs requested a review from Copilot June 25, 2025 03:26
Copilot

This comment was marked as outdated.

Sign in to view

@ylink-lfs ylink-lfs force-pushed the test/kubelet_client_ut branch 2 times, most recently from 176c5ee to 9336f45 Compare June 25, 2025 03:30
@ylink-lfs ylink-lfs requested a review from Copilot June 25, 2025 03:30
Copilot

This comment was marked as outdated.

Sign in to view

@ylink-lfs ylink-lfs force-pushed the test/kubelet_client_ut branch from 9336f45 to 1f71e67 Compare June 25, 2025 03:34
@ylink-lfs ylink-lfs requested a review from Copilot June 25, 2025 03:34
Copilot
Copilot AI reviewed Jun 25, 2025
View reviewed changes
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR significantly increases unit test coverage for the kubelet/client package by adding helper utilities and covering various transport and node connection code paths.

  • Added kubeletTestCertHelper and kubeletTestRoundTripHelper to DRY up certificate setup and request validation.
  • Expanded tests for MakeTransport, MakeInsecureTransport, and lookup-based dialing.
  • Introduced tests for NewNodeConnectionInfoGetter and its GetConnectionInfo method under multiple node scenarios.
Comments suppressed due to low confidence (2)

pkg/kubelet/client/kubelet_client_test.go:94

  • Consider adding a functional round-trip check (using httptest.NewTLSServer) in TestMakeTransportValid to verify the transport can successfully perform HTTP requests, improving coverage and catching configuration issues.
	if rt == nil {

pkg/kubelet/client/kubelet_client_test.go:99

  • The test name suggests it's testing MakeTransport with Lookup, but it actually calls MakeInsecureTransport. Consider renaming the test function to TestMakeInsecureTransportWithLookup or updating it to invoke MakeTransport as intended.
func TestMakeTransportWithLookUp(t *testing.T) {

@ylink-lfs ylink-lfs requested a review from tallclair June 25, 2025 03:37
bart0sh
bart0sh reviewed Jun 25, 2025
View reviewed changes
bart0sh
bart0sh reviewed Jun 25, 2025
View reviewed changes
bart0sh
bart0sh reviewed Jun 25, 2025
View reviewed changes
@ylink-lfs ylink-lfs force-pushed the test/kubelet_client_ut branch from 1f71e67 to ec6ea22 Compare June 25, 2025 05:39
@ylink-lfs ylink-lfs requested a review from bart0sh June 25, 2025 05:41
@bart0sh
Copy link
Contributor

bart0sh commented Jun 25, 2025

/lgtm

/assign @SergeyKanzhelev @mrunalp
for approval

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 25, 2025
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 8f549f7604c4c0da0fb57cf6496390d745b29698

@bart0sh bart0sh moved this from Needs Reviewer to Needs Approver in SIG Node: code and documentation PRs Jun 25, 2025
@tallclair
Copy link
Member

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tallclair, ylink-lfs

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

The pull request process is described here

Needs approval from an approver in each of these files:

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 k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 25, 2025
@k8s-ci-robot k8s-ci-robot merged commit 17c5e75 into kubernetes:master Jun 26, 2025
12 of 14 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.34 milestone Jun 26, 2025
@github-project-automation github-project-automation bot moved this from Needs Approver to Done in SIG Node: code and documentation PRs Jun 26, 2025
@ylink-lfs ylink-lfs deleted the test/kubelet_client_ut branch June 30, 2025 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@bobbypage bobbypage Awaiting requested review from bobbypage

@Random-Liu Random-Liu Awaiting requested review from Random-Liu

@tallclair tallclair Awaiting requested review from tallclair

@bart0sh bart0sh Awaiting requested review from bart0sh

Assignees

@mrunalp mrunalp

@SergeyKanzhelev SergeyKanzhelev

@bart0sh bart0sh

@tallclair tallclair

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubelet cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. release-note-none Denotes a PR that doesn't merit a release note. sig/node Categorizes an issue or PR as relevant to SIG Node. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
SIG Node: code and documentation PRs
Status: Done
Milestone
v1.34
Development

Successfully merging this pull request may close these issues.
6 participants
@ylink-lfs @k8s-ci-robot @bart0sh @tallclair @mrunalp @SergeyKanzhelev