@mheon not sure if this is the proper way of doing this but it seems like a happy medium of getting resource functionality without completely ripping out the guts here.

also, I am running into persistent cross build errors due to undefined unix entities in runc, any way to avoid these?

I think I am going to need to make ..._linux files for basically all of the resource handlers. Otherwise, the unix specific entities will always be included, @mheon WDYT?

leaving this here as a note for myself: the reason this is broken right now is because our cgroup creation process needs to be modified more from the start rather than each time we update the cgroup. The specific resource files need to exist which currently do not.

@giuseppe PTAL

@giuseppe correct me if I am wrong but I think the issue right now is our Apply functions for each handler need a if cgroupv2 condition that uses the fs2 functions and cgroupv1 uses fs1

I've tried to vendor the current PR into podman but I get:

libpod/stats.go:60:41: cgroupStats.CPU undefined (type *"github.com/opencontainers/runc/libcontainer/cgroups".Stats has no field or method CPU)
libpod/stats.go:66:30: cgroupStats.CPU undefined (type *"github.com/opencontainers/runc/libcontainer/cgroups".Stats has no field or method CPU)
libpod/stats.go:71:30: cgroupStats.Memory undefined (type *"github.com/opencontainers/runc/libcontainer/cgroups".Stats has no field or method Memory)
libpod/stats.go:76:27: cgroupStats.Pids undefined (type *"github.com/opencontainers/runc/libcontainer/cgroups".Stats has no field or method Pids)
libpod/stats.go:79:29: cgroupStats.CPU undefined (type *"github.com/opencontainers/runc/libcontainer/cgroups".Stats has no field or method CPU)
libpod/stats.go:80:35: cgroupStats.CPU undefined (type *"github.com/opencontainers/runc/libcontainer/cgroups".Stats has no field or method CPU)
libpod/stats.go:124:33: undefined: "github.com/containers/common/pkg/cgroups".Metrics
libpod/stats.go:137:30: undefined: "github.com/containers/common/pkg/cgroups".Metrics
libpod/stats.go:80:35: too many errors
make: *** [Makefile:321: bin/podman] Error 2

I am afraid all the new dependencies (~21000 new lines of code) will increase significantly the podman binary size.

Could we pick just the pieces we need?

How difficult would be to implement the resource limits part in our library?

@kolyshkin FYI

@giuseppe it was about 4,000 lines without the inclusion of fs2, but I needed that for the creation. I think figuring out proper cgrouo creation (if we need it at all) is what is necessary here. The resource handlers should probably be imported directly from runc... Not sure though

Ok... I now have this working locally, I am going to try and pick parts out until we get to a reasonable mix of c/common code and runc code

@giuseppe I removed all references to fs2. This brings down the total import to 4000 lines. The cgroupv2 code is pretty much a copy of runc without some of their weird native functions.

Cgroupsv1 uses the runc functions directly just for simplicity. I though that bringing in the cgroupsv2 code directly made sense so we have direct control over it in the future.

/approve
LGTM
@giuseppe PTAL

so we are using libcontainer for cgroupv1 but our code for cgroupv2?

@kolyshkin PTAL

It would probably be best if we used libcontainer for both, if possible.

@rhatdan @giuseppe the V2 implementation is basically libcontainers code minus their usage if the manager interface. Importing their V2 code brings this PR to 20k lines

Upgrading to go 1.17 should not be an issue.

strange... why is this linting things I did not even edit?

I think the linter fires on any file that you change, and forces you to fix other issues.

@cdoern how about this (untested nor spellchecked):

diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml
index 5eae1b05..8988512b 100644
--- a/.github/workflows/validate.yml
+++ b/.github/workflows/validate.yml
@@ -41,11 +41,15 @@ jobs:
       uses: golangci/golangci-lint-action@537aa1903e5d359d0b27dbc19ddd22c5087f3fbc # v3
       with:
         version: "${{ env.LINT_VERSION }}"
+        only-new-issues: true
     # Extra linters, only checking new code from a pull request.
+    - name: Extra golangci-lint config. switcharoo
+      run: mv .gilangci-extra.yml golangci.yml
     - name: lint-extra
-      if: github.event_name == 'pull_request'
-      run: |
-        golangci-lint run --config .golangci-extra.yml --new-from-rev=HEAD~1 --out-format=github-actions
+      uses: golangci/golangci-lint-action@537aa1903e5d359d0b27dbc19ddd22c5087f3fbc # v3
+      with:
+        version: "${{ env.LINT_VERSION }}"
+        only-new-issues: true
 
     - name: validate seccomp
       run: ./tools/validate_seccomp.sh ./pkg/seccomp

@giuseppe @rhatdan PTAL

LGTM

@cevich @giuseppe @vrothberg PTAL

vendored into podman: ls -ln bin/podman -rwxrwxr-x. 1 1000 1000 45730352 Jun 8 09:46 bin/podman

head of upstream/main podman: ls -ln bin/podman -rwxrwxr-x. 1 1000 1000 45325592 Jun 8 09:47 bin/podman

very similar binary sizes.

vendored into podman: ls -ln bin/podman -rwxrwxr-x. 1 1000 1000 45730352 Jun 8 09:46 bin/podman

does this happen with disable_cgroup_devices?

@giuseppe I think @kolyshkin 's most recent version of the PR removes the build tag and separates the packages in such a way that you do not import the devices code unless you excitability use it. so yes, this is without the devices,

The github-action changes LGTM

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cdoern, giuseppe, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

/lgtm