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

feat(schema): basic storage settings #1455

Merged
merged 15 commits into from
Aug 27, 2024

Conversation

joseivanlopez
Copy link
Contributor

@joseivanlopez joseivanlopez commented Jul 11, 2024

Extend JSON schema to partially support the new storage config described in this document: https://github.com/openSUSE/agama/blob/master/doc/auto_storage.md.

This PR only extends the schema for the most basic settings (i.e., boot, drives and partitions). Settings for advanced searches, delete devices, creeate RAIDs, etc are not considered yet.

Notes:

@joseivanlopez joseivanlopez force-pushed the storage-schema-basic branch 2 times, most recently from 57e5499 to ef24d7f Compare July 16, 2024 13:31
@joseivanlopez joseivanlopez marked this pull request as ready for review July 16, 2024 13:53
"snapshots": true
}
},
"size": [1024, "5 Gib"],
Copy link
Contributor

Choose a reason for hiding this comment

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

can json have comments? This will really deserve comment what this array size means. Is 1024 minimal size or block lenght? Or something else. Examples should be crystal clear.

Copy link
Contributor Author

@joseivanlopez joseivanlopez Jul 17, 2024

Choose a reason for hiding this comment

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

JSON does not allow comments. The schema defines what it means: [min, max].

Copy link
Contributor

Choose a reason for hiding this comment

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

@joseivanlopez true, but the schema also allows a {"min": min, "max": max } alternative. In light of @jreidinger 's comment, maybe we should remove the pair alternative, or discourage it.
https://github.com/openSUSE/agama/pull/1455/files#diff-e7dcc0a7344de41b0cc7ca87ba9624848c4160406f9d618a2ae025751f6c7bf7R601-R627

@ancorgs

This comment was marked as resolved.

doc/auto_storage.md Outdated Show resolved Hide resolved
doc/auto_storage.md Outdated Show resolved Hide resolved
"filesystem": "xfs",
"size": {
"min": "5 GiB",
"max": "20 GiB"
Copy link
Contributor

Choose a reason for hiding this comment

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

... which is illustrated here

@coveralls
Copy link

coveralls commented Aug 19, 2024

Coverage Status

coverage: 71.557%. remained the same
when pulling 9ddaa37 on joseivanlopez:storage-schema-basic
into 46f89e2 on openSUSE:master.

Copy link
Contributor

@ancorgs ancorgs left a comment

Choose a reason for hiding this comment

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

LGTM

@joseivanlopez joseivanlopez merged commit 10093da into agama-project:master Aug 27, 2024
1 check passed
joseivanlopez added a commit that referenced this pull request Aug 27, 2024
A JSON schema for the storage config is defined by
#1455. This PR adds support to the
D-Bus API to allow setting a JSON config following that new JSON schema:

~~~
org.opensuse.Agama.Storage1
  #SetConfig s
  #GetConfig s
~~~

## `#SetConfig`

For now, the `#SetConfig` D-Bus method admits 3 different JSON formats
for the given JSON config:

* Guided config
~~~
{ "storage": { "guided": ... } }
~~~

* New storage config
~~~
{ "storage": { ... } }
~~~

* Legacy AutoYaST config

~~~
{ "legacyAutoyastStorage": [ ... ] }
~~~

The plan is to make `#SetConfig` and `#GetConfig` only to work with a
*storage config* or a *legacy AutoYaST config*. The *guided config* is
still accepted by `#SetConfig` because the new *storage config* does not
offer all the options offered by *guided config* yet.
 
In the future, the *guided config* will be neither accepted nor reported
by `#SetConfig` and `#GetConfig` respectively. Nevertheless, a D-Bus
method accepting the *guided config* will still be offered as a
simplification of the *storage settings* to make easier to work with the
use cases supported by the web UI.

## `#GetConfig`

Currently, `#GetConfig` reports different config formats depending on
how the proposal was calculated:

* For the initial proposal: reports the *guided config*.
* If `#SetConfig` was called with *guided config*: reports *guided
config* (extended with mandatory volumes, etc).
* If `#SetConfig` was called with *storage config*: reports exactly the
given *storage config*.
* If `#SetConfig` was called with *legacy AutoYaST config*: reports
exactly the given *legacy AutoYaST config*.
* If `Storage1.Proposal#Calculate` was called: reports the *guided
config* (extended with mandatory volumes, etc).

In the future, `#GetConfig` will only report either the *storage config*
or the *legacy AutoYaST config*.
@imobachgs imobachgs mentioned this pull request Sep 20, 2024
imobachgs added a commit that referenced this pull request Sep 20, 2024
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