Skip to content

Refactoring factory/container to remove references of lib/sandbox #8601

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 17, 2024
Merged

Refactoring factory/container to remove references of lib/sandbox #8601

merged 1 commit into from
Oct 17, 2024

Conversation

@xw19
Copy link
Member

@xw19 xw19 commented Sep 12, 2024 •
edited
Loading

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Instead of passing whole sandbox object we will pass parameters that are needed This will remove interdependencies for future refactor. Also ManagedNamespace extracted to its own package inside lib so that it can be referenced for both sandbox and container.

Which issue(s) this PR fixes:

Part of #8289

Special notes for your reviewer:

Does this PR introduce a user-facing change?

No

Restructuring of packages.

@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 12, 2024
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 12, 2024

Hi @xw19. Thanks for your PR.

I'm waiting for a cri-o member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@xw19 xw19 marked this pull request as ready for review September 12, 2024 21:02
@xw19 xw19 requested a review from mrunalp as a code owner September 12, 2024 21:02
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 12, 2024
@xw19 xw19 force-pushed the feat/8289-remove-sandbox-references-from-factory-container-SM branch from 95a57f3 to 60855f3 Compare September 12, 2024 21:04
@codecov
Copy link

codecov bot commented Sep 12, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.81%. Comparing base (f8f4414) to head (937d243).
Report is 11 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #8601      +/-   ##
==========================================
- Coverage   48.86%   48.81%   -0.06%     
==========================================
  Files         153      153              
  Lines       17376    17360      -16     
==========================================
- Hits         8491     8474      -17     
  Misses       7822     7822              
- Partials     1063     1064       +1

@kwilczynski
Copy link
Contributor

/ok-to-test

@openshift-ci openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 12, 2024
@xw19
Copy link
Member Author

xw19 commented Sep 13, 2024

/test ci-rhel-critest

@xw19
Copy link
Member Author

xw19 commented Sep 13, 2024

/retest-required

1 similar comment
@xw19
Copy link
Member Author

xw19 commented Sep 13, 2024

/retest-required

@xw19 xw19 marked this pull request as draft September 13, 2024 08:18
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 13, 2024
@xw19 xw19 force-pushed the feat/8289-remove-sandbox-references-from-factory-container-SM branch 2 times, most recently from 5417aa6 to fba9dd3 Compare September 13, 2024 08:44
@xw19
Copy link
Member Author

xw19 commented Sep 13, 2024

This PR allows to move factory/sandbox to move over to lib/snadbox

@xw19 xw19 marked this pull request as ready for review September 13, 2024 08:51
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 13, 2024
@xw19
Copy link
Member Author

xw19 commented Sep 13, 2024

/retest-required

@xw19
Copy link
Member Author

xw19 commented Sep 13, 2024

/test e2e-gcp-ovn

@xw19
Copy link
Member Author

xw19 commented Sep 14, 2024

/test ci-fedora-integration

@xw19
Copy link
Member Author

xw19 commented Sep 14, 2024

/test ci-fedora-kata

@xw19
Copy link
Member Author

xw19 commented Sep 15, 2024 

/test ci-fedora-integration

@xw19 xw19 force-pushed the feat/8289-remove-sandbox-references-from-factory-container-SM branch from dbfbfaf to 615499c Compare September 16, 2024 14:59
@xw19
Copy link
Member Author

xw19 commented Sep 16, 2024

/test e2e-gcp-ovn

@xw19
Copy link
Member Author

xw19 commented Sep 16, 2024

/retest-required

@xw19 xw19 force-pushed the feat/8289-remove-sandbox-references-from-factory-container-SM branch from 615499c to 5ac6b30 Compare September 16, 2024 17:38
@xw19
Copy link
Member Author

xw19 commented Sep 16, 2024

/test ci-cgroupv2-e2e-features

@xw19
Copy link
Member Author

xw19 commented Sep 16, 2024

/test e2e-gcp-ovn

@xw19
Copy link
Member Author

xw19 commented Sep 16, 2024

/test periodics-images

@xw19
Copy link
Member Author

xw19 commented Sep 16, 2024

/test ci-cgroupv2-e2e-features

@xw19
Copy link
Member Author

xw19 commented Sep 16, 2024

/test ci-e2e-evented-pleg

1 similar comment
@xw19
Copy link
Member Author

xw19 commented Sep 18, 2024

/test ci-e2e-evented-pleg

@xw19
Copy link
Member Author

xw19 commented Sep 18, 2024

/test ci-cgroupv2-e2e-features

@haircommander
Copy link
Member

/approve

LGTM, thanks @xw19 , @cri-o/cri-o-maintainers PTAL

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 18, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: haircommander, xw19

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:

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

@openshift-ci openshift-ci bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Sep 18, 2024
@xw19 xw19 force-pushed the feat/8289-remove-sandbox-references-from-factory-container-SM branch from 5ac6b30 to 5e0f466 Compare September 19, 2024 13:04
@xw19
Refactoring factory/container to remove references of snadbox
937d243 
Instead of passing whole sandbox object we will pass parameters that are needed
This will remove interdependencies for future refactor.
Also ManagedNamespace extracted to its own package inside lib so that it can be referenced for
both sandbox and container.
Moved the constant string to its own package so that it also can be referenced without creating cyclic dependency

Signed-off-by: Sourav Moitra <[email protected]>
@xw19 xw19 force-pushed the feat/8289-remove-sandbox-references-from-factory-container-SM branch from 5e0f466 to 937d243 Compare October 13, 2024 19:26
@xw19
Copy link
Member Author

xw19 commented Oct 15, 2024

/retest

@haircommander
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 15, 2024
@xw19
Copy link
Member Author

xw19 commented Oct 17, 2024

/test ci-e2e-evented-pleg

@xw19
Copy link
Member Author

xw19 commented Oct 17, 2024

/test e2e-gcp-ovn

@openshift-merge-bot openshift-merge-bot bot merged commit ebf9616 into cri-o:main Oct 17, 2024
82 checks passed
@github-actions github-actions bot mentioned this pull request Dec 22, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@haircommander haircommander haircommander left review comments

@mrunalp mrunalp Awaiting requested review from mrunalp mrunalp is a code owner

@kwilczynski kwilczynski Awaiting requested review from kwilczynski

@QiWang19 QiWang19 Awaiting requested review from QiWang19

@littlejawa littlejawa Awaiting requested review from littlejawa

@sohankunkerkar sohankunkerkar Awaiting requested review from sohankunkerkar

Assignees

@haircommander haircommander

@xw19 xw19

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. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@xw19 @kwilczynski @haircommander