Operation Workflow Specification & Implementation

[didier-wenzek]

Proposed changes

Specification for MQTT-driven workflows that let the user extend the builtin operation support provided by thin-edge.

See the POC for more details on the idea.

• Operation overview
• MQTT topics
• Workflow Definition
• Workflow Implementation
• Workflow Overriding
• Workflow Actions
• Workflow Composition
• Security
• Backward Compatibility

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
• Documentation Update (if none of the other choices apply)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue

Checklist

• I have read the CONTRIBUTING doc
• I have signed the CLA (in all commits with git commit -s)
• I ran cargo fmt as mentioned in CODING_GUIDELINES
• I used cargo clippy as mentioned in CODING_GUIDELINES
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

Further comments

[reubenmiller]

@didier-wenzek. I've added only a few minor comments. I really like the direction we're going 👍

[albinsuresh]

So the contract is like, if the script execution succeeds, the operation is moved to the first state in the next array and on failure it is moved to the second one?

[didier-wenzek]

I still have to describe the contract.

The idea is that the script is given the current state as an argument and that it has to output the new state on stdout. If the output of the script cannot be decoded as a state, the command is moved to "failed".

[albinsuresh]

Does the tedge-agent do anything with this owner info, other than just determining if it is "tedge" or not? I mean, it doesn't care about the value as long as it is not tedge, right?

[didier-wenzek]

Indeed in the POC nothing is done if the owner is not tedge.

However, if the MQTT identifier of this owner is given (i.e. device/main/service/tedge-configuration-plugin) a sophisticated implementation could use this to check if thing has a chance to work.

=> This needs to be clarified

[albinsuresh]

Even with this priority mechanism, it's not fully clear to me as to how we can use different workflows for different targets of the same operation type. For example, with the config-update operation, there'd be different kinds of custom scripts to perform updates of different types of configs. So, how would the workflow be handed over to the respective script for that given type?

[didier-wenzek]

For example, with the config-update operation, there'd be different kinds of custom scripts to perform updates of different types of configs.

The proposal is to have a single set of actions for a (command, status) pair. If you want to behave differently depending on some property of the state payload (here the config type) then this has to be handle inside the script. If you want to handle this at the workflow level, you can introduce new state (e.g. update_mosquitto_config and update_other_config) but you need then to run a script on the state update_config that move to one or the other state depending on the config type.

[rina23q]

Few questions 👍

[rina23q]

Provided tedge-configuration-plguin is a daemon, this owner field is just indicating the daemon name for logging purpose or like that? I assume that the state machine doesn't really care about the process owner?

[didier-wenzek]

Correct. See #2071 (comment)

[didier-wenzek]

Something still needs to be clarified.

Indeed, the tedge-configuration-plugin can be implemented as a:

• as a daemon - as of today
• as a script - as you suggested here
• as an actor running inside the agent - as implemented by the POC

These 3 alternatives have different pros/cons as well as implementation impacts. I will dive into these.

[albinsuresh]

LGTM. The comments are mostly rewording suggestions and a few queries for clarification. Happy to approve once those queries are clarified.

[albinsuresh]

The states mentioned in this file are not the same that were described in the example above. Keeping them consistent would be nice.

[reubenmiller]

Though this is in a section called "workflow overriding", so it is meant to show different states.

[albinsuresh]

Okay, in that case it might be a good idea to add a workflow file example conforming to that same simple example somewhere earlier: may be, in the Operation API section itself, right after the intro of the APIs, so that we have something basic before getting into advanced overriding here.

[reubenmiller]

Yeah we can add this in later.

[albinsuresh]

It may be obvious from these examples, but it might be better to explicitly document that the first value is the target state on a successful state action and the second one is for failure.

[reubenmiller]

The order of values does not mean anything at this point in time.

[albinsuresh]

I'm missing something here then. My understanding was that, if the script exits with non-zero exit code, without any JSON output with explicit status, then the workflow moves to the first state mentioned in the array. In the case of a script execution failure, it moves to the second. Isn't that the case?

[reubenmiller]

That isn't implemented yet, and we are still deciding on what to do there. This will be done in a follow up PR

[didier-wenzek]

For now, this list is mostly documentation.

[albinsuresh]

LGTM

[reubenmiller]

Approved. Any further improvements can be done in followup PRs

[didier-wenzek]

Here are the follow-up tasks for all the unresolved comments: #2478

[gligorisaev]

QA has thoroughly checked the feature and here are the results:

• Test for ticket exists in the test suite.
• QA has tested the function and it's functioning according description.