Review Livepatch K8s Charm

This is a review request for the Livepatch’s K8s charm. Please note the following:

  • The machine version of this charm is already listed, but this one is not.
  • The repository also includes a bundle, under the bundle subdirectory. The bundle is meant to be used for on-prem installations and provides both machine and K8s charms on via separate channels.

Summary

The Livepatch K8s charm is the easiest and the recommended way to deploy the Livepatch server on K8s. This charm configures and runs the Livepatch server, which serves Livepatch-es and metadata attached to them to the clients. Canonical Livepatch patches high and critical linux kernel vulnerabilities, removing the immediate need to reboot to upgrade the kernel, instead allowing the downtime to be scheduled. It is a part of the Ubuntu Pro offering.

Review PR

You can find the charm review PR at:
https://github.com/canonical/livepatch-k8s-operator/pull/22

(Due to branch protection rules some CI workflows might not run/pass for the above PR. In the summary below I’ve put the links to the latest runs.)

Metadata links

CI Links

Documentation Links

Thanks @babakks for the review request! @miaaltieri can you please help with this review? You can go through and tick off the items on the checklistand post the result in this thread. This prior listing request could serve as an example of how to go about it. Please ping @review-coordinators for any questions.

1 Like

Thanks, @tony-meyer. Any updates on this yet?

Sorry for the delay here, still getting used to doing this and it slipped off my radar :frowning:. Will follow-up ASAP.

Just checked with Mia and it’ll be done by the end of the week. Totally my fault that it’s been slow, not hers.

1 Like

Hi @babakks and @tony-meyer

Thank you for your patience and understanding here. I took a quick glance of the charm and although it looks nice there are a few things I noticed:

Most importantly, without documentation I do not know how to verify that the charm has its intended functionality. Would you mind adding some documentation. i.e. tutorial / how-to / overview so I can learn how to use the charm and test it :slight_smile:

Once these three points are addressed I will give a thorough review of all the necessary criteria.

If you’d like to have a short call about any of these points feel free to book me and I am more than happy to facilitate.

Thank you, Mia

1 Like

No worries, @tony-meyer. We understand you’re so busy supervising all charms across the company. :heart:

Thanks for the review, @miaaltieri.

Regarding the points please consider the following, and let me know what you think:

  • Some workflows are failing only on the review PR; because the repo has no branch protection rule for the charm-review-base branch. The latest actions (which have all the same code as the review PR branch) are:
    • Unit tests.
    • Integration tests.
    • Publish charm.
    • The [Release bundle] action is failing at the moment. That’s because, within the bundle.yaml, we set the series key to jammy but this release is not supported by the juju-bundle tool. I investigated this further, and noticed the underlying Rust library for Juju, has not been updated for while and the latest version it supports is Eoan (19.10). So, I have to submit a few PRs to fix the juju-bundle.
  1. We really don’t have diataxis-based documentation for Livepatch charms. Livepatch docs are based on diataxis but we have nothing like that for charm-specific docs. The thing is, normally, customers do not deploy Livepatch server on their own. Instead our field engineers do this for them, and they follow Livepatch docs for this purpose. So, do you still think we should update the docs for now? I can speak with the team and probably we’ll bring that as an epic for the next cycle.

  2. For linting, we use the tests action from the operator-workflows repo. It calls various tox commands and checks the output/exit-code. Here you can see the latest run that applied the linter. If you think it’s not sufficient to rely on this action, I can create a separate action to invoke the linter.

2 Likes

Hi @babakks ,

Thanks for the quick + thorough response :star_struck:

  1. Unit Tests Failing - ACK - sounds good. You’ve clearly demonstrated that the tests are working (minus the release one) - no worries on that front. Automated release is part of the review criteria, so just let me know when your PR lands

  2. Thanks for this information regarding docs. Given your use case I can understand why you don’t have diataxis based documentation. But I still think it would be nice to have a simple guide on:

  • how to deploy / configure it
  • what charms it can be related to
  • what a user would expect to see

That would be less work than implementing the entire diataxis framework.

The reason I request a small guide is that the charm is missing documentation for usage (requirement for the checklist + something I need in order to verify that the charm works, since I don’t know how to use it). It also might make fields job easier in the future?

But again, I can see why it might be overkill here / not applicable. If you believe documentation isn’t applicable in your use case, then maybe we can make a case for not including it in the check-list of criteria. If this is the case please let me know :slight_smile:

  1. Lint + Format - ACK - thank you for this. Your repo supports Lint + Format. This list item can be considered completed

Thanks again and Enjoy your weekend! Mia

1 Like

Thanks, @miaaltieri.

I’m currently working on the release bundle issue and will find a workaround for it. Either updating the underlying rust-libjuju (and then juju-bundle tool), or I’ll use an ordinary (i.e., non-bundle) charm publish procedure. I’ll inform you here.

Thanks for understanding our situation regarding the documentations. I totally agree with having a simple guide. What do you think about the repo’s README.md? I mean, Can I use that as a simple guide here?

@miaaltieri I just added the simple guide to the charm. You can see it here (currently on the edge channel). Please let me know if it misses anything.

Hi @babakks

Thank you for adding that on there :smiley:

It would be helpful to add some extra parts regarding what users should expect to see (i.e. relation data or in grafana) This will also be useful for me when I go to manually validate the charm as part of the checklist

Thank you! Mia

1 Like

@miaaltieri Thanks for the hint. Regarding the relations please note that Livepatch is basically a user of other pre-defined interfaces (both for requires endpoints, like database, or provides endpoints like ingress). I mean, we just comply with other charms’ requirements and implement what they need. So, nothing is actually dictated/defined by Livepatch.

But your point regarding the Grafana dashboard is correct. I just made the changes (both to the charm’s README.md and homepage) and you can now see the changes here, under the Grafana heading.

Also, we managed to fix the Release Bundle workflow, and I just released a new revision. You can see the action here.