Refactor the (container) run command flagging process

[Laitr0n]

What is the problem you're trying to solve

I'm currently refactoring the process of handling flags for the (container) run command. However, it is very complex and I would like to discuss it further.

Describe the solution you'd like

First, we should split the createContainer function into smaller functions. As seen in the code at https://github.com/containerd/nerdctl/blob/1bbfe6930c56f3d69fe05b300f7c2817b828c024/cmd/nerdctl/container_run.go#L408-753 We will divide it into three tasks:

1. Move all helper functions to pkg/cmd/*util;
2. Add a function processInternalLabels;
3. Move all flag retrieval to processCreateContainerFlags.

Next, we will move createContainer to pkg/cmd/containerutil.

Finally, we will create ContainerRunOptions in pkg/api/types/container_types.go and Run in pkg/cmd/container/run.go.

Due to the complexity of the createContainer function and its numerous helper functions, it will take more time to handle.

@AkihiroSuda @djdongjin @Zheaoli , WDYT? Can you share your ideas?

Additional context

No response

[djdongjin]

@Laitr0n If it's me I'll start from nerdctl create and start from creating ContainerStartOptions and processContainerStartOptions, and in this step keep the code in the original place (not move from cmd -> pkg).

Why start from nerdctl create? nerdctl create is basically part of nerdctl run, and nerdctl create only uses createContainer.

Why start from creating ContainerStartOptions and processContainerStartOptions? This will make every func shorter, by eliminating things like:

    flagT, err := cmd.Flags().GetBool("tty")
    if err != nil {
        return err
    }

After this, it'll be more clear how we should relocate different funcs to pkg.

[Laitr0n]

@Laitr0n If it's me I'll start from nerdctl create and start from creating ContainerStartOptions and processContainerStartOptions, and in this step keep the code in the original place (not move from cmd -> pkg).

I got you mean that it start from nerdctl create, ContainerCreateOptions (not ContainerStartOptions) and processContainerCreateOptions (not processContainerStartOptions).

Attach it in PR #1970.

[Laitr0n]

@djdongjin createContainer process has many flag options. I would like to split them into different structures in processContainerCreateOptions based on the nerdctl run doc . This will result in the creation of BasicFlags, PlatformFlags, InitProcessFlags, IsolationFlags, NetworkFlags, ResourceFlags, IntelRDTFlags, UserFlags, SecurityFlags, RuntimeFlags, VolumeFlags, RootfsFlags, Env Flags, MetadataFlags, LoggingFlags, SharedMemoryFlags, GPUFlags, UlimitFlags, VerifyFlags and IPFSFlags structures.

The ContainerCreateOptions will be comprised of these flag structures and ContainerRunOptions could reuse them. In this way, as you mentioned, the flag retrieval process will be eliminated in createContainer and other help functions, and making it shorter. And we can do next work clearly.

But with so many flag structures, is it reasonable?"

[suyanhanx]

@djdongjin createContainer process has many flag options. I would like to split them into different structures in processContainerCreateOptions based on the nerdctl run doc . This will result in the creation of BasicFlags, PlatformFlags, InitProcessFlags, IsolationFlags, NetworkFlags, ResourceFlags, IntelRDTFlags, UserFlags, SecurityFlags, RuntimeFlags, VolumeFlags, RootfsFlags, Env Flags, MetadataFlags, LoggingFlags, SharedMemoryFlags, GPUFlags, UlimitFlags, VerifyFlags and IPFSFlags structures.

The ContainerCreateOptions will be comprised of these flag structures and ContainerRunOptions could reuse them. In this way, as you mentioned, the flag retrieval process will be eliminated in createContainer and other help functions, and making it shorter. And we can do next work clearly.

But with so many flag structures, is it reasonable?"

I feel that this will make options struct complex and difficult to use directly and add a direct mental overload to developers.

[djdongjin]

But with so many flag structures, is it reasonable?"

I'd suggest start with a single ContainerCreateOptions and use comments to label different regions (similar to the current setCreateFlags).

Adding too many sub structs will make ContainerCreateOptions relatively difficult to construct and use. For example, with the structure, the ContainerRunOptions will have something like ContainerRunOptions.ContainerCreateOptions. PlatformFlags.Platform.