Skip to content

Conversation

@dfr
Copy link
Contributor

@dfr dfr commented Nov 10, 2023

What type of PR is this?

/kind feature

What this PR does / why we need it:

This implements the sandbox lifecycle for FreeBSD, allowing pods to be created and destroyed. I have not included the code for container lifecycle to attempt to reduce the scope of this PR to the minimum. Part of #6492.

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

@dfr dfr requested a review from mrunalp as a code owner November 10, 2023 15:37
@openshift-ci openshift-ci bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: no Indicates the PR's author has not DCO signed all their commits. kind/feature Categorizes issue or PR as related to a new feature. labels Nov 10, 2023
@openshift-ci openshift-ci bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Nov 10, 2023
@codecov
Copy link

codecov bot commented Nov 10, 2023

Codecov Report

❗ No coverage uploaded for pull request base (main@87a309c). Click here to learn what that means.
Report is 12 commits behind head on main.
The diff coverage is 63.15%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7472   +/-   ##
=======================================
  Coverage        ?   47.98%           
=======================================
  Files           ?      146           
  Lines           ?    16261           
  Branches        ?        0           
=======================================
  Hits            ?     7803           
  Misses          ?     7513           
  Partials        ?      945           

@dfr
Copy link
Contributor Author

dfr commented Nov 14, 2023

/retest-required

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 30, 2023
@dfr
Copy link
Contributor Author

dfr commented Nov 30, 2023

Rebased

@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 30, 2023
@dfr
Copy link
Contributor Author

dfr commented Nov 30, 2023

/retest

@github-actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

@github-actions github-actions bot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Dec 31, 2023
@dfr dfr force-pushed the freebsd-sandbox branch from da9388d to 8907d24 Compare January 24, 2024 16:47
@dfr
Copy link
Contributor Author

dfr commented Jan 24, 2024

Rebased

@haircommander
Copy link
Member

/retest

dfr added 4 commits January 25, 2024 16:58
On FreeBSD, we use a jail to manage namespaces and managing its lifetime
is controlled by the jail's 'persist' flag.

Signed-off-by: Doug Rabson <[email protected]>
This uses the build's GOOS value to generate.New, allowing it to create
a FreeBSD generator in FreeBSD builds. In future, we should allow this
to be overridden since FreeBSD supports Linux emulation - in podman, we
use the image OS value.

We also avoid dereferencing config.Linux if its nil which is the case on
FreeBSD.

Signed-off-by: Doug Rabson <[email protected]>
This moves NeedsInfra to sandbox_linux.go and adds implementations for
freebsd and other platforms. FreeBSD needs an infra container to own the
pod vnet for network mode pod.  Arbitrarily, I made the generic stub
assume that infra containers are not needed.

Signed-off-by: Doug Rabson <[email protected]>
... and add a FreeBSD implementation

Signed-off-by: Doug Rabson <[email protected]>
dfr added 6 commits January 25, 2024 16:58
Initially, FreeBSD supports just the network namespace although we may
add IPC and UTS in future.

Signed-off-by: Doug Rabson <[email protected]>
This simply moves SetupShm to infra_linux.go since this is not needed on
FreeBSD and the MS_* constants are Linux-specific.

Signed-off-by: Doug Rabson <[email protected]>
This factors out configNsPath from container_server.go so that we can
find the network namespace for the pod.

Signed-off-by: Doug Rabson <[email protected]>
@dfr dfr force-pushed the freebsd-sandbox branch from 8907d24 to 7e94aa1 Compare January 25, 2024 16:59
@dfr
Copy link
Contributor Author

dfr commented Jan 25, 2024

Rebased to pick up #7709

@haircommander
Copy link
Member

/approve

LGTM, thanks @dfr

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 25, 2024
@dfr
Copy link
Contributor Author

dfr commented Jan 26, 2024

/retest-required

@dfr
Copy link
Contributor Author

dfr commented Jan 29, 2024

All the tests are green except for ci/kata-jenkins which is showing as 'Expected — Waiting for status to be reported'. Is that normal? Should I try to restart it?

@saschagrunert
Copy link
Member

@dfr yes that's normal, all tests are green

@haircommander
Copy link
Member

@cri-o/cri-o-maintainers any additional thoughts or are we good to merge?

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 1, 2024
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 1, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dfr, haircommander, 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 [haircommander,saschagrunert]

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

@dfr
Copy link
Contributor Author

dfr commented Feb 1, 2024

/retest-required

@openshift-merge-bot openshift-merge-bot bot merged commit c37c116 into cri-o:main Feb 1, 2024
@dfr dfr deleted the freebsd-sandbox branch February 1, 2024 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants