-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Support sighup reload configuration files #7311
Support sighup reload configuration files #7311
Conversation
This also needs to reload the Libpod state to account for potential changed
storage/database paths. We also potentially loaded a fresh database, so we
need to re-do the Libpod initialization process (checking if the alive file
is created and refreshing the state + creating it if it is not present).
…On Wed, Aug 12, 2020 at 7:12 PM OpenShift CI Robot ***@***.***> wrote:
[APPROVALNOTIFIER] This PR is *NOT APPROVED*
This pull-request has been approved by: *QiWang19
<#7311#>*
To complete the pull request process
<https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process>,
please assign *rhatdan*
You can assign the PR to them by writing /assign @rhatdan in a comment
when ready.
The full list of commands accepted by this bot can be found here
<https://go.k8s.io/bot-commands?repo=containers%2Fpodman>.
Needs approval from an approver in each of these files:
- *OWNERS <https://github.com/containers/podman/blob/master/OWNERS>*
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#7311 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB3AOCB4ILHJHDRX2DIQZP3SAMOW3ANCNFSM4P5NX4XQ>
.
|
Also, the configured and default OCI runtimes may have changed, so we need
to refresh those as well.
…On Wed, Aug 12, 2020 at 8:52 PM Matthew Heon ***@***.***> wrote:
This also needs to reload the Libpod state to account for potential
changed storage/database paths. We also potentially loaded a fresh
database, so we need to re-do the Libpod initialization process (checking
if the alive file is created and refreshing the state + creating it if it
is not present).
On Wed, Aug 12, 2020 at 7:12 PM OpenShift CI Robot <
***@***.***> wrote:
> [APPROVALNOTIFIER] This PR is *NOT APPROVED*
>
> This pull-request has been approved by: *QiWang19
> <#7311#>*
> To complete the pull request process
> <https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process>,
> please assign *rhatdan*
> You can assign the PR to them by writing /assign @rhatdan in a comment
> when ready.
>
> The full list of commands accepted by this bot can be found here
> <https://go.k8s.io/bot-commands?repo=containers%2Fpodman>.
> Needs approval from an approver in each of these files:
>
> - *OWNERS <https://github.com/containers/podman/blob/master/OWNERS>*
>
> Approvers can indicate their approval by writing /approve in a comment
> Approvers can cancel approval by writing /approve cancel in a comment
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <#7311 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AB3AOCB4ILHJHDRX2DIQZP3SAMOW3ANCNFSM4P5NX4XQ>
> .
>
|
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.
Nice work, @QiWang19!
Only minor nits
go func() { | ||
for { | ||
// Block until the signal is received | ||
<-ch |
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.
Could we add a debug message here as well? This could come in handy for support.
cmd/podman/utils/signals_linux.go
Outdated
|
||
// Platform specific signal synonyms | ||
var ( | ||
Hup os.Signal = unix.SIGHUP |
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.
Hup os.Signal = unix.SIGHUP | |
SIGHUP os.Signal = unix.SIGHUP |
Might be easier to understand when the identifier is consistent.
@@ -816,3 +818,48 @@ func (r *Runtime) mergeDBConfig(dbConfig *DBConfig) { | |||
func (r *Runtime) EnableLabeling() bool { | |||
return r.config.Containers.EnableLabeling | |||
} | |||
|
|||
func (r *Runtime) Reload() error { |
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 add a comment.
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 once @vrothberg 's concerns are addressed.
734676f
to
01d5695
Compare
Support podman service sighup reload configuration files(containers.conf, registries.conf, storage.conf). Signed-off-by: Qi Wang <qiwan@redhat.com>
01d5695
to
5b02b69
Compare
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: QiWang19, rhatdan 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 |
/hold cancel |
Woah woah woah. Were my comments here ignored? This does not work if anything related to the OCI runtime or storage paths changed. |
Can I have more details about what is specifically the LIbpod state and storage paths that need to update? |
We need to:
|
Does this just need to call makeruntime Line 241 in 80d2c01
|
Support podman service sighup reload configuration files(containers.conf, registries.conf, storage.conf).
Signed-off-by: Qi Wang qiwan@redhat.com