Skip to content

cgroups separation followup #4784

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.

Sign up for GitHub

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

Jump to bottom
Merged
kolyshkin merged 3 commits into from
Jun 19, 2025
Merged

cgroups separation followup #4784

kolyshkin merged 3 commits into from
Jun 19, 2025

Conversation

@kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Jun 17, 2025

This is a followup to #4638, fixing a few minor issues I found just now.

See individual commits for details.

@kolyshkin kolyshkin marked this pull request as draft June 19, 2025 00:59
@kolyshkin
Copy link
Contributor Author

kolyshkin commented Jun 19, 2025 •
edited
Loading

Seeing a weird golangci-lint warning:

Error: libcontainer/criu_linux.go:186:61: directive `//nolint:staticcheck // SA1019: strings.Title is deprecated` is unused for linter "staticcheck" (nolintlint)
	return "extRoot" + strings.Title(configs.NsName(t)) + "NS" //nolint:staticcheck // SA1019: strings.Title is deprecated
	                                                           ^

which I am not seeing locally or in any other PRs.

@kolyshkin kolyshkin closed this Jun 19, 2025
@kolyshkin kolyshkin reopened this Jun 19, 2025
@kolyshkin kolyshkin closed this Jun 19, 2025
kolyshkin added 2 commits June 18, 2025 18:14
@kolyshkin
libct/configs: fix/improve deprecation notices
b25bcaa 
The per-file deprecation in cgroup_deprecated.go is not working,
let's replace it.

Link to Hooks.Run in Hook.Run deprecation notice.

Signed-off-by: Kir Kolyshkin <[email protected]>
@kolyshkin
libct/configs: stop using deprecated id
d22a421 
Signed-off-by: Kir Kolyshkin <[email protected]>
@kolyshkin kolyshkin reopened this Jun 19, 2025
@kolyshkin
Copy link
Contributor Author

kolyshkin commented Jun 19, 2025

Seeing a weird golangci-lint warning:

Error: libcontainer/criu_linux.go:186:61: directive `//nolint:staticcheck // SA1019: strings.Title is deprecated` is unused for linter "staticcheck" (nolintlint)
	return "extRoot" + strings.Title(configs.NsName(t)) + "NS" //nolint:staticcheck // SA1019: strings.Title is deprecated
	                                                           ^

which I am not seeing locally or in any other PRs.

It went away once I've forcefully removed golangci-lint-action cache from GHA CI. This is very wrong.

@kolyshkin kolyshkin marked this pull request as ready for review June 19, 2025 01:31
rata
rata approved these changes Jun 19, 2025
View reviewed changes
Copy link
Member

@rata rata left a comment

Choose a reason for hiding this comment

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

LGTM, just one question that is trivial to address

libcontainer/configs/config.go
// Cgroups specifies specific cgroup settings for the various subsystems that the container is
// placed into to limit the resources the container has available.
Cgroups *Cgroup `json:"cgroups"`
Cgroups *cgroups.Cgroup `json:"cgroups"`
Copy link
Member

@rata rata Jun 19, 2025

Choose a reason for hiding this comment

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

Do we want to keep the Cgroup type on this package, then?

Copy link
Contributor Author

@kolyshkin kolyshkin Jun 19, 2025

Choose a reason for hiding this comment

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

Do we want to keep the Cgroup type on this package, then?

I think we do. There are libcontainer users who might need time to switch to opencontainers/cgroups properly.

Now, Cgroup is (now) clearly marked as deprecated, hinting that they should switch, and by not removing it we're giving users time to do so.

@kolyshkin kolyshkin added the backport/1.3-todo A PR in main branch which needs to be backported to release-1.3 label Jun 19, 2025
@kolyshkin kolyshkin merged commit 71bd84f into opencontainers:main Jun 19, 2025
59 checks passed
@kolyshkin kolyshkin mentioned this pull request Jun 19, 2025
Merged
@kolyshkin
Copy link
Contributor Author

kolyshkin commented Jun 19, 2025

1.3 backport: #4788

@kolyshkin kolyshkin added backport/1.3-done A PR in main branch which has been backported to release-1.3 and removed backport/1.3-todo A PR in main branch which needs to be backported to release-1.3 labels Jun 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rata rata rata approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

Assignees

No one assigned

Labels

backport/1.3-done A PR in main branch which has been backported to release-1.3

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kolyshkin @rata @AkihiroSuda