Add memory policy support

[askervin]

Implement support for Linux memory policy in OCI spec PR: opencontainers/runtime-spec#1282

[askervin]

@AkihiroSuda, @giuseppe, do you think it would be good to keep this PR as Draft until opencontainers/runtime-spec#1282 is merged? Or should I make it Ready for review (despite the "replace" in go.mod) in order to get reviews and possibly get this tagged into v1.3?

[AkihiroSuda]

Please keep this draft until the runtime spec PR gets merged

[askervin]

@AkihiroSuda, as the memory policy field is now in place in the runtime-spec (opencontainers/runtime-spec#1282 merged), I updated this PR to use the latest runtime-spec and removed the Draft status. Rebased and fixed lint issues. Good for review now. :)

[h-vetinari]

Careful with the milestones. According to the new release policy, features should by merged by rc1, which is due by the end of the month.

[askervin]

@kolyshkin, sorry to bother you folks with this, but I'd have a question on staticcheck lint in the CI. It runs twice, but with different set of checks. That causes an issue: one cannot disable staticcheck for a line in the case that the line passes one check but not the other. Disabling a check that does not cause an error is itself an error, which makes sense.

In order to workaround this issue, I added commit 1b32673 in this PR, so that we can see all checks pass. But I don't know what is the best way forward. Should I remove the commit before this PR gets merged?

[lifubang]

Should I remove the commit before this PR gets merged?

Yes, please remove this commit. The CI is fully passing in #4900.

[askervin]

Should I remove the commit before this PR gets merged?

Yes, please remove this commit. The CI is fully passing in #4900.

Removed.

[kolyshkin]

@askervin somewhat related to this PR; can you please review https://go-review.googlesource.com/c/sys/+/706917/2?

[lifubang]

Should I remove the commit before this PR gets merged?

Yes, please remove this commit. The CI is fully passing in #4900.

Removed.

Could you please help fix the lint errors?

Error: libcontainer/configs/memorypolicy.go:7:2: ST1003: should not use ALL_CAPS in Go names; use CamelCase instead (staticcheck)
	MPOL_DEFAULT = iota
	^
Error: libcontainer/configs/memorypolicy.go:8:2: ST1003: should not use ALL_CAPS in Go names; use CamelCase instead (staticcheck)
	MPOL_PREFERRED
	^
Error: libcontainer/configs/memorypolicy.go:9:2: ST1003: should not use ALL_CAPS in Go names; use CamelCase instead (staticcheck)
	MPOL_BIND
	^

[askervin]

Should I remove the commit before this PR gets merged?

Yes, please remove this commit. The CI is fully passing in #4900.

Removed.

Could you please help fix the lint errors?

Error: libcontainer/configs/memorypolicy.go:7:2: ST1003: should not use ALL_CAPS in Go names; use CamelCase instead (staticcheck)
	MPOL_DEFAULT = iota
	^
Error: libcontainer/configs/memorypolicy.go:8:2: ST1003: should not use ALL_CAPS in Go names; use CamelCase instead (staticcheck)
	MPOL_PREFERRED
	^
Error: libcontainer/configs/memorypolicy.go:9:2: ST1003: should not use ALL_CAPS in Go names; use CamelCase instead (staticcheck)
	MPOL_BIND
	^

@lifubang, I fixed these errors with this commit: 1b32673

The root cause is that CI runs the staticcheck lint twice, first without and then with ST1003. The first run checks all the code, the latter check only changed parts. If I disable this in code (nolint), I'll get an error from the first run: you're disabling a check without a reason. If I don't disable it, I'll get the errors quoted above. Options that I see are:

1. modify both CI staticchecks always run without ST1003 (implemented in the commit).
2. modify both CI staticchecks always run with ST1003. This requires many changes in the code base to fix existing errors.
3. stop using ALL_CAPS in these constants. This would be against practices in this code base and in the go standard library (sys/unix) where all C flags are use ALL_CAPS without exception. So I wouldn't really prefer this option, as we are working around CI lint issues by breaking the code.
4. merge the commit even if CI lint gives these warnings and fix CI lint later.

Of course there may be more options that I just fail to see. What would you suggest?

[lifubang]

What would you suggest?

In fact, we always use CamelCase for const names in runc, but as you say, it will be in go standard library (sys/unix), so I suggest:
//nolint:revive // this will matches the unix.* name in the future

[askervin]

What would you suggest?

In fact, we always use CamelCase for const names in runc, but as you say, it will be in go standard library (sys/unix), so I suggest: //nolint:revive // this will matches the unix.* name in the future

@lifubang, //nolint:revive does not silence staticcheck error. And //nolint:revive,staticcheck does not work because nolintlint complains about disabling staticcheck without effect as it does not produce an error in the first pass (when ST1003 is disabled in .golangci.yml). However, it turned out that disabling all three works. That's why you will see //nolint:revive,staticcheck,nolintlint. Included your suggestion to the comment in disabling these lint modules. Thanks for pointing me back to this, happy to get it fixed without tweaking CI lint configs.

A side note: ALL_CAPS C header constants in the runc code base do exist, like in libcontainer/configs/{mount.go,namespaces_linux.go}. And if we'd like to enable ST1003 in the first pass, we would also need to fix ALL_CAPS internal constants in utils_linux.go:

	CT_ACT_CREATE CtAct = iota + 1
	CT_ACT_RUN
	CT_ACT_RESTORE

However, fixing (or nolinting) ALL_CAPS cases would not suffice, as ST1003 wants few other naming changes, too. Like "id"s capitalized (like ttyUid -> ttyUID), but that's another story.