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

kpod create should not do an OCI Init #96

Closed
wants to merge 1 commit into from

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Dec 3, 2017

We need to differentiate between a kpod create and a kpod start
kpod create should create all of the data for libpod, but kpod start should
generate content for OCI Runtime (runc) in order to run.

Signed-off-by: Daniel J Walsh dwalsh@redhat.com

@rhatdan
Copy link
Member Author

rhatdan commented Dec 3, 2017

My only concern with this change is how it will effect kpod inspect. We need to make sure we record the information used in kpod create so inspect can be able to retrieve the data at a later time.

@umohnani8 @mheon PTAL

@rhatdan rhatdan force-pushed the create branch 2 times, most recently from 4e1d1ad to e7beabb Compare December 3, 2017 14:02
@@ -416,6 +416,10 @@ func (c *Container) Init() (err error) {
return err
}

if c.state.State == ContainerStateCreated {
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove this and instead ignore ErrCtrExists when doing the Init() in start.go?

We need to differentiate between a kpod create and a kpod start
kpod create should create all of the data for libpod, but kpod start should
generate content for OCI Runtime (runc) in order to run.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
@rhatdan
Copy link
Member Author

rhatdan commented Dec 4, 2017

@TomSweeneyRedHat
Copy link
Member

LGTM

1 similar comment
@umohnani8
Copy link
Member

LGTM

@mheon
Copy link
Member

mheon commented Dec 4, 2017

LGTM
@rh-atomic-bot r+

@rh-atomic-bot
Copy link
Collaborator

📌 Commit ddc525e has been approved by mheon

@rh-atomic-bot
Copy link
Collaborator

⌛ Testing commit ddc525e with merge b7ba665...

rh-atomic-bot pushed a commit that referenced this pull request Dec 4, 2017
We need to differentiate between a kpod create and a kpod start
kpod create should create all of the data for libpod, but kpod start should
generate content for OCI Runtime (runc) in order to run.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

Closes: #96
Approved by: mheon
@rh-atomic-bot
Copy link
Collaborator

💔 Test failed - status-papr

@mheon
Copy link
Member

mheon commented Dec 4, 2017

Looks like a flake

@mheon
Copy link
Member

mheon commented Dec 4, 2017

@rh-atomic-bot retry

@rh-atomic-bot
Copy link
Collaborator

⌛ Testing commit ddc525e with merge 95cb7a1...

@rh-atomic-bot
Copy link
Collaborator

☀️ Test successful - status-papr
Approved by: mheon
Pushing 95cb7a1 to master...

wking added a commit to wking/libpod that referenced this pull request May 11, 2018
containernetworking/plugins@a0eac8d7 (pkg/ns: remove namespace
creation, 2018-03-16) removed NewNS, which we use in
libpod/networking.go.  Pinning to the previous commit,
containernetworking/plugins@1fb94a42 (Merge pull request containers#96 from
DennisDenuto/denuto/master, 2018-03-14), allows us to run vndr without
breaking our build.  This is a short term fix; moving forward we'll
want to either drop this dependency or catch up with the new upstream
API.

The upstream package seems to have been fairly stable in the meantime,
because even with the new pinned version, a vndr re-vendor generates
no changes:

  $ vndr github.com/containernetworking/plugins

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/libpod that referenced this pull request May 11, 2018
containernetworking/plugins@a0eac8d7 (pkg/ns: remove namespace
creation, 2018-03-16) removed NewNS, which we use in
libpod/networking.go.  Pinning to the previous commit,
containernetworking/plugins@1fb94a42 (Merge pull request containers#96 from
DennisDenuto/denuto/master, 2018-03-14), allows us to run vndr without
breaking our build.  This is a short term fix; moving forward we'll
want to either drop this dependency or catch up with the new upstream
API.

The upstream package seems to have been fairly stable in the meantime,
because even with the new pinned version, a vndr re-vendor generates
no changes:

  $ vndr github.com/containernetworking/plugins

Signed-off-by: W. Trevor King <wking@tremily.us>
rh-atomic-bot pushed a commit that referenced this pull request May 11, 2018
containernetworking/plugins@a0eac8d7 (pkg/ns: remove namespace
creation, 2018-03-16) removed NewNS, which we use in
libpod/networking.go.  Pinning to the previous commit,
containernetworking/plugins@1fb94a42 (Merge pull request #96 from
DennisDenuto/denuto/master, 2018-03-14), allows us to run vndr without
breaking our build.  This is a short term fix; moving forward we'll
want to either drop this dependency or catch up with the new upstream
API.

The upstream package seems to have been fairly stable in the meantime,
because even with the new pinned version, a vndr re-vendor generates
no changes:

  $ vndr github.com/containernetworking/plugins

Signed-off-by: W. Trevor King <wking@tremily.us>

Closes: #751
Approved by: mheon
baude pushed a commit to baude/podman that referenced this pull request Aug 31, 2019
plugins/meta/bandwidth: traffic shaping plugin
@rhatdan rhatdan deleted the create branch December 1, 2022 22:02
@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 19, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
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.

None yet

5 participants