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

Release rc11 #2364

Closed
wants to merge 2 commits into from
Closed

Release rc11 #2364

wants to merge 2 commits into from

Conversation

mrunalp
Copy link
Contributor

@mrunalp mrunalp commented Apr 29, 2020

We have had quite a few fixes in the area of cgroups v2 and we should get a release out
so other repositories can pick up those fixes by vendoring the release.

@mrunalp
Copy link
Contributor Author

mrunalp commented Apr 29, 2020

@opencontainers/runc-maintainers ptal

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's finish #2315

@kolyshkin
Copy link
Contributor

kolyshkin commented Apr 29, 2020

Are we doing any kind of changelog somewhere?

@h-vetinari
Copy link

Let's finish #2315

That's basically #2281 and #2352, right? #2350 still needs work on the cilium side IIUC?

#2271 & #2292 have been ready for a while as well AFAICT.

@cyphar
Copy link
Member

cyphar commented Apr 29, 2020

Regarding cgroupv2, there's also a bug in the devices cgroup (it cannot update the device list, and there are actually a few complications if we want to be able to update the device list -- we need to keep the fd of the program around, and we don't have an in-project container monitor process to do that, and I refuse to re-live the pain of --console-socket again). So we should probably fix that too...

I also didn't see a vote email on the ML (I know the current process is silly, but we should still use it until we agree on an alternative system for releases).

Are we doing any kind of changelog somewhere?

I usually write the changelog by hand, which is a little bit annoying. I think we discussed that there should probably be a CHANGELOG.md which PR authors should update, but the problem is that we probably don't have the time to back-fill it with all the other pre-1.0 releases.

@AkihiroSuda
Copy link
Member

it cannot update the device list,

Please open an issue? Also, who is actually using that feature?

@cyphar
Copy link
Member

cyphar commented Apr 30, 2020

Please open an issue? Also, who is actually using that feature?

Will do. Kubernetes does cgroup updates fairly regularly (and given #2205, it's probably a real issue -- we have customers that are hitting that problem in particular under Kubernetes). And you can also do devices cgroup updates from Docker unless I'm mistaken.

@AkihiroSuda AkihiroSuda marked this pull request as draft April 30, 2020 01:29
@AkihiroSuda
Copy link
Member

BTW did we complete supporting Go 1.14? (#2258)

@dims
Copy link
Contributor

dims commented May 6, 2020

@cyphar @mrunalp fyi we are holding an update to google/cadvisor and kubernetes/kubernetes for rc11 to land ( google/cadvisor#2469 )

@h-vetinari
Copy link

Some unexpected progress on #2229 - which is now aiming at adding all the new hooks. Seems like the possibility to feature-freeze with rc11 is moving into view after all.

@thaJeztah
Copy link
Member

I'm proposing to tag upcoming releases as v1.0.1-rc.X. See my motivation in #2399

@h-vetinari
Copy link

h-vetinari commented May 12, 2020

There's also the former plan to:

We will try to make sure that runc and the OCI specification major versions stay in lockstep. This means that runc 1.0.0 should implement the 1.0[.0?] version of the specification.

Since the runtime spec is at 1.0.2 already, one might argue to skip 1.0.1 as well.

OTOH, runc will very likely keep having a much higher release cadence than the spec, so maybe version parity with the spec needs to be reconsidered anyway (especially if following SemVer is a goal)

@cyphar
Copy link
Member

cyphar commented May 12, 2020

@h-vetinari I would put incredibly strong emphasis on the word "former" in "former plan". As soon as we release 1.0.0 GA, we're going to completely deviate from the existing versioning and release scheme because it's been an absolute nightmare.

However, we simply cannot release (our first) post-1.0.0 version of runc if we know that it doesn't implement the specification properly. So such a solution is IMHO untenable.

@thaJeztah
Copy link
Member

I don't think we can match the version to the runtime-spec version, as that would mean that a bugfix (x.x.n+1) in runc would require a version update of the runtime-spec; question is if the tags on the repository should reflect the binary releases, or the golang releases; they may not have the same stability (a new feature in the Golang API may not introduce new functionality in the binary). Using separate versioning for submodules (e.g. for libcontainer) could be a solution for that, but will likely be a big pain to maintain in the same repository

@cyphar
Copy link
Member

cyphar commented May 12, 2020

@thaJeztah Don't worry, we're definitely not going to match releases -- for the reasons you've outlined as well as many others (one being that we would be stuck in rc releases for long periods because runtime-spec releases are few and far between -- this was actually the original reason why we stalled 1.0.0 ~3 years ago).

@giuseppe
Copy link
Member

Using separate versioning for submodules (e.g. for libcontainer) could be a solution for that, but will likely be a big pain to maintain in the same repository

That is an interesting point. The current git version has a few breaking changes in the API (e.g. functions/types in libcontainer/cgroups that were public are private now), should we consider having a v2?

@cyphar
Copy link
Member

cyphar commented May 13, 2020

At the moment, I'm not sure I'm willing to consider libcontainer as something people should be using outside of runc. So much of it is incredibly internal stuff, and in order to use it you'd need to reimplement most of the code we have in the main runc package.

Not to mention that out of all the changes we've had since 1.0.0-rc1 (which was 4 years ago), the cgroup ones are probably one of the smaller breakages -- --console-socket was a much more invasive change in terms of API breakages. And if we're going to play the SemVer game, technically we haven't broken any backwards compatibility because pre-releases don't count towards the backward-compatibility requirements of SemVer.

Also, having a v2 before we've had a v1 would make us look like even bigger cowboys when it comes to versioning. 😉

@thaJeztah
Copy link
Member

I've done some work on reducing dependencies on libcontainer in a couple of projects, but there's definitely still quite some uses outside of runc itself; https://grep.app/search?q=github.com/opencontainers/runc/libcontainer&filter[lang][0]=Go

We could drill that list further down to see if there's candidates for splitting out (e.g. libcontainer/user, but libcontainer/cgroups also seems to have some wide use)

Also, having a v2 before we've had a v1 would make us look like even bigger cowboys when it comes to versioning. 😉

Not sure what's "cleaner" at this point; it would be more "honest", but I guess (hope!) dependency projects by now have learned to "pin" to a version, and not to upgrade without having a good look for possibly breaking changes.

@giuseppe
Copy link
Member

At the moment, I'm not sure I'm willing to consider libcontainer as something people should be using outside of runc. So much of it is incredibly internal stuff, and in order to use it you'd need to reimplement most of the code we have in the main runc package.

Not to mention that out of all the changes we've had since 1.0.0-rc1 (which was 4 years ago), the cgroup ones are probably one of the smaller breakages -- --console-socket was a much more invasive change in terms of API breakages. And if we're going to play the SemVer game, technically we haven't broken any backwards compatibility because pre-releases don't count towards the backward-compatibility requirements of SemVer.

I see. The issue I am facing now is that cAdvisor and the kubelet are using it, both to read statistics and to create new cgroups; the entire cgroup v2 support in Kubernetes depends on it.

@derekwaynecarr
Copy link
Contributor

Is it possible to move libcontainer or just the cgroup part of it into a separate vendorable asset? The kubelet and cAdvisor could probably better scope what we have vendored in the past. If it was never appropriate to vendor libcontainer, apologies as that was not well-understood in Kubernetes community in years past. I know I have made selective PR fixes specific to Kubernetes usage in the past.

@mrunalp
Copy link
Contributor Author

mrunalp commented May 13, 2020

As @derekwaynecarr pointed out, kubernetes has relied on using libcontainer cgroups library for a long time now. @giuseppe is helping to make cgroups v2 a reality in k8s and @kolyshkin has been refactoring and improving the cgroups code in libcontainer and runc. I think we should find a path to get a quick iteration of testing cgroups v2 in k8s as it gives us a valuable feedback loop to improve cgroups v2 support in runc. We can't possibly assume we have fixed everything in cgroups v2 without that feedback. So, I think it makes sense for us to cut rc releases frequently to help facilitate updates in k8s. Once we get enough signal from there, we can start thinking of factoring out libcontainer/cgroups as a separate library.

@h-vetinari
Copy link

@cyphar: However, we simply cannot release (our first) post-1.0.0 version of runc if we know that it doesn't implement the specification properly. So such a solution is IMHO untenable.

I understand the emotional value of v1.0.0 as a maintainer, but I think you'll find that users of the library don't care much about the actual version string**, as long as they can use the best-available, up-to-date, well-maintained software.

Achieving a completely bugfree implementation of the spec (even aside from the question that some aspects of the new hooks were left underspecified to finally unblock progress) is IMO an almost unreachable threshold for releasing 1.0.0. More likely (and despite everyone's best effort), there will be a 1.0.1 very soon afterwards, as well as a 1.1.0 (assuming SemVer), since the feature appetite will most likely remain high - and that's OK.

So from that POV, even though it doesn't feel that way, 1.0.0 it's a pretty arbitrary line in the sand. To wit,rc6 was once "an intermediate release which will mark a freeze-on-everything-except-spec-compliance-bugs", but 1.5 years later, one could now reasonably argue that the release should wait for more complete cgroups v2 support. It's a game that can be played indefinitely.

@cyphar: Also, having a v2 before we've had a v1 would make us look like even bigger cowboys when it comes to versioning. 😉

There's a decent number of projects that skip a major version (e.g. openssl/openssl), or even v1 (e.g. containers/conmon, cython/cython), and nowhere have I heard a derogatory comment about something like this. It can be a valid choice for several reasons (aside from being trivial to deal with downstream), and it's arguably less version-cowboy-y than the current one.

** aside from information about features/breaking changes; but that's also not being served by the rc's.

@cyphar
Copy link
Member

cyphar commented May 14, 2020

The issue is that we have (loosely speaking) an actual OCI Charter requirement to have runc 1.0 be a correct reference implementation of the runtime-spec. Obviously bugs may exist (and in fact, I found one in #2391), but releasing a post-1.0 runc which we know to not implement the spec is acting in bad faith when it comes to our responsibilities within the OCI. Believe me, if that wasn't a requirement I believe we would've released 1.0 about 3 years ago, and we'd be on runc 3.54 or something now.

Note that the hooks issue has been the only blocking issue for 1.0 support for the past 2 years -- we've known about it since #1811. It's just that it took us this long to actually get to a PR being ready. If the hooks issue hadn't come up, we would've released 1.0 back then. And as for any additional features, I actually don't care at all if cgroupv2 works in runc 1.0. I will call a vote for the next runc release the minute we merge #2229.

And yes, I know that releasing 1.0.1-rc.1 technically means we didn't release a 1.0.0 but I'm not sure I buy that argument completely.

and it's arguably less version-cowboy-y than the current one

I don't really disagree, tbh. I was just making a joke. 😉

@cyphar
Copy link
Member

cyphar commented May 14, 2020

@giuseppe @derekwaynecarr @mrunalp

Regarding my comments above about libcontainer, I do get that libcontainer/cgroups (and libcontainer/user to a lesser extent) are "special" in that folks really do use them and we should support them better. But I think splitting these out into separate libraries really would help us have a clearer picture that libcontainer is something entirely internal to runc and shouldn't be used by anymore who actually wants proper support from us. It would also allow us to version them properly.

Because in my view, the scope of SemVer on runc is purely in terms of the command-line API because that is the way the project is meant to be used. I would prefer if we could actually make that clear without freaking out everyone who depends on some of our internal components (including Kubernetes). However right now the TOB has our hands full with cleaning up the OCI Charter, and we should be pushing to 1.0 with as few distractions as possible. So we can table this discussion until we get past 1.0, IMHO.

@h-vetinari
Copy link

Aiming for spec compliance is totally a valid reason for waiting with the release. I was commenting more about the "tried, tested, proven" implication of "properly implemented", which was reinforced by @mrunalp's comment. That's IMO not a reasonable goal for 1.0.0.

In any case, there finally is light at the end of the tunnel in terms of spec compliance. 🥳

I will call a vote for the next runc release the minute we merge #2229.

With the deepest respect for everyone's packed schedules, I don't really understand how this sort of alacrity doesn't translate into reviews of that PR (the written medium is not great for transmitting nuance, so let me emphasise that this observation contains not even an iota of criticism, I just genuinely don't get it).

@AkihiroSuda
Copy link
Member

Splitting libcontainer/cg repo will cause vendor hell and make it almost untestable.
If we want to split go mod, can we do that without splitting repo?

@thaJeztah
Copy link
Member

Splitting libcontainer/cg repo will cause vendor hell and make it almost untestable.
If we want to split go mod, can we do that without splitting repo?

I agree it should not be a pain to deal with in this repository (which should be the primary consumer of the package), just (reiterating) we should likely avoid submodules as they may be more of a pain than a separate repository.

@AkihiroSuda you may be more familiar with it than I am; how does https://github.com/containerd/cgroups compare to libcontainer/cgroups ? Is that a viable alternative? (assuming it's up-to-date, and kept up-to-date with the latest cgroupsv2 changes)

@AkihiroSuda
Copy link
Member

AkihiroSuda commented May 14, 2020

Is that a viable alternative? (assuming it's up-to-date, and kept up-to-date with the latest cgroupsv2 changes)

Yes but the assumption is not true :p
Notably it lacks systemd and tests systemd support and tests are incomplete

@crosbymichael
Copy link
Member

Splitting libcontainer/cg repo will cause vendor hell and make it almost untestable.
If we want to split go mod, can we do that without splitting repo?

I agree it should not be a pain to deal with in this repository (which should be the primary consumer of the package), just (reiterating) we should likely avoid submodules as they may be more of a pain than a separate repository.

@AkihiroSuda you may be more familiar with it than I am; how does https://github.com/containerd/cgroups compare to libcontainer/cgroups ? Is that a viable alternative? (assuming it's up-to-date, and kept up-to-date with the latest cgroupsv2 changes)

This is where the code should have landed! Don’t make yet another pkg.

@cyphar
Copy link
Member

cyphar commented May 15, 2020

@crosbymichael I guess it's time for me to port the devices.Emulator changes to that. 😉

@mrunalp
Copy link
Contributor Author

mrunalp commented Jun 11, 2020

Closing this since we can open a new one for rc91.

@mrunalp mrunalp closed this Jun 11, 2020
@AkihiroSuda
Copy link
Member

@mrunalp Would you like to open a rc91 PR and call for voting?

@mrunalp
Copy link
Contributor Author

mrunalp commented Jun 30, 2020

Yeah, I am opening a PR for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants