Repository: kubernetes-sigs/security-profiles-operator
Pull request #346 (2021-03-04 16:13:29)

Re-enable SELinux by default on OCP
<!--  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?
/kind bug

<!--
Add one of the following kinds:
/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:
Prior to using CRDs, we were using kustomize to change the enableSelinux
key of the config CM to true. This no longer works since we switched to
CRD as it appears that kustomize doesn't handle CRD:
    https://github.com/kubernetes-sigs/kustomize/blob/1d4b3fa36c8918ad884c7aaa6a055fa2d0fa9bf9/examples/transformerconfigs/crd/README.md

Let's re-enable SELinux by default on OpenShift by just using sed.
Long-term we want to switch to auto-detect the capabilities anyway.


#### Which issue(s) this PR fixes:
None

<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.

Fixes #

or

-->

#### Does this PR have test?
N/A

<!--
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
NONE

```

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/346#" title="Author self-approved">jhrozek</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **hasheddan** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @hasheddan` 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/master/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":["hasheddan"]} -->
k8s-ci-robot: Hi @jhrozek. 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>

JAORMX: /ok-to-test
/lgtm
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/346#" title="Author self-approved">jhrozek</a>*, *<a href="https://github.com/kubernetes-sigs/security-profiles-operator/pull/346#pullrequestreview-604288616" title="LGTM">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/master/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/Makefile b/Makefile
index da0a95c70..0fcbf2c27 100644
--- a/Makefile
+++ b/Makefile
@@ -272,6 +272,7 @@ do-deploy-openshift-dev: $(BUILD_DIR)/kustomize
 	oc wait --for=condition=Ready pod -lapp.kubernetes.io/instance=cert-manager -ncert-manager
 	@echo "Building custom operator.yaml"
 	$(BUILD_DIR)/kustomize build --reorder=none deploy/overlays/openshift-dev -o deploy/operator.yaml
+	sed -i 's/enableSelinux: false/enableSelinux: true/' deploy/operator.yaml
 	@echo "Deploying"
 	oc apply -f deploy/operator.yaml
 	@echo "Setting triggers to track image"
diff --git a/deploy/base/config.yaml b/deploy/base/config.yaml
index 647311b6b..e52390928 100644
--- a/deploy/base/config.yaml
+++ b/deploy/base/config.yaml
@@ -7,3 +7,13 @@ metadata:
 spec:
   enableSelinux: false
   enableLogEnricher: false
+  tolerations:
+    - key: "node-role.kubernetes.io/master"
+      operator: "Exists"
+      effect: "NoSchedule"
+    - key: "node-role.kubernetes.io/control-plane"
+      operator: "Exists"
+      effect: "NoSchedule"
+    - key: "node.kubernetes.io/not-ready"
+      operator: "Exists"
+      effect: "NoExecute"
diff --git a/deploy/namespace-operator.yaml b/deploy/namespace-operator.yaml
index c668cd827..9b4b2abfd 100644
--- a/deploy/namespace-operator.yaml
+++ b/deploy/namespace-operator.yaml
@@ -768,6 +768,16 @@ metadata:
 spec:
   enableLogEnricher: false
   enableSelinux: false
+  tolerations:
+  - effect: NoSchedule
+    key: node-role.kubernetes.io/master
+    operator: Exists
+  - effect: NoSchedule
+    key: node-role.kubernetes.io/control-plane
+    operator: Exists
+  - effect: NoExecute
+    key: node.kubernetes.io/not-ready
+    operator: Exists
 ---
 apiVersion: v1
 data:
diff --git a/deploy/operator.yaml b/deploy/operator.yaml
index f32564257..6ba563f28 100644
--- a/deploy/operator.yaml
+++ b/deploy/operator.yaml
@@ -766,6 +766,16 @@ metadata:
 spec:
   enableLogEnricher: false
   enableSelinux: false
+  tolerations:
+  - effect: NoSchedule
+    key: node-role.kubernetes.io/master
+    operator: Exists
+  - effect: NoSchedule
+    key: node-role.kubernetes.io/control-plane
+    operator: Exists
+  - effect: NoExecute
+    key: node.kubernetes.io/not-ready
+    operator: Exists
 ---
 apiVersion: v1
 data:
diff --git a/deploy/overlays/openshift-dev/enable-selinux.yaml b/deploy/overlays/openshift-dev/enable-selinux.yaml
deleted file mode 100644
index 3ca6c6b6d..000000000
--- a/deploy/overlays/openshift-dev/enable-selinux.yaml
+++ /dev/null
@@ -1,3 +0,0 @@
-- op: replace
-  path: /spec/enableSelinux
-  value: "true"
diff --git a/deploy/overlays/openshift-dev/kustomization.yaml b/deploy/overlays/openshift-dev/kustomization.yaml
index 3e64b9db2..90a26d509 100644
--- a/deploy/overlays/openshift-dev/kustomization.yaml
+++ b/deploy/overlays/openshift-dev/kustomization.yaml
@@ -12,11 +12,6 @@ patchesJson6902:
       kind: ConfigMap
       name: security-profiles-operator-profile
     path: operator-profile.yaml
-  - target:
-      version: security-profiles-operator.x-k8s.io/v1alpha1
-      kind: SecurityProfilesOperatorDaemon
-      name: spod
-    path: enable-selinux.yaml
 
 images:
   - name: gcr.io/k8s-staging-sp-operator/security-profiles-operator
diff --git a/installation-usage.md b/installation-usage.md
index 788d6ceae..74c0b97da 100644
--- a/installation-usage.md
+++ b/installation-usage.md
@@ -208,7 +208,7 @@ hooks_dir = [
 ```
 
 The hook references a [path][path] to the actual binary which gets executed on
-`prestart`. Please note that at least CRI-O v1.20.0 is required to let the hook
+`prestart`. Please note that at least CRI-O v1.21.0 is required to let the hook
 and CRI-O work nicely together.
 
 [bpf-hook]: https://github.com/containers/oci-seccomp-bpf-hook
@@ -249,17 +249,17 @@ multiple times.
 Now we can start the recording by running a workload which contains the label:
 
 ```
-$ kubectl run --rm -it alpine --image=alpine --labels app=alpine -- sh
+$ kubectl run --rm -it my-pod --image=alpine --labels app=alpine -- sh
 / # mkdir test
 ```
 
 If we exit the workload, then it automatically will be removed because of the
 `--rm` CLI flag. Once the workload is removed, the operator will create the CRD
-for us:
+for us. The name of the CRD is suffixed with the pod name:
 
 ```
-$ kubectl describe seccompprofile test-recording
-Name:         test-recording
+$ kubectl describe seccompprofile test-recording-my-pod
+Name:         test-recording-my-pod
 Namespace:    security-profiles-operator
 …
 Spec:
@@ -272,10 +272,10 @@ Spec:
       …[other syscalls]…
       mkdir
       …[other syscalls]…
-  Target Workload:
+  Target Workload: my-pod
 Status:
-  Localhost Profile:  operator/security-profiles-operator/test-recording.json
-  Path:               /var/lib/kubelet/seccomp/operator/security-profiles-operator/test-recording.json
+  Localhost Profile:  operator/security-profiles-operator/my-pod/test-recording-my-pod.json
+  Path:               /var/lib/kubelet/seccomp/operator/security-profiles-operator/my-pod/test-recording-my-pod.json
   Status:             Active
 Events:
   Type    Reason                 Age                From             Message
@@ -297,6 +297,46 @@ if anything goes wrong:
 11s         Normal    SavedSeccompProfile            seccompprofile/test-recording                             Successfully saved profile to disk
 ```
 
+It is also possible to record profiles from multiple containers, for example by
+using this recording and Pod manifest:
+
+```
+---
+apiVersion: security-profiles-operator.x-k8s.io/v1alpha1
+kind: ProfileRecording
+metadata:
+  name: recording
+spec:
+  kind: SeccompProfile
+  podSelector:
+    matchLabels:
+      app: my-app
+---
+apiVersion: v1
+kind: Pod
+metadata:
+  name: my-pod
+  labels:
+    app: my-app
+spec:
+  containers:
+  - name: nginx
+    image: quay.io/security-profiles-operator/test-nginx:1.19.1
+  - name: redis
+    image: quay.io/security-profiles-operator/redis:6.2.1
+  restartPolicy: Never
+```
+
+If the workload gets created and removed again, then the recorder will produce
+two seccomp profiles for each container:
+
+```
+> kubectl get sp -o wide
+NAME              STATUS   AGE   LOCALHOSTPROFILE
+recording-nginx   Active   32s   operator/default/my-pod/recording-nginx.json
+recording-redis   Active   32s   operator/default/my-pod/recording-redis.json
+```
+
 Please note that we encourage you to only use this recording approach for
 development purposes. It is not recommended to use the OCI hook in production
 clusters because it runs as highly privileged process to trace the container
diff --git a/internal/pkg/daemon/profilerecorder/profilerecorder.go b/internal/pkg/daemon/profilerecorder/profilerecorder.go
index cf60f28e7..029c14b31 100644
--- a/internal/pkg/daemon/profilerecorder/profilerecorder.go
+++ b/internal/pkg/daemon/profilerecorder/profilerecorder.go
@@ -19,6 +19,7 @@ package profilerecorder
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -64,7 +65,7 @@ type RecorderReconciler struct {
 	log           logr.Logger
 	record        event.Recorder
 	nodeAddresses []string
-	podsToWatch   map[string]string
+	podsToWatch   map[string][]string
 }
 
 // +kubebuilder:rbac:groups=core,resources=nodes,verbs=get;list;watch
@@ -103,7 +104,7 @@ func Setup(ctx context.Context, mgr ctrl.Manager, l logr.Logger) error {
 		log:           l,
 		nodeAddresses: nodeAddresses,
 		record:        event.NewAPIRecorder(mgr.GetEventRecorderFor(name)),
-		podsToWatch:   map[string]string{},
+		podsToWatch:   map[string][]string{},
 	}
 
 	return ctrl.NewControllerManagedBy(mgr).
@@ -139,11 +140,17 @@ func (r *RecorderReconciler) isPodWithTraceAnnotation(obj runtime.Object) bool {
 		return false
 	}
 
-	return p.Annotations[config.SeccompProfileRecordAnnotationKey] != ""
+	for key := range p.Annotations {
+		if strings.HasPrefix(key, config.SeccompProfileRecordAnnotationKey+"/") {
+			return true
+		}
+	}
+
+	return false
 }
 
 func (r *RecorderReconciler) Reconcile(_ context.Context, req reconcile.Request) (reconcile.Result, error) {
-	logger := r.log.WithValues("recorder", req.Name, "namespace", req.Namespace)
+	logger := r.log.WithValues("pod", req.Name, "namespace", req.Namespace)
 
 	ctx, cancel := context.WithTimeout(context.Background(), reconcileTimeout)
 	defer cancel()
@@ -162,11 +169,12 @@ func (r *RecorderReconciler) Reconcile(_ context.Context, req reconcile.Request)
 	}
 
 	if pod.Status.Phase == corev1.PodPending {
-		if r.podsToWatch[req.NamespacedName.String()] != "" {
+		if len(r.podsToWatch[req.NamespacedName.String()]) > 0 {
 			// We're tracking this pod already
 			return reconcile.Result{}, nil
 		}
-		outputFile, err := parseAnnotation(pod.Annotations[config.SeccompProfileRecordAnnotationKey])
+
+		outputFiles, err := parseAnnotations(pod.Annotations)
 		if err != nil {
 			// Malformed annotations could be set by users directly, which is
 			// why we are ignoring them.
@@ -174,14 +182,12 @@ func (r *RecorderReconciler) Reconcile(_ context.Context, req reconcile.Request)
 			r.record.Event(pod, event.Warning(reasonAnnotationParsing, err))
 			return reconcile.Result{}, nil
 		}
-		if !strings.HasPrefix(outputFile, config.ProfileRecordingOutputPath) {
-			logger.Info("Ignoring profile outside standard output path")
-			return reconcile.Result{}, nil
-		}
 
-		logger.Info("Recording seccomp profile", "path", outputFile, "pod", pod.Name)
-		r.podsToWatch[req.NamespacedName.String()] = outputFile
-		r.record.Event(pod, event.Normal(reasonProfileRecording, "Recording seccomp profile"))
+		logger.Info(fmt.Sprintf(
+			"Recording seccomp profiles: %s", strings.Join(outputFiles, ", "),
+		))
+		r.podsToWatch[req.NamespacedName.String()] = outputFiles
+		r.record.Event(pod, event.Normal(reasonProfileRecording, "Recording seccomp profiles"))
 	}
 
 	if pod.Status.Phase == corev1.PodSucceeded {
@@ -194,68 +200,90 @@ func (r *RecorderReconciler) Reconcile(_ context.Context, req reconcile.Request)
 }
 
 func (r *RecorderReconciler) collectProfile(ctx context.Context, name types.NamespacedName) error {
-	profilePath := r.podsToWatch[name.String()]
-	if profilePath == "" {
-		// ignoring this error for now
-		return nil
-	}
-
-	r.log.Info("Collecting profile", "path", r.podsToWatch[name.String()])
 	defer delete(r.podsToWatch, name.String())
-	data, err := ioutil.ReadFile(profilePath)
-	if err != nil {
-		r.log.Error(err, "Failed to read profile")
-		return errors.Wrap(err, "read profile")
-	}
 
-	// Remove the file extension and timestamp
-	base := filepath.Base(profilePath)
-	lastIndex := strings.LastIndex(base, "-")
-	if lastIndex == -1 {
-		return errors.Errorf("malformed profile path: %s", profilePath)
-	}
-	profileName := base[:lastIndex]
+	for _, profilePath := range r.podsToWatch[name.String()] {
+		r.log.Info("Collecting profile", "path", profilePath)
 
-	profile := &v1alpha1.SeccompProfile{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      profileName,
-			Namespace: name.Namespace,
-		},
-	}
-	res, err := controllerutil.CreateOrUpdate(ctx, r.client, profile, func() error {
-		err := json.Unmarshal(data, &profile.Spec)
-		return errors.Wrap(err, "unmarshal profile spec JSON")
-	})
-	if err != nil {
-		r.log.Error(err, "cannot create seccompprofile resource")
-		r.record.Event(profile, event.Warning(reasonProfileCreationFailed, err))
-		return errors.Wrap(err, "create seccompProfile resource")
+		data, err := ioutil.ReadFile(profilePath)
+		if err != nil {
+			r.log.Error(err, "Failed to read profile")
+			return errors.Wrap(err, "read profile")
+		}
+
+		// Remove the file extension and timestamp
+		base := filepath.Base(profilePath)
+		lastIndex := strings.LastIndex(base, "-")
+		if lastIndex == -1 {
+			return errors.Errorf("Malformed profile path: %s", profilePath)
+		}
+		profileName := base[:lastIndex]
+
+		profile := &v1alpha1.SeccompProfile{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      profileName,
+				Namespace: name.Namespace,
+			},
+			Spec: v1alpha1.SeccompProfileSpec{
+				TargetWorkload: name.Name,
+			},
+		}
+		res, err := controllerutil.CreateOrUpdate(ctx, r.client, profile,
+			func() error {
+				return errors.Wrap(
+					json.Unmarshal(data, &profile.Spec),
+					"unmarshal profile spec JSON",
+				)
+			},
+		)
+		if err != nil {
+			r.log.Error(err, "Cannot create seccompprofile resource")
+			r.record.Event(profile, event.Warning(reasonProfileCreationFailed, err))
+			return errors.Wrap(err, "create seccompProfile resource")
+		}
+		r.log.Info("Created/updated profile", "action", res, "name", profileName)
+		r.record.Event(
+			profile,
+			event.Normal(reasonProfileCreated, "seccomp profile created"),
+		)
 	}
-	r.log.Info("Created/updated profile", "action", res, "name", profileName)
-	r.record.Event(profile, event.Normal(reasonProfileCreated, "seccomp profile created"))
+
 	return nil
 }
 
-// parseAnnotation parses the provided annotation and extracts the mandatory
-// output file.
-func parseAnnotation(annotation string) (string, error) {
+// parseAnnotations parses the provided annotations and extracts the mandatory
+// output files.
+func parseAnnotations(annotations map[string]string) (res []string, err error) {
 	const prefix = "of:"
 
-	if !strings.HasPrefix(annotation, prefix) {
-		return "", errors.Wrapf(errors.New(errInvalidAnnotation),
-			"must start with %q prefix", prefix)
-	}
+	for key, value := range annotations {
+		if !strings.HasPrefix(key, config.SeccompProfileRecordAnnotationKey+"/") {
+			continue
+		}
 
-	outputFile := strings.TrimSpace(strings.TrimPrefix(annotation, prefix))
-	if !filepath.IsAbs(outputFile) {
-		return "", errors.Wrapf(errors.New(errInvalidAnnotation),
-			"output file path must be absolute: %q", outputFile)
-	}
+		if !strings.HasPrefix(value, prefix) {
+			return nil, errors.Wrapf(errors.New(errInvalidAnnotation),
+				"must start with %q prefix", prefix)
+		}
+
+		outputFile := strings.TrimSpace(strings.TrimPrefix(value, prefix))
+		if !filepath.IsAbs(outputFile) {
+			return nil, errors.Wrapf(errors.New(errInvalidAnnotation),
+				"output file path must be absolute: %q", outputFile)
+		}
+
+		if outputFile == "" {
+			return nil, errors.Wrap(errors.New(errInvalidAnnotation),
+				"providing output file is mandatory")
+		}
+		if !strings.HasPrefix(outputFile, config.ProfileRecordingOutputPath) {
+			// Ignoring profile outside standard output path, it may be
+			// user-defined
+			continue
+		}
 
-	if outputFile == "" {
-		return "", errors.Wrap(errors.New(errInvalidAnnotation),
-			"providing output file is mandatory")
+		res = append(res, outputFile)
 	}
 
-	return outputFile, nil
+	return res, nil
 }
diff --git a/internal/pkg/webhooks/recording/recording.go b/internal/pkg/webhooks/recording/recording.go
index 7f64545ab..401ccc590 100644
--- a/internal/pkg/webhooks/recording/recording.go
+++ b/internal/pkg/webhooks/recording/recording.go
@@ -122,7 +122,7 @@ func (p *podSeccompRecorder) Handle(
 		}
 
 		if selector.Matches(podLabels) {
-			podChanged = p.addAnnotation(pod, &items[i])
+			podChanged = p.addAnnotations(pod, &items[i])
 		}
 
 		if podChanged {
@@ -145,41 +145,54 @@ func (p *podSeccompRecorder) Handle(
 	return admission.PatchResponseFromRaw(req.Object.Raw, marshaledPod)
 }
 
-func (p *podSeccompRecorder) addAnnotation(
+func (p *podSeccompRecorder) addAnnotations(
 	pod *corev1.Pod,
 	profileRecording *profilerecordingv1alpha1.ProfileRecording,
 ) (podChanged bool) {
-	value, ok := pod.GetAnnotations()[config.SeccompProfileRecordAnnotationKey]
-
-	targetValue := fmt.Sprintf(
-		"of:%s/%s-%d.json",
-		config.ProfileRecordingOutputPath,
-		profileRecording.GetName(),
-		time.Now().Unix(),
-	)
+	ctrs := []corev1.Container{}
+	ctrs = append(ctrs, pod.Spec.InitContainers...)
+	ctrs = append(ctrs, pod.Spec.Containers...)
+
+	for i := range ctrs {
+		ctr := &ctrs[i]
+		key := fmt.Sprintf("%s/%s", config.SeccompProfileRecordAnnotationKey, ctr.Name)
+
+		value := fmt.Sprintf(
+			"of:%s/%s-%s-%d.json",
+			config.ProfileRecordingOutputPath,
+			profileRecording.GetName(),
+			ctr.Name,
+			time.Now().Unix(),
+		)
 
-	if !ok {
-		if pod.Annotations == nil {
-			pod.Annotations = make(map[string]string)
+		existingValue, ok := pod.GetAnnotations()[key]
+		if !ok {
+			if pod.Annotations == nil {
+				pod.Annotations = make(map[string]string)
+			}
+			pod.Annotations[key] = value
+			log.Info(fmt.Sprintf(
+				"adding seccomp recording annotation %s=%s to pod %s",
+				key, value, pod.Name,
+			))
+			podChanged = true
+			continue
 		}
-		pod.Annotations[config.SeccompProfileRecordAnnotationKey] = targetValue
-		log.Info("adding seccomp recording annotation to pod", "Pod", pod.Name)
-		return true
-	}
 
-	if value != targetValue {
-		log.Error(
-			errors.New("existing annotation"),
-			fmt.Sprintf(
-				"Workload %s already has annotation %s (not mutating to %s).",
-				pod.Name,
-				value,
-				targetValue,
-			),
-		)
+		if existingValue != value {
+			log.Error(
+				errors.New("existing annotation"),
+				fmt.Sprintf(
+					"workload %s already has annotation %s (not mutating to %s).",
+					pod.Name,
+					existingValue,
+					value,
+				),
+			)
+		}
 	}
 
-	return false
+	return podChanged
 }
 
 func (p *podSeccompRecorder) addPod(


Test patch:
diff --git a/test/e2e_test.go b/test/e2e_test.go
index 0848f802b..e7c00f3f3 100644
--- a/test/e2e_test.go
+++ b/test/e2e_test.go
@@ -107,6 +107,7 @@ func (e *e2e) TestSecurityProfilesOperator() {
 	e.Run("cluster-wide: Seccomp: Verify profile recording", func() {
 		e.testCaseProfileRecordingStaticPod()
 		e.testCaseProfileRecordingKubectlRun()
+		e.testCaseProfileRecordingMultiContainer()
 	})
 
 	e.cleanupWebhook(webhookManifest)
diff --git a/test/tc_profilerecordings_test.go b/test/tc_profilerecordings_test.go
index 23b4929a7..23050da2c 100644
--- a/test/tc_profilerecordings_test.go
+++ b/test/tc_profilerecordings_test.go
@@ -44,8 +44,7 @@ metadata:
 spec:
   containers:
   - image: quay.io/security-profiles-operator/test-nginx:1.19.1
-    name: recording
-    resources: {}
+    name: nginx
   restartPolicy: Never
 `
 	testPodFile, err := ioutil.TempFile(os.TempDir(), "recording-pod*.yaml")
@@ -61,8 +60,9 @@ spec:
 	e.waitFor("condition=ready", "pod", "recording")
 	e.kubectl("delete", "pod", "recording")
 
-	profile := e.kubectl("get", "sp", recordingName, "-o", "yaml")
-	defer e.kubectl("delete", "sp", recordingName)
+	resourceName := recordingName + "-nginx"
+	profile := e.kubectl("get", "sp", resourceName, "-o", "yaml")
+	defer e.kubectl("delete", "sp", resourceName)
 	e.Contains(profile, "mkdir")
 }
 
@@ -77,10 +77,55 @@ func (e *e2e) testCaseProfileRecordingKubectlRun() {
 	e.kubectl(
 		"run", "--rm", "-it", "--restart=Never", "--labels=app=alpine",
 		"--image=registry.fedoraproject.org/fedora-minimal:latest",
-		"test", "--", "echo", "test",
+		"fedora", "--", "echo", "test",
 	)
 
-	profile := e.kubectl("get", "sp", recordingName, "-o", "yaml")
+	resourceName := recordingName + "-fedora"
+	profile := e.kubectl("get", "sp", resourceName, "-o", "yaml")
+	defer e.kubectl("delete", "sp", resourceName)
 	e.Contains(profile, "prctl")
 	e.NotContains(profile, "mkdir")
 }
+
+func (e *e2e) testCaseProfileRecordingMultiContainer() {
+	e.profileRecordingTestCase()
+
+	e.logf("Creating recording")
+	e.kubectl("create", "-f", exampleRecordingPath)
+	defer e.kubectl("delete", "-f", exampleRecordingPath)
+
+	e.logf("Creating test pod")
+	const testPod = `
+apiVersion: v1
+kind: Pod
+metadata:
+  name: my-pod
+  labels:
+    app: alpine
+spec:
+  containers:
+  - name: nginx
+    image: quay.io/security-profiles-operator/test-nginx:1.19.1
+  - name: redis
+    image: quay.io/security-profiles-operator/redis:6.2.1
+  restartPolicy: Never
+`
+	testPodFile, err := ioutil.TempFile(os.TempDir(), "recording-pod*.yaml")
+	e.Nil(err)
+	defer os.Remove(testPodFile.Name())
+	_, err = testPodFile.Write([]byte(testPod))
+	e.Nil(err)
+	err = testPodFile.Close()
+	e.Nil(err)
+	e.kubectl("create", "-f", testPodFile.Name())
+
+	e.logf("Waiting for test pod to be initialized")
+	e.waitFor("condition=ready", "pod", "my-pod")
+	e.kubectl("delete", "pod", "my-pod")
+
+	profileRedis := e.kubectl("get", "sp", recordingName+"-redis", "-o", "yaml")
+	profileNginx := e.kubectl("get", "sp", recordingName+"-nginx", "-o", "yaml")
+	defer e.kubectl("delete", "sp", "--all")
+	e.Contains(profileNginx, "unlink")
+	e.Contains(profileRedis, "nanosleep")
+}