Skip to content

Try prevent flaking by placing org and ws on same shard#3931

Open
ntnn wants to merge 1 commit intokcp-dev:mainfrom
ntnn:kcp-test-workspace-deletion_create-and-clean-workspace
Open

Try prevent flaking by placing org and ws on same shard#3931
ntnn wants to merge 1 commit intokcp-dev:mainfrom
ntnn:kcp-test-workspace-deletion_create-and-clean-workspace

Conversation

@ntnn
Copy link
Member

@ntnn ntnn commented Mar 20, 2026

Summary

create and clean workspace has been flaking due to eventually timeouts, while the other subtest hasn't flaked. The other subtest places the org and workspace on the root shard explicitly, this test didn't.

My guess is that the cross-shard operation (plus all the other tests) is just enough delay for the test to fail every now and then. It's worth a try - at worst the two tests will have a similar setup.

What Type of PR Is This?

/kind flake

Related Issue(s)

Fixes #

Release Notes

NONE

@ntnn ntnn added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Mar 20, 2026
@kcp-ci-bot kcp-ci-bot added release-note-none Denotes a PR that doesn't merit a release note. kind/flake Categorizes issue or PR as related to a flaky test. dco-signoff: yes Indicates the PR's author has signed the DCO. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 20, 2026
@ntnn ntnn added this to tbd Mar 20, 2026
@github-project-automation github-project-automation bot moved this to Backlog in tbd Mar 20, 2026
name: "create and clean workspace",
work: func(ctx context.Context, t *testing.T, server runningServer) {
orgPath, _ := kcptesting.NewWorkspaceFixture(t, server, core.RootCluster.Path(), kcptesting.WithType(core.RootCluster.Path(), "organization"))
orgPath, _ := kcptesting.NewWorkspaceFixture(t, server, core.RootCluster.Path(), kcptesting.WithRootShard(), kcptesting.WithType(core.RootCluster.Path(), "organization"))
Copy link
Contributor

@gman0 gman0 Mar 20, 2026

Choose a reason for hiding this comment

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

Maybe add a comment for the rationale behind this change? Something like in the PR's description would suffice.

@mjudeikis
Copy link
Contributor

/lgtm
/approve

@kcp-ci-bot kcp-ci-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 20, 2026
@kcp-ci-bot
Copy link
Contributor

LGTM label has been added.

DetailsGit tree hash: 2ffbdfa2ccda9bc95c195cd13f74c1a26313b948

@kcp-ci-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mjudeikis

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

@kcp-ci-bot kcp-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 20, 2026
`create and clean workspace` has been flaking due to eventually timeouts, while the other subtest hasn't flaked.
The other subtest places the org and workspace on the root shard explicitly, this test didn't.

My guess is that the cross-shard operation (plus all the other tests) is just enough delay for the test to fail every now and then.
It's worth a try - at worst the two tests will have a similar setup.

Also added a note explaining the use of the .WithRootShard to the second subtest with a note on future refactoring.

Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com>
Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
@ntnn ntnn force-pushed the kcp-test-workspace-deletion_create-and-clean-workspace branch from b269ee9 to b8abff3 Compare March 20, 2026 19:38
@kcp-ci-bot kcp-ci-bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 20, 2026
@kcp-ci-bot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@kcp-ci-bot kcp-ci-bot requested a review from mjudeikis March 20, 2026 19:38
@kcp-ci-bot kcp-ci-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 20, 2026
@ntnn
Copy link
Member Author

ntnn commented Mar 20, 2026

@gman0 I like the suggestion; I also put in a comment in the second test. I think we could refactor both at some point to not require pinning the shard in either.

@ntnn
Copy link
Member Author

ntnn commented Mar 20, 2026

/retest

The failures are all related to the APIBindings not being ready yet, see: #3934

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has signed the DCO. kind/flake Categorizes issue or PR as related to a flaky test. release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants