Skip to content
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 script debugging options #156

Merged
merged 1 commit into from
Sep 15, 2017
Merged

Add script debugging options #156

merged 1 commit into from
Sep 15, 2017

Conversation

zultron
Copy link
Contributor

@zultron zultron commented Sep 1, 2017

Add two environment variables that may be defined with docker run -e
to aid debugging:

  • Define $DEBUG_TRACE to turn on init-data and
    ipa-server-configure-first script tracing

  • Define $DEBUG_NO_EXIT to disable container exit after failures

@adelton
Copy link
Collaborator

adelton commented Sep 7, 2017

The $DEBUG_TRACE part LGTM.

As for the $DEBUG_NO_EXIT, would it be possible to do it without touching /usr? Eventually we want to be able to run the container read-only, so /etc might be better here. I also wonder if addressing it in systemd-poweroff.service.d/ might be more appropriate than in the individual user services -- you could start something like sleep infinity there.

Add two environment variables that may be defined with `docker run -e`
to aid debugging:

- Define `$DEBUG_TRACE` to turn on `init-data` and
  `ipa-server-configure-first` script tracing

- Define `$DEBUG_NO_EXIT` to disable container exit after failures
@zultron
Copy link
Contributor Author

zultron commented Sep 8, 2017

@adelton, you're right, I should have used systemd unit drop-ins from the beginning. Take a look at the revised PR.

I ended up sticking with adjusting the FailureAction in ipa-server-*.service. With ExecStartPre=sleep infinity in the systemd-poweroff.service unit, other units get killed, so it's harder to reproduce the failure environment.

Thanks for the review.

@adelton
Copy link
Collaborator

adelton commented Sep 11, 2017

Untested but looks good to me.

@felipevolpone
Copy link
Member

@zultron thanks for the patch. I'm kind busy right now, but I'll find some time to test it this week and then, merge it.

@felipevolpone
Copy link
Member

Tested and it works properly. LGTM.
Do you see any restriction here @stlaz ?

@stlaz
Copy link
Contributor

stlaz commented Sep 15, 2017

Tested both the env. variables and they really work great. Thanks!

@stlaz stlaz merged commit e48e76f into freeipa:master Sep 15, 2017
@adelton
Copy link
Collaborator

adelton commented Jul 17, 2018

Note, in #219 I now propose to put the overrides to /run/systemd, to avoid polluting /data.

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

Successfully merging this pull request may close these issues.

4 participants