-
Notifications
You must be signed in to change notification settings - Fork 1.4k
🌱 Set terminationMessagePolicy to FallbackToLogsOnError for all managers #10580
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
🌱 Set terminationMessagePolicy to FallbackToLogsOnError for all managers #10580
Conversation
chrischdi
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.
Can we also have this in the following files for consistency:
./test/extension/config/default/manager.yaml
./test/infrastructure/docker/config/manager/manager.yaml
./test/infrastructure/inmemory/config/manager/manager.yaml
No strong opinion if we want this. I think its good.
This ensures we have a useful termination message in the Pod if a manager exits unexpectedly.
7819436 to
08956ad
Compare
damdo
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 @mdbooth
/lgtm
|
LGTM label has been added. DetailsGit tree hash: 894c289a1922a9515d5239f3637098d79f2df53d |
|
/assign @chrischdi |
chrischdi
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.
/lgtm
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enxebre The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Probably would have been nice to document it in v1.7-to-v1.8.md |
I wasn't actually aware of this very useful documentation. It seems targeted at provider implementers rather than end-users, though. Are you sure this is the right fit? I don't think this change requires providers to do anything. Might be worthy of an end-user release note. I'm happy to submit a follow-up if you let me know where. Perhaps something in the implementers guide where we suggest this configuration as good practise? Although I'm not sure what would prompt existing implementations to read that. A release note feels like the best solution in practise. |
|
I was thinking of adding it there because in the past we added notes there about things that could be useful for providers to pick up. It's absolutely fine to add optional things to this doc. Apart from this doc I'm not aware of any other mechanism to write some sort of "release note" for providers (in fact this doc is basically the release notes for providers for thing they can do or have to do) If we find an additional good place in the implementers guide it definitely wouldn't hurt there. But I'm not sure if there is a good place (I think the implementers guide is very far from complete today) I'm also not aware of a mechanism to write end-user release notes apart from PR titles (@kubernetes-sigs/cluster-api-release-team) |
This ensures we have a useful termination message in the Pod if a manager exits unexpectedly. Check: kubernetes-sigs/cluster-api#10580 Signed-off-by: Kashif Khan <[email protected]>
This ensures we have a useful termination message in the Pod if a manager exits unexpectedly. Check: kubernetes-sigs/cluster-api#10580 Signed-off-by: Kashif Khan <[email protected]>
This ensures we have a useful termination message in the Pod if a manager exits unexpectedly. Check: kubernetes-sigs/cluster-api#10580 Signed-off-by: Kashif Khan <[email protected]>
This ensures we have a useful termination message in the Pod if a manager exits unexpectedly.
/area logging