Deprecate UserNSSize, since we don't use it #1238
Conversation
|
@giuseppe @anotherwon PTAL |
|
I was working on exactly the same fix :) LGTM I'll prepare a patch for Podman to honor the settings after |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
docs/containers.conf.5.md
Outdated
| Default way to to create a USER namespace for the container. | ||
| Options are: | ||
| `private` Create private USER Namespace for the container. | ||
| `host` Share host USER Namespace with the container. | ||
|
|
There was a problem hiding this comment.
This is removing the wrong description.
pkg/config/config.go
Outdated
| @@ -213,7 +213,7 @@ type ContainersConfig struct { | |||
| // UserNS indicates how to create a User namespace for the container | |||
| UserNS string `toml:"userns,omitempty"` | |||
|
|
|||
| // UserNSSize how many UIDs to allocate for automatically created UserNS | |||
| // UserNSSize Deprecated, no user of this field is known. | |||
There was a problem hiding this comment.
You should use this format
// UserNSSize how many UIDs to allocate for automatically created UserNS
// Deprecated: no user of this field is known
UserNSSize int `toml:"userns_size,omitempty,omitzero"`
This will make certain linters complain if it used anywhere.
pkg/config/default.go
Outdated
| @@ -157,7 +157,7 @@ const ( | |||
| DefaultRootlessSignaturePolicyPath = "containers/policy.json" | |||
| // DefaultShmSize is the default upper limit on the size of tmpfs mounts. | |||
| DefaultShmSize = "65536k" | |||
| // DefaultUserNSSize indicates the default number of UIDs allocated for user namespace within a container. | |||
| // DefaultUserNSSize Deprecated | |||
There was a problem hiding this comment.
same as the other comment, fix the formatting here
Podman and Buildah do not use this field, and I know of no users of it, remove it from docs and the default conf file, so users will not expect it to do anything. Leaving implementation in the slight chance someone has used it in a non containers project. Fixes: containers/podman#16562 Signed-off-by: Daniel J Walsh <[email protected]>
|
Seems okay to me, though I don't know the codebase. |
Podman and Buildah do not use this field, and I
know of no users of it, remove it from docs and
the default conf file, so users will not expect
it to do anything.
Leaving implementation in the slight chance someone has used it in a non containers project.
Fixes: containers/podman#16562
Signed-off-by: Daniel J Walsh [email protected]