-
Notifications
You must be signed in to change notification settings - Fork 714
add global options to healthcheck command #4575
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
Conversation
61bf0a1 to
df9ff8c
Compare
AkihiroSuda
left 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.
df9ff8c to
27318a4
Compare
654a56b to
e1b158d
Compare
AkihiroSuda
left 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.
Thanks
| // 1. It was explicitly changed via CLI (highest priority), OR | ||
| // 2. It has a non-default value (from TOML config) | ||
| // This ensures both CLI flags and TOML config values are propagated | ||
| if f.Changed || (val != f.DefValue && val != "") { |
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.
This condition i think should not be there, the function seems to return all global flags, irrespective of changed or not changed. Not sure if this condition is required.
@AkihiroSuda Do you happen to know why we have the condition for compose.
Signed-off-by: Arjun Raja Yogidas <arjunry@amazon.com>
Signed-off-by: Arjun Raja Yogidas <arjunry@amazon.com>
Signed-off-by: Arjun Raja Yogidas <arjunry@amazon.com>
Signed-off-by: Arjun Raja Yogidas <arjunry@amazon.com>
e1b158d to
89fc035
Compare
|
Rebased and added logic to pass through any config file env variables to the systemd commands. This should enable clients like finch to utilize the nerdctl healthchecks. |
|
Line 13 in 2165e30
Should we |
Currently the global flags passed to nerdclt is not passed to the healthcheck command, this results in container not found errors while running healthchecks in a namespaced environement
This PR passes the global options to the healthcheck command as well