RFC: Offline Updates

[cajun-rat]

I'd like to extend Aktualizr to support offline updates. I understand that the Uptane standard is likely to be extended
soon to support this.

Looking at the Aktualizr code, I think there are some significant refactors needed to implement this. The purpose of this ticket is to open up discussion on what will be necessary and to make an initial proposal for those changes.

These changes are independent of the updated Uptane metadata evolution rules that will also be need to implemented (although probably after this refactor).

This refactor would be in the following areas:

• Initializer
• CommandQueue
• SotaUptaneClient
• the main Aktualizr class

Initializer

At the moment SotaUptaneClient doesn't really 'start' until the Initializer class has completed, which requires online communication with the server. I propose that we refactor the logic from the Initializer ctor into an 'void AttemptInitialize()' member function that makes one attempt at initialization and provides status (OK / Perm Failure / Temporary Failure) and an error description string via member functions.

For backwards compatibility this would be called on the entry to SotaUptaneClient::fetchMeta() (and the other external entry points to SotaUptaneClient.

Command Queue

For safety, I think it is important to make a very clear distinction between member functions that should be called via the command queue and those which can be called on any thread. One solution here would be to make SotaUptaneClient explicitly not thread-safe in all cases, and make the Aktualizr class provide thread safety on all calls (serializing via the Command Queue if necessary).

In think there is an opportunity here to simplify Aktualizr::RunForever() here: instead of calling UptaneCycle() in a loop (in a separate thread!) we can have a smarter CommandQueue that automatically schedules Update cycles when it has nothing in the queue. This can also have logic to attempt both online and offline updates.

SotaUptaneClient

I'm thinking that a good way to handle the complexity in this class is to add comments that link the member functions to parts of the Uptane spec. There are already a good number of references throughout the codebase, but I wonder if we can make more member function calls map 1:1 to steps in the Uptane spec.

We'd then add a new set of member functions that map to the upcoming Uptane Offline updates PURE, and finally wire it into the top level Aktualizr class.

Does that make sense?

[pattivacek]

Hey! This stuff is all pretty interesting. It's not clear to me, though, how all these things fit together.

These changes are independent of the updated Uptane metadata evolution rules that will also be need to implemented (although probably after this refactor).

Is there a draft text or proposal somewhere around for this? I'm not aware of how that might work, so it's hard for me to judge the appropriateness of these changes relative to enabling support for that. I like the idea, and this isn't the first time offline updates in libaktualizr have come up, but I'm not quite sure why an offline update couldn't be provided with a new API call and some new internal functionality.

I propose that we refactor the logic from the Initializer ctor into an 'void AttemptInitialize()' member function that makes one attempt at initialization and provides status (OK / Perm Failure / Temporary Failure) and an error description string via member functions.

Why? Is this necessary if you want to have an offline update before any online updates have taken place?

For backwards compatibility this would be called on the entry to SotaUptaneClient::fetchMeta() (and the other external entry points to SotaUptaneClient.

I don't think it's necessary to retry to initialize on every external call into SotaUptaneClient. fetchMeta should be enough. I wonder if this is something that should be configurable somehow, since some users might not want offline updates at all.

For safety, I think it is important to make a very clear distinction between member functions that should be called via the command queue and those which can be called on any thread.

I don't think I understand why the CommandQueue concept is necessary. What is that bringing?

One solution here would be to make SotaUptaneClient explicitly not thread-safe in all cases, and make the Aktualizr class provide thread safety on all calls

libaktualizr is pretty messy with thread safety. However, I don't think it was expected that there'd be any reason to want to do multiple things at the same time. I still don't really see why you would. Is the problem you envision that you want to use RunForever but also be able to insert offline updates at will? If so, why not just do the equivalent of RunForever in the wrapper code? Seems like a lot of internal complexity for something that could be handled smarter outside the library. Or am I missing some other gains here?

I'm thinking that a good way to handle the complexity in this class is to add comments that link the member functions to parts of the Uptane spec. There are already a good number of references throughout the codebase, but I wonder if we can make more member function calls map 1:1 to steps in the Uptane spec.

That was definitely a goal at one point in time. I'm very much in favor of doing a better job with some of this. Also beware that some of the numbering schemes might've diverged some over time.

[cajun-rat]

Hey! This stuff is all pretty interesting. It's not clear to me, though, how all these things fit together.

These changes are independent of the updated Uptane metadata evolution rules that will also be need to implemented (although probably after this refactor).

Is there a draft text or proposal somewhere around for this? I'm not aware of how that might work, so it's hard for me to judge the appropriateness of these changes relative to enabling support for that. I like the idea, and this isn't the first time offline updates in libaktualizr have come up, but I'm not quite sure why an offline update couldn't be provided with a new API call and some new internal functionality.

I think @tkfu is working on the proposed spec change, but the rough approach is to have a set of metadata similar to the Director, but which applies to any matching ECU rather than a specific ECU. I understand the PURE be ready very soon, so I won't try a poor summary here. Overall I'd hope the change can be reasonably self-contained, modulo a bit of refactoring :)

I propose that we refactor the logic from the Initializer ctor into an 'void AttemptInitialize()' member function that makes one attempt at initialization and provides status (OK / Perm Failure / Temporary Failure) and an error description string via member functions.

Why? Is this necessary if you want to have an offline update before any online updates have taken place?

Yes. Supporting offline updates for a device that has never seen the Internet is a key use case (and drives quite a bit of the complexity).

For backwards compatibility this would be called on the entry to SotaUptaneClient::fetchMeta() (and the other external entry points to SotaUptaneClient.

I don't think it's necessary to retry to initialize on every external call into SotaUptaneClient. fetchMeta should be enough. I wonder if this is something that should be configurable somehow, since some users might not want offline updates at all.

That makes sense. The other public calls that depend on initialization can do a simple check and return a failure if initialization isn't complete yet. I haven't got through them all in detail to figure out which do/don't yet, but I think it will be pretty obvious.

For safety, I think it is important to make a very clear distinction between member functions that should be called via the command queue and those which can be called on any thread.

I don't think I understand why the CommandQueue concept is necessary. What is that bringing?

I'm referring to the existing CommandQueue in aktualizr/src/libaktualizr/utilities/apiqueue.h. Personally I don't find it a particularly clean bit of code, but I didn't want to chop the codebase around too much :) I think the intention is to support embedding libaktualizr into other code in a flexible way, and to allow a mix of background polling and explicit 'check now' and 'install now' calls to be made from 3rd party code.

The repeated code like this from aktualizr.cc is an unfortunate effect of CommandQueue

    std::future<result::Install> Aktualizr::Install(const std::vector<Uptane::Target> &updates) {
      std::function<result::Install()> task([this, updates] { return uptane_client_->uptaneInstall(updates); });
      return api_queue_->enqueue(task);
    }

One solution here would be to make SotaUptaneClient explicitly not thread-safe in all cases, and make the Aktualizr class provide thread safety on all calls

libaktualizr is pretty messy with thread safety. However, I don't think it was expected that there'd be any reason to want to do multiple things at the same time. I still don't really see why you would. Is the problem you envision that you want to use RunForever but also be able to insert offline updates at will? If so, why not just do the equivalent of RunForever in the wrapper code? Seems like a lot of internal complexity for something that could be handled smarter outside the library. Or am I missing some other gains here?

One thing that we have to be careful here is that the Aktualizr class already moves a bunch of calls to a background thread, so we are probably stuck with having to deal we multiple threads. I agree that we should try and avoid parallelism where possible, but what about the case of an updater that polls in the background but also has a 'Check For Updates Now' button. This would require:

1. A user interface that wants to show the current update state, which will needs an answer in ~ms
2. A background task inside Aktualizr that does the polling (so Aktualizr 'just works' in the simple case),
3. A way to trigger a check now (interlocked with the background task)

Did you have any thoughts on how that should work?

I'm thinking that a good way to handle the complexity in this class is to add comments that link the member functions to parts of the Uptane spec. There are already a good number of references throughout the codebase, but I wonder if we can make more member function calls map 1:1 to steps in the Uptane spec.

That was definitely a goal at one point in time. I'm very much in favor of doing a better job with some of this. Also beware that some of the numbering schemes might've diverged some over time.

Ack. While I've got the lid off I will try and drop those in.

[pattivacek]

Thanks for all the explanations. I got a brief overview of the offline update concept from Jon at the Uptane meeting, so I have a decent idea of what's going on here. I want to be really careful with opening up new doors, but I also think this is a feature that's long been needed.

Supporting offline updates for a device that has never seen the Internet is a key use case (and drives quite a bit of the complexity).

I see. As it is now, calling Initialize is required before you can do anything else. You could just move the essential parts to the constructors of Aktualizr and SotaUptaneClient and leave Initialze purely for provisioning with the server. Then the assumption would just be that you have to call that before doing online update, but offline wouldn't require it.

I'm referring to the existing CommandQueue in aktualizr/src/libaktualizr/utilities/apiqueue.h.

Oops, sorry, completely forgot that already existed!

One thing that we have to be careful here is that the Aktualizr class already moves a bunch of calls to a background thread, so we are probably stuck with having to deal we multiple threads. I agree that we should try and avoid parallelism where possible, but what about the case of an updater that polls in the background but also has a 'Check For Updates Now' button. This would require:

1. A user interface that wants to show the current update state, which will needs an answer in ~ms
2. A background task inside Aktualizr that does the polling (so Aktualizr 'just works' in the simple case),
3. A way to trigger a check now (interlocked with the background task)

Did you have any thoughts on how that should work?

Ignoring the case where two threads are calling the update code at the same time, does this not already work? If it doesn't, why not just call Shutdown, do the manual update, and then restart RunForever? To be safe, we probably should make the important calls in Aktualizr or SotaUptaneClient locked with a mutex. Is that enough?

[cajun-rat]

Perhaps the next step is to make that change to Initialize process (which I think we can see a way forward on), and I'll try and put together a more concrete proposal for the concurrency/async/threads part of the problem?

[pattivacek]

Perhaps the next step is to make that change to Initialize process (which I think we can see a way forward on), and I'll try and put together a more concrete proposal for the concurrency/async/threads part of the problem?

Sure, sounds fine to me!

[cajun-rat]

Step 1, refactor the initialization process is done in #25 (now merged)

[tkfu]

PURE-2: Offline Updates is now officially approved :).