Review Livepatch K8s Charm

charmhub requests review requests
1

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

2

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
3

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

4

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
5

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
6

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
7

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
8

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?

9

@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.

10

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
11

@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.

12

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

13

@miaaltieri Any update on this?

14

Hi @babakks - I apologise for the delay. I am looking at it now. Thanks for your patience

15

Hi @babakks - the charm is looking great! Here is an update on my progress: Other than that - this is what I have for the checklist. Once I verify those two points above I will approve

Intended functionality - :construction: - WIP / Blocked

charmhub page - :white_check_mark:

Source repository - :white_check_mark:

Coding conventions - :white_check_mark:

Release automation implementation - :white_check_mark:

Unit tests implementation - :white_check_mark:

Unit tests results - :white_check_mark:

Installation test implemented (could be part of the integration test) - :white_check_mark:

Installation test results - :white_check_mark:

Integration tests implemented - :white_check_mark:

Integration test results - :white_check_mark:

Documentation for usage - :white_check_mark:

Documentation for contributing - :white_check_mark:

Licensing statement - :white_check_mark:

I am having a hard time verifying the functionality. Perhaps you can help me and we can work together or if you have a demo, you could send it to me?

Specifically I tried following the tutorial and after doing:

juju add-model test
juju deploy canonical-livepatch-server-k8s
juju deploy postgresql

The model resulted in

Model  Controller  Cloud/Region        Version  SLA          Timestamp
test   overlord    microk8s/localhost  3.1.6    unsupported  09:07:25Z

App                             Version  Status   Scale  Charm                           Channel    Rev  Address         Exposed  Message
canonical-livepatch-server-k8s           waiting      1  canonical-livepatch-server-k8s  stable      26  10.152.183.55   no       installing agent
postgresql                               waiting      1  postgresql                      14/stable  429  10.152.183.104  no       installing agent

Unit                               Workload  Agent  Address      Ports  Message
canonical-livepatch-server-k8s/0*  blocked   idle   10.1.37.117         ✘ patch-sync token not set, run get-resource-token action
postgresql/0*                      error     idle   10.1.37.126         hook failed: "leader-elected"

I could be wrong but I think juju deploy postgresql - deploys the VM charm.

Should I: A - Deploy the postgresql-k8s charm B - Deploy postgresql on a lxd cloud and integrate it with livepatch-k8s-server as a cross model relation

In either case it would be good to update the guide to reflect this :slight_smile:

I am also thinking it might quicker/easier for us both to do the deployment part together in a 1:1 meet . Up to you. I am happy to work on it on my own and message you with the issues I have along the way :slight_smile:

1 Like
16

@miaaltieri Thanks for doing this. Yeah, you should use postgresql-k8s charm. And yes, we can pair up on this to make it quicker.

17

Just an update for those following along. After some pairing today, it appears that the charm/docs need some additional fixes in order to verify its intended functionality.

Babak will look into this and we will resync later

1 Like
18

Thank you for all the help and collaboration @babakks - the charm looks ready to go @tony-meyer :slight_smile:

2 Likes
19

Thanks @miaaltieri ! @odysseus-k This one should be good for listing!

20

Hi,

canonical-livepatch-server-k8s should now be listed.

Thanks,

Deep

1 Like