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.

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

Fix Livepatch status reporting bug #215

Merged

Conversation

srunde3
Copy link
Contributor

@srunde3 srunde3 commented Feb 15, 2024

Fix issue in which pro status inaccurately displays Livepatch status as "disabled" even if it is active. Bug is due to the landscape user not being able to run snaps properly since it has a $HOME directory outside of /home (it's in /var/lib/).

See https://bugs.launchpad.net/landscape-client/+bug/2037670

Manual Testing:

  • Spin up a VM with a Livepatch-supported kernel
  • Modify UbuntuProInfo.run_interval to be something quick like 10
  • Modify landscape-client.conf to point to a Landscape Server instance. Additionally include exchange_interval = 10` to exchange messages with server more quickly
  • sudo scripts/landscape-monitor --config landscape-client.conf
  • Toggle livepatch and ensure updates are reflected in Server sudo pro <enable/disable> livepatch

@srunde3 srunde3 added the bug label Feb 15, 2024
@silverdrake11
Copy link
Contributor

According to the this comment in get_data()

New messages will only be sent
    when the result of get_data() has changed since the last time it
    was called.

However I believe it only works for monitor DataWatcher. If it works this would cut down traffic from once every 15 minutes to once a day or less. I think it's worth caching it, but I don't think it's currently working. See _persist in landscape/client/monitor/plugin.py for how this works. I'm not sure if the best way to do it is to make a new class, make existing one work, or just define a function in ubuntupro

@silverdrake11
Copy link
Contributor

Why was net-tools added as a dependency?

@Perfect5th
Copy link
Contributor

Why was net-tools added as a dependency?

It's required for some of the unit tests to pass. It's a build dep in debian/control. I'm not sure why it's needed in DEB_REQUIRED though.

@srunde3
Copy link
Contributor Author

srunde3 commented Feb 16, 2024

That's my mistake then -- it should go in debian/control instead of DEB_REQUIRED. I added it because it was required for tests to pass

@srunde3
Copy link
Contributor Author

srunde3 commented Feb 16, 2024

Moved net-tools dependency into Makefile instead of setup_lib::DEB_REQUIRED

Copy link
Contributor

@Perfect5th Perfect5th left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, may want to wait for @silverdrake11 to re-review, though.

@silverdrake11
Copy link
Contributor

LGTM except for the name data that you are persisting to. It would be better to have it be more specific name since this is only used for ubuntu pro manager and not other kinds of data

@srunde3
Copy link
Contributor Author

srunde3 commented Feb 20, 2024

Discussed offline with @silverdrake11 -- fine to use data since persist should be unique to UbuntuProInfo based on file path.

@srunde3 srunde3 merged commit 3af82ee into canonical:master Feb 21, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants