Skip to content

ctr: enable specifying additional environment variables#902

Merged
crosbymichael merged 1 commit intocontainerd:masterfrom
ehazlett:ctr-env
May 24, 2017
Merged

ctr: enable specifying additional environment variables#902
crosbymichael merged 1 commit intocontainerd:masterfrom
ehazlett:ctr-env

Conversation

@ehazlett
Copy link
Copy Markdown
Member

@ehazlett ehazlett commented May 24, 2017

This adds the ability to specify environment variables for testing with ctr.

@codecov-io
Copy link
Copy Markdown

codecov-io commented May 24, 2017 •
edited
Loading

Codecov Report

Merging #902 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #902   +/-   ##
=======================================
  Coverage   59.55%   59.55%           
=======================================
  Files           5        5           
  Lines         759      759           
=======================================
  Hits          452      452           
  Misses        199      199           
  Partials      108      108

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 17033dc...02fa534. Read the comment docs.

@crosbymichael
Copy link
Copy Markdown
Member

crosbymichael commented May 24, 2017

LGTM

mlaventure
mlaventure reviewed May 24, 2017
View reviewed changes
Comment thread cmd/ctr/run_unix.go Outdated
Copy link
Copy Markdown
Contributor

@mlaventure mlaventure May 24, 2017

Choose a reason for hiding this comment

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

What about windows? 👼

Although on windows I think the array actually need to only have the same variable defined once.

Copy link
Copy Markdown
Member Author

@ehazlett ehazlett May 24, 2017

Choose a reason for hiding this comment

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

ah, always forgetting windows :( i'll get it updated!

@ehazlett
Copy link
Copy Markdown
Member Author

ehazlett commented May 24, 2017 •
edited
Loading

I copied the func ReplaceOrAppendEnvValues
as windows needs this (https://github.com/moby/moby/blob/c22ad736aa2eb2b796cd9a662017e66b1cf970e0/container/container_windows.go#L37) thx @mlaventure

mlaventure
mlaventure approved these changes May 24, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@mlaventure mlaventure left a comment

Choose a reason for hiding this comment

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

LGTM

@crosbymichael crosbymichael merged commit 5157a32 into containerd:master May 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mlaventure mlaventure mlaventure approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ehazlett @codecov-io @crosbymichael @mlaventure