Skip to content

Conversation

@chancez
Copy link
Contributor

@chancez chancez commented Aug 9, 2019

Ensure yum install fails if a package is missing.

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Aug 9, 2019
@chancez chancez force-pushed the skip_missing_on_install_fail branch from 2dac391 to d1a1afb Compare August 9, 2019 21:13
yum install -y --setopt=tsflags=nodocs ${INSTALL_PKGS} && \
yum clean all && rm -rf /var/cache/*
yum clean all && rm -rf /var/cache/* && \
echo 'skip_missing_names_on_install=0' >> /etc/yum.conf
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need a leading newline?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not that I saw when I tested in the container manually.

@chancez
Copy link
Contributor Author

chancez commented Aug 9, 2019

/retest

@sdodson
Copy link
Member

sdodson commented Aug 12, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 12, 2019
Copy link

@pecameron pecameron left a comment

Choose a reason for hiding this comment

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

Do we need to set the yum.conf file before the yum install?

@sdodson
Copy link
Member

sdodson commented Aug 12, 2019

/lgtm cancel

Do we need to set the yum.conf file before the yum install?

Good idea, I think the main goal was to fix downstream images but I think reordering things here as well makes sense.

@openshift-ci-robot openshift-ci-robot removed lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 12, 2019
base/Dockerfile Outdated
yum clean all && \
mkdir -p /var/lib/origin
mkdir -p /var/lib/origin && \
echo 'skip_missing_names_on_install=0' >> /etc/yum.conf
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah move this up and then nuke rpm INSTALL_PKGS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good

Ensure yum install fails if a package is missing.
@chancez chancez force-pushed the skip_missing_on_install_fail branch from d1a1afb to 56fc2fe Compare August 12, 2019 17:14
@chancez
Copy link
Contributor Author

chancez commented Aug 12, 2019

Updated to move the config option before yum install.

@openshift-ci-robot openshift-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 12, 2019
@sdodson
Copy link
Member

sdodson commented Aug 12, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 12, 2019
@smarterclayton
Copy link
Contributor

/lgtm

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chancez, sdodson, smarterclayton

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

The pull request process is described here

Details 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

@openshift-merge-robot openshift-merge-robot merged commit 1be922d into openshift:master Aug 12, 2019
@chancez chancez deleted the skip_missing_on_install_fail branch August 12, 2019 19:51
hardys pushed a commit to hardys/ironic-image that referenced this pull request Aug 20, 2019
This is a dependency of the ironic packages (via the python2-oslo-db
package) already, and copying this dependency to the downstream el8
based dockerfile has caused issues since[1] made some of the image
building checks stricter, I assume this is because the package name
is different but I don't have a specfile reference atm.

This has led to [2] failing CI, and rather than just fix that downstream
we should probably make things consistent and remove this spuruious
install, then let RPM dependencies defined by the package maintainers
do the right thing on each platform?

[1] openshift/images#11
[2] openshift#15
hardys pushed a commit to hardys/ironic-image that referenced this pull request Aug 20, 2019
This is a dependency of the ironic packages (via the python2-oslo-db
package) already, and copying this dependency to the downstream el8
based dockerfile has caused issues since[1] made some of the image
building checks stricter, I assume this is because the package name
is different but I don't have a specfile reference atm.

This has led to [2] failing CI, and rather than just fix that downstream
we should probably make things consistent and remove this spuruious
install, then let RPM dependencies defined by the package maintainers
do the right thing on each platform?

[1] openshift/images#11
[2] openshift#15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants