-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Upgrade the docker version to 23.0.6 #5436
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM -- I agree it is better to update the containers to a current docker than to downgrade the GitHub Action runner to an antique version of docker.
Codecov Report
@@ Coverage Diff @@
## master #5436 +/- ##
===========================================
+ Coverage 58.52% 76.73% +18.21%
===========================================
Files 241 241
Lines 14632 14632
Branches 616 616
===========================================
+ Hits 8563 11228 +2665
+ Misses 6069 3404 -2665
... and 134 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@@ -58,6 +58,13 @@ jobs: | |||
uses: actions/checkout@v3 | |||
- name: "Setup" | |||
run: ./tools/github/setup.sh | |||
- name: Maximize free space |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on this workaround: actions/runner-images#2840 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since multi-runtime tests passed with this change, I applied this change to all workflows.
Since upgrading the docker version could impact many downstream, I think it would be better to separate the change to upgrade docker and the change to maximize the build space for multi-runtime tests. |
Once this PR is merged, I would rebase this PR. |
08ffecd
to
ec8e870
Compare
It turns out that this change is not directly related to the failure of multi-runtime tests. |
|
||
Using docker-runc obtains significantly better performance, but requires that the version of docker-runc within the invoker container is an exact version match to the docker-runc of the host environment. Failure to get an exact version match will results in error messages like: | ||
Using runc obtains significantly better performance, but requires that the version of runc within the invoker container is an exact version match to the runc of the host environment. Failure to get an exact version match will results in error messages like: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still a requirement with runc that versions must be an exact match? Makes it very hard to use runc in serverless environment where you may control the invoker container but not the underlying server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should not be an exact match, but some versions have breaking changes.
For example, the command's name is changed from docker-runc
to runc
.
The root volume directory was also different in the previous version.
https://github.com/apache/openwhisk/pull/4430/files#diff-072209721097df0ae37ac99015b28844fec1f9b390314782671445b4f80af622R185
I assume if there is no such breaking change, runc would be compatible, but anyway, it would be better to use the same version if possible.
LGTM this is very long overdue, are there any blockers to merging this. I believe this should only affect docker client right anyways? |
I have been thinking of running a performance test with this version. |
This PR has opened for a while, I would merge this. |
Description
The docker version of the GitHub action runner is 23.0.6.
Instead of downgrading the engine version I want to give a shot to upgrade the client version.
Related issue and scope
My changes affect the following components
Types of changes
Checklist: