Skip to content

LCOW: pass command arguments without extra quoting#33815

Merged
johnstep merged 1 commit into
moby:masterfrom
jstarks:lcow_command_args
Jun 27, 2017
Merged

LCOW: pass command arguments without extra quoting#33815
johnstep merged 1 commit into
moby:masterfrom
jstarks:lcow_command_args

Conversation

@jstarks

@jstarks jstarks commented Jun 24, 2017

Copy link
Copy Markdown
Contributor

Right now, LCOW processes do not work with arguments that need to be quoted. E.g. this does not work:

docker run busybox sh -c "echo hello world"

because it ends up double-quoting "echo hello world", so sh tries to find a binary with that name.

This change fixes this by removing the extra quoting for LCOW processes and passing the argument list to hcsshim as a JSON array instead of a single string, which is the more natural interface for Linux processes.

Signed-off-by: John Starks [email protected]

@jstarks

jstarks commented Jun 24, 2017

Copy link
Copy Markdown
Contributor Author

@jhowardmsft PTAL

johnstep
johnstep reviewed Jun 24, 2017
View reviewed changes

@johnstep johnstep left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I guess CommandLine and CommandArgs are mutually exclusive? Does HCS just know which one to use, depending on the platform?

johnstep
johnstep approved these changes Jun 24, 2017
View reviewed changes

@johnstep johnstep left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

lowenna
lowenna approved these changes Jun 25, 2017
View reviewed changes

@lowenna lowenna left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

@lowenna

lowenna commented Jun 25, 2017

Copy link
Copy Markdown
Member

Restarted janky

@thaJeztah

thaJeztah commented Jun 26, 2017

Copy link
Copy Markdown
Member

docker-py failing, but I've seen it fail on other PR's; definitely something flaky in there;

23:25:35 =================================== FAILURES ===================================
23:25:35 _________________ ServiceTest.test_create_service_with_secret __________________
23:25:35 /docker-py/tests/integration/api_service_test.py:429: in test_create_service_with_secret
23:25:35     container = self.get_service_container(name)
23:25:35 /docker-py/tests/integration/api_service_test.py:41: in get_service_container
23:25:35     all=include_stopped
23:25:35 /docker-py/docker/api/container.py:189: in containers
23:25:35     res = self._result(self._get(u, params=params), True)
23:25:35 /docker-py/docker/utils/decorators.py:46: in inner
23:25:35     return f(self, *args, **kwargs)
23:25:35 /docker-py/docker/api/client.py:183: in _get
23:25:35     return self.get(url, **self._set_request_timeout(kwargs))
23:25:35 /usr/lib/python2.7/dist-packages/requests/sessions.py:469: in get
23:25:35     return self.request('GET', url, **kwargs)
23:25:35 /usr/lib/python2.7/dist-packages/requests/sessions.py:457: in request
23:25:35     resp = self.send(prep, **send_kwargs)
23:25:35 /usr/lib/python2.7/dist-packages/requests/sessions.py:569: in send
23:25:35     r = adapter.send(request, **kwargs)
23:25:35 /usr/lib/python2.7/dist-packages/requests/adapters.py:407: in send
23:25:35     raise ConnectionError(err, request=request)
23:25:35 E   ConnectionError: ('Connection aborted.', BadStatusLine("''",))

@lowenna

lowenna commented Jun 26, 2017

Copy link
Copy Markdown
Member

@shin- ^^ Is there something up with docker-py again? Thanks.

@johnstep
johnstep merged commit bcc8b2b into moby:master Jun 27, 2017
@thaJeztah thaJeztah added the area/lcow Issues and PR's related to the experimental LCOW feature label Oct 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@johnstep johnstep johnstep approved these changes
@lowenna lowenna lowenna approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/lcow Issues and PR's related to the experimental LCOW feature status/4-merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jstarks @lowenna @thaJeztah @johnstep @GordonTheTurtle