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.

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

WIP: Add support for (make check CHECK_BASE_IMAGES='fedora centos') #46

Closed
wants to merge 3 commits into from

Conversation

mtrmac
Copy link
Collaborator

@mtrmac mtrmac commented May 7, 2016

This is intended for use in Travis, modifying the make check command to the above (but running this locally would of course also be possible). The default behavior remains the same, uses the fedora image.

At the moment, this does not work because in centos, the python-pip package is not available (perhaps in EPEL?).

(Also note that the (yum install) does not fail when one of the packages is missing.)

Anyone, feel free to take this over (adding conditionals to Dockerfiles? Maintaining a Dockerfile.fedora and Dockerfile.centos separately?) if you have the time.

@mtrmac mtrmac force-pushed the travis-centos branch 2 times, most recently from fde2f02 to 708b043 Compare May 9, 2016 20:01
@mtrmac mtrmac force-pushed the travis-centos branch 3 times, most recently from c8000f9 to b146465 Compare May 16, 2016 20:31
@cgwalters
Copy link
Contributor

FYI, my work in CentOS CI on PR testing is now triggering for skopeo.

It chains into rpmdistro-gitoverlay (i.e. it's building the way we're shipping). See
https://github.com/cgwalters/sig-atomic-buildscripts/blob/ebd950bf686b54cfdca49dfca2a305a4363cc2cd/centos-ci/jjb/sig-atomic-testing.yml#L190

I'll post more about this when it's farther along.

It might be worth looking at how rpmdistro-gitoverlay works, as I'd like it to be a convenient tool for local development too. (It's a bit away from that now, but it also has fundamental architectural advantages over Dockerfile per repo)

@mtrmac mtrmac force-pushed the travis-centos branch 2 times, most recently from 8c52122 to 00c816b Compare May 25, 2016 19:02
@mtrmac mtrmac force-pushed the travis-centos branch 3 times, most recently from e1d9083 to 3abf591 Compare June 2, 2016 21:36
@mtrmac mtrmac changed the title WIP: BROKEN DO NOT MERGE: Add support for (make check CHECK_BASE_IMAGES='fedora centos') BROKEN DO NOT MERGE: Add support for (make check CHECK_BASE_IMAGES='fedora centos') Jun 2, 2016
@mtrmac mtrmac mentioned this pull request Jun 2, 2016
@mtrmac
Copy link
Collaborator Author

mtrmac commented Jun 2, 2016

OK, this now seems to work (but, man, the compile-test-debug-edit cycle with Dockerfiles is long); except that it correctly diagnoses #89 and breaks the build. So, trying with #91 to see whether that would be enough.

@mtrmac mtrmac force-pushed the travis-centos branch 2 times, most recently from 549dfcb to d074f04 Compare June 3, 2016 21:27
@mtrmac
Copy link
Collaborator Author

mtrmac commented Jun 3, 2016

Fixed up, everything seems to work per #91. Note that on CentOS we don't run golint because it does not build against Go 1.4.2.

I will keep rebasing this and #91 from time to time, so that we have a chance to know about CentOS failures before Colin finds out ☺

@mtrmac mtrmac mentioned this pull request Jun 3, 2016
@mtrmac mtrmac force-pushed the travis-centos branch 5 times, most recently from 796eb56 to 7039055 Compare June 14, 2016 14:09
@mtrmac mtrmac force-pushed the travis-centos branch 4 times, most recently from 881bd51 to 22e0e1c Compare June 27, 2016 18:54
@mtrmac mtrmac force-pushed the travis-centos branch 2 times, most recently from cea92cd to 892498e Compare July 4, 2016 08:49
@rhatdan
Copy link
Member

rhatdan commented Aug 3, 2020

3.5 years since this has been updated? Should we close?

@mtrmac
Copy link
Collaborator Author

mtrmac commented Aug 7, 2020

Last updated less than a month ago, actually.

I’d love for this to be integrated in some sort of CI (and for the CI not to spend >15 minutes building OpenShift 1.5 every single time), but this is still better than nothing.

This is intended for use in Travis (or perhaps also locally).  The
default behavior remains the same, uses the fedora image.

(Note that the (yum install) does not fail when one of the packages
is missing.  This is not a dealbreaker, but it will make diagnosing
future bugs more difficult.)

Uses CentOS 7 because CentOS 8 is missing some necessary -devel packages.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
CentOS, as of bats-0.4.0-1.20141016git3b33a5a.el7.noarch, misses the fix
for sstephenson/bats#89 , causing (read) not
to split words at white space.

Set IFS to the default value explicitly to work around this.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@github-actions
Copy link

github-actions bot commented Jun 6, 2021

A friendly reminder that this PR had no activity for 30 days.

@cevich
Copy link
Member

cevich commented Jul 26, 2021

IMHO this PR should indeed be closed, it's no-longer relevant nor applicable given the amount of changes on the CI side. Downstream consumers need to have their own environment-independent test-suites. If those can reasonably be shared with upstream (i.e. like how we have a separate system-tests package for podman) that's fine. But it's unworkable to hold-back upstream development based on downstream (i.e. necessarily "old") environment requirements.

@mtrmac
Copy link
Collaborator Author

mtrmac commented Jul 26, 2021

But it's unworkable to hold-back upstream development based on downstream (i.e. necessarily "old") environment requirements.

(c/image is, unlike Podman, simple and environment-independent enough that adding compatibility upstream seemed easier than maintaining old stable branches; the major issue was just noticing that we broke something, hence this PR.

OTOH that was much more of an issue when still targeting RHEL 7, and that’s no longer much of a concern.)


IMHO this PR should indeed be closed, it's no-longer relevant nor applicable given the amount of changes on the CI side.

Given the recent changes, and that this originally targeted RHEL 7, I agree; especially with the recent changes to build test base images separately, imposing RHEL 7 targets on that work doesn’t make sense any more.

@mtrmac mtrmac closed this Jul 26, 2021
@mtrmac mtrmac deleted the travis-centos branch July 26, 2021 20:18
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants