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

vendor: bump containers/storage to v1.24.0 #4334

Merged
merged 1 commit into from Nov 17, 2020

Conversation

kolyshkin
Copy link
Collaborator

@kolyshkin kolyshkin commented Nov 2, 2020

What type of PR is this?

/kind bug

What this PR does / why we need it:

This is mostly to have containers/storage#757
which should fix occasional CI failures (and probably real failures, too).

Which issue(s) this PR fixes:

time="2020-11-02T19:00:03Z" level=fatal msg="creating container: rpc error: code = Unknown desc = error creating an ID-mapped copy of layer "01d6f430ddfdf1203b505fbb03c6479802a3366609d5e0231a30f85f412f873d": exit status 1: error during chown: storage-chown-by-maps: chown("lib/apk/db/scripts.tar"): lchown lib/apk/db/scripts.tar: interrupted system call"

periodically caught by CI

Special notes for your reviewer:

This is currently blocked by #4371

Backports:

Does this PR introduce a user-facing change?

Fix occasional "chown: interrupted system call" error on container creation.

@openshift-ci-robot openshift-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Nov 2, 2020
@haircommander
Copy link
Member

/approve

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 2, 2020
@kolyshkin
Copy link
Collaborator Author

Backports:

@codecov
Copy link

codecov bot commented Nov 2, 2020

Codecov Report

Merging #4334 (359c60f) into master (a004cba) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #4334   +/-   ##
=======================================
  Coverage   39.29%   39.29%           
=======================================
  Files         112      112           
  Lines        8835     8835           
=======================================
  Hits         3472     3472           
  Misses       4977     4977           
  Partials      386      386           

@mrunalp
Copy link
Member

mrunalp commented Nov 3, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 3, 2020
@mrunalp
Copy link
Member

mrunalp commented Nov 3, 2020

/test e2e-aws

@kolyshkin
Copy link
Collaborator Author

kolyshkin commented Nov 3, 2020

Failure in critest related to AppArmor:

Summarizing 3 Failures:

[Fail] [k8s.io] AppArmor runtime should support apparmor [It] should enforce a permissive profile 
/home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/validate/apparmor_linux.go:143

[Fail] [k8s.io] AppArmor runtime should support apparmor [It] should enforce a permissive profile 
/home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/validate/apparmor_linux.go:143

[Fail] [k8s.io] AppArmor runtime should support apparmor [It] should enforce a permissive profile 
/home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/validate/apparmor_linux.go:143

I have audited the logs (https://circleci.com/api/v1.1/project/github/cri-o/cri-o/105877/output/107/0?file=true&allocation-id=5fa1278c1be5695c9ebd99d7-0-build%2F7DFD59DA) but it's not clear what happened (other than the touch foo returned 1 while it is expected to return 0).

@kolyshkin
Copy link
Collaborator Author

/retest

@mrunalp
Copy link
Member

mrunalp commented Nov 3, 2020

/test e2e-aws

@haircommander
Copy link
Member

/retest

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 7, 2020
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: haircommander, kolyshkin, mrunalp

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,kolyshkin,mrunalp]

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

@kolyshkin
Copy link
Collaborator Author

rebased on top of current master; maybe this will help ci :-\

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 10, 2020
@kolyshkin
Copy link
Collaborator Author

Same apparmor failure as earlier. Most probably caused by containers/storage#743 as we touch the file in the root directory. The fix (in critest) is easy but I'm not sure the new default won't bring any other breakage.

@saschagrunert
Copy link
Member

Same apparmor failure as earlier. Most probably caused by containers/storage#743 as we touch the file in the root directory. The fix (in critest) is easy but I'm not sure the new default won't bring any other breakage.

I have the same issue in this PR: #4358

@kolyshkin

This comment has been minimized.

@kolyshkin kolyshkin closed this Nov 12, 2020
@kolyshkin
Copy link
Collaborator Author

closed by mistake

@kolyshkin kolyshkin reopened this Nov 12, 2020
@kolyshkin
Copy link
Collaborator Author

I have the same issue in this PR: #4358

@saschagrunert this is probably because your PR also bumps containers/storage to 1.23.10-dev-something, which includes containers/storage#743. Let's see if kubernetes-sigs/cri-tools#682 helps (testing via #4366)

@kolyshkin
Copy link
Collaborator Author

OK, due to containers/storage#743 this needs kubernetes-sigs/cri-tools#682 and a new critools release.

@saschagrunert
Copy link
Member

OK, due to containers/storage#743 this needs kubernetes-sigs/cri-tools#682 and a new critools release.

Hm, but isn't this a regression in containers/storage? My plan was to cut a new cri-tools release with k8s 1.20.0 but now this seems kind of urgent. :-/

@kolyshkin
Copy link
Collaborator Author

Hm, but isn't this a regression in containers/storage?

It's hard to say whether it's a regression or not. Looks more like a fortifying measure.

My plan was to cut a new cri-tools release with k8s 1.20.0 but now this seems kind of urgent. :-/

Alternatively, we can use critools master as I did in #4366. One side benefit is we'll be able to catch bugs like kubernetes-sigs/cri-tools#683 earlier.

@mrunalp @haircommander @saschagrunert WDYT?

@kolyshkin
Copy link
Collaborator Author

Alternatively, we can use critools master as I did in #4366. One side benefit is we'll be able to catch bugs like kubernetes-sigs/cri-tools#683 earlier.

See #4371

@kolyshkin kolyshkin changed the title vendor: bump containers/storage to v1.23.9 [wip] vendor: bump containers/storage to v1.23.9 Nov 14, 2020
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 14, 2020
@saschagrunert
Copy link
Member

Alternatively, we can use critools master as I did in #4366. One side benefit is we'll be able to catch bugs like kubernetes-sigs/cri-tools#683 earlier.

I'm in favor of switching to the master for now. I will cut a new cri-tools release together with Kubernetes v1.20.0 in any case.

This is mostly to have containers/storage#757
which should fix occasional CI failures.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 16, 2020
@kolyshkin kolyshkin changed the title [wip] vendor: bump containers/storage to v1.23.9 vendor: bump containers/storage to v1.24.0 Nov 16, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 16, 2020
@kolyshkin
Copy link
Collaborator Author

Rebased on top of recently merged #4371 to fix critest woes.

Assuming the CI is green (except e2e-aws which is broken globally atm), this one should be good to go.

@openshift-merge-robot
Copy link
Contributor

openshift-merge-robot commented Nov 17, 2020

@kolyshkin: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-aws 359c60f link /test e2e-aws

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

@kolyshkin
Copy link
Collaborator Author

failure in e2e_rhel ([k8s.io] [sig-node] Pods Extended [k8s.io] Pod Container Status should never report success for a pending container) is known (kubernetes/kubernetes#96565)

@kolyshkin
Copy link
Collaborator Author

/test e2e_rhel

@fidencio
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 17, 2020
@mrunalp mrunalp merged commit 3873392 into cri-o:master Nov 17, 2020
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/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. 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.

None yet

7 participants