-
Notifications
You must be signed in to change notification settings - Fork 543
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
[kubernetes] plugin improvements #3630
Conversation
Congratulations! One of the builds has completed. 🍾 You can install the built RPMs by following these steps:
Please note that the RPMs should be used only in a testing environment. |
b9ead85
to
5fc66b6
Compare
5fc66b6
to
53413af
Compare
I'll have a look at testing this for before and after, as we don't want to lose anything, especially from our perspective from a few scenarios such as In the meantime can you squash your commits based on our contribution guidelines |
@arif-ali I can't find any mention of squashing in https://github.com/sosreport/sos/wiki/Contribution-Guidelines |
you're right, it's not there, we ought to as that is what we ask all our contributors to do below is the common theme on this in our repo
In the mean time, I will have a look at updating the guidelines |
I'll squash, but to be clear there are no fixup commits, I'm just implementing the changes in small increments, making review easier IMO |
53413af
to
2667a63
Compare
@arif-ali commits are now squashed |
961c5c4
to
1b9b7ca
Compare
@arif-ali any bandwidth to do the tests you wanted to do ? |
I was due to do them today, but other things came in the way. As soon as I've done them will update here |
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.
one change required, and a comment maybe others will chime in on that
a483af9
to
c4cc392
Compare
@pmoravec any other concerns ? |
@TurboTurtle when you get a chance please review |
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.
Looks good to me, pending the redundant journal collection.
I think we should be good to remove the OCP bits from this plugin entirely, as I do not believe RH supports any of those versions anymore, but I'm good with leaving that for another PR later on.
- enable when k8s services are running - enable when kubelet package is present - add admin.conf as default kubeconfig - ensure we are not enabled if Openshift is - use 'oc' when present: this is relevant only for old OpenShift installs move limitranges / resourcequotas to the base plugin - collect journal only for existing services services - scrub collected files Signed-off-by: Etienne Champetier <e.champetier@ateme.com>
c4cc392
to
79bc7f4
Compare
I'll do a follow up PR to remove the OCP bits, but would love to have this PR in the upcoming release, so I can deploy it sooner rather than later |
Is next release expected to be in August ? |
Next upstream release is in August, yes, which will be Red Hat used to have a faster release cadence, then didn't, but then picked it up again with |
I do plan a new release/tag soon. There is some internal blocker that makes that tag useless now for Red Hat. Once it is resolved, I will create the 4.7.2 tag. I assume these "z-stream" tagging is important just for Red Hat. If somebody else is affected by it, esp. by the (non-planned) non-deterministic schedule, then let me know. I can adjust some stuff to have the schedule more solid / set in stone. |
Rework kubernetes plugin to make it collect in more cases
This PR can be split in multiple ones if prefered
Resolves #3628
Supersedes #3626
Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines