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

Prepare / Tag v1.1.0 release #1052

Closed
thaJeztah opened this issue Jul 2, 2020 · 50 comments · Fixed by #1213
Closed

Prepare / Tag v1.1.0 release #1052

thaJeztah opened this issue Jul 2, 2020 · 50 comments · Fixed by #1213
Milestone

Comments

@thaJeztah
Copy link
Member

(related to https://github.com/containerd/containerd/pull/4357/files#r448837520)

Runc tagged a new release (v1.0.0-rc91), and is currently depending on a non-tagged version of the runtime-spec. With various distribution packagers requiring tagged releases, and with go modules encouraging consumers to packages to use tagged releases, I'm wondering if a new release should be tagged.

runc is currently consuming 237cc4f, (changes since v1.0.2: v1.0.2...237cc4f), but there's a couple more changes in master (237cc4f...master)

Are there pending pull requests that should be merged before tagging v1.0.3? (perhaps #1046 would be nice to have). tags should be "cheap" so, perhaps not a blocker for a v1.0.3, but suggestions welcome 🤗

@cyphar @crosbymichael @tianon @mrunalp @vbatts

@h-vetinari
Copy link
Contributor

Not to further hold up an already long process, but #1040 should also be very close to finished (just waiting for final review).

@AkihiroSuda
Copy link
Member

#1040 should be probably v1.0.4 or later

@thaJeztah
Copy link
Member Author

Given that it's adding new features, #1040 should probably bump the minor version to stick with SemVer (so v1.1.0)

@thaJeztah
Copy link
Member Author

Thinking of that, possibly #1046 warrants a "minor" version bump as well, as it's adding new functionality.

@thaJeztah thaJeztah changed the title Prepare / Tag v1.0.3 release Prepare / Tag v1.0.3 (or v1.1.0) release Jul 2, 2020
@cyphar
Copy link
Member

cyphar commented Jul 2, 2020

Yeah, we should tag a 1.0.3. I would like to see #1046 merged (I don't think it makes sense to refer to it as a new feature -- it's just defining Go constants that were already defined by the spec -- though arguably the new hooks we added should've made the last release a 1.1.0). And yeah, #1040 should definitely wait -- I don't want to rush adding more logic to cgroup configuration until we're sure it makes sense.

@mskarbek
Copy link

mskarbek commented Jul 2, 2020

@cyphar what more do you expect from croupv2 work?

@cyphar
Copy link
Member

cyphar commented Jul 2, 2020

@mskarbek We need to have at least one implementation of it so that we can be sure that the specification changes actually work on real systems, and if they don't work we should go back and fix issues we may find in the specification. In addition, it wouldn't hurt to talk to other container runtimes (such as LXC) to get their feedback since they've supported cgroupv2 for much longer than any OCI runtime.

EDIT: Ignore this section, it's pretty much gibberish. I shouldn't write comments while I'm still half-awake...

And looking at the PR right now, we also need to address how to deal with unknown configuration values. Currently it says that:

Configuration unknown to the runtime MUST still be written to the relevant file.

But also:

The runtime MUST generate an error when the configuration refers to a cgroup controller that is not present or that cannot be enabled.

Which could in theory conflict in the future if there is a controller that requires some extra setup other than writing its name to cgroup.controllers. We need to make sure we don't cause future headaches for ourselves while also allowing people to configure cgroupv2 controllers without needing endless runtime-spec updates (which is why I was on board with the current proposal and have discussed doing it this way off-line for some time).

Remember, once something is in the runtime-spec we cannot remove it (without a major version bump) -- shoe-horning it in would be a mistake.

@AkihiroSuda
Copy link
Member

The v1-to-v2 conversion convention can be cherry picked to v1.0.3?

@thaJeztah
Copy link
Member Author

@AkihiroSuda which change is that? Do you have a specific PR?

Looking at the changes since the last release; v1.0.2...8e2f17c, those all (at a glance) look ok for a (v1.0.3) "patch" release. Having that would help projects declare that they require >= v1.0.3 in their go.mod.

@cyphar
Copy link
Member

cyphar commented Jul 3, 2020

@thaJeztah I think he's referring to the v1-to-v2 conversion text that's in #1040 right now (in other words, should we take the conversion text -- which is something runtimes are already implementing -- and leave the whole unified thing for a separate release).

@AkihiroSuda Since the text for the v1-to-v2 conversion is all gated by SHOULD and MAY, we could merge it now and change it in future releases without breaking backwards compatibility (since it's effectively just a recommendation or option for implementations, respectively). Do runc and crun already implement the semantics described (I haven't been following all of the cgroupv2 work in both projects)?

@AkihiroSuda
Copy link
Member

Do runc and crun already implement the semantics described (I haven't been following all of the cgroupv2 work in both projects)?

The v1-to-v2 conversion is already implemented, but the pure unified struct is not.

@thaJeztah
Copy link
Member Author

Thanks for clarifying, yes that sounds reasonable

@giuseppe
Copy link
Member

giuseppe commented Jul 3, 2020

And yeah, #1040 should definitely wait -- I don't want to rush adding more logic to cgroup configuration until we're sure it makes sense.

I'd prefer if it has not to wait so much longer, without #1040 the cgroup v2 support is limited to the same features present in cgroup v1

@rhatdan
Copy link
Contributor

rhatdan commented Jul 8, 2020

@vbatts @mrunalp @AkihiroSuda @cyphar @crosbymichael Can we get this done?

@cyphar
Copy link
Member

cyphar commented Jul 11, 2020

@opencontainers/runtime-spec-maintainers Do we want to have #1040 split up now so that we can quickly review and merge the cgroup v1->v2 conversion table?

@thaJeztah
Copy link
Member Author

I think it would make sense to at least prepare a PR for that

@giuseppe
Copy link
Member

I'd drop the conversion table completely. Despite being the author of it, I am still not convinced it should be part of the runtime specs. I'd just leave the generic If a controller is enabled on the cgroup v2 hierarchy but the configuration is provided for the cgroup v1 equivalent controller, the runtime MAY attempt a conversion.

@AkihiroSuda
Copy link
Member

Let's release v1.0.3 as-is and handle the cgroup v2 stuff separately

@giuseppe
Copy link
Member

I've dropped the conversion table, that can be added again later if needed

@AkihiroSuda
Copy link
Member

How can we move this forward? @opencontainers/runtime-spec-maintainers

@vbatts
Copy link
Member

vbatts commented Aug 3, 2020

I can do this, but others could as well. Open a PR of the commit with a version bump and then back to "-dev" (2 commits).
Then send an email to the list with a voting period.

@giuseppe
Copy link
Member

giuseppe commented Aug 4, 2020

I've simplified #1040

Is there any chance it can be reviewed before we cut a new release?

@AkihiroSuda
Copy link
Member

#1040 might be still controversial to rush in v1.0.3. Can it be v1.0.4?

btw #1060 seems to need prioritization for v1.0.3

@giuseppe
Copy link
Member

giuseppe commented Aug 5, 2020

#1040 might be still controversial to rush in v1.0.3. Can it be v1.0.4?

I am fine with it

@AkihiroSuda
Copy link
Member

#1059 seems also harmless for v1.0.3 and easy to review

@h-vetinari
Copy link
Contributor

Now that #1040 is also in, is there anything still necessary for this?

Looking at the open PRs since 1.0.2:

@h-vetinari
Copy link
Contributor

Friendly ping @opencontainers/runtime-spec-maintainers
@crosbymichael @mrunalp @vbatts @dqminh @tianon @hqhq @cyphar

@h-vetinari
Copy link
Contributor

h-vetinari commented Sep 22, 2020

From here, regarding deprecation of KernelMemory (which also happens to be broken on RHEL 7):

@cyphar: We will need to remove it from the runtime-spec too...

EDIT (April '21): Should be done now: #1093, opencontainers/runc#2840

@AkihiroSuda
Copy link
Member

@opencontainers/runtime-spec-maintainers
Can we move this forward?

@zhsj
Copy link

zhsj commented May 31, 2022

Any news? Currently most consumers(runc containers/common) are not using the tagged version, which makes the version of the spec meaningless..

@rhatdan
Copy link
Contributor

rhatdan commented May 31, 2022

@kolyshkin @thaJeztah Please cut a release.

@giuseppe
Copy link
Member

Please let's merge #1143 before we cut a new release

@kolyshkin
Copy link
Contributor

@kolyshkin @thaJeztah Please cut a release.

Neither me nor @thaJeztah are maintainers of this repo (as of right now, although this is being changed by #1150).

@AkihiroSuda
Copy link
Member

@kolyshkin @thaJeztah Please cut a release.

Neither me nor @thaJeztah are maintainers of this repo (as of right now, although this is being changed by #1150).

You both are maintainers now 😃.
Are we ready to cut a release?

@kolyshkin
Copy link
Contributor

Sure, let's cut 1.1.0. It seems that we would need a few PRs to be merged first. Let me work on a milestone.

@AkihiroSuda
Copy link
Member

It seems that we would need a few PRs to be merged first. Let me work on a milestone.

Which PRs?

@AkihiroSuda
Copy link
Member

@kolyshkin ping 🙏

@AkihiroSuda AkihiroSuda pinned this issue Oct 24, 2022
@AkihiroSuda AkihiroSuda changed the title Prepare / Tag v1.0.3 (or v1.1.0) release Prepare / Tag v1.1.0 release Oct 24, 2022
@giuseppe giuseppe unpinned this issue Nov 3, 2022
@h-vetinari
Copy link
Contributor

runtime-spec releases really are a sisyphean story...

There's a bunch of high-quality PRs that would suggest themselves for inclusion, but perhaps the current state of the repo should just be released to finally provide a new baseline, and then focus on getting those PRs merged for another release that's similarly just cut without a lot of ceremony?

It's a pity for those features, but it seems we're stuck in an infinite "just these couple more PRs before we release" cycle.

@AkihiroSuda
Copy link
Member

Let's move this forward

@AkihiroSuda AkihiroSuda pinned this issue Feb 1, 2023
@AkihiroSuda AkihiroSuda added this to the v1.1.0 milestone Feb 1, 2023
@h-vetinari
Copy link
Contributor

RC has been live for 6 weeks - is there anything in particular that's still necessary to happen for a release?

@AkihiroSuda
Copy link
Member

RC has been live for 6 weeks - is there anything in particular that's still necessary to happen for a release?

#1130 (comment)

@AkihiroSuda
Copy link
Member

Proposal for releasing v1.1.0-rc.2:

@utam0k
Copy link
Member

utam0k commented Mar 30, 2023

Can 1.1.0 include these at a minimum? I am not talking about the next rc.2.

@AkihiroSuda
Copy link
Member

AkihiroSuda commented May 22, 2023

Proposal for v1.1.0-rc.3

This should be the last RC for v1.1.0.

@AkihiroSuda
Copy link
Member

If there is no objection, I'd like to propose releasing v1.1 GA very soon.

@utam0k
Copy link
Member

utam0k commented Jun 22, 2023

👍

@AkihiroSuda AkihiroSuda mentioned this issue Jun 26, 2023
12 tasks
@AkihiroSuda
Copy link
Member

@AkihiroSuda AkihiroSuda unpinned this issue Jul 22, 2023
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 a pull request may close this issue.