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

Migrate to procfs 0.12.0 #69

Closed
bpetit opened this issue Feb 3, 2021 · 12 comments · Fixed by #144
Closed

Migrate to procfs 0.12.0 #69

bpetit opened this issue Feb 3, 2021 · 12 comments · Fixed by #144
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
General
Milestone
Release v0.5.0

Comments

@bpetit
Copy link
Contributor

bpetit commented Feb 3, 2021

Problem

Procfs 0.9.0 includes breaking changes compared to 0.8.1: https://github.com/eminence/procfs/releases/tag/v0.9.0

Solution

Adapt the powercap rapl sensor to be able to keep up with the procfs upstream.
@bpetit bpetit added the enhancement New feature or request label Feb 3, 2021
@bpetit bpetit added this to Triage in General Feb 8, 2021
@bpetit bpetit moved this from Triage to To do in General Mar 7, 2021
@bpetit bpetit added this to the Release v0.4.0 milestone Mar 12, 2021
@uggla
Copy link
Collaborator

uggla commented Apr 16, 2021

It would be wise to ask the author of procfs if he has still plan to change/disrupt the api. If he considers the api stable enough then we could go ahead.

@bpetit
Copy link
Contributor Author

bpetit commented Apr 17, 2021

You're right, we should start a discussion with them about that.

@uggla
Copy link
Collaborator

uggla commented May 11, 2021

eminence/procfs#69 (comment)

@eminence
Copy link

HI all, procfs maintainer here. I replied to @uggla in the issue over in my repo, but I figure I'd repeat it (and continue it) here.

The procfs API is not yet at 1.0, and so the API can still change in breaking ways.

The differences between 0.8 and 0.9 aren't that big, so I think you could remain on 0.8 if you wanted (there's actually only 1 bug fix between the two versions and I don't think you're impacted by it).

If you're concerned about the work involved, I would be happy to volunteer to do the upgrade for you, if you want.

Let me know what you think. Thanks!

@bpetit
Copy link
Contributor Author

bpetit commented May 18, 2021

Hi !

Thanks a lot for those informations. We will wait for the 1.0 then and ask you if we have issues with the upgrade :)

@bpetit bpetit changed the title Migrate to procfs 0.9.0 Migrate to procfs 1.0.0 once it's there May 18, 2021
@bpetit bpetit moved this from To do to Triage in General May 18, 2021
@bpetit bpetit removed this from the Release v0.4.0 milestone May 18, 2021
@bpetit bpetit added the help wanted Extra attention is needed label Nov 3, 2021
@rossf7 rossf7 mentioned this issue Dec 11, 2021
Closed
@rossf7
Copy link
Contributor

rossf7 commented Dec 11, 2021

Hey,
I ran into this while I was working on #143

The problem is the systemd cgroup driver for kubernetes uses : as a separator and the path gets truncated which means the container ID is missing. This was fixed in eminence/procfs#152

If I use a fork of 0.8.1 with just the fix it works great. Could we either upgrade or create a 0.8.2 patch release?

@eminence
Copy link

My general recommendation is to upgrade to the latest procfs, but if you must remain on the v0.8.x release, then I'm happy to create a 0.8.2 that has eminence/procfs#152 cherry-picked. Just let me know

@bpetit
Copy link
Contributor Author

bpetit commented Dec 14, 2021 •
edited

I'll have a look at that, as I'm working on some refactoring. I'll look into upgrading to latest procfs through that opportunity.

@bpetit bpetit moved this from Triage to In progress in General Dec 16, 2021
@bpetit bpetit added this to the Release v0.5.0 milestone Dec 16, 2021
@bpetit bpetit changed the title Migrate to procfs 1.0.0 once it's there Migrate to procfs 0.12.0 Dec 16, 2021
@bpetit bpetit linked a pull request Dec 16, 2021 that will close this issue
fix: upgrade to procfs 0.12 #144
Merged
@bpetit
Copy link
Contributor Author

bpetit commented Dec 16, 2021

Having a try with #144

Seems to work fine, feel free to run tests on your sides to.

@rossf7
Copy link
Contributor

rossf7 commented Dec 17, 2021

Thanks I tested my changes in #143 with your changes in #144 and they worked fine.

@bpetit
Copy link
Contributor Author

bpetit commented Dec 17, 2021 •
edited

I think I found a bug in #144, don't rely on it too much yet. I'll try to document and fix that this week end.

@bpetit
Copy link
Contributor Author

bpetit commented Dec 21, 2021

Bug is fixed. Merged this PR in the new dev branch. I'll work on #74, then merge in dev before merging everything to main.

@bpetit bpetit closed this as completed Jun 15, 2022
General automation moved this from In progress to Done Jun 15, 2022
@bpetit bpetit moved this from Done to Previous releases in General Jan 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
General
Previous releases
Milestone
Release v0.5.0
Development

Successfully merging a pull request may close this issue.

fix: upgrade to procfs 0.12 hubblo-org/scaphandre
4 participants
@rossf7 @eminence @bpetit @uggla