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.
Maybe we can tell the user about this and add "... and exec argument not provided" to the error message
iaguis
force-pushed
the
exec-creates-app-section
branch
from
199e5a2
to
5a3d776
Compare
|
LGTM |
|
Functional test? 😊
|
|
LGTM after a test |
|
I'll hang this on the wall so I never forget again 😅
|
| 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.
"and --exec argument is not provided"
There was a problem hiding this comment.
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
iaguis
force-pushed
the
exec-creates-app-section
branch
from
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.
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