Repository: kubernetes-sigs/security-profiles-operator
Pull request #273 (2021-02-04 11:48:17)

Reuse manager imagePullPolicy for all other workloads
#### What type of PR is this?

/kind cleanup


#### What this PR does / why we need it:
We now reuse the provided pull policy from the manager for all other
created workloads. This simplifies the configuration overhead and
deployment complexity.

#### 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?

<!--
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
None
```


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/273#" 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/273?src=pr&el=h1) Report
> Merging [#273](https://codecov.io/gh/kubernetes-sigs/security-profiles-operator/pull/273?src=pr&el=desc) (14d7bc9) into [master](https://codecov.io/gh/kubernetes-sigs/security-profiles-operator/commit/19894f350e916fd98b2a9d4da999447fa7e80c20?el=desc) (19894f3) will **increase** coverage by `0.05%`.
> The diff coverage is `0.00%`.

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

```diff
@@            Coverage Diff             @@
##           master     #273      +/-   ##
==========================================
+ Coverage   28.71%   28.77%   +0.05%     
==========================================
  Files           5        5              
  Lines         498      497       -1     
==========================================
  Hits          143      143              
+ Misses        338      337       -1     
  Partials       17       17              
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `28.77% <0.00%> (+0.05%)` | :arrow_up: |

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/273?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ernal/pkg/controllers/spod/configmap\_controller.go](https://codecov.io/gh/kubernetes-sigs/security-profiles-operator/pull/273/diff?src=pr&el=tree#diff-aW50ZXJuYWwvcGtnL2NvbnRyb2xsZXJzL3Nwb2QvY29uZmlnbWFwX2NvbnRyb2xsZXIuZ28=) | `0.00% <0.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/kubernetes-sigs/security-profiles-operator/pull/273?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/273?src=pr&el=footer). Last update [19894f3...14d7bc9](https://codecov.io/gh/kubernetes-sigs/security-profiles-operator/pull/273?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

JAORMX: /lgtm

Patch:
diff --git a/deploy/base/config.yaml b/deploy/base/config.yaml
index 87845cf0f..a7872965f 100644
--- a/deploy/base/config.yaml
+++ b/deploy/base/config.yaml
@@ -7,5 +7,4 @@ metadata:
   labels:
     security-profiles-operator/config: ""
 data:
-  SPOdImagePullPolicy: Always
   EnableSelinux: "false"
diff --git a/deploy/namespace-operator.yaml b/deploy/namespace-operator.yaml
index adb59e7e2..ad267f6a0 100644
--- a/deploy/namespace-operator.yaml
+++ b/deploy/namespace-operator.yaml
@@ -527,7 +527,6 @@ spec:
 apiVersion: v1
 data:
   EnableSelinux: "false"
-  SPOdImagePullPolicy: Always
 kind: ConfigMap
 metadata:
   labels:
diff --git a/deploy/operator.yaml b/deploy/operator.yaml
index f168c8683..49accf19c 100644
--- a/deploy/operator.yaml
+++ b/deploy/operator.yaml
@@ -524,7 +524,6 @@ spec:
 apiVersion: v1
 data:
   EnableSelinux: "false"
-  SPOdImagePullPolicy: Always
 kind: ConfigMap
 metadata:
   labels:
diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go
index aa29d626c..f47553a3f 100644
--- a/internal/pkg/config/config.go
+++ b/internal/pkg/config/config.go
@@ -57,11 +57,6 @@ const (
 	SPOcEnableSelinux = "EnableSelinux"
 )
 
-// DaemonSet configMap keys.
-const (
-	SPOdImagePullPolicy = "SPOdImagePullPolicy"
-)
-
 // GetOperatorNamespace gets the namespace that the operator is currently running on.
 func GetOperatorNamespace() string {
 	// TODO(jaosorior): Get a method to return the current operator
diff --git a/internal/pkg/controllers/spod/configmap_controller.go b/internal/pkg/controllers/spod/configmap_controller.go
index c1bdc8e78..8044e1b42 100644
--- a/internal/pkg/controllers/spod/configmap_controller.go
+++ b/internal/pkg/controllers/spod/configmap_controller.go
@@ -78,7 +78,8 @@ func (r *ReconcileSPOd) Reconcile(request reconcile.Request) (reconcile.Result,
 	}
 	// We use the same target image for the deamonset as which we have right
 	// now running.
-	targetImage := foundDeployment.Spec.Template.Spec.Containers[0].Image
+	image := foundDeployment.Spec.Template.Spec.Containers[0].Image
+	pullPolicy := foundDeployment.Spec.Template.Spec.Containers[0].ImagePullPolicy
 
 	spodKey := types.NamespacedName{
 		Name:      r.baseSPOd.GetName(),
@@ -87,7 +88,7 @@ func (r *ReconcileSPOd) Reconcile(request reconcile.Request) (reconcile.Result,
 	foundSPOd := &appsv1.DaemonSet{}
 	if err := r.client.Get(ctx, spodKey, foundSPOd); err != nil {
 		if kerrors.IsNotFound(err) {
-			return r.handleCreate(ctx, cminstance, targetImage)
+			return r.handleCreate(ctx, cminstance, image, pullPolicy)
 		}
 		return reconcile.Result{}, errors.Wrap(err, "getting spod DaemonSet")
 	}
@@ -100,9 +101,10 @@ func (r *ReconcileSPOd) handleCreate(
 	ctx context.Context,
 	cfg *corev1.ConfigMap,
 	image string,
+	pullPolicy corev1.PullPolicy,
 ) (res reconcile.Result, err error) {
 	r.log.Info("Creating operator resources")
-	newSPOd := r.getConfiguredSPOd(cfg, image)
+	newSPOd := r.getConfiguredSPOd(cfg, image, pullPolicy)
 
 	if err := controllerutil.SetControllerReference(cfg, newSPOd, r.scheme); err != nil {
 		return res, errors.Wrap(err, "setting spod controller reference")
@@ -136,7 +138,11 @@ func (r *ReconcileSPOd) handleCreate(
 	return res, nil
 }
 
-func (r *ReconcileSPOd) getConfiguredSPOd(cfg *corev1.ConfigMap, image string) *appsv1.DaemonSet {
+func (r *ReconcileSPOd) getConfiguredSPOd(
+	cfg *corev1.ConfigMap,
+	image string,
+	pullPolicy corev1.PullPolicy,
+) *appsv1.DaemonSet {
 	newSPOd := r.baseSPOd.DeepCopy()
 	templateSpec := &newSPOd.Spec.Template.Spec
 
@@ -154,12 +160,8 @@ func (r *ReconcileSPOd) getConfiguredSPOd(cfg *corev1.ConfigMap, image string) *
 			"--with-selinux=true")
 	}
 
-	imagePullPolicyStr, found := cfg.Data[config.SPOdImagePullPolicy]
-	if found {
-		pullPolicy := corev1.PullPolicy(imagePullPolicyStr)
-		for i := range templateSpec.Containers {
-			templateSpec.Containers[i].ImagePullPolicy = pullPolicy
-		}
+	for i := range templateSpec.Containers {
+		templateSpec.Containers[i].ImagePullPolicy = pullPolicy
 	}
 
 	return newSPOd


Test patch:
diff --git a/test/e2e_test.go b/test/e2e_test.go
index c3b29ffc6..683f228c5 100644
--- a/test/e2e_test.go
+++ b/test/e2e_test.go
@@ -133,10 +133,6 @@ func (e *e2e) deployOperator(manifest string) {
 		fmt.Sprintf("s;imagePullPolicy: Always;imagePullPolicy: %s;g", e.pullPolicy),
 		manifest,
 	)
-	e.run(
-		"sed", "-i", fmt.Sprintf("s;SPOdImagePullPolicy: Always;SPOdImagePullPolicy: %s;g", e.pullPolicy),
-		manifest,
-	)
 
 	// Update the image name to match the test image
 	e.run(