Skip to content

src/cmdlib.sh: Allow use of a mirrors for getting the installer ISO.#335

Merged
miabbott merged 1 commit intocoreos:masterfrom
darkmuggle:mirror_iso
Feb 12, 2019
Merged

src/cmdlib.sh: Allow use of a mirrors for getting the installer ISO.#335
miabbott merged 1 commit intocoreos:masterfrom
darkmuggle:mirror_iso

Conversation

@darkmuggle
Copy link
Contributor

Allow use of a mirror for the installer image.

@dustymabe
Copy link
Member

not sure I quite understand. https://download.fedoraproject.org will send you to a mirror. https://dl.fedoraproject.org is the URL that won't.

@cgwalters
Copy link
Member

I think is more about using a local/internal mirror.

INSTALLER_CHECKSUM=https://download.fedoraproject.org/pub/$repository_dir/releases/$release/Everything/$arch/iso/Fedora-Everything-$release-1.2-$arch-CHECKSUM
INSTALLER_MIRROR="${INSTALLER_MIRROR:-https://download.fedoraproject.org}"
INSTALLER_URL="${INSTALLER_MIRROR}/pub/$repository_dir/releases/$release/Everything/$arch/iso"
INSTALLER="${INSTALLER_URL}/Fedora-Everything-netinst-$arch-$release-1.2.iso"
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

yeah if the intent here is to use F28 instead of F29 then ti's not going to work because f28 went out on the first beta candidate (1.1) where f29 went out on the 2nd beta candidate (1.2) so there is still a difference in the filename.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intent on the PR was not to change the static filenames; I would consider that a separate PR. My intent was to allow a user-specified mirror/location for the bits.

Adding a lookup would be easy enough:

suffix=""
case $release in
    27) echo 'srly?'; suffix=1.6;;
    28) suffix=1.1;;
    29) suffix=1.2;;
      *) echo "unkown release"; exit 10;;
esac

But given that our stated intent is to remove the need for an installer URL, it seems unnecessary.

@darkmuggle
Copy link
Contributor Author

I think is more about using a local/internal mirror.

Correct. The intent is to allow someone to pick a local mirror.

@cgwalters
Copy link
Member

We should probably accept a URL to a treeinfo file: https://dl.fedoraproject.org/pub/fedora/linux/releases/29/Everything/x86_64/os/.treeinfo

And then parse that.

@dustymabe
Copy link
Member

since our goal is to get rid of anaconda and we already have the INSTALLER_OVERRIDE vars is it worth perfecting this?

@miabbott
Copy link
Member

miabbott commented Feb 7, 2019

since our goal is to get rid of anaconda and we already have the INSTALLER_OVERRIDE vars is it worth perfecting this?

Probably not. But if this change helps the RHCOS pipeline, I'm OK with merging it.

@dustymabe
Copy link
Member

But if this change helps the RHCOS pipeline, I'm OK with merging it.

+1 if it helps.. I maybe just need some more context.

@darkmuggle
Copy link
Contributor Author

darkmuggle commented Feb 8, 2019

Ugh, I have been heads down and missed this.

The short-story is the when running COSA behind the Red Hat firewall, download.fedoraproject.org is dreadfully slow. Having the ability to switch to a known internal URL would make it faster. Right now we do something like this:

        def builder_iso = "http://download.devel.redhat.com/released/F-28/GOLD/Everything/x86_64/iso/Fedora-Everything-netinst-x86_64-28-1.1.iso";
        def builder_checksum = "http://download.devel.redhat.com/released/F-28/GOLD/Everything/x86_64/iso/Fedora-Everything-28-1.1-x86_64-CHECKSUM";

        stage('Fetch ISO') {
            sh_wrap("""
            mkdir -p installer
            (cd installer && curl -L -O ${builder_iso} ${builder_checksum} )
            """);
        }

With INSTALLER_MIRROR we can just add it as an envVar during the init stage.

@dustymabe
Copy link
Member

so the additional sugar this PR adds is that we have to only define one env var vs two?

INSTALLER_MIRROR=http://download.devel.redhat.com

vs

INSTALLER_URL_OVERRIDE=http://download.devel.redhat.com/released/F-28/GOLD/Everything/x86_64/iso/Fedora-Everything-netinst-x86_64-28-1.1.iso
INSTALLER_CHECKSUM_URL_OVERRIDE=http://download.devel.redhat.com/released/F-28/GOLD/Everything/x86_64/iso/Fedora-Everything-netinst-x86_64-28-1.1.iso

?

@darkmuggle
Copy link
Contributor Author

darkmuggle commented Feb 8, 2019 via email

Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

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

If tests look good I'm 👍

@miabbott miabbott merged commit 0bec0ca into coreos:master Feb 12, 2019
@dustymabe
Copy link
Member

hit an issue with a build I was doing today:

awk: fatal: cannot open file `/srv/installer/Fedora-Everything-netinst-x86_64-29-1.2-CHECKSUM' for reading (No such file or directory)
+ rc=2
+ set +x
[dustymabe@media fedora-assembler]$ ls /srv/
[dustymabe@media fedora-assembler]$ ls installer/
Fedora-Everything-29-1.2-x86_64-CHECKSUM  Fedora-Everything-netinst-x86_64-29-1.2.iso

Looks like it might be related to this change.

dustymabe added a commit that referenced this pull request Feb 12, 2019
Revert "Merge pull request #335 from darkmuggle/mirror_iso"
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.

4 participants

Comments