Handle errors in specerror

[zhouhao3]

There are some declared errors in specerror that have not been called to validate them. Now they are listed below, and we can discuss what needs to be done and what does not.
If there's anything you need to add or remove, you can edit directly.

Bundle

• ConfigConstName: This REQUIRED file MUST be named config.json.

Config-linux

Namespace

• NSProcInPath: The runtime MUST place the container process in the namespace associated with that path.
• NSPathMatchTypeError: The runtime MUST generate an error if path is not associated with a namespace of type type.
• NSNewNSWithoutPath: If path is not specified, the runtime MUST create a new container namespace of type type.
• NSInheritWithoutType: If a namespace type is not specified in the namespaces array, the container MUST inherit the runtime namespace of that type.

User namespace mappings

• ❌ UserNSMapOwnershipRO: The runtime SHOULD NOT modify the ownership of referenced filesystems to realize the mapping.

Devices

• ❌ DevicesMajMinRequired: major, minor (int64, REQUIRED unless type is p) - major, minor numbers for the device.
• DevicesErrorOnDup: The same type, major and minor SHOULD NOT be used for multiple devices.

Cgroups

• ❌ CgroupsPathAbsOrRel: The value of cgroupsPath MUST be either an absolute path or a relative path.
• CgroupsAbsPathRelToMount: In the case of an absolute path (starting with /), the runtime MUST take the path to be relative to the cgroups mount point.
• CgroupsPathAttach: If the value is specified, the runtime MUST consistently attach to the same place in the cgroups hierarchy given the same value of cgroupsPath.
• ❌ CgroupsPathError: Runtimes MAY consider certain cgroupsPath values to be invalid, and MUST generate an error if this is the case.

Device whitelist

• DevicesApplyInOrder: The runtime MUST apply entries in the listed order.

Block IO

• BlkIOWeightOrLeafWeightExist: You MUST specify at least one of weight or leafWeight in a given entry, and MAY specify both.

IntelRdt

• ❌ IntelRdtPIDWrite: If intelRdt is set, the runtime MUST write the container process ID to the <container-id>/tasks file in a mounted resctrl pseudo-filesystem, using the container ID from start and creating the container-id directory if necessary.
• ❌ IntelRdtNoMountedResctrlError: If no mounted resctrl pseudo-filesystem is available in the runtime mount namespace, the runtime MUST generate an error.
• ❌ NotManipResctrlWithoutIntelRdt: If intelRdt is not set, the runtime MUST NOT manipulate any resctrl pseudo-filesystems.
• ❌ IntelRdtL3CacheSchemaWrite: If l3CacheSchema is set, runtimes MUST write the value to the schemata file in the <container-id> directory discussed in intelRdt.
• ❌ IntelRdtL3CacheSchemaNotWrite: If l3CacheSchema is not set, runtimes MUST NOT write to schemata files in any resctrl pseudo-filesystems.

Config-Windows

HyperV

• ❌ WindowsHyperVPresent: If present, the container MUST be run with Hyper-V isolation.
• ❌ WindowsHyperVOmit: If omitted, the container MUST be run as a Windows Server container.

Config

Root

• RootOnWindowsRequired: On Windows, for Windows Server Containers, this field is REQUIRED.

Mounts

• ❌ MountsOptionsOnWindowsROSupport: Windows: runtimes MUST support ro, mounting the filesystem read-only when ro is given.

Process

• ❌ ProcRequiredAtStart: This property is REQUIRED when start is called. (same with StartWithProcUnsetGenError)
• ❌ ProcConsoleSizeIgnore: Runtimes MUST ignore consoleSize if terminal is false or unset.

POSIX process

• PosixProcRlimitsTypeGenError: The runtime MUST generate an error for any values which cannot be mapped to a relevant kernel interface.
• ❌ PosixProcRlimitsTypeGet: For each entry in rlimits, a getrlimit(3) on type MUST succeed.
• PosixProcRlimitsSoftMatchCur: rlim.rlim_cur MUST match the configured value.
• PosixProcRlimitsHardMatchMax: rlim.rlim_max MUST match the configured value.

Linux Process

• LinuxProcCapError: Any value which cannot be mapped to a relevant kernel interface MUST cause an error.
• ❌ LinuxProcOomScoreAdjNotSet: If oomScoreAdj is not set, the runtime MUST NOT change the value of oom_score_adj.

POSIX-platform Hooks

• PosixHooksCalledInOrder: Hooks MUST be called in the listed order.
• PosixHooksStateToStdin: The state of the container MUST be passed to hooks over stdin so that they may do work appropriate to the current state of the container.

Annotations

• ❌ AnnotationsKeyValueMap: Annotations MUST be a key-value map.
• ❌ AnnotationsKeyRequired: Keys MUST NOT be an empty string.
• AnnotationsKeyReversedDomain: Keys SHOULD be named using a reverse domain notation - e.g. com.example.myKey.
• AnnotationsKeyReservedNS: Keys using the org.opencontainers namespace are reserved and MUST NOT be used by subsequent specifications.
• AnnotationsKeyIgnoreUnknown: Implementations that are reading/processing this configuration file MUST NOT generate an error if they encounter an unknown annotation key.
• ❌ AnnotationsValueString: Values MUST be strings.

Extensibility

• ExtensibilityIgnoreUnknownProp: Runtimes that are reading or processing this configuration file MUST NOT generate an error if they encounter an unknown property.

Valid values

• ValidValues: Runtimes that are reading or processing this configuration file MUST generate an error when invalid or unsupported values are encountered.

Runtime-Linux

• DefaultRuntimeLinuxSymlinks: While creating the container (step 2 in the lifecycle), runtimes MUST create default symlinks if the source file exists after processing mounts.

Runtime

• ❌ EntityOperSameContainer: The entity using a runtime to create a container MUST be able to use the operations defined in this specification against that same container.

State

• ❌ StateIDUniq: id (string, REQUIRED) is the container's ID. This MUST be unique across all containers on this host.
• ❌ StateNewStatus: Additional values MAY be defined by the runtime, however, they MUST be used to represent new runtime states not defined above.
• DefaultStateJSONPattern: When serialized in JSON, the format MUST adhere to the default pattern.

Lifecycle

• ❌ EnvCreateImplement: The container's runtime environment MUST be created according to the configuration in config.json.
• ❌ EnvCreateError: If the runtime is unable to create the environment specified in the config.json, it MUST generate an error.
• ConfigUpdatesWithoutAffect: Any updates to config.json after this step MUST NOT affect the container.
• PrestartHookFailGenError: If any prestart hook fails, the runtime MUST generate an error, stop the container, and continue the lifecycle at step 9.
• ❌ UndoCreateSteps: The container MUST be destroyed by undoing the steps performed during create phase (step 2).

Errors

• ❌ ErrorsLeaveStateUnchange: Unless otherwise stated, generating an error MUST leave the state of the environment as if the operation were never attempted - modulo any possible trivial ancillary changes such as logging.

Warnings

• ❌ WarnsLeaveFlowUnchange: Unless otherwise stated, logging a warning does not change the flow of the operation; it MUST continue as if the warning had not been logged.

Operations

• ❌ DefaultOperations: Unless otherwise stated, runtimes MUST support the default operations.

Create

• PropsApplyExceptProcOnCreate: All of the properties configured in config.json except for process MUST be applied.
• ProcArgsApplyUntilStart: process.args MUST NOT be applied until triggered by the start operation.
• PropApplyFailGenError: If the runtime cannot apply a property as specified in the configuration, it MUST generate an error.
• PropApplyFailNotCreate: If the runtime cannot apply a property as specified in the configuration, a new container MUST NOT be created.

Start

• StartWithoutIDGenError: start operation MUST generate an error if it is not provided the container ID.
• StartNonCreateHaveNoEffect: Attempting to start a container that is not created MUST have no effect on the container.
• StartNonCreateGenError: Attempting to start a container that is not created MUST generate an error.
• StartProcImplement: start operation MUST run the user-specified program as specified by process.
• StartWithProcUnsetGenError: start operation MUST generate an error if process was not set.

Kill

• KillWithoutIDGenError: kill operation MUST generate an error if it is not provided the container ID.
• KillNonCreateRunGenError: Attempting to send a signal to a container that is neither created nor running MUST generate an error.
• KillSignalImplement: kill operation MUST send the specified signal to the container process.
• KillNonCreateRunHaveNoEffect: Attempting to send a signal to a container that is neither created nor running MUST have no effect.

Delete

• DeleteWithoutIDGenError: delete operation MUST generate an error if it is not provided the container ID.
• DeleteNonStopHaveNoEffect: Attempting to delete a container that is not stopped MUST have no effect on the container.
• DeleteNonStopGenError: Attempting to delete a container that is not stopped MUST generate an error.
• DeleteResImplement: Deleting a container MUST delete the resources that were created during the create step.
• DeleteOnlyCreatedRes: Note that resources associated with the container, but not created by this container, MUST NOT be deleted.

[liangchenye]

good ! Seems we still need lots of work to do.

[liangchenye]

I think this is done

ConfigConstName: This REQUIRED file MUST be named config.json.

We have 'specConfig' in validate.go.
So these two 'MUST' are actually the same thing.

This REQUIRED file MUST reside in the root of the bundle directory and MUST be named config.json. 

[liangchenye]

About the 3 State items,
StateIDUniq --- Cannot validate it.
StateNewStatus --- Cannot validate it.
DefaultStateJSONPattern --- #575

[zhouhao3]

About Devices's items
DevicesMajMinRequired --- To wait for this pr land can be verified.
DevicesErrorOnDup --- #576

[liangchenye]

#576 is checked in. I'll set 'DevicesErrorOnDup' to 'done'.

[zhouhao3]

Set DefaultStateJSONPattern to done.

[zhouhao3]

We can add ❌ mark things that are unverifiable.

[liangchenye]

@q384566678 and I remove the '[ ]' to list valid tasks only.

[liangchenye]

'start' test is done here: #578

[zhouhao3]

Sine #577 merged. Set PosixHooksCalledInOrder to done.
Sine #578 merged. Set 'start' to done.

[zhouhao3]

I have ruled out the following errors that need not be verified:
UserNSMapOwnershipRO
CgroupsPathAbsOrRel
ProcRequiredAtStart
PosixProcRlimitsTypeGet
AnnotationsKeyValueMap
AnnotationsKeyRequired
AnnotationsValueString

I'll just mark them as ❌ , and if there's any inaccuracy, I can modify it.
What other needs to be added or modified can be directly updated here.

[liangchenye]

StartWithProcUnsetGenError is same with ProcRequiredAtStart, I added a comment to ProcRequiredAtStart.

[liangchenye]

'kill' test is mostly done here: #580
One missing spec error is mentioned in that PR.

----added on 2.27
580 is merged, set three tasks to done.

[liangchenye]

Mark 'BlkIOWeightOrLeafWeightExist' to DONE since #570 is merged.

[liangchenye]

RootOnWindowsRequired is checked in #584

[zhouhao3]

Set RootOnWindowsRequired to DONE sine #584 is merged.

[zhouhao3]

DefaultRuntimeLinuxSymlinks has implemented in runtimetest/main.go . Set it to DONE.

[liangchenye]

set PosixProcRlimitsSoftMatchCur and PosixProcRlimitsHardMatchMax to 'done' since #587 is merged

[zhouhao3]

LinuxProcCapError and PosixProcRlimitsTypeGenError is checked in #591

[liangchenye]

Mark LinuxProcCapError and PosixProcRlimitsTypeGenError to 'done'.

[zhouhao3]

set AnnotationsKeyIgnoreUnknown, AnnotationsKeyReversedDomain, AnnotationsKeyReservedNS, ValidValues and ExtensibilityIgnoreUnknownProp to done.

[zhouhao3]

I tried to write a test program for the intelRdt, but I got the following information, so I thought the contents related to the intelRdt were marked as ❌.

intelRdt is specified in config, but Intel RDT feature is not supported or enabled

[liangchenye]

@q384566678 I read the git history of 'how intelRdt being added to the runtime spec', it seems a quite new feature to the linux kernel. So I think most running servers nowadays will not support this, which means it is hard for us to verify it. I suggest we need help from the original committer @xiaochenshen.
What do you think about this? @xiaochenshen

PS,
We also need Windows/Solaris developers' help.

[xiaochenshen]

@q384566678 I read the git history of 'how intelRdt being added to the runtime spec', it seems a quite new feature to the linux kernel. So I think most running servers nowadays will not support this, which means it is hard for us to verify it. I suggest we need help from the original committer @xiaochenshen.
What do you think about this? @xiaochenshen

Current Intel RDT implementation in OCI/runc could handle these two cases: (1) hardware support Intel RDT, (2) hardware doesn't support Intel RDT.

For case (1), in my opinion, it is not difficult to find a hardware platform to verify.

Intel RDT features are supported from recent generations of Intel Xeon servers. You could find the matrix of Intel RDT features and supported hardware:
https://github.com/intel/intel-cmt-cat/blob/master/README#L108

[zhouhao3]

ProcArgsApplyUntilStart is checked in #602

[liangchenye]

mark ProcArgsApplyUntilStar done

[zhouhao3]

ConfigUpdatesWithoutAffect is checked in #604

[liangchenye]

DevicesMajMinRequired cannot be verified. The default '0' value is also valid. Mark it ❌.

[liangchenye]

Set ConfigUpdatesWithoutAffect to DONE #604

[zhouhao3]

Set NSNewNSWithoutPath and NSInheritWithoutType to DONE #620.

[liangchenye]

Set CgroupsPathAttach and CgroupsAbsPathRelToMount to DONE #631

[zhouhao3]

Because there are still some things that need to be discussed and implemented, it may take a long time, so mark it as v0.8.0 and add v0.8.0 plan to ROAMMAP. We can follow ROADMAP to do some version updates.

[zhouhao3]

Set KillNonCreateRunHaveNoEffect to DONE #607.

[zhouhao3]

Set DevicesApplyInOrder to DONE #633.

[zhouhao3]

Set NSProcInPath to DONE #628.

[zhouhao3]

set NSPathMatchTypeError to DONE #636.

[zhouhao3]

DeleteResImplement and DeleteOnlyCreatedRes haven't implement. @liangchenye @dongsupark @alban @wking I'd like to know what you think.

DeleteResImplement: Deleting a container MUST delete the resources that were created during the create step.

Does the resources here refer to the resources structure in the linux structure?
So if validation it, does it need to verify all the contents of the resources structure?

  DeleteOnlyCreatedRes: Note that resources associated with the container, but not created by this container, MUST NOT be deleted.

I don't understand this very well. I don't have any idea how to implement it.

[wking]

DeleteOnlyCreatedRes: Note that resources associated with the container, but not created by this container, MUST NOT be deleted.

I don't understand this very well.

For example, if the container joins an existing cgroup, the runtime should not attempt to delete that cgroup when it deletes the container.

On the scoping side, I think this is "resources" broadly, not just linux.resources. For example, when joining an existing mount namespace (which we don't support well anyway), the runtime should probably attempt to unmount anything it mounted, but not anything it didn't mount during create.

I don't have any idea how to implement it.

Yeah, it's going to be hard to cover this completely, because there are many resources that the runtime could be releasing. I'd just pick one (cgroups?) to start, and we can add others if/when someone requests them.

[dongsupark]

@q384566678
When I read these specerrors for the first time, I thought that they were described in a too broad and vague way. Though on the other hand, there must be probably good reasons for keeping the spec that way.
Reading discussions in the past, for example this, I think in theory resources need to cover not only cgroups but also other resources like namespaces.
Though it's true that runtime-tools validation tests cannot cover every possible cases.
I agree that we can just test only cgroups in the runtime-tools tests, as @wking said.

[zhouhao3]

For example, if the container joins an existing cgroup, the runtime should not attempt to delete that cgroup when it deletes the container.

@wking I know the container will create the cgroup during create process, but I don't know how to join an existing cgroup, can you explain in detail or give me some example? Thanks.

[zhouhao3]

Set DeleteResImplement and DeleteOnlyCreatedRes to DONE #654.