New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add generate systemd -e/--env option #15584
Add generate systemd -e/--env option #15584
Conversation
b4b83c4
to
c85df72
Compare
b2b277a
to
cafa12d
Compare
a8d8e75
to
3335ccd
Compare
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.
LGTM
@containers/podman-maintainers PTAL
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.
Please also add some new unit tests here:
func TestCreateContainerSystemdUnit(t *testing.T) { |
5944f6d
to
03bb57d
Compare
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.
LGTM
@Luap99 PTAL
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.
Sorry one thing I missed yesterday was that you have to escape the arguments if required, see
podman/pkg/systemd/generate/containers.go
Line 487 in 43baf49
info.ExtraEnvs = append(info.ExtraEnvs, escapeSystemdArg(containerEnv)) |
and
Environment=FOO=abc "BAR=my test" USER=%%a |
When the env var contains a space or a special character such as % the systemd unit would likely fail right now.
fd978ac
to
a9a411f
Compare
-e/--env option sets environment variables to the systemd unit files. Fixes: containers#15523 Signed-off-by: Toshiki Sonoda <sonoda.toshiki@fujitsu.com>
@Luap99 PTAL again. |
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.
LGTM, thanks
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99, sstosh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
-e/--env option sets environment variables to the systemd unit files.
Fixes: #15523
Signed-off-by: Toshiki Sonoda sonoda.toshiki@fujitsu.com
Does this PR introduce a user-facing change?