Fix the c8y custom operation status handling

[rina23q]

Proposed changes

A custom operation managed in /etc/tedge/operations/c8y could be marked successful unconditionally and the status had been sent out before the operation has completed.

To fix, now the Cumulocity converter holds a mqtt publisher and sends executing and successful/failed message from the converter. Either successful or failed is determined by the exit code (0 or not).

Also, stdout is reported when successful, and stderr for a failed case.

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

#1603

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

[rina23q]

[Solved] cargo test never ends for some reason although it passes on my dev machine. I will work on that.

[didier-wenzek]

Is this useful to send the whole output of the command to the cloud?

If yes, one needs to escape any character misleading for CSV (notably the quotes ") and to conform to what is supported by Cumulocity (are multiline stdout supported?).

[rina23q]

Discussed with @reubenmiller, what to report to the cloud and how the message should look.

For successful case, sending the stdout contents as much as possible (within the size limit of c8y) with some sanitising. The detailed, you can find from #1603 "Result/failure message parsing rules".

My commit e76e172 changed this part as described in the original ticket.

[didier-wenzek]

One needs to escape any character misleading for CSV (notably the quotes ") and to conform to what is supported by Cumulocity (are multiline stderr supported?).

[rina23q]

Discussed with @reubenmiller, what to report to the cloud and how the message should look.

For error case, sending the last line of stdout as failure reason. The detailed, you can find from #1603 "Result/failure message parsing rules".

My commit e76e172 changed this part as described in the original ticket.

[albinsuresh]

Although I agree that sending this executing status here before the actual command itself is executed(which could take time to complete), provides better UX from the cloud, it is done at the cost of breaking the "converter" abstraction of it "just being a message converter without any MQTT business".

Since this is just a short term hack anyway, I was wondering why not keep it simple like this:

1. Run the command and wait for the exit code.
2. If exit code is 0, return both EXECUTING and SUCCESSFUL messages in the response Vec<Message>
3. If exit code is 1, return both EXECUTING and FAILED messages in the response Vec<Message>.

I understand that the executing response will be delayed for long running operations and results in suboptimal UX. So, we need to choose between a "complex hack for better UX" for vs "simple hack for suboptimal UX".

[albinsuresh]

On a second thought, I feel that your current approach is closer to the actor model that we are going to, where the C8yMapperActor would receive the conversion response from the C8yConvertorActor over a channel, like the mqtt_publisher that you're sharing between the mapper and the converter.

[didier-wenzek]

To sanitize an utf8 string I would work at the char level and not to byte level.

Something along the lines:

input.chars().filter( ...).map(...).take(max).collect()

There are then 2 tricky points:

• How to sanitize a non-utf8 input? One can use the current code.
• How to cut the utf8 output to fit the maximum size that is a byte count? I would try to use an iter::scan that accumulates the number of bytes used so far.

        input
            .chars()
            .filter(...)
            .flat_map(...)
            .scan(0, |bytes_count, char| {
                *bytes_count = *bytes_count + number_bytes(char);
                Some( (*bytes_count, char))
            })
            .take_while(|(size, _)| { size <= max })
            .map(|(_,char)| {char})
            .collect();

[rina23q]

To sanitize an utf8 string I would work at the char level and not to byte level.

Something along the lines:

input.chars().filter( ...).map(...).take(max).collect()

There are then 2 tricky points:

• How to sanitize a non-utf8 input? One can use the current code.
• How to cut the utf8 output to fit the maximum size that is a byte count? I would try to use an iter::scan that accumulates the number of bytes used so far.

        input
            .chars()
            .filter(...)
            .flat_map(...)
            .scan(0, |bytes_count, char| {
                *bytes_count = *bytes_count + number_bytes(char);
                Some( (*bytes_count, char))
            })
            .take_while(|(size, _)| { size <= max })
            .map(|(_,char)| {char})
            .collect();

Thanks @didier-wenzek. You are absolutely right. Now all sanitising functions were re-written.

[didier-wenzek]

Approved - with two comments.

[didier-wenzek]

Suggested change

	/// - Double quote is escaped as `\"`.
	/// - Double quote is escaped as `\"\"`.

[didier-wenzek]

This intermediate collect could be avoid by using a flat_map instead of the replace.