Skip to content

fix(test/setupTestSandbox): wait until initial objects are propagated to informers#1878

Merged
ingvagabund merged 1 commit into
kubernetes-sigs:masterfrom
ingvagabund:setupTestSandbox-fix
May 18, 2026
Merged

fix(test/setupTestSandbox): wait until initial objects are propagated to informers#1878
ingvagabund merged 1 commit into
kubernetes-sigs:masterfrom
ingvagabund:setupTestSandbox-fix

Conversation

@ingvagabund
Copy link
Copy Markdown
Contributor

@ingvagabund ingvagabund commented May 18, 2026

Description

ssia

Checklist

Please ensure your pull request meets the following criteria before submitting
for review, these items will be used by reviewers to assess the quality and
completeness of your changes:

  • Code Readability: Is the code easy to understand, well-structured, and consistent with project conventions?
  • Naming Conventions: Are variable, function, and structs descriptive and consistent?
  • Code Duplication: Is there any repeated code that should be refactored?
  • Function/Method Size: Are functions/methods short and focused on a single task?
  • Comments & Documentation: Are comments clear, useful, and not excessive? Were comments updated where necessary?
  • Error Handling: Are errors handled appropriately ?
  • Testing: Are there sufficient unit/integration tests?
  • Performance: Are there any obvious performance issues or unnecessary computations?
  • Dependencies: Are new dependencies justified ?
  • Logging & Monitoring: Is logging used appropriately (not too verbose, not too silent)?
  • Backward Compatibility: Does this change break any existing functionality or APIs?
  • Resource Management: Are resources (files, connections, memory) managed and released properly?
  • PR Description: Is the PR description clear, providing enough context and explaining the motivation for the change?
  • Documentation & Changelog: Are README and docs updated if necessary?

Copilot AI review requested due to automatic review settings May 18, 2026 14:39
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 18, 2026
@ingvagabund ingvagabund mentioned this pull request May 18, 2026
Merged
@a7i
Copy link
Copy Markdown
Contributor

a7i commented May 18, 2026

/approve

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented May 18, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: a7i

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 18, 2026
@ingvagabund
Copy link
Copy Markdown
Contributor Author

ingvagabund commented May 18, 2026

/override pull-descheduler-verify-master

@ingvagabund ingvagabund added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 18, 2026
Copilot AI reviewed May 18, 2026
View reviewed changes
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the kube client sandbox test setup to wait for initial objects to reach the sandbox’s fake informer indexers before tests proceed, reducing races in sandbox-related tests.

Changes:

  • Adds polling in setupTestSandbox to confirm initial objects are visible in the fake indexer.
  • Adds meta import for object metadata logging during propagation waits.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/descheduler/kubeclientsandbox_test.go
exists, err := sandbox.hasRuntimeObjectInIndexer(obj)
if err != nil {
t.Logf("Error checking if object propagated to fake indexer: %v", err)
return false, nil
Comment thread pkg/descheduler/kubeclientsandbox_test.go
Comment on lines +67 to +68
// Wait for initial objects to propagate from real client to fake client via event handlers.
for _, obj := range initialObjects {
Comment thread pkg/descheduler/kubeclientsandbox_test.go
// Wait for initial objects to propagate from real client to fake client via event handlers.
for _, obj := range initialObjects {
if err := wait.PollUntilContextTimeout(ctx, 10*time.Millisecond, 5*time.Second, true, func(ctx context.Context) (bool, error) {
exists, err := sandbox.hasRuntimeObjectInIndexer(obj)
Comment thread pkg/descheduler/kubeclientsandbox_test.go Outdated
@ingvagabund ingvagabund force-pushed the setupTestSandbox-fix branch from 2673e34 to 66918dc Compare May 18, 2026 14:46
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 18, 2026
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented May 18, 2026

New changes are detected. LGTM label has been removed.

@ingvagabund ingvagabund added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 18, 2026
@ingvagabund
Copy link
Copy Markdown
Contributor Author

ingvagabund commented May 18, 2026

/override pull-descheduler-verify-master

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented May 18, 2026

@ingvagabund: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-descheduler-verify-master 66918dc link true /test pull-descheduler-verify-master
pull-descheduler-test-e2e-k8s-master-1-36 66918dc link true /test pull-descheduler-test-e2e-k8s-master-1-36

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

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

@ingvagabund ingvagabund merged commit d6720f5 into kubernetes-sigs:master May 18, 2026
6 of 9 checks passed
@ingvagabund ingvagabund deleted the setupTestSandbox-fix branch May 18, 2026 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@a7i a7i Awaiting requested review from a7i

@ricardomaraschini ricardomaraschini Awaiting requested review from ricardomaraschini

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ingvagabund @a7i @k8s-ci-robot