Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parse: allow parsing anomaly non-human value for memory control group #3679

Merged

Conversation

flouthoc
Copy link
Collaborator

Memory control group accepts an anomaly value of -1 for setting swap to unlimited.
Buildah must accepts this value if specified.

Reference: https://github.com/opencontainers/runtime-spec/blob/main/config-linux.md#memory

Closes: #3675

@flouthoc
Copy link
Collaborator Author

flouthoc commented Dec 15, 2021

We use docker helpers to parse human readable value for swap which is ok for memory but swap also accepts exception of -1 which is not honored by the same helper. So PR handles that.

@flouthoc
Copy link
Collaborator Author

flouthoc commented Dec 15, 2021

I think buildah static build is facing the same issue as: containers/crun#812

@flouthoc
Copy link
Collaborator Author

I think buildah static build is facing the same issue: containers/crun#812

@cevich @edsantiago I suspect this a nixos/nix:latest regression any suggestions if we should do that same for buildah or not. Let me know if there is any better approach.

@edsantiago
Copy link
Collaborator

I'm totally fine with disabling nix, it seems to be a constant source of problems. I think this is how @cevich disabled it in podman:

diff --git a/.cirrus.yml b/.cirrus.yml
index 37d102fa..fa064bfe 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -201,6 +201,8 @@ static_build_task:
     name: "Static Build"
     alias: static_build
     only_if: *not_docs
+    # 2021-12-15 nix is dead broken and unmaintained
+    allow_failures: $CI == $CI
     depends_on:
       - unit

@rhatdan
Copy link
Member

rhatdan commented Dec 15, 2021

LGTM

@cevich
Copy link
Member

cevich commented Dec 15, 2021

I think this is how @cevich disabled it in podman

That will simply ignore the results. I suggest just nuke the whole task (and remove it from success also).

@TomSweeneyRedHat
Copy link
Member

Changes LGTM
@hswong3i any thoughts on the nix issue(s)?

@hswong3i
Copy link
Collaborator

@saschagrunert PTAL

@saschagrunert
Copy link
Member

I'm not blocking on removing the static builds.

flouthoc added a commit to flouthoc/buildah that referenced this pull request Dec 16, 2021
Remove `static_build` from ci as it is becoming hard to keep up with
`nix` hiccups in CI.

------
Before Deciding to remove static_build from CI we tried things below :
Things decided below worked but in order to keep maintaince simple
consensus was made to remove static_build. Read more here:
containers#3679

It seems nixos/nix:latest expects nixbld user to be added and specified in
build-users-group but this should be only needed for multi-user
mode.

Hence I suspect latest push has a regression. Lock the nix to last working
image.

[NO NEW TESTS NEEDED]

Signed-off-by: Aditya Rajan <arajan@redhat.com>
Memory control group accepts an anomaly value of `-1` for setting `swap` to unlimited.
Buildah must accepts this value if specified.

Reference: https://github.com/opencontainers/runtime-spec/blob/main/config-linux.md#memory

Signed-off-by: Aditya Rajan <arajan@redhat.com>
@flouthoc
Copy link
Collaborator Author

Rebased after: #3680

@rhatdan
Copy link
Member

rhatdan commented Dec 16, 2021

/approve
/lgtm
/hold

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 16, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc, rhatdan

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@flouthoc
Copy link
Collaborator Author

/hold cancel

@openshift-merge-robot openshift-merge-robot merged commit f05a4dc into containers:main Dec 16, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid value for memory-swap: invalid size: '-1'
8 participants