Add liveness/startup probe to SPOD

[saschagrunert]

What type of PR is this?

/kind feature

What this PR does / why we need it:

We now add a liveness probe for giving the SPOD more time on startup to
become ready. Every controller should at least run one Reconile to be
marked as ready.

We now also add a new interface Controller
(internal/pkg/controller/controller.go), which has to be fulfilled by
every SPOD controller. This allows to simplify the implementation in
main.

Which issue(s) this PR fixes:

None

Does this PR have test?

None

Special notes for your reviewer:

None

Does this PR introduce a user-facing change?

Added liveness and startup probe to operator daemon set to streamline the operator stratup.

[jhrozek]

First, I really like the new shared Controller interface, that's a really nice refactor.

About the probes, maybe a silly question, but I'm very good at asking those -- aren't the Reconcile loops reactive and invoked only when the watcher hits something? Meaning, would the loop go through at least once if there are no selinux/seccomp profiles installed by default or if all are removed by the admin?

[saschagrunert]

First, I really like the new shared Controller interface, that's a really nice refactor.

Thank you! 🙏

About the probes, maybe a silly question, but I'm very good at asking those -- aren't the Reconcile loops reactive and invoked only when the watcher hits something? Meaning, would the loop go through at least once if there are no selinux/seccomp profiles installed by default or if all are removed by the admin?

Indeed the reconcile will not be hit if no objects to react on are available. That's exactly the reason why I cannot tell right now when the recording reconciler is ready. For seccomp and selinux profiles I just assume that we will definitely ship some default ones, which can be then used as indicator for readiness.

The other way around, do you have anything in mind how we could indicate readiness in a better way?

[codecov-commenter]

Codecov Report

Merging #430 (8dc95bf) into master (30f83f7) will increase coverage by 0.31%.
The diff coverage is 70.00%.

@@            Coverage Diff             @@
##           master     #430      +/-   ##
==========================================
+ Coverage   38.58%   38.90%   +0.31%     
==========================================
  Files          22       23       +1     
  Lines         990     1000      +10     
==========================================
+ Hits          382      389       +7     
- Misses        593      596       +3     
  Partials       15       15              

[jhrozek]

On Mon, May 03, 2021 at 01:05:34AM -0700, Sascha Grunert wrote: > First, I really like the new shared Controller interface, that's a really nice refactor. Thank you! :pray: > About the probes, maybe a silly question, but I'm very good at asking those -- aren't the Reconcile loops reactive and invoked only when the watcher hits something? Meaning, would the loop go through at least once if there are no selinux/seccomp profiles installed by default or if all are removed by the admin? Indeed the reconcile will not be hit if no objects to react on are available. That's exactly the reason why I cannot tell right now when the recording reconciler is ready. For seccomp and selinux profiles I just assume that we will definitely ship some default ones, which can be then used as indicator for readiness.

OK. I was wondering a bit whether to create some default objects straight up in the code, but I guess creating them with the manifests is as good.

The other way around, do you have anything in mind how we could indicate readiness in a better way?

For readiness no, not really. I was wondering a bit about thinkgs like trying to contact selinuxd for the liveness probe in the selinux controller, but that's handled in the controller anyway. Thanks for the replies.

[saschagrunert]

Let's see if I can get the e2e tests happy.

[saschagrunert]

/retest

[saschagrunert]

/retest

[saschagrunert]

Decided to carve-out the controller logic into a separate PR (#437) and will rebase this one later.

[saschagrunert]

Rebased on top of the latest master.

[saschagrunert]

Ready for review ✔️

[saschagrunert]

@JAORMX @jhrozek please take a look.

[JAORMX]

/lgtm

This is looking great! Since this only addresses the Seccomp controller, we should track as issues the controllers that are missing so they get addressed. We're hoping to onboard someone to the SPO soon and those would be some great introductory tasks.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JAORMX, saschagrunert

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 [saschagrunert]

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

[saschagrunert]

…we should track as issues the controllers that are missing so they get addressed.

Sounds good, opened #442 and #443