Endpoint redirection documentation #447

noursaidi · 2022-09-08T11:13:31Z

https://noursaidi.github.io/udmi/docs/specs/sequences/endpoint_reconfiguration

noursaidi · 2022-09-08T11:15:27Z

@johnrandolph PTAL only at the sequence diagrams - are they correct with what the sequencer tests? Quality of other information can be sorted later. If you click "View File" it should render the diagrams

grafnu · 2022-09-09T15:16:30Z

docs/specs/sequences/endpoint_reconfiguration.md

+
+### Valid Endpoint (Successful) Reconfiguration
+
+```mermaid


Is there a place we can go to see a rendered version of this?

https://noursaidi.github.io/udmi/docs/specs/sequences/endpoint_reconfiguration.html

noursaidi · 2022-09-10T11:34:21Z

@grafnu - https://noursaidi.github.io/udmi/docs/specs/sequences/endpoint_reconfiguration

But the states final and applied don't appear to be in the schema so either I might be wrong about those?

grafnu · 2022-09-10T14:56:18Z

Don't worry about the schema -- getting the sequence diagrams correct will be easier (since we can see all the parts and how they relate), then we go back to update the schema. Just a few notes -- I wouldn't put the device in the center of the two endpoints -- generally the device is on the left. Yah, some arrows may be longer but it's really confusing to have the main thing in the middle! The final state should be the same as the intended config. Right now the config is "final" and state is "applied" -- but it should be a static description of the end state. So "final" in this case. The intermediate state should be something like "apply" to keep the same word tense as "final" -- and it's not really preparing anything, it's actually applying the changes... We should also have a CONFIG message from the new endpoint in there. I think we'll want some cloud-side locking mechanism (how do you know you go to the right endpoint?) -- but at the very least just acknowledge that there is another CONFIG. But overall the sequences look great!

…

On Sat, Sep 10, 2022 at 4:34 AM Noureddine ***@***.***> wrote: @grafnu <https://github.com/grafnu> - https://noursaidi.github.io/udmi/docs/specs/sequences/endpoint_reconfiguration But the states final and applied don't appear to be in the schema so either I might be wrong about those? — Reply to this email directly, view it on GitHub <#447 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIEPD4EYURLWD3NQXM7PPDV5RW4PANCNFSM6AAAAAAQHUDTC4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

noursaidi · 2022-09-12T11:50:33Z

@grafnu comments incorporated.

Re the locking mechanism, is that an additional test represented as another sequence diagram - "If the config which initiates the redirect includes a nonce, only commit the new endpoint if the device receives a config from the new endpoint with the same nonce"? Or is it core functionality?

tests/config.tests/endpoint_reconfiguration.json

tests/state.tests/endpoint_reconfiguration.json

docs/specs/sequences/endpoint_reconfiguration.md

grafnu

How come I never just have one simple comment for anything??!?!

grafnu · 2022-09-14T18:37:27Z

docs/specs/sequences/endpoint_reconfiguration.md

    D-->>E':CONNECTION ATTEMPT
    E'->>D:CONFIG MESSAGE
-    D->>E':STATE MESSAGE<BR>blobset.blobs._iot_endpoint_config.blob.phase = "final"
+    D->>E':STATE MESSAGE<BR>blobset.blobs._iot_endpoint_config.phase = "final"


I think we should add an additional config/state sequence at the end that starts with config.blobset.blobs._iot_endpoint_config = null and then we verify that the state.blobset.blobs._iot_endpoint_config == null The fundamental reason is that we want to verify that the state block only shows up when expected.

grafnu · 2022-09-14T18:38:19Z

docs/specs/sequences/endpoint_reconfiguration.md

@@ -22,11 +22,11 @@ sequenceDiagram
    participant D as Device
    participant E as Original Endpoint
    participant E' as New Endpoint
-    E->>D:CONFIG MESSAGE<br>blobset.blobs._iot_endpoint_config.base64 = <ENDPOINT><br>blobset.blobs._iot_endpoint.blob.phase = "final"
-    D->>E:STATE MESSAGE<BR>blobset.blobs._iot_endpoint_config.blob.phase = "apply"
+    E->>D:CONFIG MESSAGE<br>blobset.blobs._iot_endpoint_config.base64 = <ENDPOINT><br>blobset.blobs._iot_endpoint.phase = "final"


pedantically there should be a check at the start that verifies that state.blobset.blobs._iot_endpoint_config == null (as the config block is empty so there should be no corresponding state).

grafnu · 2022-09-14T18:39:40Z

docs/specs/sequences/endpoint_reconfiguration.md

    D-->>E':CONNECTION ATTEMPT
    E'->>D:CONFIG MESSAGE
-    D->>E':STATE MESSAGE<BR>blobset.blobs._iot_endpoint_config.blob.phase = "final"
+    D->>E':STATE MESSAGE<BR>blobset.blobs._iot_endpoint_config.phase = "final"


there should be a check in here that the system.last_update time of the state message matches the config timestamp from the new endpoint (so might need to indicate that too). Goal is to check that they don't just connect and then use the wrong reply.

How long does phase: final stay in the state message for? Atleast 1? Noting also that the new endpoint probably won't have a blobset.blob._iot_endpoint_config blob

Added last_update and timestamps for config and state messages

grafnu · 2022-09-14T18:41:58Z

docs/specs/sequences/endpoint_reconfiguration.md

+    E->>D:CONFIG MESSAGE<br>blobset.blobs._iot_endpoint_config.blob = <ENDPOINT><br>blobset.blobs._iot_endpoint.blob.phase = "final"
+    D->>E:STATE MESSAGE<BR>blobset.blobs._iot_endpoint_config.phase = "apply"
+    D-->>E':CONNECTION ATTEMPT
+    note over D: Failure, e.g. endpoint doesn't exist, incorrect credentials, ...


what about a time limit here? I'd be worried that somebody implements a "try for 3h before reporting failure" scenario... so we should just have a fixed time-bound (which will be needed for the test anyway)... 30s? We can always increase it later, and devices can always report failure faster.

Added note for 30 seconds

grafnu · 2022-09-14T18:43:50Z

docs/specs/sequences/endpoint_reconfiguration.md

+    note over D: Failure, e.g. endpoint doesn't exist, incorrect credentials, ...
+    D-->>E:CONNECTION ATTEMPT
+    E->>D:CONFIG MESSAGE
+    D->>E:STATE MESSAGE<BR>blobset.blobs._iot_endpoint_config.phase = "final"<BR/>blobset.blobs._iot_endpoint_config.status.level=500 (ERROR) 


somewhere we need to introduce a category(s) for this too...

Do we differentiate for different types of failure? Or is it a single endpoint.reconfig.failed or similar?
A pretty common error I've encountered before is an endpoint "unreachable" because of firewall or network restrictions. Would that be its own category, or is that level of detail left to the message?

Category added

docs/specs/sequences/endpoint_reconfiguration.md

grafnu · 2022-09-15T17:03:02Z

I try to think of it as the categories delineate the various phases something would go through -- e.g. for config there's { receive, parse, apply } -- but, the actual "why did parsing fail?" it left up to the message. E.g. it could be a base64 parse error, or JSON parse error, or... --- no need to categorize all that. So for this, I think there's the obvious categories of { receive, parse, apply } -- like, I received the update, I could understand the update, and then I could apply the update -- but the actual underlying reasons (firewall bad) wouldn't be categories. Also, it's highly unlikely that a device would know that a firewall is causing a problem. So... not something that would be a useful categorization from that perspective.

…

On Thu, Sep 15, 2022 at 9:30 AM Noureddine ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In docs/specs/sequences/endpoint_reconfiguration.md <#447 (comment)>: > +### Invalid Endpoint (Unsuccessful Reconfiguration) + +```mermaid +%%{wrap}%% +sequenceDiagram + autonumber + participant D as Device + participant E as Original Endpoint + participant E' as New Endpoint + E->>D:CONFIG MESSAGE blobset.blobs._iot_endpoint_config.blob = <ENDPOINT> blobset.blobs._iot_endpoint.blob.phase = "final" + D->>E:STATE MESSAGE blobset.blobs._iot_endpoint_config.phase = "apply" + D-->>E':CONNECTION ATTEMPT + note over D: Failure, e.g. endpoint doesn't exist, incorrect credentials, ... + D-->>E:CONNECTION ATTEMPT + E->>D:CONFIG MESSAGE + D->>E:STATE MESSAGE blobset.blobs._iot_endpoint_config.phase = "final" blobset.blobs._iot_endpoint_config.status.level=500 (ERROR) Do we differentiate for different types of failure? Or is it a single endpoint.reconfig.failed or similar? A pretty common error I've encountered before is an endpoint "unreachable" because of firewall or network restrictions. Would that be its own category, or is that level of detail left to the message? — Reply to this email directly, view it on GitHub <#447 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIEPDZZQKGI3G4H5QGPCITV6NFLLANCNFSM6AAAAAAQHUDTC4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

grafnu · 2022-09-15T17:06:54Z

The final should be there as long as the blob is in the config. If the blob data is what is applied, then it's final. If the data is different, then it's initial or some other phase. If the new endpoint has the blob config or not is, well, up to the backend. So, if the new endpoint wants the blob state to be defined then it needs to have it in the config. If it doesn't care then it doesn't need it there. Basically, we should be able to clearly articulate the device logic: 1) blob should be in state if and only if it is in state 2) If blob matches the finalized state (fully applied or errored) then it's final, otherwise it's not final.

…

On Thu, Sep 15, 2022 at 9:32 AM Noureddine ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In docs/specs/sequences/endpoint_reconfiguration.md <#447 (comment)>: > D-->>E':CONNECTION ATTEMPT E'->>D:CONFIG MESSAGE - D->>E':STATE MESSAGE blobset.blobs._iot_endpoint_config.blob.phase = "final" + D->>E':STATE MESSAGE blobset.blobs._iot_endpoint_config.phase = "final" How long does phase: final stay in the state message for? Atleast 1? Noting also that the new endpoint probably won't have a blobset.blob._iot_endpoint_config blob — Reply to this email directly, view it on GitHub <#447 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIEPD7G2ETQJSMPDGYSW6LV6NFTJANCNFSM6AAAAAAQHUDTC4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

noursaidi · 2022-09-16T08:56:15Z

So for this, I think there's the
obvious categories of { receive, parse, apply } -- like, I received the
update, I could understand the update, and then I could apply the update --
but the actual underlying reasons (firewall bad) wouldn't be categories.
Okay, using blobset.blob as the prefix e.g. blobset.blob.apply

Also, it's highly unlikely that a device would know that a firewall is
causing a problem. So... not something that would be a useful
categorization from that perspective.
Yes, butthe device can differentiate "could not reach host", "could connect to host but no MQTT broker" and "MQTT broker denied connection (usually incorrect credentials)". But I think you're notes are fine, the message is sufficient and it's not anything that can't be deduced from other means too.

noursaidi · 2022-09-16T10:01:59Z

@johnrandolph @grafnu I think I've incorporated comments, please take a look.

bin/gencode_docs_examples

docs/specs/categories.md

docs/specs/sequences/endpoint_reconfiguration.md

grafnu · 2022-09-16T13:19:17Z

docs/specs/sequences/endpoint_reconfiguration.md

+    D-->>E':CONNECTION ATTEMPT
+    E'->>D:CONFIG MESSAGE<br>timestamp = t2<br/>blobset.blobs._iot_endpoint_config.base64 = <ENDPOINT><br>blobset.blobs._iot_endpoint_config.phase = "final"
+    D->>E':STATE MESSAGE<br/>system.last_update = t2<br/>blobset.blobs._iot_endpoint_config.phase = "final"
+    E->>D:CONFIG MESSAGE<br/>timestamp = t3<br/>blobset.blobs._iot_endpoint_config = null (unset)


I'm not sure all the timestamp signaling in here makes anything clearer. It's explicitly required for ALL config/state messages, and there's nothing special about these cases. What value or specific thing are we trying to indicate by showing it all here? Just "because it's true" isn't sufficient b/c then you could argue that we should indicate that the VERSION should be included :-).

I was using t0, t1, etc.. to indicate a new different config message. The specific intent was to show the state message sent after a reconnect is in response to the config message sent from the new endpoint

Maybe just a note to say last_update matches timestamp of the config message received from the new endpoint?

I've removed all the timestamps/last_updates except for the first config from the new endpoint.

But it's entirely redundant. Every config message is followed by a state message with a matching timestamp. What problem are you trying to "fix" by having that information there? What loss of information is there if it's taken out? My point is that 1) it causes more questions than it answers, and 2) is just more "noise" without much signal behind it. Every other line in the sequence is indicating something that's specific/unique to endpoint sequences... except for that.

@grafnu I was trying to address this comment - #447 (comment)

docs/specs/sequences/endpoint_reconfiguration.md

grafnu · 2022-09-20T19:56:34Z

docs/specs/sequences/endpoint_reconfiguration.md

+
+**Notes**
+- `<NEW_ENDPOINT>` is a **base64** encoded endpoint object
+- `blobset.blobs_iot_endpoint_config` is present in a device's state message if, and only if,   the last received config message has a `blobset.blobs_iot_endpoint_config` block.


missing . after blobs (two places)

grafnu · 2022-09-20T20:19:56Z

Ah, well it seemed like a better idea when I said it the first time :-). What might work better would be a note over the "new endpoint" that says something like "state.timestamp matches new endpoint config.timestamp" -- to be explicit about it. As it stands now, although you're 100% right, it kinda goes unnoticed (even by me!) because it does look just like normal message flow. We don't really indicate all the usual checks (like for the phase, etc...) and I don't think we should, but this seems like a special case? Sorry for all the confusion and back/forth...

…

On Tue, Sep 20, 2022 at 1:08 PM Noureddine ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In docs/specs/sequences/endpoint_reconfiguration.md <#447 (comment)>: > + +```mermaid +%%{wrap}%% +sequenceDiagram + autonumber + participant D as Device + participant E as Original Endpoint + participant E' as New Endpoint + E->>D:CONFIG MESSAGE timestamp = t0 blobset.blobs._iot_endpoint_config = null (unset) + D->>E:STATE MESSAGE system.last_update = t0 blobset.blobs._iot_endpoint_config = null (unset) + E->>D:CONFIG MESSAGE timestamp = t1 blobset.blobs._iot_endpoint_config.base64 = <ENDPOINT> blobset.blobs._iot_endpoint_config.phase = "final" + D->>E:STATE MESSAGE system.last_update = t1 blobset.blobs._iot_endpoint_config.phase = "apply" + D-->>E':CONNECTION ATTEMPT + E'->>D:CONFIG MESSAGE timestamp = t2 blobset.blobs._iot_endpoint_config.base64 = <ENDPOINT> blobset.blobs._iot_endpoint_config.phase = "final" + D->>E':STATE MESSAGE system.last_update = t2 blobset.blobs._iot_endpoint_config.phase = "final" + E->>D:CONFIG MESSAGE timestamp = t3 blobset.blobs._iot_endpoint_config = null (unset) @grafnu <https://github.com/grafnu> I was trying to address this comment - #447 (comment) <#447 (comment)> — Reply to this email directly, view it on GitHub <#447 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIEPDZUGFFSHZ32VJYCN7DV7IKUVANCNFSM6AAAAAAQHUDTC4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

grafnu

Just one minor comment about the position of the state comparison note, but otherwise all looks good to me!

grafnu · 2022-09-21T17:34:19Z

docs/specs/sequences/endpoint_reconfiguration.md

+    D-->>E':CONNECTION ATTEMPT
+    end
+    E'->>D:CONFIG MESSAGE<br/>blobset.blobs._iot_endpoint_config.base64 = <NEW_ENDPOINT><br/>blobset.blobs._iot_endpoint_config.phase = "final"
+    note over E': system.last_update in state matches timestamp of config from new endpoint


I'd move this one line down so it happens AFTER the state is received, since that's when the comparison would be made.

grafnu · 2022-10-11T07:41:19Z

I think it would just be one diagram that shows the overall flow. I'm not sure of the utility in breaking it out into smaller things unless the smaller things have some meaning behind them (e.g. ability to restart is useful independent of redirection). I added in some fields I think would work well for this -- they're borrowed from the restart mechanism (mode and last_start). Everything *underlined* is new, everything else is the same. Some of the system config messages (e.g. mode and last_start) aren't part of the redirect sequence, but they're necessary things that need to be here at this point (but could be earlier) -- so not 100% sure how to diagram them. CONFIG MESSAGE *system.mode = "active"* *system.last_start = <LAST_START>* blobset.blobs._iot_endpoint.blob = <ENDPOINT> blobset.blobs._iot_endpoint.blob.phase = "final" STATE MESSAGE blobset.blobs._iot_endpoint.blob.phase = "apply" *system.mode = "reconnect"* CONNECTION ATTEMPT *CONNECTION* SUCCESS CONFIG *MESSAGE* *system.last_start = <LAST_START>* STATE MESSAGE blobset.blobs._iot_endpoint.blob.phase = "applied" *system.mode = "active"*

…

On Mon, Sep 12, 2022 at 4:50 AM Noureddine ***@***.***> wrote: @grafnu <https://github.com/grafnu> comments incorporated. Re the locking mechanism, is that an additional test represented as another sequence diagram - "If the config which initiates the redirect includes a nonce, only commit the new endpoint if the new endpoint if the device receives a config from the new endpoint with the same nonce"? Or is it core functionality? — Reply to this email directly, view it on GitHub <#447 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIEPD4ZW4BAI53D3O5PHADV54KJJANCNFSM6AAAAAAQHUDTC4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

initial commit

1eda294

noursaidi requested a review from johnrandolph September 8, 2022 11:13

noursaidi assigned johnrandolph Sep 8, 2022

noursaidi marked this pull request as draft September 8, 2022 11:13

noursaidi requested a review from grafnu September 9, 2022 13:59

grafnu reviewed Sep 9, 2022

View reviewed changes

noursaidi added 3 commits September 10, 2022 12:01

add inline examples

f952a9f

Merge branch 'inline' into redirect

0dff9f8

add additional examples, fix mermaid display

9fb0d38

comments

76b61ba

noursaidi force-pushed the redirect branch from df0bd9d to 76b61ba Compare September 12, 2022 11:47

noursaidi added 2 commits September 12, 2022 16:24

save commit and push changes ..

85314a0

rename _iot_endpoint to _iot_endpoint_config

f9166a7

johnrandolph reviewed Sep 14, 2022

View reviewed changes

tests/config.tests/endpoint_reconfiguration.json Show resolved Hide resolved

tests/state.tests/endpoint_reconfiguration.json Show resolved Hide resolved

docs/specs/sequences/endpoint_reconfiguration.md Outdated Show resolved Hide resolved

noursaidi added 5 commits September 14, 2022 14:48

change preparing -> apply

968707d

Comments

5bc4a31

Add failure status

6cd768f

example

4e8be1f

remove duplicate blob

60109f6

johnrandolph reviewed Sep 14, 2022

View reviewed changes

docs/specs/sequences/endpoint_reconfiguration.md Outdated Show resolved Hide resolved

grafnu reviewed Sep 14, 2022

View reviewed changes

PR comments

5475e3e

last timestamp

ae5d8ac

grafnu reviewed Sep 16, 2022

View reviewed changes

noursaidi added 3 commits September 20, 2022 19:41

comments

9b175ef

more comments

1e8cfa9

remove timestamps

8ff7d97

noursaidi requested review from grafnu and johnrandolph September 20, 2022 19:09

commit gencode_docs_examples

b88b56a

grafnu reviewed Sep 20, 2022

View reviewed changes

noursaidi marked this pull request as ready for review September 21, 2022 10:52

comments - last update new endpoint

258a3e9

noursaidi force-pushed the redirect branch from 461a211 to 258a3e9 Compare September 21, 2022 11:11

noursaidi requested a review from grafnu September 21, 2022 16:29

grafnu approved these changes Sep 21, 2022

View reviewed changes

move note down one line

93e3664

noursaidi merged commit 640f035 into faucetsdn:master Sep 21, 2022

noursaidi mentioned this pull request Oct 11, 2022

Gencode to include inline examples from test directory #431

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Endpoint redirection documentation #447

Endpoint redirection documentation #447

noursaidi commented Sep 8, 2022 •

edited

Loading

noursaidi commented Sep 8, 2022 •

edited

Loading

grafnu Sep 9, 2022

grafnu Sep 12, 2022

noursaidi commented Sep 10, 2022

grafnu commented Sep 10, 2022 via email

noursaidi commented Sep 12, 2022 •

edited

Loading

grafnu left a comment

grafnu Sep 14, 2022

grafnu Sep 14, 2022

noursaidi Sep 16, 2022

grafnu Sep 14, 2022

noursaidi Sep 15, 2022

noursaidi Sep 16, 2022

grafnu Sep 14, 2022

noursaidi Sep 16, 2022

grafnu Sep 14, 2022

noursaidi Sep 15, 2022

noursaidi Sep 16, 2022

grafnu commented Sep 15, 2022 via email

grafnu commented Sep 15, 2022 via email

noursaidi commented Sep 16, 2022

noursaidi commented Sep 16, 2022

grafnu Sep 16, 2022

noursaidi Sep 20, 2022

noursaidi Sep 20, 2022

noursaidi Sep 20, 2022

grafnu Sep 20, 2022

noursaidi Sep 20, 2022

grafnu Sep 20, 2022

grafnu commented Sep 20, 2022 via email

grafnu left a comment

grafnu Sep 21, 2022

grafnu commented Oct 11, 2022 via email

Endpoint redirection documentation #447

Endpoint redirection documentation #447

Conversation

noursaidi commented Sep 8, 2022 • edited Loading

noursaidi commented Sep 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

noursaidi commented Sep 10, 2022

grafnu commented Sep 10, 2022 via email

noursaidi commented Sep 12, 2022 • edited Loading

grafnu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grafnu commented Sep 15, 2022 via email

grafnu commented Sep 15, 2022 via email

noursaidi commented Sep 16, 2022

noursaidi commented Sep 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grafnu commented Sep 20, 2022 via email

grafnu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grafnu commented Oct 11, 2022 via email

noursaidi commented Sep 8, 2022 •

edited

Loading

noursaidi commented Sep 8, 2022 •

edited

Loading

noursaidi commented Sep 12, 2022 •

edited

Loading