Repository: kubernetes-sigs/security-profiles-operator
Pull request #824 (2022-03-08 14:55:41)

fix: keep retrieving the remaining profiles when a PID is no longer found

<!--  Thanks for sending a pull request!  Here are some tips for you:

- If this is your first time, please read our contributor guidelines: https://git.k8s.io/community/contributors/guide#your-first-contribution and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
- Please label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. For reference on required PR/issue labels, read here:
https://git.k8s.io/community/contributors/devel/sig-release/release.md#issuepr-kind-label
- If you want *faster* PR reviews, read how: https://git.k8s.io/community/contributors/guide/pull-requests.md#best-practices-for-faster-reviews
- If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
-->

#### What type of PR is this?

There are sometime init containers in a pod for which the PID is not longer
active at the point when the pod is removed. In this situation, we still want
to retrieve the remaining seccomp profiles.

<!--
Add one of the following kinds:
/kind bug
/kind cleanup
/kind documentation
/kind feature
/kind design

Optionally add one or more of the following kinds if applicable:
/kind api-change
/kind deprecation
/kind failing-test
/kind flake
/kind regression
-->

#### What this PR does / why we need it:

#### Which issue(s) this PR fixes:

<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.

Fixes #

or

None
-->

#### Does this PR have test?

<!--
If tests aren't applicable just write N/A.
-->

#### Special notes for your reviewer:

#### Does this PR introduce a user-facing change?

<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".

For more information on release notes see: https://git.k8s.io/community/contributors/guide/release-notes.md
-->

```release-note
Keep retrieving the remaining profiles when a PID is no longer found.
```


linux-foundation-easycla[bot]:  <a href="https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/20677499/258468842/824/#/?version=2"><img src="https://s3.amazonaws.com/cla-project-logo-prod/cla-missing-id.svg" alt="CLA Missing ID" align="left" height="28" width="328"></a> <a href="https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/20677499/258468842/824/#/?version=2"><img src="https://s3.amazonaws.com/cla-project-logo-prod/cla-not-signed.svg" alt="CLA Not Signed" align="left" height="28" width="328"></a><br/><br /><ul><li> :x: The commit (03854f41dcbfd0b667343bcbd7f889314188e354). This user is missing the User's ID, preventing the EasyCLA check. <a href='https://help.github.com/en/github/committing-changes-to-your-project/why-are-my-commits-linked-to-the-wrong-user' target='_blank'>Consult GitHub Help</a> to resolve.For further assistance with EasyCLA, <a href='https://jira.linuxfoundation.org/servicedesk/customer/portal/4' target='_blank'>please submit a support request ticket</a>.</li></ul>
k8s-ci-robot: Welcome @ccojocar! <br><br>It looks like this is your first PR to <a href='https://github.com/kubernetes-sigs/security-profiles-operator'>kubernetes-sigs/security-profiles-operator</a> 🎉. Please refer to our [pull request process documentation](https://git.k8s.io/community/contributors/guide/pull-requests.md) to help your PR have a smooth ride to approval. <br><br>You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. [Here is the bot commands documentation](https://go.k8s.io/bot-commands). <br><br>You can also check if kubernetes-sigs/security-profiles-operator has [its own contribution guidelines](https://github.com/kubernetes-sigs/security-profiles-operator/tree/master/CONTRIBUTING.md). <br><br>You may want to refer to our [testing guide](https://git.k8s.io/community/contributors/devel/sig-testing/testing.md) if you run into trouble with your tests not passing. <br><br>If you are having difficulty getting your pull request seen, please follow the [recommended escalation practices](https://github.com/kubernetes/community/blob/master/contributors/guide/pull-requests.md#why-is-my-pull-request-not-getting-reviewed). Also, for tips and tricks in the contribution process you may want to read the [Kubernetes contributor cheat sheet](https://git.k8s.io/community/contributors/guide/contributor-cheatsheet/README.md). We want to make sure your contribution gets all the attention it needs! <br><br>Thank you, and welcome to Kubernetes. :smiley:
k8s-ci-robot: Hi @ccojocar. Thanks for your PR.

I'm waiting for a [kubernetes-sigs](https://github.com/orgs/kubernetes-sigs/people) member to verify that this patch is reasonable to test. If it is, they should reply with `/ok-to-test` on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should [join the org](https://git.k8s.io/community/community-membership.md#member) to skip this step.

Once the patch is verified, the new status will be reflected by the `ok-to-test` label.

I understand the commands that are listed [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fsecurity-profiles-operator).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>

k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/security-profiles-operator/pull/824#" title="Author self-approved">ccojocar</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **saschagrunert** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @saschagrunert` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fsecurity-profiles-operator).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubernetes-sigs/security-profiles-operator/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["saschagrunert"]} -->
saschagrunert: Thank you for the contribution @ccojocar! Can you please sign-off your commits? ☺️
saschagrunert: /ok-to-test 
ccojocar: Thanks @saschagrunert,  I requested from my company to sign the CLA as an organisation. It'll take a bit. 
k8s-ci-robot: @ccojocar: 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-security-profiles-operator-test-unit | 03854f41dcbfd0b667343bcbd7f889314188e354 | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_security-profiles-operator/824/pull-security-profiles-operator-test-unit/1501211597358501888) | true | `/test pull-security-profiles-operator-test-unit`
pull-security-profiles-operator-test-e2e | 03854f41dcbfd0b667343bcbd7f889314188e354 | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_security-profiles-operator/824/pull-security-profiles-operator-test-e2e/1501211597438193664) | true | `/test pull-security-profiles-operator-test-e2e`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=security-profiles-operator&pr=824). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Accojocar). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/security-profiles-operator/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
k8s-ci-robot: @ccojocar: 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-security-profiles-operator-test-e2e | 03854f41dcbfd0b667343bcbd7f889314188e354 | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_security-profiles-operator/824/pull-security-profiles-operator-test-e2e/1501211597438193664) | true | `/test pull-security-profiles-operator-test-e2e`
pull-security-profiles-operator-test-unit | 636acddd5df9b548243d014e11fc687001f2f29d | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_security-profiles-operator/824/pull-security-profiles-operator-test-unit/1501215376191524864) | true | `/test pull-security-profiles-operator-test-unit`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=security-profiles-operator&pr=824). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Accojocar). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/security-profiles-operator/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
saschagrunert: /retest
k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/security-profiles-operator/pull/824#" title="Author self-approved">ccojocar</a>*, *<a href="https://github.com/kubernetes-sigs/security-profiles-operator/pull/824#pullrequestreview-903991779" title="Approved">saschagrunert</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fsecurity-profiles-operator).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubernetes-sigs/security-profiles-operator/blob/main/OWNERS)~~ [saschagrunert]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/internal/pkg/daemon/profilerecorder/profilerecorder.go b/internal/pkg/daemon/profilerecorder/profilerecorder.go
index db98790ee..bab99926f 100644
--- a/internal/pkg/daemon/profilerecorder/profilerecorder.go
+++ b/internal/pkg/daemon/profilerecorder/profilerecorder.go
@@ -654,16 +654,15 @@ func (r *RecorderReconciler) collectBpfProfiles(
 			recorderClient, ctx, &bpfrecorderapi.ProfileRequest{Name: profile.name},
 		)
 		if err != nil {
-			// Something went wrong during BPF event collection, which also
-			// means that we do not have to retry this failure case.
+			// PID was not found for this profile, this might be an init container
+			// which is not longer active. Let's skip here and keep processing the
+			// next profile.
 			if status.Convert(err).Message() == bpfrecorder.ErrNotFound.Error() {
-				r.log.Error(err, "Recorded profile not found, giving up")
-				if err := r.stopBpfRecorder(); err != nil {
-					r.log.Error(err, "Unable to stop bpf recorder")
-				}
-				return nil
+				r.log.Error(err, "Recorded profile not found", profile.name)
+				continue
+			} else {
+				return errors.Wrap(err, "get syscalls for profile")
 			}
-			return errors.Wrap(err, "get syscalls for profile")
 		}
 
 		arch, err := r.goArchToSeccompArch(response.GoArch)


Test patch:
diff --git a/internal/pkg/daemon/profilerecorder/profilerecorder_test.go b/internal/pkg/daemon/profilerecorder/profilerecorder_test.go
index 4c2d81976..e21bb3469 100644
--- a/internal/pkg/daemon/profilerecorder/profilerecorder_test.go
+++ b/internal/pkg/daemon/profilerecorder/profilerecorder_test.go
@@ -458,7 +458,7 @@ func TestReconcile(t *testing.T) {
 				}, nil)
 				mock.DialBpfRecorderReturns(nil, func() {}, nil)
 				mock.SyscallsForProfileReturns(nil, bpfrecorder.ErrNotFound)
-				mock.StopBpfRecorderReturns(errTest) // will be ignored
+				mock.StopBpfRecorderReturns(nil)
 			},
 			assert: func(sut *RecorderReconciler, err error) {
 				assert.Nil(t, err)