-
Notifications
You must be signed in to change notification settings - Fork 40k
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
Add unit tests for Kubernetes apiclient waiter functions #128592
Conversation
This commit introduces comprehensive unit tests for critical functions in the `apiclient` package, specifically within `wait.go`. These tests are designed to ensure the reliability and correctness of various waiter functions that are essential for monitoring and managing Kubernetes control plane components. ### Added Tests: 1. **TestGetControlPlaneComponents**: Validates the `getControlPlaneComponents` function by checking that it correctly constructs control plane component URLs based on configuration inputs. 2. **TestGetStaticPodSingleHash**: Ensures that `getStaticPodSingleHash` retrieves the correct hash value from pod annotations. 3. **TestGetStaticPodSingleHash_PodNotFound**: Verifies that `getStaticPodSingleHash` returns an appropriate error when the specified pod does not exist. 4. **TestWaitForStaticPodSingleHash**: Confirms that `WaitForStaticPodSingleHash` waits for and retrieves the correct hash value for a static pod. 5. **TestWaitForPodsWithLabel**: Checks that `WaitForPodsWithLabel` only returns successfully when all pods with a specific label are in the `Running` phase, simulating different pod states. 6. **TestWaitForStaticPodHashChange**: Validates that `WaitForStaticPodHashChange` accurately detects changes in the hash of a static pod, which is critical for determining if the pod has been restarted. 7. **TestWaitForStaticPodHashChange_Timeout**: Ensures that `WaitForStaticPodHashChange` times out appropriately if the pod hash does not change. 8. **TestWaitForStaticPodControlPlaneHashes**: Verifies that `WaitForStaticPodControlPlaneHashes` successfully retrieves hashes for all control plane static pods. ### Implementation Notes: - **Mock Kubernetes Client**: These tests use `client-go/fake` to simulate interactions with the Kubernetes API server, allowing for isolated and predictable testing without requiring an actual cluster. - **HTTP Server Simulation**: `httptest` is used to create mock HTTP responses for the health endpoints, testing the HTTP polling logic in functions like `WaitForControlPlaneComponents` and `WaitForAPI`. - **Timeout Handling**: All tests are written with specific timeouts to avoid hanging indefinitely. Short timeouts are used in certain cases to simulate realistic polling intervals. These tests enhance the robustness of the `apiclient` package by ensuring the core waiter functions handle expected scenarios and edge cases effectively. The coverage gained by these tests will assist in catching regressions and improve confidence in code modifications going forward.
This commit introduces comprehensive unit tests for critical functions in the `apiclient` package, specifically within `wait.go`. These tests are designed to ensure the reliability and correctness of various waiter functions that are essential for monitoring and managing Kubernetes control plane components. ### Added Tests: 1. **TestGetControlPlaneComponents**: Validates the `getControlPlaneComponents` function by checking that it correctly constructs control plane component URLs based on configuration inputs. 2. **TestGetStaticPodSingleHash**: Ensures that `getStaticPodSingleHash` retrieves the correct hash value from pod annotations. 3. **TestGetStaticPodSingleHash_PodNotFound**: Verifies that `getStaticPodSingleHash` returns an appropriate error when the specified pod does not exist. 4. **TestWaitForStaticPodSingleHash**: Confirms that `WaitForStaticPodSingleHash` waits for and retrieves the correct hash value for a static pod. 5. **TestWaitForPodsWithLabel**: Checks that `WaitForPodsWithLabel` only returns successfully when all pods with a specific label are in the `Running` phase, simulating different pod states. 6. **TestWaitForStaticPodHashChange**: Validates that `WaitForStaticPodHashChange` accurately detects changes in the hash of a static pod, which is critical for determining if the pod has been restarted. 7. **TestWaitForStaticPodHashChange_Timeout**: Ensures that `WaitForStaticPodHashChange` times out appropriately if the pod hash does not change. 8. **TestWaitForStaticPodControlPlaneHashes**: Verifies that `WaitForStaticPodControlPlaneHashes` successfully retrieves hashes for all control plane static pods. ### Implementation Notes: - **Mock Kubernetes Client**: These tests use `client-go/fake` to simulate interactions with the Kubernetes API server, allowing for isolated and predictable testing without requiring an actual cluster. - **HTTP Server Simulation**: `httptest` is used to create mock HTTP responses for the health endpoints, testing the HTTP polling logic in functions like `WaitForControlPlaneComponents` and `WaitForAPI`. - **Timeout Handling**: All tests are written with specific timeouts to avoid hanging indefinitely. Short timeouts are used in certain cases to simulate realistic polling intervals. These tests enhance the robustness of the `apiclient` package by ensuring the core waiter functions handle expected scenarios and edge cases effectively. The coverage gained by these tests will assist in catching regressions and improve confidence in code modifications going forward.
@gnarayan1337: The label(s) In response to this:
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. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: gnarayan1337 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The 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. |
Hi @gnarayan1337. 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 Once the patch is verified, the new status will be reflected by the 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the PR, please use the sub test approach.
// Call the function under test | ||
actual := getControlPlaneComponents(tc.cfg) | ||
// Compare the expected and actual results |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these comments are not needed
|
||
kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" | ||
) | ||
|
||
// TestGetControlPlaneComponents tests that getControlPlaneComponents returns the correct control plane components and their URLs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't add comments above test functions
|
||
// TestGetStaticPodSingleHash tests that getStaticPodSingleHash correctly retrieves the static pod hash from annotations. | ||
func TestGetStaticPodSingleHash(t *testing.T) { | ||
// Initialize a fake clientset |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all of these comments are not needed as the test code is rather simple.
} | ||
|
||
// TestGetStaticPodSingleHash_PodNotFound tests that getStaticPodSingleHash returns an error when the pod is not found. | ||
func TestGetStaticPodSingleHash_PodNotFound(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we use subtests with testcases instead of seperate functions that do a different test.
check how TestGetControlPlaneComponents is doing it.
podName := fmt.Sprintf("%s-%s", component, nodeName) | ||
expectedHash := "abc123" | ||
|
||
// Create annotations with the expected hash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here and everywhere else.
/release-note-none to test locally type:
|
please limit the number of commits to 1. |
PR needs rebase. 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. |
@gnarayan1337 are you going to continue work on this PR? |
/close |
@neolit123: Closed this PR. In response to this:
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. |
What type of PR is this?
/kind cleanup /kind testing
What this PR does / why we need it:
This PR introduces unit tests for key functions in the apiclient package, specifically within wait.go. These tests enhance the reliability and maintainability of the package by verifying the behavior of core waiter functions used in managing Kubernetes control plane components. The tests cover essential functionality, ensuring that functions perform as expected under various scenarios, including both normal and edge cases.
Which issue(s) this PR fixes:
Added unit tests
Special notes for your reviewer:
The tests use client-go/fake to simulate the Kubernetes API server, and httptest to mock HTTP health check responses. Each test includes detailed assertions for expected outcomes, enabling robust validation of each function’s behavior. Please see individual test cases for specific scenarios covered.
Does this PR introduce a user-facing change? No
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: None