Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add contract partial based instruction generation #1101

Merged
merged 2 commits into from
Feb 16, 2023
Merged

Conversation

mehalter
Copy link
Contributor

@mehalter mehalter commented Jun 10, 2021

This is an initial implementation of a basic API call to interpolate partials within a contract. Discussion is needed to address the HostOS contract that is currently hard-coded (should be pulled from somewhere else). Also, this functionality applies not just to device types, but any contract could potentially have partials that need to be interpolated. There currently isn't a place to handle such abstract functionality in the SDK (to my knowledge) and it seems like this would be a good place to use Contrato as it is going to be (should be) the de facto implementation of the Contract data structure and operations surrounding them. It might be better for the partial interpolation to live there and then anything that is contract based in the SDK have the ability to use Contrato types and functionality.

This will also be able to easily fix the current issue where the instructions are not correct in BalenaHub and open fleets since new devices added do not appear in the dashboard. This will allow different interfaces and uses of the partials to provide their own overarching template (Discussion for this specific issue on Flowdock: https://www.flowdock.com/app/rulemotion/balenahub/threads/UQ29ow3_IXlkXMOnzT-ea9H6V-q).
https://balena.zulipchat.com/#narrow/stream/349104-balena-io.2Fcontracts/topic/contract.20partial.20based.20instruction.20generation

TODO

  • Updating the contracts repo to have necessary partials
  • Improve implementation to handle the few edge cases that don't fall under the usual Etcher based install methods
    • Fix cases with nested instructions like having different instructions for linux/mac/windows

Depends-on: balena-io/contracts#236
Change-type: minor


Contributor checklist
  • Includes tests
  • Includes typings
  • Includes updated documentation
  • Includes updated build output

@mehalter mehalter self-assigned this Jun 10, 2021
@mehalter
Copy link
Contributor Author

@balena-ci rebase

@ghost ghost force-pushed the instructions branch from dfcdc6c to d7efe40 Compare June 17, 2021 16:07
@mehalter
Copy link
Contributor Author

mehalter commented Feb 2, 2022

@balena-ci rebase

@mehalter
Copy link
Contributor Author

@balena-ci rebase

@ghost ghost force-pushed the instructions branch from 0c69d44 to ddba646 Compare June 14, 2022 18:12
@mehalter mehalter force-pushed the instructions branch 2 times, most recently from 85f63cc to 7b718e5 Compare October 25, 2022 16:14
@mehalter mehalter marked this pull request as ready for review October 25, 2022 17:54
@mehalter mehalter force-pushed the instructions branch 6 times, most recently from 5add372 to 1bb297e Compare October 27, 2022 16:02
@mehalter
Copy link
Contributor Author

Tested this iterating over the getAllSupported result and got all of the instructions:

(await sdk.models.deviceType.getAllSupported()).forEach(async (x, i) => {
  var instructions = await sdk.models.deviceType.getInstructions(x.slug)
  console.log("Instructions for " + x.slug + ":\n");
  console.log(instructions);
});

There are a couple devices that get returned by getAllSupported that do not have contracts (and therefore no partials): smartcube-kbox-a150 and smartcube-kbox-a250. I'm not sure where these are coming from and why they are being returned, but I also don't see them as an option in Balena Cloud. These also do correctly throw an error indicating that the instruction partials are not defined rather than failing in an unexpected way.

I have also attached the following text file with the complete output from running the above code snippet (with a bit of prettying up of the formatting for easier reading and checking: all_instructions.txt

Copy link

@alexgg alexgg left a comment

Choose a reason for hiding this comment

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

A couple of comments so we can move the partials into the OS contracts - I'll leave the ts review to someone more versed in the SDK.

`Remove the {{deviceType.data.media.defaultBoot}} from the host machine.`,
`Insert the freshly flashed {{deviceType.data.media.defaultBoot}} into the {{deviceType.name}}.`,
`{{{deviceType.partials.bootDevice}}} to boot the device.`,
],
Copy link

Choose a reason for hiding this comment

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

@mehalter the partials above should go into the existing balenaOS contract in https://github.com/balena-io/contracts/blob/master/contracts/sw.os/balenaos/contract.json.

This is already used by the OS build scripts so when added the contract will be deployed with the OS release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awesome, I'll get this moved over. Is this accessible through the API as well?

Copy link

Choose a reason for hiding this comment

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

@mehalter it's deployed with the OS release. For example if you do:

await sdk.pine.get({
    resource: 'release',
    id: nnnnn,
});

You will get something like:

belongs_to__application: {__id: 1520936, __deferred: {…}}
build_log:null
commit:"2c3a3a6c020e3ca5a2ae008f9de59643"
composition:{version: '2.1', networks: {…}, volumes: {…}, services: {…}}
contract:
"{\"name\":\"Balena OS for Raspberry Pi 4 (using 64bit OS)\",\"type\":\"sw.block\",\"description\":\"Balena OS for a Raspberry Pi 4 (using 64bit OS)\",\"provides\":[{\"type\":\"sw.os\",\"slug\":\"balena-os\"},{\"type\":\"hw.device-type\",\"slug\":\"raspberrypi4-64\"}],\"composedOf\":[\"balena-os\",\"raspberrypi4-64\"],\"version\":\"2.105.1+rev1\"}"

As you see at the moment the contract is very basic, but once you add the instructions they will appear there.

},
},
};

Copy link

Choose a reason for hiding this comment

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

Both jetsonFlash and edisonFlash are device specific and they do not belong in the OS contract. They need to be included into something like sw.device-family, and then pulled in when the specific device type contract is built.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, each of these partials are supported installation methods of the operating system and are not necessarily device specific. Any device that supports using the jetson flash tool should be allowed to get these instructions. These are just Balena OS's way of saying, "I support being installed with the jetson flash tool, and this is how. It doesn't provide any information that is specific to a device type. If that makes sense. Basically it's the BalenaOS's part in the flash process of telling the user "how to handle the file that I am giving you.

Copy link

Choose a reason for hiding this comment

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

Jetson is a device family, no other vendor will use Jetson tools. And the same goes for edison.

Why would a contract for the RaspberryPi need to mention other vendors like Jetson or edison? In my view this corresponds to a sw.device-family contract that is then pulled in only when building the Jetson/Edison families.

lib/models/device-type.ts Outdated Show resolved Hide resolved
lib/models/device-type.ts Outdated Show resolved Hide resolved
lib/models/device-type.ts Outdated Show resolved Hide resolved
lib/models/device-type.ts Outdated Show resolved Hide resolved
lib/models/device-type.ts Outdated Show resolved Hide resolved
lib/models/device-type.ts Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
lib/types/device-type-json.ts Outdated Show resolved Hide resolved
lib/types/device-type-json.ts Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
lib/models/config.ts Outdated Show resolved Hide resolved
lib/models/device-type.ts Outdated Show resolved Hide resolved
lib/models/device-type.ts Outdated Show resolved Hide resolved
lib/models/device-type.ts Outdated Show resolved Hide resolved
lib/models/device-type.ts Outdated Show resolved Hide resolved
lib/models/device-type.ts Outdated Show resolved Hide resolved
lib/models/device-type.ts Outdated Show resolved Hide resolved
lib/models/device-type.ts Outdated Show resolved Hide resolved
@vipulgupta2048
Copy link
Member

Let me know what you think of the changes made here @thgreasi if they are good to go from this point, then I will start writing tests. I also considered use loadash's template instead of handlebars and decided against it as I still haven't gotten to the point of figuring out what the output will produce.

Change-type: minor
Signed-off-by: Micah Halter <micah@balena.io>
@vipulgupta2048
Copy link
Member

@thgreasi PR ready to go. The methods have been tested and I will open a PR in docs to show how this looks on production for the final review.

Signed-off-by: Vipul Gupta (@vipulgupta2048) <vipul@balena.io>
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