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

Add Dockerfiles.centos7 and job-id files for CentOS image building #9864

Merged

Conversation

tdawson
Copy link
Member

@tdawson tdawson commented Jul 14, 2016

These files should not interfere with any non-CentOS work.
They make it possible to build OpenShift Origin images in CentOS's enviroment.

@sdodson
Copy link
Member

sdodson commented Jul 15, 2016

Is there anyway that we can merge .centos7 and .rhel7 files in some way to keep from having 3 Dockerfiles for each image? Or at least we should make sure that the two differ as little as possible other than the top level base image.

@tdawson
Copy link
Member Author

tdawson commented Jul 15, 2016

@sdodson the .rhel7 and .centos7 have different image names. If anything, we might be able to whittle this down to only have Dockerfile.centos7 images when it varies from the main Dockerfile.

@mohammedzee1000 could the CentOS image build system check to see if there is a Dockerfile.centos7 file and use that, otherwise use the plain Dockerfile?

@brenton
Copy link
Contributor

brenton commented Jul 15, 2016

LGTM. @smarterclayton, are you OK with these new .centos7 Dockerfiles?

@smarterclayton
Copy link
Contributor

Yes, ok with them but I want to make sure someone is on the hook to test them and keep them current

@tdawson
Copy link
Member Author

tdawson commented Jul 15, 2016

That would be me and/or @mohammedzee1000.
I'm already doing a Monday, Wed, Fri check to see if the normal Dockerfiles change, so I am the logical choice for now.

@kbsingh
Copy link

kbsingh commented Jul 15, 2016

@tdawson For builds that dont need a custom / changed Dockerfile, just skip adding in the .centos7 ones, and we can directly reference the Dockerfile there.

@tdawson
Copy link
Member Author

tdawson commented Jul 15, 2016

@kbsingh That sounds like a plan, that will save alot of space and work. I'll update this pull request with the duplicate Dockerfiles removed.

@tdawson tdawson force-pushed the 2016-07-dockerfile.centos-1.3 branch from a9889d1 to 5c53722 Compare July 15, 2016 20:53
@tdawson
Copy link
Member Author

tdawson commented Jul 15, 2016

That looks better.

@tdawson
Copy link
Member Author

tdawson commented Jul 15, 2016

Does anyone mind giving this a merge?


COPY scripts/* /usr/local/bin/

RUN INSTALL_PKGS="origin-sdn-ovs libmnl libnetfilter_conntrack openvswitch \
Copy link
Contributor

Choose a reason for hiding this comment

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

How big is origin-SDN-OVS? If nothing else changes I would just put this in Openshift/origin and that's one less image

Copy link
Member

Choose a reason for hiding this comment

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

origin-sdn-ovs itself is a few KiB, but it pulls in openvswitch and all of its dependencies. openshift3/node is 689.1MB, openshift/ose is 483.1MB.

Copy link
Contributor

Choose a reason for hiding this comment

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

@tdawson tdawson force-pushed the 2016-07-dockerfile.centos-1.3 branch 2 times, most recently from d1c9bcf to 943e562 Compare July 18, 2016 20:05
@tdawson
Copy link
Member Author

tdawson commented Jul 18, 2016

Changed the released Dockerfile to ensure that bsdtar is installed.

@@ -15,7 +15,7 @@ ENV VERSION=1.6 \
ENV PATH=$PATH:$GOROOT/bin:$GOPATH/bin

RUN mkdir $TMPDIR && \
INSTALL_PKGS="make gcc zip mercurial krb5-devel" && \
Copy link
Contributor

Choose a reason for hiding this comment

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

Changes to the release dockerfile need to be replicated to all the specific sub versions.

@tdawson tdawson force-pushed the 2016-07-dockerfile.centos-1.3 branch 2 times, most recently from 8db071b to 80b8bc8 Compare July 18, 2016 21:54
@tdawson
Copy link
Member Author

tdawson commented Jul 18, 2016

All release/Dockerfile and release/*/Dockerfile ensure that bsdtar is installed.
removed enabling centosplus repo from custom-docker-builder, enabling me to remove the Dockerfile.centos7 from there.

@smarterclayton
Copy link
Contributor

[test]

@mohammedzee1000
Copy link
Contributor

@tdawson Hey, yes there is an entry in the index called target-file, where you can specify a filename, and if that is not specified, it falls back to using Dockerfile.

@tdawson
Copy link
Member Author

tdawson commented Jul 19, 2016

[test]

@tdawson
Copy link
Member Author

tdawson commented Jul 19, 2016

Those failures look like some type of networking issue ... testing again.

@tdawson
Copy link
Member Author

tdawson commented Jul 19, 2016

[test]

@tdawson
Copy link
Member Author

tdawson commented Jul 19, 2016

Looking at other jobs, it looks like all tests (not just this pull request) are failing today on test_pull_requests_origin_integration and test_pull_requests_origin_conformance

@smarterclayton
Copy link
Contributor

Yes, AWS is blowed up.

On Tue, Jul 19, 2016 at 3:43 PM, Troy Dawson notifications@github.com
wrote:

Looking at other jobs, it looks like all tests (not just this pull
request) are failing today on test_pull_requests_origin_integration and
test_pull_requests_origin_conformance


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#9864 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABG_p2TKCRx0RB73DjPoT1M_y2Bh7d8Mks5qXSjygaJpZM4JM3e8
.

@tdawson
Copy link
Member Author

tdawson commented Jul 20, 2016

[test]

1 similar comment
@tdawson
Copy link
Member Author

tdawson commented Jul 21, 2016

[test]

@tdawson
Copy link
Member Author

tdawson commented Jul 21, 2016

I really cannot tell if the test failures are because of this pull request or not. It keeps failling in the same section, but it seems to have a different failure each time.
Can someone look at the failures to determine if we've broken anything.
If not, can we get a merge?

@smarterclayton
Copy link
Contributor

It's not broken, but this is behind another flake reducing PR we're trying
to land. Please stand by

On Jul 21, 2016, at 5:36 PM, Troy Dawson notifications@github.com wrote:

I really cannot tell if the test failures are because of this pull request
or not. It keeps failling in the same section, but it seems to have a
different failure each time.
Can someone look at the failures to determine if we've broken anything.
If not, can we get a merge?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#9864 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABG_p3TYv0FufH8Y2c4VvMp5l6fkx4jZks5qX-ZLgaJpZM4JM3e8
.

@smarterclayton
Copy link
Contributor

Ok, the other PR merged - be aware that openvswitch changed its from image.

On Thu, Jul 21, 2016 at 9:07 PM, Clayton Coleman ccoleman@redhat.com
wrote:

It's not broken, but this is behind another flake reducing PR we're trying
to land. Please stand by

On Jul 21, 2016, at 5:36 PM, Troy Dawson notifications@github.com wrote:

I really cannot tell if the test failures are because of this pull request
or not. It keeps failling in the same section, but it seems to have a
different failure each time.
Can someone look at the failures to determine if we've broken anything.
If not, can we get a merge?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#9864 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABG_p3TYv0FufH8Y2c4VvMp5l6fkx4jZks5qX-ZLgaJpZM4JM3e8
.

Add CentOS job-id files

Add Dockerfile.centos7 files

Remove Dockerfile.centos7 files when they are the same as Dockerfile

Put relavent centos changes into plain Dockerfiles

Ensure bsdtar is installed

We no longer need centosplus enabled

Remove Dockerfile.centos7 for openvswitch
@tdawson tdawson force-pushed the 2016-07-dockerfile.centos-1.3 branch from 80b8bc8 to 987b601 Compare July 22, 2016 13:21
@tdawson
Copy link
Member Author

tdawson commented Jul 22, 2016

Awesome, looks like one less Dockerfile.centos7 file needed.

@tdawson
Copy link
Member Author

tdawson commented Jul 26, 2016

[test]

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 987b601

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6927/)

@tdawson
Copy link
Member Author

tdawson commented Jul 26, 2016

I think we've taken care of all the problems, and the tests have finally passed. Can we get a merge while the tests are still working?

@tdawson
Copy link
Member Author

tdawson commented Jul 27, 2016

[merge]

@openshift-bot
Copy link
Contributor

openshift-bot commented Jul 27, 2016

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6989/) (Image: devenv-rhel7_4685)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 987b601

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.

8 participants