migrate read only tests from otp to origin#31239
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds a control-plane e2e test that verifies kube-apiserver /readyz?verbose via oc port-forward, registers the suite in staticSuites and the extended test bundle, and adds an OWNERS file for the test package. ChangesControl Plane Readyz E2E Test
Sequence DiagramsequenceDiagram
participant TestSuite
participant KubeAPI
participant KubeAPIPod as kube-apiserver Pod
participant PortFwd as oc port-forward
participant LocalHTTP as localhost:port/readyz?verbose
TestSuite->>KubeAPI: List pods in openshift-kube-apiserver (filter kube-apiserver-ip*)
KubeAPI-->>TestSuite: Pod list
TestSuite->>PortFwd: Start subprocess forwarding random localhost:port -> pod:insecureReadyzPort
PortFwd->>KubeAPIPod: Forward traffic to readyz port
TestSuite->>LocalHTTP: Poll localhost:port/readyz?verbose
LocalHTTP-->>TestSuite: 200 response containing "ok"
TestSuite->>TestSuite: Assert response contains "ok"
TestSuite->>PortFwd: Cleanup: cancel/kill port-forward subprocess
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 2 warnings, 1 inconclusive)
✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/assign @gangwgr |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
test/extended/otp/otp_tests.go (2)
68-68: 💤 Low valueConsider checking port availability before use.
Random port selection without availability checking could lead to conflicts, though the risk is low since the suite runs serially (
Parallelism: 1). Consider using a more robust approach like binding to:0and letting the OS assign an available port, or checking availability before use.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/extended/otp/otp_tests.go` at line 68, The test currently picks a random port with localPort := rand.Intn(65534-1025) + 1025 which can collide; replace that approach by letting the OS pick an available port (use a temporary TCP listener on ":0" to obtain the assigned port) or explicitly check availability before using localPort. Concretely, in otp_tests.go change the random port allocation to create a net.Listen("tcp", ":0"), read the listener's Addr to set localPort (or use the listener directly for server startup), then close or reuse the listener as appropriate so the test always uses an available port.
99-99: ⚖️ Poor tradeoffHTTP client lacks explicit timeout but is protected by polling timeout.
The
http.Getcall doesn't specify a timeout, which could potentially hang. However, since it's wrapped inwait.PollUntilContextTimeoutwith a 30-second timeout, the risk is mitigated. For better control and faster failure detection on hung requests, consider using an HTTP client with an explicit shorter timeout.♻️ Optional improvement with explicit HTTP timeout
+ // Create HTTP client with timeout + httpClient := &http.Client{ + Timeout: 5 * time.Second, + } + var responseBody string var lastErr error err = wait.PollUntilContextTimeout(ctx, 500*time.Millisecond, portForwardTimeout, true, func(ctx context.Context) (bool, error) { - resp, err := http.Get(fmt.Sprintf("http://localhost:%d/readyz?verbose", localPort)) + resp, err := httpClient.Get(fmt.Sprintf("http://localhost:%d/readyz?verbose", localPort))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/extended/otp/otp_tests.go` at line 99, The http.Get call in otp_tests.go can hang; replace it with an explicit http.Client that sets a reasonable Timeout (e.g., a few seconds) and call client.Get(fmt.Sprintf("http://localhost:%d/readyz?verbose", localPort)) inside the existing wait.PollUntilContextTimeout block (keep using wait.PollUntilContextTimeout with its 30s overall timeout), and ensure the response body is closed after reading; this gives faster failed-request detection while preserving the polling behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/extended/otp/otp_tests.go`:
- Line 106: The call to io.ReadAll(resp.Body) currently ignores its error;
change the assignment to capture the error (e.g., body, err :=
io.ReadAll(resp.Body)) and handle it immediately (for example with
require.NoError(t, err) or t.Fatalf) before using the response body so the test
fails clearly if the read fails; update the usage in the test function that
reads resp.Body to reference the new err variable and ensure the response body
is only used after the error check.
---
Nitpick comments:
In `@test/extended/otp/otp_tests.go`:
- Line 68: The test currently picks a random port with localPort :=
rand.Intn(65534-1025) + 1025 which can collide; replace that approach by letting
the OS pick an available port (use a temporary TCP listener on ":0" to obtain
the assigned port) or explicitly check availability before using localPort.
Concretely, in otp_tests.go change the random port allocation to create a
net.Listen("tcp", ":0"), read the listener's Addr to set localPort (or use the
listener directly for server startup), then close or reuse the listener as
appropriate so the test always uses an available port.
- Line 99: The http.Get call in otp_tests.go can hang; replace it with an
explicit http.Client that sets a reasonable Timeout (e.g., a few seconds) and
call client.Get(fmt.Sprintf("http://localhost:%d/readyz?verbose", localPort))
inside the existing wait.PollUntilContextTimeout block (keep using
wait.PollUntilContextTimeout with its 30s overall timeout), and ensure the
response body is closed after reading; this gives faster failed-request
detection while preserving the polling behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 5d064165-f32d-4679-acf6-482dcefeb28a
📒 Files selected for processing (4)
pkg/testsuites/standard_suites.gotest/extended/include.gotest/extended/otp/OWNERStest/extended/otp/otp_tests.go
| e2e.Logf("Found %d kube-apiserver pods to test", len(targets)) | ||
|
|
||
| for _, target := range targets { | ||
| target := target // capture loop variable |
There was a problem hiding this comment.
This line prevents a common Go bug with loop variables and closures.
The Problem:
In Go, when loop variable is used inside a function that executes later (like deferred cleanup or goroutines), all iterations share the same variable. Without capturing it, they all end
up using the last value from the loop.
Example of the bug:
for _, target := range targets {
g.DeferCleanup(func() {
// BUG: This will use the LAST target from the loop for ALL cleanups!
cleanup(target.podName)
})
}
The fix:
for _, target := range targets {
target := target // Creates a NEW variable for THIS iteration
g.DeferCleanup(func() {
// Now this captures the correct target for this iteration
cleanup(target.podName)
})
}
In this test:
The g.DeferCleanup() function registers cleanup code that runs after the test completes. Without target := target, all three cleanup functions would try to clean up the same (last) pod
instead of their respective pods.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
test/extended/otp/otp_tests.go (1)
112-123:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse a context/timeout-aware HTTP request for the readyz probe (and handle ignored errors).
http.Getin thePollUntilContextTimeoutcallback ignores the callback context and has no client timeout, so a stalled request can hang/flake beyondportForwardTimeout.
- Replace with
http.NewRequestWithContext(ctx, ...)and anhttp.Client{Timeout: ...}.- Don’t ignore
io.ReadAll(resp.Body)errors (body, err := ...).- Don’t discard
Process.Kill()errors in theDeferCleanupcleanup.Proposed fix
- err = wait.PollUntilContextTimeout(ctx, 500*time.Millisecond, portForwardTimeout, true, func(ctx context.Context) (bool, error) { - resp, err := http.Get(fmt.Sprintf("http://localhost:%d/readyz?verbose", localPort)) + httpClient := &http.Client{Timeout: 5 * time.Second} + err = wait.PollUntilContextTimeout(ctx, 500*time.Millisecond, portForwardTimeout, true, func(ctx context.Context) (bool, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, fmt.Sprintf("http://localhost:%d/readyz?verbose", localPort), nil) + if err != nil { + lastErr = err + return false, nil + } + resp, err := httpClient.Do(req) if err != nil { lastErr = err return false, nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/extended/otp/otp_tests.go` around lines 112 - 123, The readyz probe uses http.Get inside wait.PollUntilContextTimeout which ignores the callback context and lacks a request timeout; replace the http.Get call with an HTTP request created via http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("http://localhost:%d/readyz?verbose", localPort), nil) and execute it with an http.Client that has a sensible Timeout (e.g., derived from portForwardTimeout or a fraction thereof) so requests are canceled by the provided ctx; capture and handle errors returned by io.ReadAll (assign body, err := io.ReadAll(resp.Body) and return the error if non-nil) and ensure the DeferCleanup that calls Process.Kill() checks and logs/returns that error instead of discarding it so failures to kill the process are not ignored.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/extended/otp/otp_tests.go`:
- Around line 103-105: The cleanup currently ignores the error from
portForwardCmd.Process.Kill(); change it to capture the returned error, and if
err != nil and err != os.ErrProcessDone then log or fail the test (e.g., using
the test logger or t.Errorf/t.Logf) so failures are not swallowed. Locate the
Kill call on portForwardCmd.Process (in the OTP test cleanup block) and replace
the blank-assignment with error handling that compares to os.ErrProcessDone and
reports any other error.
---
Duplicate comments:
In `@test/extended/otp/otp_tests.go`:
- Around line 112-123: The readyz probe uses http.Get inside
wait.PollUntilContextTimeout which ignores the callback context and lacks a
request timeout; replace the http.Get call with an HTTP request created via
http.NewRequestWithContext(ctx, "GET",
fmt.Sprintf("http://localhost:%d/readyz?verbose", localPort), nil) and execute
it with an http.Client that has a sensible Timeout (e.g., derived from
portForwardTimeout or a fraction thereof) so requests are canceled by the
provided ctx; capture and handle errors returned by io.ReadAll (assign body, err
:= io.ReadAll(resp.Body) and return the error if non-nil) and ensure the
DeferCleanup that calls Process.Kill() checks and logs/returns that error
instead of discarding it so failures to kill the process are not ignored.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 7e2062e6-d6c1-41bb-8692-339a733990b0
📒 Files selected for processing (4)
pkg/testsuites/standard_suites.gotest/extended/include.gotest/extended/otp/OWNERStest/extended/otp/otp_tests.go
✅ Files skipped from review due to trivial changes (1)
- test/extended/otp/OWNERS
🚧 Files skipped from review as they are similar to previous changes (1)
- test/extended/include.go
|
/test images |
1 similar comment
|
/test images |
|
/test okd-scos-images |
| This suite comprises the generic control plane tests from the openshift-tests-private repo. | ||
| `), | ||
| Qualifiers: []string{ | ||
| withStandardEarlyOrLateTests(`name.contains("[Suite:openshift/controlplaneotp]")`), |
| @@ -0,0 +1,130 @@ | |||
| package otp | |||
There was a problem hiding this comment.
rename file and directory to controlplane
| @@ -0,0 +1,8 @@ | |||
| approvers: | |||
|
Scheduling required tests: |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
test/extended/controlplane/controlplane_tests.go (2)
86-86: 💤 Low valueRemove redundant defer.
The
defer pfCancel()at line 86 is redundant becauseg.DeferCleanupat line 102 already callspfCancel(). SinceDeferCleanupis the proper Ginkgo cleanup mechanism and handles both success and failure cases, the explicit defer adds no value and could cause confusion.♻️ Proposed fix
// Create context with timeout for port-forward pfCtx, pfCancel := context.WithCancel(ctx) - defer pfCancel()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/extended/controlplane/controlplane_tests.go` at line 86, Remove the redundant defer by deleting the explicit "defer pfCancel()" call and rely on the Ginkgo cleanup registration already present via "g.DeferCleanup(pfCancel)" (or the existing "g.DeferCleanup" call) to invoke pfCancel; ensure only the cleanup registration (g.DeferCleanup with pfCancel) remains so pfCancel is executed once through the Ginkgo cleanup mechanism.
113-113: 💤 Low valueConsider adding HTTP client timeout.
The
http.Getcall uses the default client, which has no timeout. If the port-forward is established but the readyz endpoint hangs, the request could block until the outer poll timeout expires, making debugging harder.⏱️ Proposed improvement
+ httpClient := &http.Client{Timeout: 5 * time.Second} err = wait.PollUntilContextTimeout(ctx, 500*time.Millisecond, portForwardTimeout, true, func(ctx context.Context) (bool, error) { - resp, err := http.Get(fmt.Sprintf("http://localhost:%d/readyz?verbose", localPort)) + resp, err := httpClient.Get(fmt.Sprintf("http://localhost:%d/readyz?verbose", localPort))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/extended/controlplane/controlplane_tests.go` at line 113, The http.Get call in the test (the readyz probe in controlplane_tests.go) uses the default client with no timeout; replace it with a request using an http.Client with a Deadline/Timeout or create the request with context.WithTimeout and use http.DefaultClient.Do to enforce a short timeout (e.g., a few seconds) so the test won't hang; ensure you still check and close resp.Body after Do to avoid leaks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/extended/controlplane/controlplane_tests.go`:
- Line 120: The call to io.ReadAll(resp.Body) currently ignores its error;
update that line to capture the error (e.g., body, err := io.ReadAll(resp.Body))
and handle it by failing the test with a clear message (for example using
t.Fatalf("reading response body: %v", err) or require.NoError(t, err)) so any
read failure is reported instead of proceeding with an empty body; locate the
io.ReadAll(resp.Body) usage in controlplane_tests.go and apply this change.
- Around line 103-105: The cleanup currently ignores the error returned by
portForwardCmd.Process.Kill(); update the cleanup block that references
portForwardCmd and portForwardCmd.Process to call Kill() and check its error:
import the os package if missing, call err := portForwardCmd.Process.Kill(),
then if err != nil && !errors.Is(err, os.ErrProcessDone) log or report the error
(using the existing test logger or t.Logf/t.Errorf) so only os.ErrProcessDone is
ignored and all other Kill errors are surfaced.
---
Nitpick comments:
In `@test/extended/controlplane/controlplane_tests.go`:
- Line 86: Remove the redundant defer by deleting the explicit "defer
pfCancel()" call and rely on the Ginkgo cleanup registration already present via
"g.DeferCleanup(pfCancel)" (or the existing "g.DeferCleanup" call) to invoke
pfCancel; ensure only the cleanup registration (g.DeferCleanup with pfCancel)
remains so pfCancel is executed once through the Ginkgo cleanup mechanism.
- Line 113: The http.Get call in the test (the readyz probe in
controlplane_tests.go) uses the default client with no timeout; replace it with
a request using an http.Client with a Deadline/Timeout or create the request
with context.WithTimeout and use http.DefaultClient.Do to enforce a short
timeout (e.g., a few seconds) so the test won't hang; ensure you still check and
close resp.Body after Do to avoid leaks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 8e2d47b4-2c0a-46d4-aa02-aa9ccf41ae29
📒 Files selected for processing (4)
pkg/testsuites/standard_suites.gotest/extended/controlplane/OWNERStest/extended/controlplane/controlplane_tests.gotest/extended/include.go
✅ Files skipped from review due to trivial changes (1)
- test/extended/controlplane/OWNERS
| if portForwardCmd.Process != nil { | ||
| _ = portForwardCmd.Process.Kill() | ||
| } |
There was a problem hiding this comment.
Handle Process.Kill() errors in port-forward cleanup.
The cleanup code ignores the error from portForwardCmd.Process.Kill(). While os.ErrProcessDone can be safely ignored, other errors (e.g., permission issues) should be logged so cleanup failures aren't silently lost. As per coding guidelines, Go code must never ignore error returns.
🛡️ Proposed fix to handle the error
Import the os package at the top of the file if not already present:
import (
"context"
"fmt"
"io"
"math/rand"
"net/http"
+ "os"
"os/exec"Then update the cleanup logic:
g.DeferCleanup(func() {
e2e.Logf("Cleaning up port-forward for pod %s", target.podName)
pfCancel()
if portForwardCmd.Process != nil {
- _ = portForwardCmd.Process.Kill()
+ if err := portForwardCmd.Process.Kill(); err != nil && err != os.ErrProcessDone {
+ e2e.Logf("Warning: failed to kill port-forward process for pod %s: %v", target.podName, err)
+ }
}
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if portForwardCmd.Process != nil { | |
| _ = portForwardCmd.Process.Kill() | |
| } | |
| if portForwardCmd.Process != nil { | |
| if err := portForwardCmd.Process.Kill(); err != nil && err != os.ErrProcessDone { | |
| e2e.Logf("Warning: failed to kill port-forward process for pod %s: %v", target.podName, err) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/extended/controlplane/controlplane_tests.go` around lines 103 - 105, The
cleanup currently ignores the error returned by portForwardCmd.Process.Kill();
update the cleanup block that references portForwardCmd and
portForwardCmd.Process to call Kill() and check its error: import the os package
if missing, call err := portForwardCmd.Process.Kill(), then if err != nil &&
!errors.Is(err, os.ErrProcessDone) log or report the error (using the existing
test logger or t.Logf/t.Errorf) so only os.ErrProcessDone is ignored and all
other Kill errors are surfaced.
| } | ||
| defer resp.Body.Close() | ||
|
|
||
| body, _ := io.ReadAll(resp.Body) |
There was a problem hiding this comment.
Handle error from io.ReadAll.
The error returned by io.ReadAll is ignored. If reading the response body fails, the test will proceed with an empty responseBody and potentially produce a misleading assertion failure. As per coding guidelines, Go code must never ignore error returns.
🛡️ Proposed fix to handle the error
- body, _ := io.ReadAll(resp.Body)
- responseBody = string(body)
+ body, err := io.ReadAll(resp.Body)
+ if err != nil {
+ lastErr = fmt.Errorf("failed to read response body: %w", err)
+ return false, nil
+ }
+ responseBody = string(body)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/extended/controlplane/controlplane_tests.go` at line 120, The call to
io.ReadAll(resp.Body) currently ignores its error; update that line to capture
the error (e.g., body, err := io.ReadAll(resp.Body)) and handle it by failing
the test with a clear message (for example using t.Fatalf("reading response
body: %v", err) or require.NoError(t, err)) so any read failure is reported
instead of proceeding with an empty body; locate the io.ReadAll(resp.Body) usage
in controlplane_tests.go and apply this change.
| } | ||
| }) | ||
|
|
||
| g.It("OCP-24698 should have accessible HTTP readyz endpoints for all kube-apiserver pods", func(ctx context.Context) { |
| Qualifiers: []string{ | ||
| withStandardEarlyOrLateTests(`name.contains("[Suite:openshift/controlplane]")`), | ||
| }, | ||
| Parallelism: 1, |
| withStandardEarlyOrLateTests(`name.contains("[Suite:openshift/controlplane]")`), | ||
| }, | ||
| Parallelism: 1, | ||
| TestTimeout: 15 * time.Minute, |
There was a problem hiding this comment.
remove this, I think not needed
|
/lgtm |
|
/test okd-scos-images |
|
/retest |
|
Scheduling required tests: |
|
/retest |
1 similar comment
|
/retest |
|
/lgtm |
|
/verified by CI. |
openshift-ci-robot
added
the
verified
|
@sandeepknd: This PR has been marked as verified by DetailsIn 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/approve |
|
/assign @cpmeadors |
|
/assign @xueqzhan |
|
/assign @bertinatto |
xueqzhan
left a comment
xueqzhan
left a comment
There was a problem hiding this comment.
Can you help me understand the added value for this test? Isn't readyz port being sampled constantly by kubelet? Do we not have other tests acting on the missing readyz events? I just want to make sure we are adding tests that are really increasing coverage.
| namespace string | ||
| } | ||
|
|
||
| var _ = g.Describe("[sig-api-machinery][Suite:openshift/controlplane] Control Plane", func() { |
There was a problem hiding this comment.
I suggest not to use [Suite:] in your test name. You can just use [openshift/controlplane] and then match it in the Suite selection. [Suite:] is a standard one used by the framework.
There was a problem hiding this comment.
I followed the existing suites format. All those use "Suite" in the name.
More importantly, when I removed it, its unable to detect the test case.
$ OPENSHIFT_SKIP_EXTERNAL_TESTS=1 ./openshift-tests run-test "[sig-api-machinery][openshift/controlplane][OTP] OCP-24698 should have accessible HTTP readyz endpoints for all kube-apiserver pods"
I0603 01:05:58.665617 151043 factory.go:195] Registered Plugin "containerd"
I0603 01:05:59.107432 151043 binary.go:80] Found 8895 test specs
I0603 01:05:59.112124 151043 binary.go:97] 1226 test specs remain, after filtering out k8s
openshift-tests v4.1.0-11404-g58ddcec
Error: no such tests: [sig-api-machinery][openshift/controlplane][OTP] OCP-24698 should have accessible HTTP readyz endpoints for all kube-apiserver pods
error: no such tests: [sig-api-machinery][openshift/controlplane][OTP] OCP-24698 should have accessible HTTP readyz endpoints for all kube-apiserver pods
There was a problem hiding this comment.
Do you intend to include your test in [Suite:openshift/conformance/parallel] at all? If not, then your way is ok. But if you intend to include this test in the default conformance test, you might want to work with existing framework.
The reason your local run couldn't find the test was because appendSuiteNames appends to your test the following string "[Suite:openshift/conformance/parallel]"
| { | ||
| Name: "openshift/controlplane", | ||
| Description: templates.LongDesc(` | ||
| This suite comprises the generic control plane tests from the openshift-tests-private repo. |
There was a problem hiding this comment.
Do you intend to use this for non OTP test as well? Maybe make this generic enough and do not restrict it to openshift-tests-private in case you will add new tests here later?
|
New changes are detected. LGTM label has been removed. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: gangwgr, sandeepknd The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Its primarily a read-only test that checks the kube-api-server readyz status. It can be considered a basic day0 test. |
|
Scheduling required tests: |
|
@sandeepknd: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
@benluddy I might need your help here. Across control plane's OTP migration effort, do we have guidelines as which tests should be migrated and which ones should not? For example, the test added in this PR simply tunnels through requests directly to the kube-apiserver's readyz endpoints and make sure it works. Are we adding extra coverage with this? We are struggling with too many tests running in our CI jobs already. But it does add extra coverage, we should definitely port. |
7 participants
Our intention is to migrate the read-only tests of control plane modules from openshift-tests-private to origin.
Started with test OCP-24698 .
Please find the execution logs, with the latest review comments addressed.
Summary by CodeRabbit
Release Notes
Tests
Chores