-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
runc: release 1.2.0-rc.1 #3963
runc: release 1.2.0-rc.1 #3963
Conversation
450ec61
to
1b550e1
Compare
1b550e1
to
c8681a6
Compare
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 there are still a few too many things in the milestone for 1.2.0 to start doing RCs (time namespace, removing bindfd, full idmapped mounts support). The bindfd one is pretty important because the current state of main
is such that quite a few people have reported significant performance issues with the new setup. I would want to get at least #3931 and #3876 merged before an -rc1, and I think #3953 needs to go into 1.2.0-rc2 at the latest (but I'd prefer to get it into -rc1).
I don't want us to have more than 2 or 3 rcs for 1.2, so we should get the big bits of code done and do a feature freeze after rc1 to make sure we don't have a repeat of 1.0.
@cyphar But the issues you mention are also in the 1.1 release branch, right? Not creating a release will not fix any issue either, IIUC. Can't we just release runc 1.3.0 when those PRs are merged and release 1.2.0 now? Do you think those PRs can be merged soon (like next week)? Or what do you estimate (realistically)? Not having a runc release also is complicating moving forward in Kubernetes and containerd with userns support (that requires idmap mounts. The full idmap mount support that you mention is not needed by any user for now). Kubernetes requires idmap mounts since 1.27 already (now 1.28 is about to be released in the next days), and it can't be used with a runc release currently. And some PRs to containerd that require idmap mounts support in runc are also halted due to a missing release (containerd/containerd#8287 (review)). |
c8681a6
to
fab4a13
Compare
#3931 and #3876 are both ready to merge IMHO. I would be fine with a -rc1 release with just those if we really do need to rush it that badly. #3953 will probably be ready next week (there is a CRIU test failure that only happens in CI that I'm currently debugging -- aside from that, all of the code works and full idmapped-mount support is implemented). But I need to point out that your suggestion appears to be to have everyone ship -rcs? We did that for 1.0-rc9999999 but that's because we forgot how RCs are supposed to work. I would not expect distributions to ship -rc1 to everyone indiscriminately. |
@cyphar Ok, so I assume the release 1.1 still has the same issue.
Okay, so wanna include those in -rc.1 if they are merged by next week and otherwise we release 1.2.0-rc.1 AND 1.2.0 without them?
No, I don't propose to keep doing rc releases. My original proposal in the PR is to release without them, and release again when those are merged. Not delay any final release. Nor rc release to include PRs, as you just proposed ;) |
Can we just release beta.1? Or alpha.1 |
I agree with @cyphar -- we need to merge these big things first, and also I see no point in release alpha or beta, if there will be big changes coming up. OTOH I agree we need the 1.2.0 release soon (and, in general, make new releases more frequent -- which we're already doing with x.y.z -stable releases, but alas not the main branch). If that's the target, we can release what we have as 1.2 and move the features we're still working on to 1.3 milestone. |
No, #3599 was never backported to 1.1.
I don't want a 1.2 release without full idmapped mount support when there is already a PR that has working code and is nearly ready for merging.
I don't really see any point to doing a 1.2 release and then doing a 1.3 release a few weeks later, when we can instead get a few remaining PRs in and then do a 1.2 release. But more importantly, I don't want a release with partial idmapped mount support if it's not necessary (and since #3953 exists and the code works, it seems extremely unnecessary).
I completely agree. We need to start doing time-based releases, like we discussed around the time of 1.1. Maybe we can aim for a minor release every 3 months, kind of like Linux? |
I just noticed the JSON field names for the idmap mounts don't match the style of the other fields. This is something we cannot change after we do a runc release, so this needs to be fixed first as well. I will fix that in #3953 as well. |
fab4a13
to
df30986
Compare
Exactly. But that fixes an issue present in 1.1, right? If there are no known issues in main that are not present in 1.1, then we won't knowingly cause regressions by doing a release.
I completely agree with that, that is what I proposed :). I'm ok with either doing a rc release "now" or waiting 1 more week and do an rc release. If we have pending PRs that are very useful, don't make it for 1.2 and fix issues users are reporting, I think that is nice too because we will do an 1.3 release in 3 months. And hopefully that helps to build the muscle :)
So, @cyphar does it sound good to you to do an rc release on Aug 17th, that would gives us time to merge the PRs that are very close, do any pass we want to do before and whatever big changes are not merged for the rc release, will be included in 1.3 in 3 months? |
df30986
to
7e9c8fd
Compare
7e9c8fd
to
f795a41
Compare
a755754
to
0be0cd0
Compare
10fb4e0
to
bf3ae7e
Compare
I'll be afk for a few weeks starting tomorrow. Please feel free to carry-this forward if you need to. The changelog is updated until the current main as of today |
I think there's a clash between the mindset of "we shouldn't release until feature X or refactor Y lands" and time-based releases -- you cannot1 have both, and so far the former has clearly won, while people (including runc developers it seems) want the latter. Given the developer/review capacity and resulting velocity in this repo, I think that a 3 months release cadence is way too ambitious (case in point: this PR is almost 3 months old). It might work with 6 months, where the last two months before a release are for finalization (presumably on a release branch, so that development on Just my 2c... PS. Another case in point, citing 1.1.7 release notes:
Footnotes |
Just add a comparison, we cut main to a release branch every 6 weeks in Prometheus. The difference is we go from cut, to rc, to release in a few days. The regular cadence actually makes it easier to release. |
bf3ae7e
to
170be41
Compare
Updated to current main, forward ported the changelog for other runc 1.1 releases and added entries for blocker PRs for 1.2.0-rc.1 that are not yet merged. |
@h-vetinari I understand your point of view, however:
The PRs I've linked are fixes for a broken feature, which required a significant rewrite to fix. The only two options are to revert the feature entirely (which @rata wouldn't like, because that's the main feature he wants in 1.2) or to wait for the fix to land. If we had an agreed-to time-based release cadence, I wouldn't have merged the idmap code in the state it was when we merged it. I merged it because I knew we had time to rework it (though it turns out it required a larger rework than I expected). So yes, if things were different, then things would be different. 😸
In the defense of the other maintainers, that was my plan at the beginning of the year. Obviously, things didn't go to plan. |
@cyphar Does the broken feature affect users who are not using it? If it does, I would revert for sure. If it's only broken for people using it, I would put something in the Go docs. But if the rework is as complicated as you suggest, I would probably revert it. The change + rework can be done separately, and a new release can be cut at any time to release it. Getting 1.2.0 out the door is important for a lot of other features. For example, we've been waiting quite a long time for cgroup v2 PSI support. While we could use a git tag downstream, having a release is probably better practice considering how many downstream users of Kubernetes there are. |
The code is already ready in #3985. Reverting all of the changes I've done over the past 4 months to fix this code is going to be significantly more work than finishing the review of 1 PR. |
e1aba7f
to
9f7cf5d
Compare
given #3985 is merged, is there anything else required before a RC can be created? |
|
LGTM after rebase |
An rc1 needs to include the changelog in #4218 since it includes warnings about possible breakages. I can prepare an rc1 update PR with that included though. |
Signed-off-by: Rodrigo Campos <[email protected]>
Signed-off-by: Rodrigo Campos <[email protected]>
9f7cf5d
to
8c54e7a
Compare
@cyphar thanks! I've rebased, added that (removed the parenthesis about the mapping restriction, that wasn't never in a release), added this includes all the fixes to runc 1.1.12 and forward-ported those changelog entries too. I added a tentative date of next week for the release, I can adjust that too, of course. EDIT: I've also had a looked at the PRs that got merged since I last updated this (Dec 18), and it seems nothing was worth mentioning. So this should be up to date until now. @cyphar @AkihiroSuda PTAL |
b38377d
to
f4afe3d
Compare
Signed-off-by: Rodrigo Campos <[email protected]>
f4afe3d
to
c246dbc
Compare
I have several other changelog fixes that I've applied to my own branch (wording fixes and your copy of the 1.1.z changes is missing the links and a few other things), and to be honest it's simpler for me to do a release from my own branch. Closing in favour of #4221. |
As discussed in the mailing-list, here is the changelog for the 1.2.0-rc.1 release.
For future reference (in case it helps others), this is what I did to create it: I started to create this by looking at all the merge commits since 1.1.0, but that took several hours and I only went through a few months. Then I started to look at github PRs with the impact/changelog label. I used these queries:
This changelog is basically the result of those queries, plus 1-2 others that I found looking manually.
The release date is of course aspirational :)