Add support for containers.conf #4698

rhatdan · 2019-12-13T19:02:02Z

vendor in c/common config pkg for containers.conf

Signed-off-by: Qi Wang [email protected]
Signed-off-by: Daniel J Walsh [email protected]

openshift-ci-robot · 2019-12-13T19:02:06Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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:

~~OWNERS~~ [rhatdan]

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

rhatdan · 2019-12-13T19:02:14Z

Replaces #4569

rh-atomic-bot · 2019-12-13T20:20:47Z

☔ The latest upstream changes (presumably #4699) made this pull request unmergeable. Please resolve the merge conflicts.

rh-atomic-bot · 2019-12-19T18:21:23Z

☔ The latest upstream changes (presumably #4730) made this pull request unmergeable. Please resolve the merge conflicts.

rh-atomic-bot · 2020-01-08T12:17:17Z

☔ The latest upstream changes (presumably #4592) made this pull request unmergeable. Please resolve the merge conflicts.

rh-atomic-bot · 2020-01-08T15:59:44Z

☔ The latest upstream changes (presumably #4816) made this pull request unmergeable. Please resolve the merge conflicts.

mheon · 2020-03-19T16:39:10Z

Please don't merge today, before I cut 1.8.2 final. Will do a review this afternoon.

rhatdan · 2020-03-19T17:34:03Z

/hold
@mheon is in charge of cancelling, unless I get sent back to the drawing board

mheon · 2020-03-19T19:55:26Z

/hold cancel

Full review delayed until tomorrow

rhatdan · 2020-03-21T13:32:22Z

@mheon @vrothberg @giuseppe @TomSweeneyRedHat @QiWang19 @baude Could some give this a review, so we can finally get it merged...

vrothberg

Just a couple of nits. It's very hard to review such a gigantic PR but the green tests (in addition with the new tests) give me a good feeling.

What worries me a bit is that the CLI and API v2 are diverging with all the new defaults being set in cmd/podman/.... I didn't catch up on all recent developments and I am certain that's already being worked on but it would be great to consolidate container creation (and let the backend do the validation in contrast to cmd/podman/...).

Other than that LGTM.

vrothberg · 2020-03-23T09:28:04Z

cmd/podman/shared/create.go

+		// Check for . and dns-search domains
+		if util.StringInSlice(".", dnsSearches) {
+			if len(dnsSearches) > 1 {
+				return nil, errors.Errorf("cannot pass additional search domains when also specifying '.'")


We could move the check down to if dom == "." in the following loop. Would avoid iterating twice.

vrothberg · 2020-03-23T09:31:25Z

docs/source/markdown/podman-run.1.md

@@ -867,6 +869,8 @@ Set the user namespace mode for the container.  It defaults to the **PODMAN_USER
 - **host**: run in the user namespace of the caller. This is the default if no user namespace options are set. The processes running in the container will have the same privileges on the host as any other process launched by the calling user.
 - **keep-id**: creates a user namespace where the current rootless user's UID:GID are mapped to the same values in the container. This option is ignored for containers created by the root user.
 - **ns**: run the container in the given existing user namespace.
+- **private**: create a new namespace for the container (default)
+- **private**:


This line can be deleted.

vrothberg · 2020-03-23T09:36:10Z

pkg/util/utils.go

@@ -609,3 +609,46 @@ func Tmpdir() string {

 	return tmpdir
 }
+
+// ValidateSysctls validates a list of sysctl and returns it.
+func ValidateSysctls(strSlice []string) (map[string]string, error) {


Looks like a candidate for a c/common/pkg/sysctl.

WIll open a different PR for this, but I would prefer to get this merged and fixed later.

Completely agree 👍

containers/common#97

QiWang19 · 2020-03-23T14:52:16Z

libpod/config/config.go

-	}
-
-	// If we need to, switch to cgroupfs and logger=file on rootless.
-	config.checkCgroupsAndLogger()


should use func (c *Config) CheckCgroupsAndAdjustConfig() directly in podman. containers/common#80 removed it from NewConfig

Good Catch,
Fixed

rhatdan · 2020-03-23T21:20:22Z

@mheon @vrothberg @QiWang19 @TomSweeneyRedHat @baude @jwhonce PTAL
Fixed up issues.

umohnani8 · 2020-03-23T21:46:07Z

LGTM

rhatdan · 2020-03-24T17:19:21Z

@mheon @vrothberg @QiWang19 @TomSweeneyRedHat @baude @jwhonce @giuseppe PTAL
Could we get this in...

vendor in c/common config pkg for containers.conf Signed-off-by: Qi Wang [email protected] Signed-off-by: Daniel J Walsh <[email protected]>

baude · 2020-03-27T19:51:39Z

/lgtm

vrothberg · 2020-03-30T12:25:05Z

@baude @jwhonce @mheon @rhatdan ... while working on pull I found that the v2 binary is now failing for all invocations with Error: eventer creation: No support for journald logging. Looks like this PR introduced it.

I will look into it but we really need some sort of CI for podmanV2.

vrothberg · 2020-03-30T12:36:12Z

K. It turns out we just need to specify the systemd build tag.

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 13, 2019

openshift-ci-robot requested review from baude and mrunalp December 13, 2019 19:02

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 13, 2019

openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Dec 13, 2019

rhatdan force-pushed the containers.conf branch from 7b844a1 to 2e60cc0 Compare December 13, 2019 19:34

rhatdan mentioned this pull request Dec 13, 2019

WIP: Added support for log policy and log rate limit in conmon #4663

Closed

rhatdan force-pushed the containers.conf branch from 2e60cc0 to bd88eda Compare December 13, 2019 19:50

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 13, 2019

rhatdan force-pushed the containers.conf branch from bd88eda to 2abe2d1 Compare December 16, 2019 16:52

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 16, 2019

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 19, 2019

rhatdan force-pushed the containers.conf branch from 2abe2d1 to da0ff59 Compare December 19, 2019 19:11

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 19, 2019

rhatdan force-pushed the containers.conf branch 3 times, most recently from 242b1aa to 8384542 Compare January 6, 2020 22:34

vrothberg mentioned this pull request Jan 8, 2020

Error: could not get runtime: operation not permitted with stable podman image #3464

Closed

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 8, 2020

rhatdan force-pushed the containers.conf branch from 8384542 to c537c5e Compare January 8, 2020 12:42

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 8, 2020

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 8, 2020

rhatdan force-pushed the containers.conf branch from c537c5e to 9edb1b4 Compare January 15, 2020 01:05

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 15, 2020

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 19, 2020

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 19, 2020

vrothberg reviewed Mar 23, 2020

View reviewed changes

rhatdan force-pushed the containers.conf branch from 6887554 to 1e5c6cd Compare March 23, 2020 13:13

QiWang19 reviewed Mar 23, 2020

View reviewed changes

rhatdan force-pushed the containers.conf branch 2 times, most recently from ccc88dd to c634565 Compare March 23, 2020 18:28

rhatdan force-pushed the containers.conf branch 3 times, most recently from 0ec7c52 to c1caffb Compare March 27, 2020 17:00

Add support for containers.conf

4352d58

vendor in c/common config pkg for containers.conf Signed-off-by: Qi Wang [email protected] Signed-off-by: Daniel J Walsh <[email protected]>

rhatdan force-pushed the containers.conf branch from c1caffb to 4352d58 Compare March 27, 2020 18:36

openshift-ci-robot assigned baude Mar 27, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 27, 2020

openshift-merge-robot merged commit 3336b10 into containers:master Mar 27, 2020

This was referenced Mar 27, 2020

podmanv2 checkpoint and restore #5638

Merged

userns: support --userns=auto #5478

Merged

edsantiago mentioned this pull request Aug 31, 2022

Cirrus: use image with fewer downloaded dependencies containers/skopeo#1739

Closed

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 25, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for containers.conf #4698

Add support for containers.conf #4698

rhatdan commented Dec 13, 2019

openshift-ci-robot commented Dec 13, 2019

rhatdan commented Dec 13, 2019

rh-atomic-bot commented Dec 13, 2019

rh-atomic-bot commented Dec 19, 2019

rh-atomic-bot commented Jan 8, 2020

rh-atomic-bot commented Jan 8, 2020

mheon commented Mar 19, 2020

rhatdan commented Mar 19, 2020

mheon commented Mar 19, 2020

rhatdan commented Mar 21, 2020

vrothberg left a comment

vrothberg Mar 23, 2020

rhatdan Mar 23, 2020

vrothberg Mar 23, 2020

rhatdan Mar 23, 2020

vrothberg Mar 23, 2020

rhatdan Mar 23, 2020

vrothberg Mar 23, 2020

rhatdan Mar 23, 2020

QiWang19 Mar 23, 2020

rhatdan Mar 23, 2020

rhatdan commented Mar 23, 2020

umohnani8 commented Mar 23, 2020

rhatdan commented Mar 24, 2020

baude commented Mar 27, 2020

vrothberg commented Mar 30, 2020 •

edited

Loading

vrothberg commented Mar 30, 2020

Add support for containers.conf #4698

Add support for containers.conf #4698

Conversation

rhatdan commented Dec 13, 2019

openshift-ci-robot commented Dec 13, 2019

rhatdan commented Dec 13, 2019

rh-atomic-bot commented Dec 13, 2019

rh-atomic-bot commented Dec 19, 2019

rh-atomic-bot commented Jan 8, 2020

rh-atomic-bot commented Jan 8, 2020

mheon commented Mar 19, 2020

rhatdan commented Mar 19, 2020

mheon commented Mar 19, 2020

rhatdan commented Mar 21, 2020

vrothberg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rhatdan commented Mar 23, 2020

umohnani8 commented Mar 23, 2020

rhatdan commented Mar 24, 2020

baude commented Mar 27, 2020

vrothberg commented Mar 30, 2020 • edited Loading

vrothberg commented Mar 30, 2020

vrothberg commented Mar 30, 2020 •

edited

Loading