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

Implement Podman Container Clone #13059

Merged
merged 1 commit into from
Feb 22, 2022
Merged

Conversation

cdoern
Copy link
Contributor

@cdoern cdoern commented Jan 28, 2022

podman container clone takes the id of an existing continer and creates a specgen from the given container's config
recreating all proper namespaces and overriding spec options like resource limits and the container name if given in the cli options

this command utilizes the common function DefineCreateFlags meaning that we can funnel as many create options as we want
into clone over time allowing the user to clone with as much or as little of the original config as they want.

the current supported flags are:

--id (ctr to clone)
--destroy (remove the original container)
--name (new ctr name)
--cpus (sets cpu period and quota)
--cpuset-cpus
--cpu-period
--cpu-rt-period
--cpu-rt-runtime
--cpu-shares
--cpuset-mems
--memory

Signed-off-by: cdoern [email protected]

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 28, 2022
@cdoern cdoern force-pushed the clone branch 2 times, most recently from 0766f36 to 792e011 Compare January 28, 2022 03:54
pkg/specgen/specgen.go Outdated Show resolved Hide resolved
@cdoern cdoern force-pushed the clone branch 2 times, most recently from 7bc7c2a to 73ed0cc Compare February 1, 2022 19:43
@rhatdan rhatdan added the 4.1 label Feb 1, 2022
@cdoern cdoern force-pushed the clone branch 2 times, most recently from b53d4ef to 42e5df6 Compare February 2, 2022 05:04
@cdoern cdoern marked this pull request as ready for review February 2, 2022 20:05
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 2, 2022
@cdoern
Copy link
Contributor Author

cdoern commented Feb 2, 2022

@containers/podman-maintainers PTAL, this works and is set up in such a way that more options can basically be dropped in over time.

FillOutSpecGen needed to be retrofitted so the spec fields are only updated if they contain their default values OR if the container create options contain something other than their default values this is because Clone needs to create the spec from the old container's config before filling out new options.

libpod/container_config.go Outdated Show resolved Hide resolved
libpod/container_config.go Outdated Show resolved Hide resolved
@cdoern
Copy link
Contributor Author

cdoern commented Feb 11, 2022

@containers/podman-maintainers PTAL, I think I am done making major changes to this before v1, let me know if there are any glaring issues.

ctrClone.CreateOpts.Name = args[1]
case 3:
ctrClone.CreateOpts.Name = args[1]
ctrClone.Image = args[2]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this properly handle image environment variables, healthchecks, etc? I think it should merge the ones from the old container with the ones from the new container, but the exact details could be difficult?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well this is only if the user specified a new image to use in the clone, would the env variables just get picked up from the other image by default? I am not really familiar with how that works

@cdoern cdoern force-pushed the clone branch 4 times, most recently from d641020 to 2a9303c Compare February 17, 2022 02:50
@rhatdan
Copy link
Member

rhatdan commented Feb 17, 2022

Fixes: #9540

@rhatdan
Copy link
Member

rhatdan commented Feb 17, 2022

@mheon need another pass?

test/e2e/container_clone_test.go Show resolved Hide resolved
pkg/specgenutil/createparse.go Outdated Show resolved Hide resolved
@cdoern cdoern force-pushed the clone branch 2 times, most recently from 04c0c3e to c4c2981 Compare February 18, 2022 21:25
podman container clone takes the id of an existing continer and creates a specgen from the given container's config
recreating all proper namespaces and overriding spec options like resource limits and the container name if given in the cli options

this command utilizes the common function DefineCreateFlags meaning that we can funnel as many create options as we want
into clone over time allowing the user to clone with as much or as little of the original config as they want.

container clone takes a second argument which is a new name and a third argument which is an image name to use instead of the original container's

the current supported flags are:

--destroy (remove the original container)
--name (new ctr name)
--cpus (sets cpu period and quota)
--cpuset-cpus
--cpu-period
--cpu-rt-period
--cpu-rt-runtime
--cpu-shares
--cpuset-mems
--memory
--run

resolves containers#10875

Signed-off-by: cdoern <[email protected]>
Signed-off-by: cdoern <[email protected]>
Signed-off-by: cdoern <[email protected]>
@rhatdan
Copy link
Member

rhatdan commented Feb 21, 2022

@mheon @Luap99 PTAL

@mheon
Copy link
Member

mheon commented Feb 22, 2022

LGTM

Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Luap99
Copy link
Member

Luap99 commented Feb 22, 2022

/approve
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 22, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 22, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cdoern, Luap99

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 22, 2022
@openshift-merge-robot openshift-merge-robot merged commit fab82a7 into containers:main Feb 22, 2022
@github-actions 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 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants