Skip to content
This repository has been archived by the owner on Feb 24, 2020. It is now read-only.

stage0/app-add: CLI args should override image ones #3566

Merged
merged 1 commit into from
Feb 1, 2017

Conversation

lucab
Copy link
Member

@lucab lucab commented Jan 27, 2017

This commit uniforms app args and exec command, so that in both cases
CLI parameters always override image ones.

@lucab
Copy link
Member Author

lucab commented Jan 27, 2017

This introduces a minor CLI breaking change for run. The rationale for this is:

  • we were previously not completely coherent (--exec already overrides additional flags)
  • this brings our UX in line with Docker one
  • This is allowed by an explicit MAY in appc spec

@lucab
Copy link
Member Author

lucab commented Jan 30, 2017

An alternative path here is to just change the app-add behavior for the moment, announce the behavioral change for run on the ML and switch it later (eg. in 2 release cycles).

@s-urbaniak
Copy link
Contributor

Since this is mostly needed for CRI/rktlet I am voting for announcing this breaking change for rkt run via ML and change app add behavior right now.

@s-urbaniak s-urbaniak added this to the v1.24.0 milestone Jan 30, 2017
@lucab lucab force-pushed the to-upstream/stage0-args-override branch from 59280ab to 9c7a66f Compare January 30, 2017 16:37
@lucab
Copy link
Member Author

lucab commented Jan 30, 2017

Ack. I dropped the run part out of here, so now this only changes app-add behavior. PTAL.

Copy link
Contributor

@s-urbaniak s-urbaniak left a comment

Choose a reason for hiding this comment

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

LGTM on green

@lucab lucab changed the title stage0: CLI args should override image ones stage0/app-add: CLI args should override image ones Jan 30, 2017
@squeed
Copy link
Contributor

squeed commented Jan 30, 2017

I'll take "stupid slice tricks for $200, Alex". Nice.

This commit uniforms app args and exec command, so that in both cases
CLI parameters always override image ones.
@lucab lucab force-pushed the to-upstream/stage0-args-override branch from 9c7a66f to aa4e4bf Compare January 31, 2017 09:18
@lucab
Copy link
Member Author

lucab commented Jan 31, 2017

(re-pushed as CI hooks didn't trigger)

@s-urbaniak
Copy link
Contributor

ok to test

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants