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.

Sign up for GitHub

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

Jump to bottom

Update autoinstall-reference.rst #1933

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Update autoinstall-reference.rst #1933

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
367069f
Update autoinstall-reference.rst
ben-ba Mar 11, 2024
d0012b7
switching order of arches (opt) and uri (req)
Mar 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions doc/reference/autoinstall-reference.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,9 @@ The default is:
primary:
- country-mirror
- arches: [i386, amd64]
uri: "http://archive.ubuntu.com/ubuntu"
- uri: "http://archive.ubuntu.com/ubuntu"
Copy link
Member

Choose a reason for hiding this comment

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

The change does not look right to me. The key called "primary" expects an array of elements, where each element can either be "country-mirror" or an object with a URI + optionally a set of architectures if applies to.

If we translate the YAML into JSON, we have:

primary:
  - arches: [i386, amd64]
    uri: http://archive.ubuntu.com/ubuntu

equivalent to

{
   "primary": [
        {
            "arches": ["i386", "amd64"],
            "uri": "http://archive.ubuntu.com/ubuntu"
        }
    ]
}

and

primary:
  - arches: [i386, amd64]
  - uri: "http://archive.ubuntu.com/ubuntu"

equivalent to

{
   "primary": [
        {
            "arches": ["i386", "amd64"]
        }, {
            "uri": "http://archive.ubuntu.com/ubuntu"
        }
    ]
}

Copy link
Author

@ben-ba ben-ba Mar 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

  1. if you scroll down in the documentation you will see the uri already with an hyphen. -> https://canonical-subiquity.readthedocs-hosted.com/en/latest/reference/autoinstall-reference.html#geoip
  2. If i use arches + uri the hyphen for uri isn't necessary, but if i use only uri, the error mentioned get thrown.

my user-data looks like following for apt;
working one

apt:
    mirror-selection:
      primary:
         - uri: "http://archive.ubuntu.com/ubuntu/"
    fallback: offline-install

failed one, error see 1 comment

apt:
    mirror-selection:
      primary:
         uri: "http://archive.ubuntu.com/ubuntu/"
    fallback: offline-install

Copy link
Author

@ben-ba ben-ba Mar 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

got it;

primary is an array of country-mirror and an object with two properties; uri and arches

so each list needs an hyphen, so when uri is needed, the documentation should switch to the following;

why? with the following example in the docuentation u couldn't get in trouble if you only use the necessary property of the list.

primary:
          - country-mirror
          - uri: "http://archive.ubuntu.com/ubuntu"
            arches: [i386, amd64]

Copy link
Contributor

Choose a reason for hiding this comment

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

if you scroll down in the documentation you will see the uri already with an hyphen.

Right, the issue in the proposed change is having arches on its own line with a hyphen.

- uri: my_uri
  arches: [amd64]

and

- arches: [amd64]
  uri: my_uri

are semantically equivalent. The primary key should be an array of items. Either containing the string country-mirror or an object which has a string property uri and (optionally) an array of arches arches which list the supported architectures for that uri.

The reason your second example fails is because it's missing the hyphen. In the following example:

primary:
    uri: "http://archive.ubuntu.com/ubuntu/"

primary is an object with some string property uri. Whereas in:

  primary:
     - uri: "http://archive.ubuntu.com/ubuntu/"

primary is an array of objects with one item. Where that one item has a string property uri.

Copy link
Contributor

Choose a reason for hiding this comment

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

I hadn't refreshed and didn't see your comment, apologies! I also had to look at the code for this one. Perhaps we should update the docs to better reflect what's expected.

Copy link
Member

Choose a reason for hiding this comment

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

@ben-ba I'd be happy to accept this as a doc improvement. Would you like to update this PR with the switched arches / uri statements?

Copy link
Member

@ogayot ogayot Mar 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We'd also need you to sign the CLA before merging the change. We have some instructions on how to do so:

https://github.com/canonical/subiquity/blob/main/CONTRIBUTING.md#contributor-license-agreement

Copy link
Author

Choose a reason for hiding this comment

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

PR updated, CLA signed.

Copy link
Author

@ben-ba ben-ba Mar 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I would be really helpfully to read something about the validation of a autoinstall configuration with
./scripts/validate-autoinstall-user-data.py ../autoinstall/simple/user-data

Complaining is to easy, so i made an PR for this.
#1941

- arches: [s390x, arm64, armhf, powerpc, ppc64el, riscv64]
uri: "http://ports.ubuntu.com/ubuntu-ports"
- uri: "http://ports.ubuntu.com/ubuntu-ports"
fallback: abort
geoip: true

Expand Down
Loading