CNTRLPLANE-1579: E2E test: Add support for event-ttl in Kube API Server Operator #1967
Conversation
|
@gangwgr: This pull request references CNTRLPLANE-1579 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.21.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
jira/valid-reference
|
Warning Rate limit exceeded@gangwgr has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 42 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
WalkthroughMade test command and registry construction error-aware; replaced static test registration with auto-discovery and dynamic Go-test adapter; added an EventTTL e2e test; updated go.mod with ginkgo/gomega and related dependency changes. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: gangwgr 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 |
![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/e2e/event-ttl.go (1)
209-422: Consider polling for event deletion instead of fixedSleepto reduce test duration and flakinessThe per‑TTL test flow is clear and well‑structured, but the TTL validation currently:
- Sleeps for the full
waitDuration := (ttl + bufferMinutes) * time.Minute(Line 407).- Then checks once that the event is gone and errors contain
"not found".This has a few effects:
- Tests always run for the full TTL+buffer even if the event expires much earlier.
- If the event is briefly missing/available due to eventual consistency, a single
Getright after the sleep can be a bit brittle.- For the 15‑minute case, this guarantees at least an 18‑minute wait just for the TTL portion.
You could instead poll for deletion with a reasonable interval and a hard timeout, which both shortens successful runs and gives you more observability when the TTL isn’t honored. For example:
- time.Sleep(waitDuration) - - // Verify event is deleted - actualExpirationTime := time.Now() - _, err = kubeClient.CoreV1().Events(testNamespace).Get(ctx, eventName, metav1.GetOptions{}) - o.Expect(err).To(o.HaveOccurred(), "event should be deleted after TTL") - o.Expect(err.Error()).To(o.ContainSubstring("not found"), "event should return 'not found' error") + pollInterval := 30 * time.Second + var actualExpirationTime time.Time + err = wait.PollUntilContextTimeout(ctx, pollInterval, waitDuration, false, func(cxt context.Context) (bool, error) { + _, getErr := kubeClient.CoreV1().Events(testNamespace).Get(cxt, eventName, metav1.GetOptions{}) + if getErr != nil && strings.Contains(getErr.Error(), "not found") { + actualExpirationTime = time.Now() + return true, nil + } + return false, nil + }) + o.Expect(err).NotTo(o.HaveOccurred(), "event should be deleted within TTL+buffer")This keeps the upper bound the same but allows the test to finish earlier on success and provides more structured handling if deletion is delayed.
cmd/cluster-kube-apiserver-operator-tests-ext/main.go (1)
63-165: Suite partitioning and spec post‑processing look consistent; double‑check Serial+Slow routingThe new OTE wiring looks thoughtfully structured:
conformance/parallel: anything without[Serial],[Slow], or[Timeout:].conformance/serial:[Serial]tests that are not[Slow], withParallelism: 1.optional/slow: all[Slow]tests, plus[Timeout:]tests that are not[Serial], also withParallelism: 1.all: catch‑all suite.Combined with the
[Disruptive]→[Serial][Disruptive]normalization, this ensures disruptive tests can’t accidentally land in the parallel suite, and long‑running/timeout‑heavy tests are isolated in the slow suite.One behavioral detail to be intentional about: tests tagged with both
[Serial]and[Slow](like the new EventTTL test) are excluded from conformance/serial and run only in theoptional/slowsuite (still serial due toParallelism: 1). If that’s the desired classification, this setup looks good; if you want Serial+Slow tests to appear in both serial and slow suites, the serial suite qualifier would need to drop the!name.contains("[Slow]")condition.The timeout extraction from
[Timeout:...]intospec.Tags["timeout"]is straightforward and should work fine for the usual[Timeout:Xm]style tags used in these suites.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
⛔ Files ignored due to path filters (283)
go.sumis excluded by!**/*.sumvendor/github.com/go-logr/logr/.golangci.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/go-logr/logr/funcr/funcr.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/.editorconfigis excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/.gitattributesis excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/Taskfile.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/crypto.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/date.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/defaults.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/dict.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/functions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/list.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/network.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/numeric.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/reflect.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/regex.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/strings.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/url.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/CONTRIBUTING.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/Makefileis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/OWNERSis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/RELEASING.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/config/deprecated.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/core_dsl.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/core_dsl_patch.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/decorator_dsl.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/deprecated_dsl.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/formatter/colorable_others.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/formatter/colorable_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/formatter/formatter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/build/build_command.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/command/abort.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/command/command.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/command/program.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/boostrap_templates.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/bootstrap_command.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/generate_command.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/generate_templates.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/generators_common.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/compile.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/gocovmerge.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/profiles_and_reports.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/run.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/test_suite.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/utils.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/verify_version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/labels/labels_command.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/main.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/ginkgo.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/import.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/outline.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/outline_command.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/run/run_command.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/unfocus/unfocus_command.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/delta.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/delta_tracker.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/dependencies.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/package_hash.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/package_hashes.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/suite.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/watch_command.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo_cli_dependencies.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo_t_dsl.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/counter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/failer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/focus.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/global/init.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/group.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/interrupt_handler/interrupt_handler.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/interrupt_handler/sigquit_swallower_unix.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/interrupt_handler/sigquit_swallower_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/node.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/ordering.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_unix.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_wasm.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_win.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/client_server.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/http_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/http_server.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/rpc_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/rpc_server.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/server_handler.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_bsd.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_unix.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_wasm.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_win.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_reporter_manager.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/report_entry.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/spec.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/spec_context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/spec_patch.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/suite.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/suite_patch.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/testingtproxy/testing_t_proxy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/tree.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/writer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/default_reporter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/deprecated_reporter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/json_report.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/junit_report.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/reporter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/teamcity_report.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/reporting_dsl.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/table_dsl.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/code_location.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/deprecated_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/deprecation_support.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/enum_support.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/file_filter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/flags.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/label_filter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/report_entry.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/types_patch.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/CONTRIBUTING.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/RELEASING.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/format/format.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/gomega_dsl.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/internal/assertion.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/internal/async_assertion.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/internal/duration_bundle.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/internal/gomega.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/internal/gutil/post_ioutil.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/internal/gutil/using_ioutil.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/internal/polling_signal_error.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/internal/vetoptdesc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/and.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/assignable_to_type_of_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/attributes_slice.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_a_directory.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_a_regular_file.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_an_existing_file.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_closed_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_comparable_to_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_element_of_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_empty_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_equivalent_to_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_false_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_identical_to.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_key_of_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_nil_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_numerically_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_sent_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_temporally_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_true_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_zero_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/consist_of.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/contain_element_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/contain_elements_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/contain_substring_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/equal_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_cap_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_each_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_exact_elements.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_existing_field_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_field.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_http_body_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_http_header_with_value_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_http_status_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_key_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_key_with_value_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_len_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_occurred_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_prefix_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_suffix_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_value.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/internal/miter/type_support_iter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/internal/miter/type_support_noiter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/match_error_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/match_json_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/match_regexp_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/match_xml_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/match_yaml_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/not.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/or.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/panic_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/receive_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/satisfy_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/semi_structured_data_support.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/succeed_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/support/goraph/bipartitegraph/bipartitegraph.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/support/goraph/bipartitegraph/bipartitegraphmatching.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/support/goraph/edge/edge.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/support/goraph/node/node.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/support/goraph/util/util.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/type_support.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/with_transform.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/types/types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo/logging.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo/parallel.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo/util.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/net/html/atom/atom.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/net/html/atom/table.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/net/html/charset/charset.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/net/html/const.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/net/html/doc.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/net/html/doctype.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/net/html/entity.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/net/html/escape.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/net/html/foreign.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/net/html/iter.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/net/html/node.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/net/html/parse.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/net/html/render.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/net/html/token.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/charmap/charmap.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/charmap/tables.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/encoding.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/htmlindex/htmlindex.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/htmlindex/map.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/htmlindex/tables.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/internal/identifier/identifier.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/internal/identifier/mib.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/internal/internal.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/japanese/all.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/japanese/eucjp.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/japanese/iso2022jp.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/japanese/shiftjis.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/japanese/tables.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/korean/euckr.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/korean/tables.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/simplifiedchinese/all.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/simplifiedchinese/gbk.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/simplifiedchinese/hzgb2312.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/simplifiedchinese/tables.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/traditionalchinese/big5.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/traditionalchinese/tables.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/unicode/override.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/encoding/unicode/unicode.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/internal/utf8internal/utf8internal.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/runes/cond.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/text/runes/runes.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/cover/profile.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ast/edge/edge.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ast/inspector/cursor.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ast/inspector/inspector.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ast/inspector/iter.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ast/inspector/typeof.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ast/inspector/walk.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/encoding/protowire/wire.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/internal/editiondefaults/editions_defaults.binpbis excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/internal/filedesc/editions.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/internal/filedesc/presence.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/internal/genid/descriptor_gen.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/internal/impl/codec_message_opaque.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/internal/impl/message_opaque.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/internal/impl/presence.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/internal/strs/strings_unsafe.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/internal/strs/strings_unsafe_go120.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/internal/version/version.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/proto/merge.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/reflect/protoreflect/source_gen.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/reflect/protoreflect/value_unsafe.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/reflect/protoreflect/value_unsafe_go120.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/types/descriptorpb/descriptor.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/types/gofeaturespb/go_features.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/types/known/anypb/any.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/types/known/durationpb/duration.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/types/known/emptypb/empty.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/types/known/fieldmaskpb/field_mask.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/types/known/structpb/struct.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/types/known/timestamppb/timestamp.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/types/known/wrapperspb/wrappers.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/modules.txtis excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (3)
cmd/cluster-kube-apiserver-operator-tests-ext/main.go(3 hunks)go.mod(5 hunks)test/e2e/event-ttl.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
test/e2e/event-ttl.gogo.modcmd/cluster-kube-apiserver-operator-tests-ext/main.go
🔇 Additional comments (3)
go.mod (1)
53-80: Confirm ginkgo fork compatibility and keep module metadata in syncThe additional testing deps and the
github.com/onsi/ginkgo/v2replace to the OpenShift fork look consistent with the new E2E suite, but the required version (v2.25.1) differs from the fork tag (v2.6.1-…). It’s fine as long as the fork is API‑compatible with what this code uses, but that mismatch can make future upgrades confusing.Please verify that:
go test ./...passes using this replace.go mod tidyhas been run sogo.sumstays consistent with these changes.Also applies to: 119-139
test/e2e/event-ttl.go (2)
428-557: Helpers for feature gate / rollout waiting look robust and debuggable
waitForFeatureGateEnabledandwaitForAPIServerRolloutusewait.PollUntilContextTimeoutwith clear logging, track attempts, and avoid treating transient API errors as hard failures. The rollout helper’s requirement that all kube‑apiserver pods be new (created afterrolloutStartTime) and running is a good safety check to ensure a full rollout, and the logging of non‑running pods and revisions should make diagnosing flakes much easier.No changes needed here from a correctness or maintainability standpoint.
559-644: ConfigMap‑based verification ofevent-ttlis pragmatic and defensiveThe
verifyEventTTLInConfigFilehelper does a good job of:
- Polling the
configConfigMap to handle races between pod readiness and config updates.- Requiring at least one running kube‑apiserver pod before checking config, which guards against checking during outages.
- Matching
event-ttlin several possible serialized forms and logging nearby context when the value doesn’t match.This should make config drift around
event-ttlrelatively easy to debug. Looks good as‑is.
| "k8s.io/client-go/kubernetes" | ||
| ) | ||
|
|
||
| var _ = g.Describe("[Jira:kube-apiserver][sig-api-machinery][FeatureGate:EventTTL] Event TTL Configuration", g.Ordered, func() { |
There was a problem hiding this comment.
is there a requirement to use the ginkgo framework to write the test cases ?
could we use standard golang framework ?
There was a problem hiding this comment.
It is requirement to use like this by ote team
There was a problem hiding this comment.
okay, from what I’ve been able to gather, it looks like there might be a way to run standard go tests with OTE. (xref: https://redhat-internal.slack.com/archives/C07RDCVEYJG/p1763540247211469)
The issue is that no one has actually tried it, since everyone seems to be using ginkgo. If we want to continue writing tests using the standard go testing framework, or avoid rewriting the existing tests, then we need to build an adapter (if that’s even possible).
I’d like to hear from @benluddy, @bertinatto, @sanchezl and @dinhxuanvu to get consensus on which framework we would like to use.
There was a problem hiding this comment.
Is there a problem with continue to use gingko to write test cases? If all of current test cases use gingko then I don't see why we need to switch to standard go testing framework unless there are some limitations that I don't know about?
I'm not sure how many tests we have in this repo but if there ain't too many, it may be faster jus to convert them all to not use gingko if we intend to use standard go testing moving forward. Having two different testing libraries and using an adapter seem to be a maintainance nightmare in the long run.
There was a problem hiding this comment.
it is the other way around. the existing tests use the standard go testing framework and this PR proposes to use ginkgo.
There was a problem hiding this comment.
@p0lyn0mial By creating new adapter able to identify new non ginkgo cases with ote. But facing 2 problems
Go does not include _test.go files in go build, so OTE cannot see or run them. Only regular .go files are included in the OTE binary.
We have two options:
- Keep tests separate:
Existing _test.go tests run with go test.
New OTE tests use .go files and run through OTE. - Migrate all tests to OTE:
Rename _test.go → .go
Change TestX(t *testing.T) → testX(t adapter.T)
Register tests using adapter.RunTests().
And _test.go files cannot be used with OTE because Go excludes them during go build.
There was a problem hiding this comment.
@p0lyn0mial able to run old go cases with ote
make build
go build -mod=vendor -trimpath -ldflags "-X github.com/openshift/cluster-kube-apiserver-operator/pkg/version.versionFromGit="v4.0.0-alpha.0-2271-gd5d7299" -X github.com/openshift/cluster-kube-apiserver-operator/pkg/version.commitFromGit="d5d729906" -X github.com/openshift/cluster-kube-apiserver-operator/pkg/version.gitTreeState="clean" -X github.com/openshift/cluster-kube-apiserver-operator/pkg/version.buildDate="2025-11-20T11:00:45Z" " github.com/openshift/cluster-kube-apiserver-operator/cmd/cluster-kube-apiserver-operator
go build -mod=vendor -trimpath -ldflags "-X github.com/openshift/cluster-kube-apiserver-operator/pkg/version.versionFromGit="v4.0.0-alpha.0-2271-gd5d7299" -X github.com/openshift/cluster-kube-apiserver-operator/pkg/version.commitFromGit="d5d729906" -X github.com/openshift/cluster-kube-apiserver-operator/pkg/version.gitTreeState="clean" -X github.com/openshift/cluster-kube-apiserver-operator/pkg/version.buildDate="2025-11-20T11:00:46Z" " github.com/openshift/cluster-kube-apiserver-operator/cmd/cluster-kube-apiserver-operator-tests-ext
go build -mod=vendor -trimpath -ldflags "-X github.com/openshift/cluster-kube-apiserver-operator/pkg/version.versionFromGit="v4.0.0-alpha.0-2271-gd5d7299" -X github.com/openshift/cluster-kube-apiserver-operator/pkg/version.commitFromGit="d5d729906" -X github.com/openshift/cluster-kube-apiserver-operator/pkg/version.gitTreeState="clean" -X github.com/openshift/cluster-kube-apiserver-operator/pkg/version.buildDate="2025-11-20T11:00:46Z" " github.com/openshift/cluster-kube-apiserver-operator/cmd/cluster-kube-apiserver-operator-tests-ext/adapter
rgangwar@rgangwar-mac cluster-kube-apiserver-operator % ./cluster-kube-apiserver-operator-tests-ext run-test "[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e/bound_sa_token_test.go:TestTokenRequestAndReview [Serial]"
Running Suite: - /Users/rgangwar/Downloads/backupoffice/cluster-kube-apiserver-operator
========================================================================================
Random Seed: 1763636546 - will randomize all specs
Will run 1 of 1 specs
------------------------------
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e/bound_sa_token_test.go:TestTokenRequestAndReview [Serial]
github.com/openshift/cluster-kube-apiserver-operator/cmd/cluster-kube-apiserver-operator-tests-ext/adapter/gotest_wrapper.go:141
STEP: Running go test on test/e2e/bound_sa_token_test.go:TestTokenRequestAndReview @ 11/20/25 16:32:26.19
STEP: Executing: go test -v -json -run TestTokenRequestAndReview bound_sa_token_test.go (in /Users/rgangwar/Downloads/backupoffice/cluster-kube-apiserver-operator/test/e2e) @ 11/20/25 16:32:26.191
=== RUN TestTokenRequestAndReview
Found configuration for host https://api.rg-a9.qe.devcluster.openshift.com:6443.
--- PASS: TestTokenRequestAndReview (1.96s)
PASS: TestTokenRequestAndReview (1.96s)
STEP: Results: 1 passed, 0 failed, 0 skipped @ 11/20/25 16:32:31.251
STEP: All tests passed for test/e2e/bound_sa_token_test.go:TestTokenRequestAndReview @ 11/20/25 16:32:31.251
• [5.060 seconds]
------------------------------
Ran 1 of 1 Specs in 5.060 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 0 Skipped
[
{
"name": "[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e/bound_sa_token_test.go:TestTokenRequestAndReview [Serial]",
"lifecycle": "blocking",
"duration": 5060,
"startTime": "2025-11-20 11:02:26.190664 UTC",
"endTime": "2025-11-20 11:02:31.251100 UTC",
"result": "passed",
"output": " STEP: Running go test on test/e2e/bound_sa_token_test.go:TestTokenRequestAndReview @ 11/20/25 16:32:26.19\n STEP: Executing: go test -v -json -run TestTokenRequestAndReview bound_sa_token_test.go (in /Users/rgangwar/Downloads/backupoffice/cluster-kube-apiserver-operator/test/e2e) @ 11/20/25 16:32:26.191\n=== RUN TestTokenRequestAndReview\nFound configuration for host https://api.rg-a9.qe.devcluster.openshift.com:6443.\n--- PASS: TestTokenRequestAndReview (1.96s)\nPASS: TestTokenRequestAndReview (1.96s)\n STEP: Results: 1 passed, 0 failed, 0 skipped @ 11/20/25 16:32:31.251\n STEP: All tests passed for test/e2e/bound_sa_token_test.go:TestTokenRequestAndReview @ 11/20/25 16:32:31.251\n"
}
]% rgangwar@rgangwar-mac cluster-kube-apiserver-operator % ./cluster-kube-apiserver-operator-tests-ext run-test "[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e/bound_sa_token_test.go:TestBoundTokenSignerController [Serial]"
Running Suite: - /Users/rgangwar/Downloads/backupoffice/cluster-kube-apiserver-operator
========================================================================================
Random Seed: 1763636567 - will randomize all specs
Will run 1 of 1 specs
------------------------------
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e/bound_sa_token_test.go:TestBoundTokenSignerController [Serial]
github.com/openshift/cluster-kube-apiserver-operator/cmd/cluster-kube-apiserver-operator-tests-ext/adapter/gotest_wrapper.go:141
STEP: Running go test on test/e2e/bound_sa_token_test.go:TestBoundTokenSignerController @ 11/20/25 16:32:47.091
STEP: Executing: go test -v -json -run TestBoundTokenSignerController bound_sa_token_test.go (in /Users/rgangwar/Downloads/backupoffice/cluster-kube-apiserver-operator/test/e2e) @ 11/20/25 16:32:47.091
=== RUN TestBoundTokenSignerController
Found configuration for host https://api.rg-a9.qe.devcluster.openshift.com:6443.
=== RUN TestBoundTokenSignerController/operand-secret-deletion
bound_sa_token_test.go:54:
=== RUN TestBoundTokenSignerController/configmap-deletion
bound_sa_token_test.go:63:
=== RUN TestBoundTokenSignerController/operator-secret-deletion
bound_sa_token_test.go:80:
--- PASS: TestBoundTokenSignerController (0.74s)
--- SKIP: TestBoundTokenSignerController/operand-secret-deletion (0.00s)
SKIP: TestBoundTokenSignerController/operand-secret-deletion
--- SKIP: TestBoundTokenSignerController/configmap-deletion (0.00s)
SKIP: TestBoundTokenSignerController/configmap-deletion
--- SKIP: TestBoundTokenSignerController/operator-secret-deletion (0.00s)
SKIP: TestBoundTokenSignerController/operator-secret-deletion
PASS: TestBoundTokenSignerController (0.74s)
STEP: Results: 1 passed, 0 failed, 3 skipped @ 11/20/25 16:32:49.759
STEP: All tests passed for test/e2e/bound_sa_token_test.go:TestBoundTokenSignerController @ 11/20/25 16:32:49.759
• [2.668 seconds]
------------------------------
Ran 1 of 1 Specs in 2.668 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 0 Skipped
[
{
"name": "[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e/bound_sa_token_test.go:TestBoundTokenSignerController [Serial]",
"lifecycle": "blocking",
"duration": 2667,
"startTime": "2025-11-20 11:02:47.091506 UTC",
"endTime": "2025-11-20 11:02:49.759277 UTC",
"result": "passed",
"output": " STEP: Running go test on test/e2e/bound_sa_token_test.go:TestBoundTokenSignerController @ 11/20/25 16:32:47.091\n STEP: Executing: go test -v -json -run TestBoundTokenSignerController bound_sa_token_test.go (in /Users/rgangwar/Downloads/backupoffice/cluster-kube-apiserver-operator/test/e2e) @ 11/20/25 16:32:47.091\n=== RUN TestBoundTokenSignerController\nFound configuration for host https://api.rg-a9.qe.devcluster.openshift.com:6443.\n=== RUN TestBoundTokenSignerController/operand-secret-deletion\n bound_sa_token_test.go:54: \n=== RUN TestBoundTokenSignerController/configmap-deletion\n bound_sa_token_test.go:63: \n=== RUN TestBoundTokenSignerController/operator-secret-deletion\n bound_sa_token_test.go:80: \n--- PASS: TestBoundTokenSignerController (0.74s)\n --- SKIP: TestBoundTokenSignerController/operand-secret-deletion (0.00s)\nSKIP: TestBoundTokenSignerController/operand-secret-deletion\n --- SKIP: TestBoundTokenSignerController/configmap-deletion (0.00s)\nSKIP: TestBoundTokenSignerController/configmap-deletion\n --- SKIP: TestBoundTokenSignerController/operator-secret-deletion (0.00s)\nSKIP: TestBoundTokenSignerController/operator-secret-deletion\nPASS: TestBoundTokenSignerController (0.74s)\n STEP: Results: 1 passed, 0 failed, 3 skipped @ 11/20/25 16:32:49.759\n STEP: All tests passed for test/e2e/bound_sa_token_test.go:TestBoundTokenSignerController @ 11/20/25 16:32:49.759\n"
}
]%
There was a problem hiding this comment.
./cluster-kube-apiserver-operator-tests-ext list tests 2>/dev/null | jq -r '.[].name'
[Jira:kube-apiserver][sig-api-machinery][FeatureGate:EventTTL] Event TTL Configuration should configure and validate eventTTLMinutes=5m [Timeout:60m][Slow][Serial][OTP]
[Jira:kube-apiserver][sig-api-machinery][FeatureGate:EventTTL] Event TTL Configuration should configure and validate eventTTLMinutes=10m [Timeout:60m][Slow][Serial][OTP]
[Jira:kube-apiserver][sig-api-machinery][FeatureGate:EventTTL] Event TTL Configuration should configure and validate eventTTLMinutes=15m [Timeout:60m][Slow][Serial][OTP]
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e/bound_sa_token_test.go:TestBoundTokenSignerController [Serial]
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e/bound_sa_token_test.go:TestTokenRequestAndReview [Serial]
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e/certrotation_test.go:TestCertRotationTimeUpgradeable [Serial]
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e/certrotation_test.go:TestCertRotationStompOnBadType [Serial]
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e/deprecated_api_test.go:TestAPIRemovedInNextReleaseInUse [Serial]
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e/deprecated_api_test.go:TestAPIRemovedInNextEUSReleaseInUse [Serial]
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e/encryption_test.go:TestEncryptionTypeAESCBC [Serial]
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e/operator_test.go:TestOperatorNamespace [Serial]
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e/operator_test.go:TestOperandImageVersion [Serial]
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e/operator_test.go:TestRevisionLimits [Serial]
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e/serviceaccountissuer_test.go:TestServiceAccountIssuer [Serial]
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e/user_certs_test.go:TestNamedCertificates [Serial]
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e/user_client_ca_test.go:TestUserClientCABundle [Serial]
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e/user_cors_test.go:TestAdditionalCORSAllowedOrigins [Serial]
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e-encryption/encryption_test.go:TestEncryptionTypeIdentity [Serial]
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e-encryption/encryption_test.go:TestEncryptionTypeUnset [Serial]
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e-encryption/encryption_test.go:TestEncryptionTurnOnAndOff [Serial]
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e-encryption-perf/encryption_perf_test.go:TestPerfEncryption [Serial]
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e-encryption-rotation/encryption_rotation_test.go:TestEncryptionRotation [Serial]
[sig-api-machinery] kube-apiserver operator Standard Go Tests test/e2e-sno-disruptive/sno_disruptive_test.go:TestFallback [Serial]
rgangwar@rgangwar-mac cluster-kube-apiserver-operator % ./cluster-kube-apiserver-operator-tests-ext list tests 2>/dev/null | jq -r '.[].name' | wc -l
23
There was a problem hiding this comment.
We can use both ginkgo and standard go test cases.
I prefer future cases to use ginkgo.
There was a problem hiding this comment.
with normal go
cd test/e2e && go test -v -run TestOperatorNamespace
=== RUN TestOperatorNamespace
Found configuration for host https://xxxxx.
--- PASS: TestOperatorNamespace (0.70s)
PASS
ok github.com/openshift/cluster-kube-apiserver-operator/test/e2e 1.435s
| // no-op, logic is provided by the OTE framework | ||
| if err := cmd.Help(); err != nil { | ||
| klog.Fatal(err) | ||
| fmt.Fprintf(os.Stderr, "Error: %v\n", err) |
There was a problem hiding this comment.
When I refactored the error handling earlier, I changed it to be consistent with the new registry error handling pattern, I think that time I got some compiling issue let me try again
| cmd.AddCommand(otecmd.DefaultExtensionCommands(registry)...) | ||
|
|
||
| return cmd | ||
| code := cli.Run(cmd) |
There was a problem hiding this comment.
during refactoring I have changed for consistency with standard ote main.go not any specific reason
| registry := oteextension.NewRegistry() | ||
| extension := oteextension.NewExtension("openshift", "payload", "cluster-kube-apiserver-operator") | ||
|
|
||
| // Suite: conformance/parallel (fast, parallel-safe) |
There was a problem hiding this comment.
which suites are required to run the EventTTL test ?
There was a problem hiding this comment.
It runs with suite openshift/cluster-kube-apiserver-operator/optional/slow
| "k8s.io/client-go/kubernetes" | ||
| ) | ||
|
|
||
| var _ = g.Describe("[Jira:kube-apiserver][sig-api-machinery][FeatureGate:EventTTL] Event TTL Configuration", g.Ordered, func() { |
There was a problem hiding this comment.
which CI job will run this test?
There was a problem hiding this comment.
It is long duration cases I will create new ci job for that.
| } | ||
| }) | ||
|
|
||
| // Ignore obsolete tests |
There was a problem hiding this comment.
why do we need this code ?
There was a problem hiding this comment.
@p0lyn0mial this is for if we want to stop cases for sometime from running cases in ci.
| }) | ||
|
|
||
| // Ignore obsolete tests | ||
| extension.IgnoreObsoleteTests( |
There was a problem hiding this comment.
why do we need this code ?
| }) | ||
|
|
||
| // Extract timeout from test name if present (e.g., [Timeout:50m]) | ||
| specs = specs.Walk(func(spec *et.ExtensionTestSpec) { |
There was a problem hiding this comment.
why do we need this code ?
There was a problem hiding this comment.
this is needed because some cases we have longduration cases like we have cases which have duration 30-40mins
but job has default has case timeout only 15mins to extended that we need Timeout tag, same we do in origin cases
| }) | ||
|
|
||
| // Preserve original-name labels for renamed tests | ||
| specs = specs.Walk(func(spec *et.ExtensionTestSpec) { |
There was a problem hiding this comment.
why do we need this code ?
There was a problem hiding this comment.
It extracts the original name from a label like original-name:SomeOldTestName and populates the OriginalName field of the test spec. You rename tests but want to preserve historical test results that's why we are using. We can remove that but it is standard main.go shared by ote team
| // Build ginkgo test specs from the test/e2e package | ||
| specs, err := g.BuildExtensionTestSpecsFromOpenShiftGinkgoSuite() | ||
| if err != nil { | ||
| panic(fmt.Sprintf("couldn't build extension test specs from ginkgo: %+v", err.Error())) |
There was a problem hiding this comment.
why not to return the error ? why do we have to panic ?
|
|
||
| // Initialize environment before running any tests | ||
| specs.AddBeforeAll(func() { | ||
| // do stuff |
There was a problem hiding this comment.
why do we need this code ?
| } | ||
|
|
||
| // Ensure [Disruptive] tests are also [Serial] | ||
| specs = specs.Walk(func(spec *et.ExtensionTestSpec) { |
There was a problem hiding this comment.
why do we need this code ?
There was a problem hiding this comment.
@p0lyn0mial some qe cases we have tag disruptive to considered those are serial. that's why I will remove that it is waste, we have serial tag
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
test/e2e/event-ttl.go (2)
378-422: Consider polling for event deletion instead of a single longSleepto reduce runtime and flakinessThe current TTL check waits for
ttl + bufferMinutesusing a singletime.Sleep(waitDuration)and then performs oneGetto assert the event is gone. This is functionally correct for detecting “TTL not applied” (the event will still exist if it’s stuck at the default 3h), but it forces each test case to wait the full TTL+buffer (up to ~18 minutes here) even if the event expired much earlier and gives no signal if it expired significantly before the configured TTL.You could make this more efficient and slightly more robust by polling for deletion with a shorter interval, exiting as soon as you observe
NotFound, and optionally asserting that deletion didn’t occur too early (e.g. beforettlminutes have elapsed). That would shorten successful runs and provide tighter bounds on TTL behavior while preserving the overall semantics.
463-560: Rollout detection could be made more resilient by leveraging revision labels instead of creation time
waitForAPIServerRolloutcurrently treats “new” pods as those withCreationTimestampafterrolloutStartTimeand requires that all listed pods meet that condition and areRunning. This generally works when you call it immediately after applying a config change, but it does rely on call timing: if a rollout starts and completes very quickly before this helper is invoked, those already‑rolled‑out pods will be treated as “old”, and the condition may never become true, leading to timeouts despite a healthy cluster.Since you already collect
initialRevisionand log the current revision from pod labels, you could make this more robust by driving success off revision changes (e.g., all pods Running and theirrevisionlabel different frominitialRevision) instead of or in addition to creation time. That would decouple the check from when the helper is called and reduce the chance of false negatives in fast or out‑of‑band rollouts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
cmd/cluster-kube-apiserver-operator-tests-ext/main.go(2 hunks)test/e2e/event-ttl.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
cmd/cluster-kube-apiserver-operator-tests-ext/main.gotest/e2e/event-ttl.go
🔇 Additional comments (2)
cmd/cluster-kube-apiserver-operator-tests-ext/main.go (1)
10-26: Registry wiring, suite qualifiers, and timeout tagging look consistent and robustThe move to
prepareOperatorTestsRegistry()with error returns, the Cobra/cli.Runintegration, and the OTE registry/suite setup all look correct. The suite qualifiers and[Timeout:...]tag extraction are consistent with the described behavior and should correctly route long‑running[Slow][Serial][Timeout:...]tests like EventTTL into theoptional/slow(andall) suites without overlaps. I don’t see any functional or maintainability blockers here.Also applies to: 28-56, 63-144
test/e2e/event-ttl.go (1)
25-211: Overall EventTTL suite, feature gate lifecycle, and helpers look solidThe Ordered Ginkgo suite, feature‑gate enable/restore logic, rollout waits, and verification helpers are all structured carefully and defensively (good use of read‑modify‑write, explicit restore path, cluster stabilization waits, and guarded logging). The per‑TTL tests correctly snapshot and restore
spec.EventTTLMinutes, isolate work in a dedicated namespace, and validate both config propagation and eventual event deletion. I don’t see correctness or safety issues that would block merging from this file.Also applies to: 432-648
gangwgr
force-pushed
the
test-infrastructure-setup-v2
branch
2 times, most recently
from
a636219 to
1ff12c1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
test/e2e/event-ttl.go (2)
463-561: Rollout detector may hang when rollout is already complete or is a no‑op
waitForAPIServerRolloutonly reports success when all current pods were created afterrolloutStartTime(Line 492). If:
- kube‑apiserver has already fully rolled out before this helper is called (e.g., after the feature gate change while
waitForFeatureGateEnabledis still polling), or- a given spec change doesn’t actually trigger a new revision (no new pods are created),
then no pods will ever satisfy
pod.CreationTimestamp.After(rolloutStartTime), and the function will poll until thetimeoutexpires even though the cluster is already stable.Because this helper is used after feature gate changes and in cleanup paths, that can turn harmless no‑op transitions into 20–25 minute false failures.
Consider making success detection revision‑based instead of wall‑clock‑based, e.g.:
- Capture
initialRevisionfrom therevisionlabel in the initial pod list.- In the poll loop, consider rollout complete when:
- all kube‑apiserver pods are
Running, and- they all share a single non‑empty revision label, and
- that revision differs from
initialRevision(orinitialRevisionwas empty), or some explicit “no‑rollout needed” condition is met.This avoids depending on “new pods must be younger than when we started waiting” and should make the helper robust both when rollout is fast and when a change is a no‑op.
213-427: Re‑evaluate number of TTL variants and stricttime.Sleepto keep CI runtime under controlThe test loops over
[]int32{5, 10, 15}and, for each value, performs:
- a full kube‑apiserver rollout wait (up to 20 minutes), plus
- a hard
time.Sleep(waitDuration)wherewaitDuration = (ttl + 3) * time.Minute(Lines 379–381, 411).Run sequentially, the worst‑case wall‑clock time for this single describe block can easily exceed what most jobs tolerate, especially if rollout is slow.
To keep the optional/slow job healthy while still validating the feature, consider:
- Reducing the number of tested TTL values (e.g., a single representative value such as 5 or 10 minutes), or
- Keeping multiple TTLs but replacing the single long
time.Sleepwith a poll (e.g.,wait.PollUntilContextTimeout) that checks for deletion and bails out as soon as the event disappears, within a capped maximum ofttl+buffer.This keeps coverage but avoids “always wait full ttl+buffer for each variant” and reduces the impact of occasional slow rollouts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
cmd/cluster-kube-apiserver-operator-tests-ext/main.go(2 hunks)test/e2e/event-ttl.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
test/e2e/event-ttl.gocmd/cluster-kube-apiserver-operator-tests-ext/main.go
🔇 Additional comments (1)
cmd/cluster-kube-apiserver-operator-tests-ext/main.go (1)
39-151: Registry wiring and error propagation look solidThe refactor to have
newOperatorTestCommand/prepareOperatorTestsRegistryreturn errors, plus the suite definitions and[Timeout:…]tag extraction, all look consistent with OTE patterns and give you clearer failure surfaces. The qualifiers also correctly route[Slow][Serial][Timeout:…]tests like the EventTTL one into the optional/slow suite only.No changes requested here.
gangwgr
force-pushed
the
test-infrastructure-setup-v2
branch
from
1ff12c1 to
d5d7299
Compare
|
/test e2e-aws-ovn-upgrade |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
test/e2e/event-ttl.go (2)
42-52: Make stdout redirection safer and less brittleRebinding
os.Stdoutat process scope is fragile, andos.Open(os.DevNull)errors are ignored (could leaveos.Stdoutnil and cause panics). Consider:
- Checking the
os.Openerror and failing early if it cannot be opened.- Using a
deferto restoreos.Stdoutand closedevNull, so future edits don’t accidentally miss the restore path.For example:
oldStdout := os.Stdout devNull, err := os.Open(os.DevNull) o.Expect(err).NotTo(o.HaveOccurred(), "failed to open os.DevNull") defer func() { os.Stdout = oldStdout _ = devNull.Close() }() os.Stdout = devNull kubeConfig, err := libgotest.NewClientConfigForTest()This keeps the hack localized and safer while preserving the intent of suppressing noisy output.
414-453: Use poll + typed NotFound check instead of fixed sleep + string matchingRight now the test:
- Always sleeps
ttl+bufferMinuteseven if the event is deleted early.- Asserts deletion by checking
err.Error()contains"not found", which is brittle against error‑message changes.It would be more robust (and often faster) to:
- Use
wait.PollUntilContextTimeout(or similar) to periodicallyGetthe event untilapierrors.IsNotFound(err)is true or a timeout is hit.- Fail on timeouts explicitly, and use
errors.IsNotFoundinstead of string matching.This reduces flakiness and better documents the intended contract with the API.
cmd/cluster-kube-apiserver-operator-tests-ext/adapter/gotest_wrapper.go (2)
192-238: Don’t silently drop non‑JSONgo test -jsonoutputIn the JSON parsing loop, any line that fails
json.Unmarshalis justcontinued, which means non‑JSON stdout fromgo testis discarded. While stderr is printed later, interesting diagnostics or panics sometimes appear on stdout, especially when tests or the go tool emit unexpected output.Consider:
- Logging lines that fail JSON parsing to
GinkgoWriter(perhaps at a lower verbosity), or- Falling back to dumping the raw
stdoutbuffer whenrunErr != nil.This will make debugging failing/disrupted suites significantly easier.
41-45: Align default lifecycle handling betweenRunGoTestFileandRunGoTestSuite
RunGoTestFiletreats a nilLifecycleasote.Informing(), whereasRunGoTestSuitepasses a nilLifecyclestraight through tog.It. That difference is subtle and could surprise future callers who expect both helpers to behave the same whenLifecycleisn’t set.Either:
- Apply the same defaulting in
RunGoTestSuite(e.g., default toote.Informing()whenLifecycleis nil), or- Remove the defaulting from
RunGoTestFileand require callers to always choose a lifecycle explicitly.Picking one consistent convention will make the adapter easier to use correctly.
Also applies to: 138-141
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (3)
cmd/cluster-kube-apiserver-operator-tests-ext/adapter/gotest_wrapper.go(1 hunks)cmd/cluster-kube-apiserver-operator-tests-ext/main.go(2 hunks)test/e2e/event-ttl.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
test/e2e/event-ttl.gocmd/cluster-kube-apiserver-operator-tests-ext/main.gocmd/cluster-kube-apiserver-operator-tests-ext/adapter/gotest_wrapper.go
🔇 Additional comments (1)
cmd/cluster-kube-apiserver-operator-tests-ext/main.go (1)
96-139: Suite and registry wiring for OTE looks consistentThe registry construction and suite qualifiers (parallel/serial/optional‑slow/all) align with the documented rules and correctly route
[Slow]/[Serial]/[Timeout:...]tests like the EventTTL cases intoopenshift/cluster-kube-apiserver-operator/optional/slow. Error‑aware construction viaprepareOperatorTestsRegistry()andnewOperatorTestCommand()also looks good.
|
/test e2e-aws-ovn-upgrade |
gangwgr
force-pushed
the
test-infrastructure-setup-v2
branch
from
deeaede to
ec9b1bc
Compare
|
/test e2e-aws-ovn-upgrade |
|
/test e2e-aws-ovn |
4 participants
Adding test cases for "Add support for event-ttl in Kube API Server Operator"