Skip to content

Conversation

@vipulgupta2048
Copy link
Member

Change-type: minor
Signed-off-by: Vipul Gupta (@vipulgupta2048) vipul@balena.io

@vipulgupta2048 vipulgupta2048 added the versionbot/pr-draft Draft PR - Don't merge this PR automatically label Feb 5, 2021
@vipulgupta2048
Copy link
Member Author

vipulgupta2048 commented Feb 9, 2021

@ab77 I need the WHY part of device registration. Why are we doing the 3 steps in the device registration part? Got any good description we can add?

@vipulgupta2048 vipulgupta2048 changed the title Add Offline Updates documentation Add Offline Upgrades documentation Feb 9, 2021
$ balena preload ${tmpimg} \
--app ${app_slug} \
--commit ${commit} \
--pin-device-to-release
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For devices with low bandwidth internet connections, the user has probably already pinned the device to the release it is running, so that the device doesn't try to start downloading any new release they create.
Given that, it might actually make sense to change this to --no-pin-device-to-release and do the pinning in the patch state endpoint script by adding

,\"should_be_running__release\":${release_id}

Such users will have to run that script then before they boot their device.
What do you think @ab77 ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thgreasi I really don't like that wall of bash text to patch state endpoints, so I don't really have an opinion. Ideally, we should go with whatever reduced the number of steps or even more ideally, have the CLI perform these as part of the preload flow (if a flag is specified presumably) cc @pdcastro

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[...] even more ideally, have the CLI perform these as part of the preload flow (if a flag is specified presumably)

Yes, sounds good to me. The wall of bash text could be replaced with a balena-cli PR. To decide on the name of the flag, what is a short description of the purpose of patching the state endpoint? Why do we need / want to patch the state endpoint? Things like --patch-state or --fix-state sound rather generic...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I am also not saying that offline updates should be delayed until more work is done on preload - up to you.

Copy link
Contributor

@pdcastro pdcastro Feb 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so that the device doesn't try to start downloading any new release they create [as soon as the preload command is executed]

This is a good point. But even moving the pinning to the patch state endpoint step (which is currently described as an optional step for low bandwidth devices) would still require coordination with the physical updating of each device in a strict order, for example:

  1. Preload the image without release pinning and flash the update media.
  2. Take the device offline for the update (rebooting would do).
  3. While the device is offline / before it boots, patch the state endpoint to pin the release.

Now, picture devices in multiple remote sites, where the operators doing the flashing do not have a laptop with them to patch the state endpoint then and there for that device...

There is a conflict here between "really offline" and "online with low bandwitdh, could download images but don't want to, it would likely fail anyway while wasting bandwidth and money". For the latter, perhaps we needed a new supervisor feature like "offline update mode - don't ever try to download images".

Copy link
Member

@thgreasi thgreasi Feb 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ab77 @pdcastro @vipulgupta2048

If the device is really always offline, then it doesn't make any difference if they --pin-device-to-release or pin it in the Patch State Endpoint or don't pin at all (but in that case there is also the question of why are they paying for balenaCloud at the first place).

On the other hand, as @pdcastro said,
If the device has partial/slow/expensive connectivity, the --pin-device-to-release would issue the device to try to fetch the new release. I realize that having to run a curl before booting the new preloaded image doesn't sound great, but:

  • it should be do-able for an operator to do that, given that the device already has limited connectivity
  • It unlocks an extra use case, which imo sounds like what customers probably need

It would of course be great to have a way to instruct a device to not try to download any new release as Paulo said.

@vipulgupta2048 vipulgupta2048 changed the title Add Offline Upgrades documentation Add Offline Updates documentation Feb 10, 2021
@vipulgupta2048
Copy link
Member Author

vipulgupta2048 commented Feb 10, 2021

@ab77 I need the WHY part of device registration. Why are we doing the 3 steps in the device registration part? Got any good description we can add?

Still need this, would be quite helpful to understand what we mean to do in device registration @ab77

$ balena preload ${tmpimg} \
--app ${app_slug} \
--commit ${commit} \
--pin-device-to-release
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thgreasi I really don't like that wall of bash text to patch state endpoints, so I don't really have an opinion. Ideally, we should go with whatever reduced the number of steps or even more ideally, have the CLI perform these as part of the preload flow (if a flag is specified presumably) cc @pdcastro

@vipulgupta2048
Copy link
Member Author

@balena-ci rebase

@ghost ghost force-pushed the vipul/offline-updates branch from cdb1386 to 5bd1a75 Compare February 10, 2021 21:06
@ab77
Copy link
Contributor

ab77 commented Feb 10, 2021

@ab77 I need the WHY part of device registration. Why are we doing the 3 steps in the device registration part? Got any good description we can add?

@vipulgupta2048 If you are referring to the part ## create offline device, then the WHY is because all the the subsequent steps require it. If the user already has a device, they just need to set uuid={{their-uuid}}.


#### Patch State Endpoint

This step is strictly not required for devices with low bandwidth internet connections. Those devices would be able to update the state endpoint automatically.
Copy link
Contributor

@pdcastro pdcastro Feb 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The word "strictly" confuses me in this sentence. Would it still mean the same if the word "strictly" was deleted?
Even then, whether or not this step is required would depend on the device having "low bandwidth", which is a loosely defined condition. As a user, how can I determine, with confidence, whether or not this step applies to me? "Just tell me whether or not I should run this" kind of thing. :-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose if users are blindly copying and pasting instructions without understanding what they do, having something like this forces some sort of consideration. Could be reworded to say that if a device never ever phones home, then this step is required (unless the user doesn't care how the device appears in the dash board at all).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be reworded to say that if a device never ever phones home, then this step is required (unless the user doesn't care how the device appears in the dash board at all).

This would definitely be clearer. 👍  For example, assuming it is correct:

#### Update the device state in the cloud

This step updates the device state in the cloud (balenaCloud or openBalena)
on behalf of the device, as if the device was online. It is only required for
devices that do not have an internet connection. If the device has an internet
connection, even if low bandwidth, this step may be skipped because the device
itself will contact the cloud to update its state.

Assuming it is correct, this also answers my other question as to what the balena preload flag might be called:

$ balena help preload
OPTIONS
  --update-cloud-state  Update the device state in the cloud (for offline updates)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a really good description. I am adding it, until someone says otherwise.
cc: @ab77 @pdcastro

@vipulgupta2048
Copy link
Member Author

vipulgupta2048 commented Feb 11, 2021

@vipulgupta2048 If you are referring to the part ## create offline device, then the WHY is because all the the subsequent steps require it. If the user already has a device, they just need to set uuid={{their-uuid}}.

@ab77 No, I don't mean this part. I meant, the actual device registration part with this heading, ### Update Device Registration(s). A description for this part would be nice to understand what this means to the user + what the next steps would entail.

Edit: Actually I wrote and edited a little of mine own. Do check that out.

@vipulgupta2048 vipulgupta2048 force-pushed the vipul/offline-updates branch 2 times, most recently from f6f8982 to 299b7da Compare February 11, 2021 08:18
Change-type: minor
Signed-off-by: Vipul Gupta (@vipulgupta2048) <vipul@balena.io>
- Lots of optional stages in the process where pre-existing applications,
devices, keys can be used by users. All of them documented accordingly.

Signed-off-by: Vipul Gupta (@vipulgupta2048) <vipul@balena.io>
Signed-off-by: Vipul Gupta (@vipulgupta2048) <vipul@balena.io>
Signed-off-by: Vipul Gupta (@vipulgupta2048) <vipul@balena.io>
- Replace 'offline updates' to 'offline upgrades'
- Remove space in ssh_key variable
- Marked state endpoint as non-optional
- Improved language and added more links
- Updated headings in TOC
- Linted, and fixed broken code snippets
- Added to sidebar nav

Signed-off-by: Vipul Gupta (@vipulgupta2048) <vipul@balena.io>
Signed-off-by: Vipul Gupta (@vipulgupta2048) <vipul@balena.io>
| sed 's/"//g' \
| sed 's/v//g')

# (e.g.) staging
Copy link
Contributor

@pdcastro pdcastro Feb 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this # (e.g.) staging comment, how about replacing the two occurrences of balena-staging.com with an environment variable? The CLI uses the BALENARC_BALENA_URL environment variable to select a different cloud domain name (openBalena, staging, ...) than the balena-cloud.com default, so I suggest:

if [ -z "$BALENARC_BALENA_URL" ]; then
    BALENARC_BALENA_URL=balena-cloud.com
fi

$ release_id=$(curl --silent \
    "https://api.${BALENARC_BALENA_URL}/v6/release?
...

$ curl --silent \
    -X 'PATCH' "https://api.${BALENARC_BALENA_URL}/v6/device(uuid='${uuid}')" \
...

I understand that default value assignment is bash-specific and less portable, hence not using it:
BALENARC_BALENA_URL={BALENARC_BALENA_URL:-balena-cloud.com}

And yes, ideally this shell code would be replaced with a new flag for the balena preload command.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only problem with this @pdcastro is that it doesn't fit into the overall command structure we have going on in the document. Everything is a shell command and here comes a bash script IF statement. I can make this into a one-liner though.

if [ -z "$BALENARC_BALENA_URL" ]; then BALENARC_BALENA_URL=balena-cloud.com; fi

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replacing stuff with this.

Signed-off-by: Vipul Gupta (@vipulgupta2048) <vipul@balena.io>
@ab77
Copy link
Contributor

ab77 commented Feb 11, 2021

@vipulgupta2048 LGTM

@ab77 ab77 self-requested a review February 11, 2021 20:14
ab77
ab77 previously approved these changes Feb 11, 2021
@ghost ghost dismissed ab77’s stale review February 11, 2021 20:15

Approval reviews not allowed in Draft PRs

@vipulgupta2048 vipulgupta2048 removed the versionbot/pr-draft Draft PR - Don't merge this PR automatically label Feb 11, 2021
Signed-off-by: Vipul Gupta (@vipulgupta2048) <vipul@balena.io>
@vipulgupta2048
Copy link
Member Author

@balena-ci I self-certify!

@ghost ghost merged commit ee675ac into master Feb 12, 2021
@ghost ghost deleted the vipul/offline-updates branch February 12, 2021 10:59
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants