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

Start using Centos 8 as base #100

Merged
merged 1 commit into from
Jan 9, 2020
Merged

Start using Centos 8 as base #100

merged 1 commit into from
Jan 9, 2020

Conversation

elfosardo
Copy link
Member

Start testing images with Centos 8 as base.

@metal3-io-bot metal3-io-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 7, 2019
@dtantsur
Copy link
Member

dtantsur commented Jan 3, 2020

There seem to be CentOS 8 packages for Train now: https://trunk.rdoproject.org/centos8-train/status_report.html. Time to continue?

dtantsur
dtantsur reviewed Jan 3, 2020
View reviewed changes
Dockerfile Outdated

RUN yum install -y python-requests && \
curl https://raw.githubusercontent.com/openstack/tripleo-repos/master/tripleo_repos/main.py | python - -b stein current-tripleo && \
RUN yum install -y python3 python3-requests && \
Copy link
Member

Choose a reason for hiding this comment

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

Do you really need to install python3 explicitly?

Copy link
Member Author

Choose a reason for hiding this comment

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

it was necessary until last time :/

dtantsur
dtantsur reviewed Jan 3, 2020
View reviewed changes
Dockerfile Outdated
RUN yum install -y python-requests && \
curl https://raw.githubusercontent.com/openstack/tripleo-repos/master/tripleo_repos/main.py | python - -b stein current-tripleo && \
RUN yum install -y python3 python3-requests && \
curl https://raw.githubusercontent.com/openstack/tripleo-repos/master/tripleo_repos/main.py | python3 - -b train current-tripleo && \
yum update -y && \
yum install -y openstack-ironic-api openstack-ironic-conductor crudini \
iproute iptables dnsmasq httpd qemu-img-ev iscsi-initiator-utils parted gdisk ipxe-bootimgs psmisc sysvinit-tools \
mariadb-server python2-chardet genisoimage && \
Copy link
Member

Choose a reason for hiding this comment

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

python3-chardet?

Copy link
Member Author

Choose a reason for hiding this comment

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

this has been already corrected in a different patch

@elfosardo elfosardo changed the title [WIP] start using Centos 8 as base Start using Centos 8 as base Jan 3, 2020
@metal3-io-bot metal3-io-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 3, 2020
@elfosardo elfosardo force-pushed the use-centos8 branch 3 times, most recently from 4b97584 to f4ff74f Compare January 3, 2020 17:05
@metal3-io-bot metal3-io-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 3, 2020
@elfosardo
Copy link
Member Author

@dtantsur python interpreter is not installed by default in Centos 8 base images, so we need to explicitly install it

dtantsur
dtantsur approved these changes Jan 7, 2020
View reviewed changes
Copy link
Member

@dtantsur dtantsur left a comment

Choose a reason for hiding this comment

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

Looks good. Did you try running it?

Also do you plan on switching the builder as well?

@metal3-io-bot metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 7, 2020
dtantsur
dtantsur reviewed Jan 7, 2020
View reviewed changes
Dockerfile
sysvinit-tools mariadb-server genisoimage python-ironic-prometheus-exporter \
python-jinja2 && \
yum clean all && \
RUN dnf install -y python3 python3-requests && \
Copy link
Member

Choose a reason for hiding this comment

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

Installing python3-requests is supposed to pull in python 3, I wonder why it's not the case..

Copy link
Member Author

Choose a reason for hiding this comment

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

this is "fun", I was also expecting that but apparently python3-requests doesn't care about python3!
The only dependencies are python3-chardet, python3-idna, python3-pysocks, python3-urllib3

@elfosardo
Copy link
Member Author

elfosardo commented Jan 7, 2020 •
edited

@dtantsur I tested the builder conversion to centos 8 but there is an issue when building the ESP image:

grub2-mkimage: error: cannot open `/usr/lib/grub/x86_64-efi/linuxefi.mod': No such file or directory

despite having the grub2-efi-x64-modules package.
I did a brief reserach and I can see that the package has /usr/lib/grub/x86_64-efi/linux.mod but not the efi one.
I believe removing linuxefi should be safe, but I'd rather keep centos7 as base for the builder for the time being and have a separate discussion on this.

@elfosardo
Copy link
Member Author

elfosardo commented Jan 8, 2020 •
edited

@dtantsur I tested this locally and it seems working fine
I did a deeper research about migrating the builder and I can confirm we can remove linuxefi as it's not part of grub2 upstream, it's just distribution specific, and I don't really think we need it.
I have the final change ready, if we want to remove it, but I'd like also @juliakreger opinion on this before.

dtantsur
dtantsur approved these changes Jan 8, 2020
View reviewed changes
Copy link
Member

@dtantsur dtantsur left a comment

Choose a reason for hiding this comment

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

Okay, let's merge this first and deal with the builder later. I'm keeping this PR open to let others have a look.

@dtantsur dtantsur mentioned this pull request Jan 8, 2020
Merged
dtantsur
dtantsur approved these changes Jan 9, 2020
View reviewed changes
Copy link
Member

@dtantsur dtantsur left a comment

Choose a reason for hiding this comment

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

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 9, 2020
@metal3-io-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dtantsur, elfosardo

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

@dtantsur dtantsur mentioned this pull request Jan 9, 2020
Closed
@metal3-io-bot metal3-io-bot merged commit c7eaddc into metal3-io:master Jan 9, 2020
elfosardo pushed a commit to elfosardo/ironic-image that referenced this pull request Aug 28, 2020
@openshift-merge-robot
Merge pull request metal3-io#100 from elfosardo/pkgs-list
a01f344 
Install packages from file content
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dtantsur dtantsur dtantsur approved these changes

Assignees

@dtantsur dtantsur

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.

None yet
3 participants
@elfosardo @dtantsur @metal3-io-bot