Skip to content

daemon: un-export internal functions, and some minor refactoring#41656

Merged
AkihiroSuda merged 5 commits intomoby:masterfrom
thaJeztah:unexport_things
Jun 8, 2021
Merged

daemon: un-export internal functions, and some minor refactoring#41656
AkihiroSuda merged 5 commits intomoby:masterfrom
thaJeztah:unexport_things

Conversation

@thaJeztah
Copy link
Member

  • daemon: un-export IsRunningSystemd()
    This utility was added after 19.03, and is only used in the daemon code
    itself, so we can un-export it, until there's an external use for it.

    Also updated the description, because the runc code already copied it
    from coreos/go-systemd, so better to describe the actual source.

  • daemon: use sync.Once for systemd detection

  • daemon: un-export VerifyCgroupDriver()

  • daemon: un-export ModifyRootKeyLimit()

  • daemon: move DefaultShimBinary, DefaultRuntimeBinary to config package

@thaJeztah thaJeztah added area/daemon Core Engine kind/refactor PR's that refactor, or clean-up code status/2-code-review labels Nov 10, 2020
Copy link
Member

Choose a reason for hiding this comment

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

Not a new in this PR, but v1 shim is no longer used by default. Can we add a godoc about that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm.. was about to update the description, but looks like it's still used if containerd is started as child process;

func (cli *DaemonCli) getPlatformContainerdDaemonOpts() ([]supervisor.DaemonOpt, error) {
opts := []supervisor.DaemonOpt{
supervisor.WithOOMScore(cli.Config.OOMScoreAdjust),
supervisor.WithPlugin("linux", &linux.Config{
Shim: daemon.DefaultShimBinary,
Runtime: daemon.DefaultRuntimeBinary,
RuntimeRoot: filepath.Join(cli.Config.Root, "runc"),
ShimDebug: cli.Config.Debug,
}),
}

Should that be updated to containerd-shim-runc-v2 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Or should it be removed altogether? I see that if the containerd-config looks like this;

[plugins]
  [plugins.linux]
    shim = "containerd-shim"
    runtime = "runc"
    runtime_root = "/var/lib/docker/runc"
    no_shim = false
    shim_debug = true

Containers are still started with containerd-shim-runc-v2, so looks like the configuration is not needed anymore?

I see containerd also sets containerd-shim and runc as defaults; https://github.com/containerd/containerd/blob/9b4967bd6b77e4a6bb4c36c86b4e3a6daaead6d4/runtime/v1/linux/runtime.go#L71-L82

So perhaps it should be removed, or is there still a path where this would be used?

thaJeztah added 5 commits May 31, 2021 19:06
This utility was added after 19.03, and is only used in the daemon code
itself, so we can un-export it, until there's an external use for it.

Also updated the description, because the runc code already copied it
from coreos/go-systemd, so better to describe the actual source.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
it's only used internally, so no need to export

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
it's only used internally, so no need to export

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

@AkihiroSuda @cpuguy83 PTAL

@thaJeztah
Copy link
Member Author

Good to go? 😅

@AkihiroSuda AkihiroSuda merged commit 0ad2293 into moby:master Jun 8, 2021
@thaJeztah thaJeztah deleted the unexport_things branch June 8, 2021 06:34
@thaJeztah thaJeztah added this to the 21.xx milestone Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/daemon Core Engine kind/refactor PR's that refactor, or clean-up code status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants