Skip to content
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.

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

CRIO changes to support split filesystem #7269

Merged
merged 1 commit into from
Dec 21, 2023

Conversation

kannon92
Copy link
Contributor

@kannon92 kannon92 commented Sep 1, 2023

What type of PR is this?

/kind api-change

What this PR does / why we need it:

Implement Image_Fs_Info as part of the separate gc KEP. We should not merge unless this KEP is approved. TODO: Add a link.

Opening this up now to gather feedback on feasibility of this implementation.

Note, since I had a few changes in other areas, I vendored these changes here. I already opened up a PR into c/storage. Kubernetes is based on KEP approval.

Which issue(s) this PR fixes:

Special notes for your reviewer:

We are having some problems with the integration tests picking up an older version of crictl. I moved the integration tests to #7269 just to move this PR through.

Does this PR introduce a user-facing change?

Added more file system information in `ImageFsInfo` as part of the garbage collection KEP.

@kannon92 kannon92 requested a review from mrunalp as a code owner September 1, 2023 18:57
@openshift-ci openshift-ci bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Sep 1, 2023
@kannon92 kannon92 changed the title Do not Merge - ImageFs implementation in CRI WIP: Do not Merge - ImageFs implementation in CRI Sep 1, 2023
@openshift-ci openshift-ci bot requested review from hasan4791 and klihub September 1, 2023 18:57
@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. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 1, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 1, 2023

Hi @kannon92. 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/test-infra repository.

@kannon92 kannon92 force-pushed the container-cri-change branch from 004a4bd to 724cd91 Compare September 1, 2023 18:58
@openshift-ci openshift-ci bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. dco-signoff: no Indicates the PR's author has not DCO signed all their commits. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Sep 1, 2023
@kannon92 kannon92 force-pushed the container-cri-change branch from 4fcea23 to 74bfc6e Compare September 5, 2023 14:43
@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 Sep 5, 2023
@kannon92 kannon92 force-pushed the container-cri-change branch from 02f5dce to 9f11b11 Compare September 7, 2023 20:18
@kannon92 kannon92 force-pushed the container-cri-change branch from 9f11b11 to 48a9e18 Compare September 12, 2023 22:06
@openshift-ci openshift-ci bot added dco-signoff: no Indicates the PR's author has not DCO signed all their commits. and removed dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Sep 12, 2023
@kannon92 kannon92 force-pushed the container-cri-change branch from 48a9e18 to 8d88d57 Compare September 12, 2023 22:07
@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 Sep 12, 2023
@github-actions
Copy link

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

@kannon92
Copy link
Contributor Author

/retest

1 similar comment
@saschagrunert
Copy link
Member

/retest

@kannon92
Copy link
Contributor Author

@sohankunkerkar I think there is a legit failure here. I'll see if I find the cause.

@kannon92 kannon92 force-pushed the container-cri-change branch from c57220f to d8bf1ee Compare December 19, 2023 19:06
@haircommander
Copy link
Member

/retest

@kannon92
Copy link
Contributor Author

@haircommander we think that this one needs #7606 as we are hitting caching issues where crictl is still picking up the old 1.28 version and we want to force a new clone so we can pick up the latest version.

@kannon92 kannon92 force-pushed the container-cri-change branch from d8bf1ee to 702ac88 Compare December 19, 2023 21:47
@sohankunkerkar
Copy link
Member

/retest

1 similar comment
@saschagrunert
Copy link
Member

/retest

@sohankunkerkar
Copy link
Member

hmm... the latest build still uses the older version of crictl:

[skunkerk@ci-op-ygyzh78h-39a11-buildhost ~]$ crictl --version
crictl version 1.28.0

@openshift-ci openshift-ci bot added dco-signoff: no Indicates the PR's author has not DCO signed all their commits. and removed dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Dec 20, 2023
Signed-off-by: Kevin Hannon <kannon1992@gmail.com>
@kannon92 kannon92 force-pushed the container-cri-change branch from 66858d1 to c30ce40 Compare December 20, 2023 20:54
@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 Dec 20, 2023
@haircommander
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 20, 2023
@sohankunkerkar
Copy link
Member

/retest

1 similar comment
@kannon92
Copy link
Contributor Author

/retest

@kannon92
Copy link
Contributor Author

@saschagrunert could you override this? The other e2e jobs pass.

we moved the integration tests to another PR so this should be mergable now.

@saschagrunert
Copy link
Member

/override ci/prow/ci-cgroupv2-e2e

Copy link
Contributor

openshift-ci bot commented Dec 21, 2023

@saschagrunert: Overrode contexts on behalf of saschagrunert: ci/prow/ci-cgroupv2-e2e

In response to this:

/override ci/prow/ci-cgroupv2-e2e

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/test-infra repository.

Copy link
Contributor

openshift-ci bot commented Dec 21, 2023

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@kannon92
Copy link
Contributor Author

/retest-required

@haircommander
Copy link
Member

/retest

@openshift-merge-bot openshift-merge-bot bot merged commit d59bbdc into cri-o:main Dec 21, 2023
61 of 62 checks passed
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/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API 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
Development

Successfully merging this pull request may close these issues.

5 participants