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

Initial support for testing with a live-iso #579

Merged
merged 2 commits into from
Dec 25, 2020
Merged

Initial support for testing with a live-iso #579

merged 2 commits into from
Dec 25, 2020

Conversation

hardys
Copy link
Member

@hardys hardys commented Dec 18, 2020

Add initial support for downloading an FCOS iso, related to
development of metal3-io/metal3-docs#150 and testing of metal3-io/baremetal-operator#754

@metal3-io-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hardys

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@metal3-io-bot metal3-io-bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 18, 2020
@hardys hardys mentioned this pull request Dec 18, 2020
Closed
@metal3-io-bot metal3-io-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 18, 2020
@hardys
Copy link
Member Author

hardys commented Dec 18, 2020

/test-integration
/test-centos-integration

@fmuyassarov
Copy link
Member

Ubuntu integration job is failing when running kubeadm init. It somehow could be related to the latest Docker version that was released 2 days ago. So, if the job fails on this PR, it might be due the common issue. Will update here once we have fixed it.

@fmuyassarov
Copy link
Member

CI should be back to normal now. Triggering integration tests.
/test-integration
/test-centos-integration

fmuyassarov
fmuyassarov reviewed Dec 20, 2020
View reviewed changes
lib/images.sh
export IMAGE_LOCATION=${IMAGE_LOCATION:-https://builds.coreos.fedoraproject.org/prod/streams/testing/builds/32.20201104.3.0/x86_64/}
elif [[ "${IMAGE_OS}" == "FCOS-ISO" ]]; then
export IMAGE_NAME=${IMAGE_NAME:-fedora-coreos-33.20201201.2.1-live.x86_64.iso}
export IMAGE_LOCATION=${IMAGE_LOCATION:-https://builds.coreos.fedoraproject.org/prod/streams/testing/builds/33.20201201.2.1/x86_64}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export IMAGE_LOCATION=${IMAGE_LOCATION:-https://builds.coreos.fedoraproject.org/prod/streams/testing/builds/33.20201201.2.1/x86_64}
export IMAGE_LOCATION=${IMAGE_LOCATION:-https://builds.coreos.fedoraproject.org/prod/streams/testing/builds/33.20201201.2.1/x86_64/}

Copy link
Member Author

Choose a reason for hiding this comment

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

@fmuyassarov thanks for the suggestion, but I don't think the trailing slash is needed:

./01_prepare_host.sh:    curl --insecure --compressed -O -L "${IMAGE_LOCATION}/${IMAGE_NAME}"

It's also used inconsistently, so we should perhaps consider removing it for existing images in a separate PR?

Copy link
Member

Choose a reason for hiding this comment

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

You are right.
+1 for removing extra slash in another PR.

lib/images.sh Outdated
Comment on lines 9 to 10
export IMAGE_NAME=${IMAGE_NAME:-fedora-coreos-32.20201104.3.0-openstack.x86_64.qcow2.xz}
export IMAGE_LOCATION=${IMAGE_LOCATION:-https://builds.coreos.fedoraproject.org/prod/streams/testing/builds/32.20201104.3.0/x86_64/}
Copy link
Member

Choose a reason for hiding this comment

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

Are we missing something in the image path ?

$ wget https://builds.coreos.fedoraproject.org/prod/streams/testing/builds/32.20201104.3.0/x86_64/fedora-coreos-32.20201104.3.0-openstack.x86_64.qcow2.xz

--2020-12-20 13:14:27--  https://builds.coreos.fedoraproject.org/prod/streams/testing/builds/32.20201104.3.0/x86_64/fedora-coreos-32.20201104.3.0-openstack.x86_64.qcow2.xz
Resolving builds.coreos.fedoraproject.org (builds.coreos.fedoraproject.org)... 205.251.219.79, 205.251.219.37, 205.251.219.127, ...
Connecting to builds.coreos.fedoraproject.org (builds.coreos.fedoraproject.org)|205.251.219.79|:443... connected.
HTTP request sent, awaiting response... 403 Forbidden
2020-12-20 13:14:28 ERROR 403: Forbidden.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah thanks, I think the update to the openstack image was a mistake, now reverted to the previous version and we can perhaps update it in a future commit.

Steven Hardy added 2 commits December 21, 2020 10:24
Initial support for testing with a live-iso
d1d6bc7 
Add initial support for downloading an FCOS iso, related to
development of metal3-io/metal3-docs#150
Catch image download errors
92d2869
@metal3-io-bot metal3-io-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 21, 2020
@fmuyassarov
Copy link
Member

/test-integration
/test-centos-integration

@fmuyassarov
Copy link
Member

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 25, 2020
@metal3-io-bot metal3-io-bot merged commit 2d0cd26 into metal3-io:master Dec 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fmuyassarov fmuyassarov fmuyassarov left review comments

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hardys @metal3-io-bot @fmuyassarov