-
Notifications
You must be signed in to change notification settings - Fork 42
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
System Update API #2100
System Update API #2100
Changes from all commits
b7b60c0
9a5ae8c
468a263
1ce8ae6
9d86f52
24ab40c
cde4429
859605c
e3e3123
4eed1d7
afe32c1
186994e
ea4bbde
38e3444
dea5ada
a7d898c
54e1f45
34ebe77
4c59334
7aa207c
b9ac8b6
c143f8a
c6f9dff
78e3fb7
8ffa99a
1eeaf4e
c5b6264
ffea521
190af7c
fde6e98
3ef6f38
f8898a8
c145d60
2660a52
b3df99c
9ec0773
e992960
3dc2bb0
0746adf
5a2b99f
a28b371
0d22dd0
53d7d03
07dea02
05635bd
9575587
6cdfe40
b7a0b93
a1772af
f109cfe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1512,6 +1512,152 @@ CREATE INDEX ON omicron.public.update_available_artifact ( | |
targets_role_version | ||
); | ||
|
||
/* | ||
* System updates | ||
*/ | ||
CREATE TABLE omicron.public.system_update ( | ||
/* Identity metadata (asset) */ | ||
id UUID PRIMARY KEY, | ||
time_created TIMESTAMPTZ NOT NULL, | ||
time_modified TIMESTAMPTZ NOT NULL, | ||
|
||
-- Because the version is unique, it could be the PK, but that would make | ||
-- this resource different from every other resource for little benefit. | ||
|
||
-- Unique semver version | ||
version STRING(64) NOT NULL, -- TODO: length | ||
-- version string with maj/min/patch 0-padded to be string sortable | ||
version_sort STRING(64) NOT NULL -- TODO: length | ||
); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using the version as the PK while still keeping the ID around is one of the stranger parts of this PR, but I landed on it after trying less weird-sounding alternatives that turned out to be uglier. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, minor nitpick, that doesn't really matter much but might help with consistency:
I think this should be functionally very similar, but it matches the pattern of other objects with "both IDs and unique names". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So the problem here was with the lookup macros. I could not figure out how to get a nice lookup-by-version with |
||
|
||
CREATE UNIQUE INDEX ON omicron.public.system_update ( | ||
version | ||
); | ||
|
||
CREATE UNIQUE INDEX ON omicron.public.system_update ( | ||
version_sort | ||
); | ||
|
||
CREATE TYPE omicron.public.updateable_component_type AS ENUM ( | ||
'bootloader_for_rot', | ||
'bootloader_for_sp', | ||
'bootloader_for_host_proc', | ||
'hubris_for_psc_rot', | ||
'hubris_for_psc_sp', | ||
'hubris_for_sidecar_rot', | ||
'hubris_for_sidecar_sp', | ||
'hubris_for_gimlet_rot', | ||
'hubris_for_gimlet_sp', | ||
'helios_host_phase_1', | ||
'helios_host_phase_2', | ||
'host_omicron' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're probably gonna add zones-updateable-separately-from-the-host-OS like CRDB + Clickhouse to this list, right? (Not in this PR, just kinda thinking out loud) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hadn't thought about it, but yeah, sounds right. I just copied these from the example tree in RFD 334. |
||
); | ||
|
||
/* | ||
* Component updates. Associated with at least one system_update through | ||
* system_update_component_update. | ||
*/ | ||
CREATE TABLE omicron.public.component_update ( | ||
/* Identity metadata (asset) */ | ||
id UUID PRIMARY KEY, | ||
time_created TIMESTAMPTZ NOT NULL, | ||
time_modified TIMESTAMPTZ NOT NULL, | ||
|
||
-- On component updates there's no device ID because the update can apply to | ||
-- multiple instances of a given device kind | ||
|
||
-- The *system* update version associated with this version (this is confusing, will rename) | ||
version STRING(64) NOT NULL, -- TODO: length | ||
-- TODO: add component update version to component_update | ||
|
||
component_type omicron.public.updateable_component_type NOT NULL | ||
); | ||
|
||
-- version is unique per component type | ||
CREATE UNIQUE INDEX ON omicron.public.component_update ( | ||
component_type, version | ||
); | ||
|
||
/* | ||
* Associate system updates with component updates. Not done with a | ||
* system_update_id field on component_update because the same component update | ||
* may be part of more than one system update. | ||
*/ | ||
CREATE TABLE omicron.public.system_update_component_update ( | ||
system_update_id UUID NOT NULL, | ||
component_update_id UUID NOT NULL, | ||
|
||
PRIMARY KEY (system_update_id, component_update_id) | ||
); | ||
|
||
-- For now, the plan is to treat stopped, failed, completed as sub-cases of | ||
-- "steady" described by a "reason". But reason is not implemented yet. | ||
-- Obviously this could be a boolean, but boolean status fields never stay | ||
-- boolean for long. | ||
CREATE TYPE omicron.public.update_status AS ENUM ( | ||
'updating', | ||
'steady' | ||
); | ||
|
||
/* | ||
* Updateable components and their update status | ||
*/ | ||
CREATE TABLE omicron.public.updateable_component ( | ||
/* Identity metadata (asset) */ | ||
id UUID PRIMARY KEY, | ||
time_created TIMESTAMPTZ NOT NULL, | ||
time_modified TIMESTAMPTZ NOT NULL, | ||
|
||
-- Free-form string that comes from the device | ||
device_id STRING(40) NOT NULL, | ||
|
||
component_type omicron.public.updateable_component_type NOT NULL, | ||
|
||
-- The semver version of this component's own software | ||
version STRING(64) NOT NULL, -- TODO: length | ||
|
||
-- The version of the system update this component's software came from. | ||
-- This may need to be nullable if we are registering components before we | ||
-- know about system versions at all | ||
system_version STRING(64) NOT NULL, -- TODO: length | ||
-- version string with maj/min/patch 0-padded to be string sortable | ||
system_version_sort STRING(64) NOT NULL, -- TODO: length | ||
|
||
status omicron.public.update_status NOT NULL | ||
-- TODO: status reason for updateable_component | ||
); | ||
|
||
-- can't have two components of the same type with the same device ID | ||
CREATE UNIQUE INDEX ON omicron.public.updateable_component ( | ||
component_type, device_id | ||
); | ||
|
||
CREATE INDEX ON omicron.public.updateable_component ( | ||
system_version_sort | ||
); | ||
|
||
/* | ||
* System updates | ||
*/ | ||
CREATE TABLE omicron.public.update_deployment ( | ||
/* Identity metadata (asset) */ | ||
id UUID PRIMARY KEY, | ||
time_created TIMESTAMPTZ NOT NULL, | ||
time_modified TIMESTAMPTZ NOT NULL, | ||
|
||
-- semver version of corresponding system update | ||
-- TODO: this makes sense while version is the PK of system_update, but | ||
-- if/when I change that back to ID, this needs to be the ID too | ||
version STRING(64) NOT NULL, | ||
|
||
status omicron.public.update_status NOT NULL | ||
-- TODO: status reason for update_deployment | ||
); | ||
|
||
CREATE INDEX on omicron.public.update_deployment ( | ||
time_created | ||
); | ||
|
||
/*******************************************************************/ | ||
|
||
/* | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does it mean for an update to be modified? The version updated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, is there a need for an index to be created for this or other tables here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really know why these would be modified. The only reason I have it on here is because it's convenient to make these
Asset
s, which are likeResource
but without name and description. I don't think it's too bad to have the field there and never use it — the alternative (not being anAsset
) is probably worse, though I haven't thought about it too hard.Yes to indexes, will put that on my to-do list.