Bug 1859134: Switch to periodic process reaper #190

sgreene570 · 2020-09-21T18:13:29Z

Switch the router process reaper to use the new periodic reaper brought in by openshift/library-go#767 as per https://bugzilla.redhat.com/show_bug.cgi?id=1859134.

This PR bumps openshift/library-go to the latest version to accomplish this. Note the function signature change in pkg/cmd/infra/router/template.go.

Previous POC PR for additional context. #111

/assign @frobware
/cc @Miciah @danehans

openshift-ci-robot · 2020-09-21T18:13:41Z

@sgreene570: This pull request references Bugzilla bug 1859134, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1859134: Switch to periodic process reaper

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.

frobware · 2020-09-22T09:00:34Z

/retest

frobware · 2020-09-22T09:02:00Z

/lgtm

openshift-bot · 2020-09-22T11:32:06Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-22T11:46:57Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-22T13:15:52Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-22T13:28:51Z

/retest

Please review the full test history for this PR and help us cut down flakes.

Miciah · 2020-09-22T13:34:36Z

vendor/github.com/openshift/library-go/pkg/proc/proc_linux.go

+		processID, err := strconv.Atoi(file.Name())
+		if err != nil {
+			break


How can we be sure there are no file names in procfs that get sorted before numerics?

Seems like there is no guarantee since ioutil.ReadDir(...) will sort by name . The Atoi err check would break out of the loop on any non-numerical named procfs files. I suppose the files list could be better sorted first, or the non-numerical filenames could be removed from the list. @Miciah do you bring this up out of time complexity concerns, or just trying to improve the vendored code?
edit: I now realize the problem... break != continue...

I'm wondering whether the vendored code is correct. For example, if /proc had a file named .foo, it would be sorted before the numeric names, and the loop would break before scanning any pids.

@mrunalp, am I missing something? Does some API guarantee exist for procfs that prohibits file names that would be sorted before numeric names?

openshift-bot · 2020-09-22T13:41:51Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-22T13:54:51Z

/retest

Please review the full test history for this PR and help us cut down flakes.

sgreene570 · 2020-09-22T14:57:37Z

/hold while we verify that the new vendor code works as intended

sgreene570 · 2020-09-22T16:13:29Z

/hold cancel

Miciah · 2020-09-22T16:14:24Z

/lgtm
But let's follow up on #190 (comment).

openshift-bot · 2020-09-22T16:44:08Z

/retest

Please review the full test history for this PR and help us cut down flakes.

sgreene570 · 2020-09-22T18:38:15Z

/test e2e

openshift-bot · 2020-09-22T18:53:52Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-22T20:11:52Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-22T23:39:41Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-23T01:23:29Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-23T01:49:27Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot · 2020-09-30T16:25:38Z

@sgreene570: This pull request references Bugzilla bug 1859134. The bug has been updated to no longer refer to the pull request using the external bug tracker. All external bug links have been closed. The bug has been moved to the NEW state.

In response to this:

Bug 1859134: Switch to periodic process reaper

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.

openshift-ci-robot · 2020-09-30T16:25:41Z

@sgreene570: This pull request references Bugzilla bug 1859134, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1859134: Switch to periodic process reaper

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.

frobware · 2020-09-30T17:06:12Z

/lgtm

openshift-ci-robot · 2020-09-30T17:06:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: frobware, Miciah, sgreene570

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~~ [Miciah,frobware,sgreene570]

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

sgreene570 · 2020-09-30T18:10:15Z

/test e2e

openshift-bot · 2020-09-30T18:34:13Z

/retest

Please review the full test history for this PR and help us cut down flakes.

sgreene570 · 2020-09-30T19:47:51Z

/test e2e

openshift-bot · 2020-09-30T20:05:06Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-30T20:57:08Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-30T21:35:20Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-30T22:27:22Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-30T23:19:21Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-01T00:11:55Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-01T01:16:18Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-01T01:42:20Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-01T02:21:18Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-01T03:13:21Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot · 2020-10-01T03:24:35Z

@sgreene570: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-metal-ipi	`ab08773`	link	`/test e2e-metal-ipi`

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

openshift-bot · 2020-10-01T03:39:19Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot · 2020-10-01T04:43:09Z

@sgreene570: All pull requests linked via external trackers have merged:

openshift/router#190

Bugzilla bug 1859134 has been moved to the MODIFIED state.

In response to this:

Bug 1859134: Switch to periodic process reaper

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.

sgreene570 · 2020-10-06T15:48:25Z

/cherry-pick release-4.5

openshift-cherrypick-robot · 2020-10-06T15:48:29Z

@sgreene570: #190 failed to apply on top of branch "release-4.5":

Applying: pkg/cmd: Switch to periodic reaper
Applying: go.mod: bump openshift/library-go
.git/rebase-apply/patch:912: trailing whitespace.
script: 
.git/rebase-apply/patch:1321: trailing whitespace.
backend        frontend       database 
warning: 2 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	go.mod
M	go.sum
A	vendor/k8s.io/klog/v2/klog.go
M	vendor/modules.txt
Falling back to patching base and 3-way merge...
Auto-merging vendor/modules.txt
CONFLICT (content): Merge conflict in vendor/modules.txt
Auto-merging vendor/k8s.io/klog/klog.go
CONFLICT (content): Merge conflict in vendor/k8s.io/klog/klog.go
Removing vendor/github.com/openshift/library-go/pkg/proc/reaper_unsupported.go
Removing vendor/github.com/openshift/library-go/pkg/proc/reaper.go
Auto-merging go.sum
CONFLICT (content): Merge conflict in go.sum
Auto-merging go.mod
CONFLICT (content): Merge conflict in go.mod
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 go.mod: bump openshift/library-go
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-4.5

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.

openshift-ci-robot assigned frobware Sep 21, 2020

openshift-ci-robot requested review from danehans and Miciah September 21, 2020 18:13

openshift-ci-robot added the bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. label Sep 21, 2020

openshift-ci-robot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 21, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 22, 2020

Miciah reviewed Sep 22, 2020

View reviewed changes

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 22, 2020

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 22, 2020

openshift-ci-robot assigned Miciah Sep 22, 2020

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Sep 30, 2020

sgreene570 closed this Sep 30, 2020

sgreene570 reopened this Sep 30, 2020

sgreene570 force-pushed the bz-1859134 branch from e8d0478 to ab08773 Compare September 30, 2020 16:26

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 30, 2020

openshift-merge-robot merged commit 189bd7f into openshift:master Oct 1, 2020

sgreene570 mentioned this pull request Oct 6, 2020

Bug 1861455: Add basic HAProxy alert rules for HAProxy status and Reload failures openshift/cluster-ingress-operator#397

Merged

sgreene570 mentioned this pull request Oct 6, 2020

[release-4.5] Bug 1885688: Back port Switch to periodic process reaper #195

Merged

Bug 1859134: Switch to periodic process reaper #190

Bug 1859134: Switch to periodic process reaper #190

Conversation

sgreene570 commented Sep 21, 2020

openshift-ci-robot commented Sep 21, 2020

frobware commented Sep 22, 2020

frobware commented Sep 22, 2020

openshift-bot commented Sep 22, 2020

openshift-bot commented Sep 22, 2020

openshift-bot commented Sep 22, 2020

openshift-bot commented Sep 22, 2020

Miciah Sep 22, 2020

Choose a reason for hiding this comment

sgreene570 Sep 22, 2020 • edited

Choose a reason for hiding this comment

Miciah Sep 22, 2020

Choose a reason for hiding this comment

Miciah Sep 22, 2020

Choose a reason for hiding this comment

openshift-bot commented Sep 22, 2020

openshift-bot commented Sep 22, 2020

sgreene570 commented Sep 22, 2020

sgreene570 commented Sep 22, 2020

Miciah commented Sep 22, 2020

openshift-bot commented Sep 22, 2020

sgreene570 commented Sep 22, 2020

openshift-bot commented Sep 22, 2020

openshift-bot commented Sep 22, 2020

openshift-bot commented Sep 22, 2020

openshift-bot commented Sep 23, 2020

openshift-bot commented Sep 23, 2020

openshift-ci-robot commented Sep 30, 2020

openshift-ci-robot commented Sep 30, 2020

frobware commented Sep 30, 2020

openshift-ci-robot commented Sep 30, 2020

sgreene570 commented Sep 30, 2020

openshift-bot commented Sep 30, 2020

sgreene570 commented Sep 30, 2020

openshift-bot commented Sep 30, 2020

openshift-bot commented Sep 30, 2020

openshift-bot commented Sep 30, 2020

openshift-bot commented Sep 30, 2020

openshift-bot commented Sep 30, 2020

openshift-bot commented Oct 1, 2020

openshift-bot commented Oct 1, 2020

openshift-bot commented Oct 1, 2020

openshift-bot commented Oct 1, 2020

openshift-bot commented Oct 1, 2020

openshift-ci-robot commented Oct 1, 2020 • edited

openshift-bot commented Oct 1, 2020

openshift-ci-robot commented Oct 1, 2020

sgreene570 commented Oct 6, 2020

openshift-cherrypick-robot commented Oct 6, 2020

sgreene570 Sep 22, 2020 •

edited

openshift-ci-robot commented Oct 1, 2020 •

edited