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.

[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.

[cyphar]

The nolintlint issue is quite annoying, I've hit it before as well. Adding nolintlint is the only way I found to work around it as well. The only real long-term solution is to go through and fix all of the lint errors in a mega-PR so we don't have these two separate runs of golangci-lint -- I started this with #4904 but plan to eventually do all of the lint failures once I have some more time.

[cyphar]

This has 3 reviews, so I'm happy to merge it.

@askervin Did you still want this in runc 1.4?

@kolyshkin @AkihiroSuda @lifubang Would you be happy for us to add this in rc2, or do you want to push this for 1.5 in April?

[kolyshkin]

I think having it in v1.4 is fine since it's a new functionality and the chance it will bring a regression is minimal.

[askervin]

@askervin Did you still want this in runc 1.4?

Sure, @cyphar! Glad to see the functionality already in v1.4.

Thanks for all the great comments, @AkihiroSuda, @lifubang, @cyphar and @kolyshkin!