Repository: kubernetes-sigs/security-profiles-operator
Pull request #370 (2021-03-15 11:04:00)

Do not throw "Profile saved on disk" event if profile not modified
#### What type of PR is this?

/kind cleanup


#### What this PR does / why we need it:
If a seccomp profile has not been modified locally then we do not throw
the event any more. This avoids spamming the event on large clusters.
#### 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
-->
None
#### Does this PR have test?
None
<!--
If tests aren't applicable just write N/A.
-->

#### Special notes for your reviewer:
None
#### 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
Throw "profile saved to disk" event only if a profile modification happened on the node.
```


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/370#" title="Author self-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/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":[]} -->
codecov-io: # [Codecov](https://codecov.io/gh/kubernetes-sigs/security-profiles-operator/pull/370?src=pr&el=h1) Report
> Merging [#370](https://codecov.io/gh/kubernetes-sigs/security-profiles-operator/pull/370?src=pr&el=desc) (548fb34) into [master](https://codecov.io/gh/kubernetes-sigs/security-profiles-operator/commit/f72243f01f6f792b54febfb5d6c67c6deaeed21a?el=desc) (f72243f) will **decrease** coverage by `0.05%`.
> The diff coverage is `54.54%`.

[![Impacted file tree graph](https://codecov.io/gh/kubernetes-sigs/security-profiles-operator/pull/370/graphs/tree.svg?width=650&height=150&src=pr&token=37VIWSZ1ZT)](https://codecov.io/gh/kubernetes-sigs/security-profiles-operator/pull/370?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #370      +/-   ##
==========================================
- Coverage   27.65%   27.60%   -0.06%     
==========================================
  Files          10       10              
  Lines         752      757       +5     
==========================================
+ Hits          208      209       +1     
- Misses        525      527       +2     
- Partials       19       21       +2     
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `27.60% <54.54%> (-0.06%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/kubernetes-sigs/security-profiles-operator/pull/370?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ternal/pkg/daemon/seccompprofile/seccompprofile.go](https://codecov.io/gh/kubernetes-sigs/security-profiles-operator/pull/370/diff?src=pr&el=tree#diff-aW50ZXJuYWwvcGtnL2RhZW1vbi9zZWNjb21wcHJvZmlsZS9zZWNjb21wcHJvZmlsZS5nbw==) | `34.84% <54.54%> (-0.27%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/kubernetes-sigs/security-profiles-operator/pull/370?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/kubernetes-sigs/security-profiles-operator/pull/370?src=pr&el=footer). Last update [f72243f...c6636ba](https://codecov.io/gh/kubernetes-sigs/security-profiles-operator/pull/370?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

k8s-ci-robot: @saschagrunert: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-security-profiles-operator-test-e2e | c6636ba4e380ff168f71a18f81d693420cf4ce09 | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_security-profiles-operator/370/pull-security-profiles-operator-test-e2e/1371417596192100352) | `/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=370). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Asaschagrunert). 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: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/security-profiles-operator/pull/370#pullrequestreview-612087853" title="LGTM">jhrozek</a>*, *<a href="https://github.com/kubernetes-sigs/security-profiles-operator/pull/370#" title="Author self-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/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":[]} -->
saschagrunert: /retest
k8s-ci-robot: @saschagrunert: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-security-profiles-operator-test-e2e | c6636ba4e380ff168f71a18f81d693420cf4ce09 | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_security-profiles-operator/370/pull-security-profiles-operator-test-e2e/1371425838158843904) | `/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=370). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Asaschagrunert). 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: New changes are detected. LGTM label has been removed.
k8s-ci-robot: @saschagrunert: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-security-profiles-operator-test-e2e | a299ad7dfb5014426f810226581ad29cd75ff462 | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_security-profiles-operator/370/pull-security-profiles-operator-test-e2e/1371463191330033664) | `/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=370). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Asaschagrunert). 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

Patch:
diff --git a/go.mod b/go.mod
index 7652b48f1..3dce2e54f 100644
--- a/go.mod
+++ b/go.mod
@@ -13,7 +13,7 @@ require (
 	k8s.io/api v0.20.4
 	k8s.io/apimachinery v0.20.4
 	k8s.io/client-go v0.20.4
-	k8s.io/klog/v2 v2.6.0
+	k8s.io/klog/v2 v2.8.0
 	k8s.io/release v0.7.0
 	sigs.k8s.io/controller-runtime v0.8.3
 	sigs.k8s.io/controller-tools v0.5.0
diff --git a/go.sum b/go.sum
index 1f2c9c3d3..2fe310ccd 100644
--- a/go.sum
+++ b/go.sum
@@ -1405,8 +1405,8 @@ k8s.io/klog v1.0.0/go.mod h1:4Bi6QPql/J/LkTDqv7R/cd3hPo4k2DG6Ptcz060Ez5I=
 k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE=
 k8s.io/klog/v2 v2.2.0/go.mod h1:Od+F08eJP+W3HUb4pSrPpgp9DGU4GzlpG/TmITuYh/Y=
 k8s.io/klog/v2 v2.4.0/go.mod h1:Od+F08eJP+W3HUb4pSrPpgp9DGU4GzlpG/TmITuYh/Y=
-k8s.io/klog/v2 v2.6.0 h1:c1wFxejFMBkp/VxCdc6kYdgrBkC2gzmcl6afuJAkJyU=
-k8s.io/klog/v2 v2.6.0/go.mod h1:hy9LJ/NvuK+iVyP4Ehqva4HxZG/oXyIS3n3Jmire4Ec=
+k8s.io/klog/v2 v2.8.0 h1:Q3gmuM9hKEjefWFFYF0Mat+YyFJvsUyYuwyNNJ5C9Ts=
+k8s.io/klog/v2 v2.8.0/go.mod h1:hy9LJ/NvuK+iVyP4Ehqva4HxZG/oXyIS3n3Jmire4Ec=
 k8s.io/kube-openapi v0.0.0-20190816220812-743ec37842bf/go.mod h1:1TqjTSzOxsLGIKfj0lK8EeCP7K1iUG65v09OM0/WG5E=
 k8s.io/kube-openapi v0.0.0-20191107075043-30be4d16710a/go.mod h1:1TqjTSzOxsLGIKfj0lK8EeCP7K1iUG65v09OM0/WG5E=
 k8s.io/kube-openapi v0.0.0-20200410145947-61e04a5be9a6/go.mod h1:GRQhZsXIAJ1xR0C9bd8UpWHZ5plfAS9fzPjJuQ6JL3E=
diff --git a/internal/pkg/daemon/seccompprofile/seccompprofile.go b/internal/pkg/daemon/seccompprofile/seccompprofile.go
index 9788cf7f5..b2641b085 100644
--- a/internal/pkg/daemon/seccompprofile/seccompprofile.go
+++ b/internal/pkg/daemon/seccompprofile/seccompprofile.go
@@ -17,6 +17,7 @@ limitations under the License.
 package seccompprofile
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
@@ -135,7 +136,7 @@ func Setup(ctx context.Context, mgr ctrl.Manager, l logr.Logger) error {
 		})
 }
 
-type saver func(string, []byte) error
+type saver func(string, []byte) (bool, error)
 
 // A Reconciler reconciles seccomp profiles.
 type Reconciler struct {
@@ -314,7 +315,8 @@ func (r *Reconciler) reconcileSeccompProfile(
 		r.record.Event(sp, event.Warning(reasonCannotUpdateProfile, err))
 		return reconcile.Result{}, errors.Wrap(err, "adding finalizer for SeccompProfile")
 	}
-	if err = r.save(profilePath, profileContent); err != nil {
+	updated, err := r.save(profilePath, profileContent)
+	if err != nil {
 		l.Error(err, "cannot save profile into disk")
 		r.record.Event(sp, event.Warning(reasonCannotSaveProfile, err))
 		return reconcile.Result{RequeueAfter: wait}, nil
@@ -339,7 +341,9 @@ func (r *Reconciler) reconcileSeccompProfile(
 		"resource version", sp.GetResourceVersion(),
 		"name", sp.GetName(),
 	)
-	r.record.Event(sp, event.Normal(reasonSavedProfile, "Successfully saved profile to disk"))
+	if updated {
+		r.record.Event(sp, event.Normal(reasonSavedProfile, "Successfully saved profile to disk"))
+	}
 	return reconcile.Result{}, nil
 }
 
@@ -558,15 +562,21 @@ func handleDeletion(sp *v1alpha1.SeccompProfile, l logr.Logger) error {
 	return nil
 }
 
-func saveProfileOnDisk(fileName string, contents []byte) error {
+func saveProfileOnDisk(fileName string, content []byte) (updated bool, err error) {
 	if err := os.MkdirAll(path.Dir(fileName), dirPermissionMode); err != nil {
-		return errors.Wrap(err, errCreatingOperatorDir)
+		return false, errors.Wrap(err, errCreatingOperatorDir)
 	}
 
-	if err := ioutil.WriteFile(fileName, contents, filePermissionMode); err != nil {
-		return errors.Wrap(err, errSavingProfile)
+	existingContent, err := ioutil.ReadFile(fileName)
+	if err == nil && bytes.Equal(existingContent, content) {
+		return false, nil
 	}
-	return nil
+
+	if err := ioutil.WriteFile(fileName, content, filePermissionMode); err != nil {
+		return false, errors.Wrap(err, errSavingProfile)
+	}
+
+	return true, nil
 }
 
 // GetProfilePath returns the full path for the provided profile name and config.
diff --git a/vendor/k8s.io/klog/v2/klog.go b/vendor/k8s.io/klog/v2/klog.go
index e850db6b0..25483fad1 100644
--- a/vendor/k8s.io/klog/v2/klog.go
+++ b/vendor/k8s.io/klog/v2/klog.go
@@ -81,6 +81,7 @@ import (
 	"math"
 	"os"
 	"path/filepath"
+	"reflect"
 	"runtime"
 	"strconv"
 	"strings"
@@ -780,7 +781,7 @@ func (l *loggingT) errorS(err error, loggr logr.Logger, filter LogFilter, depth
 		loggr.Error(err, msg, keysAndValues...)
 		return
 	}
-	l.printS(err, depth+1, msg, keysAndValues...)
+	l.printS(err, errorLog, depth+1, msg, keysAndValues...)
 }
 
 // if loggr is specified, will call loggr.Info, otherwise output with logging module.
@@ -792,12 +793,12 @@ func (l *loggingT) infoS(loggr logr.Logger, filter LogFilter, depth int, msg str
 		loggr.Info(msg, keysAndValues...)
 		return
 	}
-	l.printS(nil, depth+1, msg, keysAndValues...)
+	l.printS(nil, infoLog, depth+1, msg, keysAndValues...)
 }
 
 // printS is called from infoS and errorS if loggr is not specified.
-// if err arguments is specified, will output to errorLog severity
-func (l *loggingT) printS(err error, depth int, msg string, keysAndValues ...interface{}) {
+// set log severity by s
+func (l *loggingT) printS(err error, s severity, depth int, msg string, keysAndValues ...interface{}) {
 	b := &bytes.Buffer{}
 	b.WriteString(fmt.Sprintf("%q", msg))
 	if err != nil {
@@ -805,12 +806,6 @@ func (l *loggingT) printS(err error, depth int, msg string, keysAndValues ...int
 		b.WriteString(fmt.Sprintf("err=%q", err.Error()))
 	}
 	kvListFormat(b, keysAndValues...)
-	var s severity
-	if err == nil {
-		s = infoLog
-	} else {
-		s = errorLog
-	}
 	l.printDepth(s, logging.logr, nil, depth+1, b)
 }
 
@@ -1583,6 +1578,13 @@ type KMetadata interface {
 
 // KObj returns ObjectRef from ObjectMeta
 func KObj(obj KMetadata) ObjectRef {
+	if obj == nil {
+		return ObjectRef{}
+	}
+	if val := reflect.ValueOf(obj); val.Kind() == reflect.Ptr && val.IsNil() {
+		return ObjectRef{}
+	}
+
 	return ObjectRef{
 		Name:      obj.GetName(),
 		Namespace: obj.GetNamespace(),
diff --git a/vendor/modules.txt b/vendor/modules.txt
index 4d5543a59..2534ea978 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -400,7 +400,7 @@ k8s.io/client-go/util/workqueue
 # k8s.io/component-base v0.20.2
 k8s.io/component-base/config
 k8s.io/component-base/config/v1alpha1
-# k8s.io/klog/v2 v2.6.0
+# k8s.io/klog/v2 v2.8.0
 ## explicit
 k8s.io/klog/v2
 k8s.io/klog/v2/klogr


Test patch:
diff --git a/internal/pkg/daemon/seccompprofile/seccompprofile_test.go b/internal/pkg/daemon/seccompprofile/seccompprofile_test.go
index 2a89ca53c..d589017ef 100644
--- a/internal/pkg/daemon/seccompprofile/seccompprofile_test.go
+++ b/internal/pkg/daemon/seccompprofile/seccompprofile_test.go
@@ -99,7 +99,7 @@ func TestReconcile(t *testing.T) {
 				},
 				log:    log.Log,
 				record: event.NewNopRecorder(),
-				save:   func(_ string, _ []byte) error { return nil },
+				save:   func(_ string, _ []byte) (bool, error) { return false, nil },
 			},
 			req:        reconcile.Request{NamespacedName: types.NamespacedName{Namespace: namespace, Name: name}},
 			wantResult: reconcile.Result{},
@@ -184,7 +184,7 @@ func TestSaveProfileOnDisk(t *testing.T) {
 				tc.setup()
 			}
 
-			gotErr := saveProfileOnDisk(tc.fileName, []byte(tc.contents))
+			_, gotErr := saveProfileOnDisk(tc.fileName, []byte(tc.contents))
 			file, _ := os.Stat(tc.fileName) // nolint: errcheck
 			gotFileCreated := file != nil