stage0: create app section if we override exec #1427
Conversation
@@ -152,7 +152,7 @@ func generatePodManifest(cfg PrepareConfig, dir string) ([]byte, error) { | |||
if pm.Apps.Get(*appName) != nil { | |||
return fmt.Errorf("error: multiple apps with name %s", am.Name) | |||
} | |||
if am.App == nil { | |||
if am.App == nil && app.Exec == "" { | |||
return fmt.Errorf("error: image %s has no app section", img) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can tell the user about this and add "... and exec argument not provided" to the error message
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, thanks!
199e5a2
to
5a3d776
Compare
LGTM |
Functional test? 😊
|
LGTM after a test |
if am.App == nil { | ||
return fmt.Errorf("error: image %s has no app section", img) | ||
if am.App == nil && app.Exec == "" { | ||
return fmt.Errorf("error: image %s has no app section and --exec argument not provided", img) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"and --exec argument is not provided"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not strictly necessary here. Actually, I'd prefer without it.
On Fri, Sep 18, 2015 at 6:38 AM, Krzesimir Nowak notifications@github.com
wrote:
In stage0/run.go
#1427 (comment):@@ -152,8 +153,8 @@ func generatePodManifest(cfg PrepareConfig, dir string) ([]byte, error) {
if pm.Apps.Get(*appName) != nil {
return fmt.Errorf("error: multiple apps with name %s", am.Name)
}
if am.App == nil {
return fmt.Errorf("error: image %s has no app section", img)
if am.App == nil && app.Exec == "" {
return fmt.Errorf("error: image %s has no app section and --exec argument not provided", img)
"and --exec argument is not provided"
—
Reply to this email directly or view it on GitHub
https://github.com/coreos/rkt/pull/1427/files#r39855005.
Small nit, otherwise LFAD. |
When an image doesn't have an App section and we override exec with the --exec flag we should create a minimal App section instead of failing. This would fail with a missing app section error: rkt run image-without-app-section.aci But this would succeed and exec the specified command if it exists in the image: rkt run image-without-app-section.aci --exec /bin/sh
06fa8a1
to
da78347
Compare
stage0: create app section if we override exec
yesssssss On Fri, Sep 18, 2015 at 2:06 AM, Iago López Galeiras <
|
@@ -42,6 +54,11 @@ func TestRunOverrideExec(t *testing.T) { | |||
rktCmd: fmt.Sprintf("%s --insecure-skip-verify run --mds-register=false %s --exec /inspect-link -- --print-exec", ctx.cmd(), execImage), | |||
expectedLine: "inspect execed as: /inspect-link", | |||
}, | |||
{ | |||
// Test overriding the entrypoint with a missing app section | |||
rktCmd: fmt.Sprintf("%s --insecure-skip-verify run --mds-register=false %s --exec /inspect -- --print-exec", ctx.cmd(), execImage), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where exactly are you telling this to use noAppImage? ;-)
@iaguis I don't think the test is quite doing what it's supposed to |
Oooops 😳 |
When an image doesn't have an App section and we override exec with the
--exec flag we should create a minimal App section instead of failing.
This would fail with a missing app section error:
But this would succeed and exec the specified command if it exists in
the image:
Fixes #1421