Refactor sandbox creation function arguments

[saschagrunert]

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

• Add a dedicated list of setters for the various pod sandbox fields and use them directly on sandbox creation
• Add a new method to convert a runtime spec to a sandbox and use it on restore.
• Enable the revive linter and set it to a minimum amount of arguments.

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

None

Does this PR introduce a user-facing change?

None

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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

[codecov]

Codecov Report

Attention: Patch coverage is 54.01460% with 63 lines in your changes are missing coverage. Please review.

Project coverage is 49.43%. Comparing base (cdfc5b7) to head (983cfd1).
Report is 45 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8141      +/-   ##
==========================================
- Coverage   49.58%   49.43%   -0.16%     
==========================================
  Files         153      152       -1     
  Lines       16926    16994      +68     
==========================================
+ Hits         8393     8401       +8     
- Misses       7487     7550      +63     
+ Partials     1046     1043       -3     

[saschagrunert]

/retest

[saschagrunert]

/retest

[saschagrunert]

/retest

[saschagrunert]

@cri-o/cri-o-maintainers PTAL

[saschagrunert]

/retest

[saschagrunert]

@cri-o/cri-o-maintainers PTAL

[haircommander]

I'm thinking about this more, and I find myself wondering if the internal/factory packages were actually a mistake. Part of me wants to move these functions into that package, but doing so would either cause us to do what you're doing here where we ultimately have setters inside of the sandbox package, or it would cause us to keep the problem as-is (a large number of args in sandbox.New()), or it could cause us to overuse an intermediate struct (copying all of the values from server/ into a SandboxBuilder object, and then passing that object ultimately to the sandbox package.

Maybe instead we should really move internal/factory packages into oci and sandbox packages , and create a new file that is factory.go in each, where we define these setters/translation functions)

Ultimately, we want a way to translate CRI spec (and server configuration) to sandbox/container instance, as well as a way to restore each of those from a file

WDYT @cri-o/cri-o-maintainers? Maybe we discuss this at next week's community meeting?

[saschagrunert]

Maybe instead we should really move internal/factory packages into oci and sandbox packages , and create a new file that is factory.go in each, where we define these setters/translation functions)

Is the goal to have an immutable sandbox after its creation? Then a dedicated builder would help, which is probably a complete replacement for the factory.

This PR also clashes with #7859

/hold

[haircommander]

Is the goal to have an immutable sandbox after its creation? Then a dedicated builder would help, which is probably a complete replacement for the factory.

I mean it could be partially mutable, but I don't think we need this level of flexibility. there are definitely fields that will never change and we shouldn't expose the ability to change externally.

[openshift-merge-robot]

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.