[release-1.24] resource store: save stage of resource creation and report to callers #6263

haircommander · 2022-09-29T17:24:53Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

CRI-O now logs the stage of container or pod creation under system load. This allows users to find why their creation requests are stalling.

haircommander · 2022-09-29T17:25:00Z

cherry-pick of #6238

sonatype-lift · 2022-09-29T17:37:50Z

internal/resourcestore/resourcestore.go

 	}
 	r.watchers = append(r.watchers, watcher)
-	return watcher
+	return watcher, r.stage


typecheck: too many return values
have (chan struct{}, string)
want (chan struct{})

ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage

@sonatype-lift ignore Leave out the above finding from this PR

@sonatype-lift ignoreall Leave out all the existing findings from this PR

@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
_{Click here to add LiftBot to another repo.}

Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

sonatype-lift · 2022-09-29T17:37:51Z

internal/resourcestore/resourcestore.go

 			name:     name,
 		}
-		return watcher
+		return watcher, StageUnknown


typecheck: too many return values
have (chan struct{}, string)
want (chan struct{})

ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage

@sonatype-lift ignore Leave out the above finding from this PR

@sonatype-lift ignoreall Leave out all the existing findings from this PR

@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
_{Click here to add LiftBot to another repo.}

Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

sonatype-lift · 2022-09-29T17:54:50Z

internal/resourcestore/resourcestore.go

 // they've taken too long. Adding a watcher allows the server to slow down the client, but still
 // return the resource in a timely manner once it's actually created.
-func (rc *ResourceStore) WatcherForResource(name string) chan struct{} {
+func (rc *ResourceStore) WatcherForResource(name string) (chan struct{}, string) {


gocritic: unnamedResult: consider giving a name to these results

ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage

@sonatype-lift ignore Leave out the above finding from this PR

@sonatype-lift ignoreall Leave out all the existing findings from this PR

@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
_{Click here to add LiftBot to another repo.}

Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

codecov · 2022-09-29T17:58:32Z

Codecov Report

Merging #6263 (c1d8146) into release-1.24 (ac6f687) will increase coverage by 0.04%.
The diff coverage is 63.63%.

Additional details and impacted files

@@               Coverage Diff                @@
##           release-1.24    #6263      +/-   ##
================================================
+ Coverage         42.85%   42.89%   +0.04%     
================================================
  Files               124      124              
  Lines             12690    12716      +26     
================================================
+ Hits               5438     5455      +17     
- Misses             6730     6739       +9     
  Partials            522      522

stages are a mechanims that a watcher can see the stage a pod or container creation is at. This will help aid in debugging under load. Signed-off-by: Peter Hunt <pehunt@redhat.com>

Signed-off-by: Peter Hunt <pehunt@redhat.com>

haircommander · 2022-10-06T19:42:45Z

@saschagrunert PTAL

saschagrunert

/lgtm

openshift-ci · 2022-10-07T06:47:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: haircommander, saschagrunert

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [haircommander,saschagrunert]

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

haircommander · 2022-10-07T15:03:00Z

/override ci/prow/e2e-gcp

openshift-ci · 2022-10-07T15:03:11Z

@haircommander: Overrode contexts on behalf of haircommander: ci/prow/e2e-gcp

In response to this:

/override ci/prow/e2e-gcp

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/test-infra repository.

haircommander · 2022-10-10T16:11:41Z

/cherry-pick release-1.23

openshift-cherrypick-robot · 2022-10-10T16:12:30Z

@haircommander: #6263 failed to apply on top of branch "release-1.23":

Applying: resource store: introduce stages
Applying: resource store: return stage when a watcher is requested
Using index info to reconstruct a base tree...
M	internal/resourcestore/resourcestore.go
M	internal/resourcestore/resourcestore_test.go
M	server/utils.go
Falling back to patching base and 3-way merge...
Auto-merging server/utils.go
Auto-merging internal/resourcestore/resourcestore_test.go
Auto-merging internal/resourcestore/resourcestore.go
CONFLICT (content): Merge conflict in internal/resourcestore/resourcestore.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 resource store: return stage when a watcher is requested
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-1.23

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/test-infra repository.

haircommander requested review from mrunalp and runcom as code owners September 29, 2022 17:24

openshift-ci bot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Sep 29, 2022

openshift-ci bot added kind/feature Categorizes issue or PR as related to a new feature. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Sep 29, 2022

openshift-ci bot requested review from QiWang19 and wgahnagl September 29, 2022 17:25

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 29, 2022

sonatype-lift bot reviewed Sep 29, 2022

View reviewed changes

haircommander force-pushed the resource-stages-1.24 branch from d8612ab to b0ce842 Compare September 29, 2022 17:40

sonatype-lift bot reviewed Sep 29, 2022

View reviewed changes

haircommander force-pushed the resource-stages-1.24 branch from b0ce842 to 3a50a56 Compare September 29, 2022 17:58

haircommander force-pushed the resource-stages-1.24 branch from 3a50a56 to 269e8cf Compare September 30, 2022 13:35

haircommander added 4 commits September 30, 2022 12:37

resource store: introduce stages

ba0ab40

stages are a mechanims that a watcher can see the stage a pod or container creation is at. This will help aid in debugging under load. Signed-off-by: Peter Hunt <pehunt@redhat.com>

resource store: return stage when a watcher is requested

cf021d4

Signed-off-by: Peter Hunt <pehunt@redhat.com>

server: update stages according to progress with resource creation

0cbf0d1

Signed-off-by: Peter Hunt <pehunt@redhat.com>

resourcestore: add test for stages

c1d8146

Signed-off-by: Peter Hunt <pehunt@redhat.com>

haircommander force-pushed the resource-stages-1.24 branch from 269e8cf to c1d8146 Compare September 30, 2022 16:38

haircommander mentioned this pull request Oct 6, 2022

[1.24] version: bump to 1.24.3 #6283

Merged

saschagrunert approved these changes Oct 7, 2022

View reviewed changes

openshift-ci bot assigned saschagrunert Oct 7, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 7, 2022

openshift-merge-robot merged commit 0e72422 into cri-o:release-1.24 Oct 7, 2022

This was referenced Oct 22, 2023

[release-1.24] OCPBUGS-20596: build(deps): bump google.golang.org/grpc from 1.43.0 to 1.58.3 #7421

Merged

[release-1.24] OCPBUGS-20596: build(deps): bump golang.org/x/net from 0.0.0-20220225172249-27dd8689420f to 0.17.0 #7422

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[release-1.24] resource store: save stage of resource creation and report to callers #6263

[release-1.24] resource store: save stage of resource creation and report to callers #6263

Uh oh!

haircommander commented Sep 29, 2022

Uh oh!

haircommander commented Sep 29, 2022

Uh oh!

sonatype-lift bot Sep 29, 2022

Uh oh!

sonatype-lift bot Sep 29, 2022

Uh oh!

sonatype-lift bot Sep 29, 2022

Uh oh!

codecov bot commented Sep 29, 2022 •

edited

Loading

Uh oh!

haircommander commented Oct 6, 2022

Uh oh!

saschagrunert left a comment

Uh oh!

openshift-ci bot commented Oct 7, 2022

Uh oh!

haircommander commented Oct 7, 2022

Uh oh!

openshift-ci bot commented Oct 7, 2022

Uh oh!

haircommander commented Oct 10, 2022

Uh oh!

openshift-cherrypick-robot commented Oct 10, 2022

Uh oh!

Uh oh!

Command	Usage
`@sonatype-lift ignore`	Leave out the above finding from this PR
`@sonatype-lift ignoreall`	Leave out all the existing findings from this PR
`@sonatype-lift exclude <file\|issue\|path\|tool>`	Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file

[release-1.24] resource store: save stage of resource creation and report to callers #6263

[release-1.24] resource store: save stage of resource creation and report to callers #6263

Uh oh!

Conversation

haircommander commented Sep 29, 2022

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Uh oh!

haircommander commented Sep 29, 2022

Uh oh!

sonatype-lift bot Sep 29, 2022

Choose a reason for hiding this comment

Uh oh!

sonatype-lift bot Sep 29, 2022

Choose a reason for hiding this comment

Uh oh!

sonatype-lift bot Sep 29, 2022

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Sep 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

haircommander commented Oct 6, 2022

Uh oh!

saschagrunert left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Oct 7, 2022

Uh oh!

haircommander commented Oct 7, 2022

Uh oh!

openshift-ci bot commented Oct 7, 2022

Uh oh!

haircommander commented Oct 10, 2022

Uh oh!

openshift-cherrypick-robot commented Oct 10, 2022

Uh oh!

Uh oh!

codecov bot commented Sep 29, 2022 •

edited

Loading