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.

Sign up for GitHub

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

Jump to bottom

[kubernetes] plugin improvements #3630

Merged
merged 1 commit into from May 15, 2024
Merged

[kubernetes] plugin improvements #3630

merged 1 commit into from May 15, 2024

Conversation

champtar
Copy link
Contributor

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

  • Is the commit message split over multiple lines and hard-wrapped at 72 characters?
  • Is the subject and message clear and concise?
  • Does the subject start with [plugin_name] if submitting a plugin patch or a [section_name] if part of the core sosreport code?
  • Does the commit contain a Signed-off-by: First Lastname email@example.com?
  • Are any related Issues or existing PRs properly referenced via a Closes (Issue) or Resolved (PR) line?
  • Are all passwords or private data gathered by this PR obfuscated?

@packit-as-a-service Packit-as-a-Service
Copy link

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo yum install -y dnf-plugins-core on RHEL 8
  • sudo dnf install -y dnf-plugins-core on Fedora
  • dnf copr enable packit/sosreport-sos-3630
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

@champtar champtar force-pushed the kubernetes-kubeadm branch 2 times, most recently from b9ead85 to 5fc66b6 Compare April 29, 2024 21:39
@champtar champtar marked this pull request as ready for review April 30, 2024 15:10
@champtar champtar mentioned this pull request Apr 30, 2024
Closed
6 tasks
@arif-ali
Copy link
Member

arif-ali commented May 2, 2024

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 microk8s and sunbeam

In the meantime can you squash your commits based on our contribution guidelines

@champtar
Copy link
Contributor Author

champtar commented May 2, 2024

@arif-ali I can't find any mention of squashing in https://github.com/sosreport/sos/wiki/Contribution-Guidelines

@arif-ali
Copy link
Member

arif-ali commented May 2, 2024

@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

we don't allow fixup commits when merging (keeps the git history cleaner and much easier to read).

In the mean time, I will have a look at updating the guidelines

@champtar
Copy link
Contributor Author

champtar commented May 2, 2024

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

@champtar
Copy link
Contributor Author

champtar commented May 3, 2024

@arif-ali commits are now squashed

@champtar champtar force-pushed the kubernetes-kubeadm branch 2 times, most recently from 961c5c4 to 1b9b7ca Compare May 3, 2024 19:13
@champtar
Copy link
Contributor Author

champtar commented May 8, 2024

@arif-ali any bandwidth to do the tests you wanted to do ?

@arif-ali
Copy link
Member

arif-ali commented May 8, 2024

@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

arif-ali
arif-ali requested changes May 9, 2024
View reviewed changes
Copy link
Member

@arif-ali arif-ali left a 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

sos/report/plugins/kubernetes.py Outdated Show resolved Hide resolved
sos/report/plugins/kubernetes.py Outdated Show resolved Hide resolved
@champtar champtar force-pushed the kubernetes-kubeadm branch 3 times, most recently from a483af9 to c4cc392 Compare May 9, 2024 13:40
@champtar
Copy link
Contributor Author

champtar commented May 9, 2024

@pmoravec any other concerns ?

@champtar champtar requested a review from pmoravec May 9, 2024 17:20
@champtar champtar changed the title Kubernetes plugin improvements [kubernetes] plugin improvements May 11, 2024
@champtar
Copy link
Contributor Author

@TurboTurtle when you get a chance please review

TurboTurtle
TurboTurtle reviewed May 15, 2024
View reviewed changes
Copy link
Member

@TurboTurtle TurboTurtle left a 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.

sos/report/plugins/kubernetes.py Outdated Show resolved Hide resolved
@champtar
[kubernetes] plugin improvements
79bc7f4 
- 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>
@champtar
Copy link
Contributor Author

champtar commented May 15, 2024 •
edited

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

@TurboTurtle TurboTurtle merged commit 38dc631 into sosreport:main May 15, 2024
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arif-ali arif-ali arif-ali approved these changes

@TurboTurtle TurboTurtle TurboTurtle approved these changes

@pmoravec pmoravec pmoravec approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[RFE] collect kubernetes informations for kubeadm installs
4 participants
@champtar @arif-ali @TurboTurtle @pmoravec