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 hack/build-dind-images.sh #12192

Merged
merged 1 commit into from
Dec 15, 2016

Conversation

marun
Copy link
Contributor

@marun marun commented Dec 8, 2016

Building the dind images as part of a ci job is a frequent source of flakes caused by package installation failure via dnf. This change ensures the dind images are built by hack/build-base-images.sh so they can be baked into the ami.

Intended to help with #11452.

cc: @stevekuznetsov @openshift/networking

@stevekuznetsov
Copy link
Contributor

Seems reasonable to me - this change will still result in them being built by the tests if they are not present, right? There will be a momentary disruption until we can get a new base_ami built.

@marun
Copy link
Contributor Author

marun commented Dec 8, 2016

The images will still be built by the test runner. I'm counting on the test runner build being a no-op when the ami contains the correct built image, but if it changes it should rebuild.

@openshift-bot
Copy link
Contributor

[Test]ing while waiting on the merge queue

@dcbw
Copy link
Contributor

dcbw commented Dec 10, 2016

@marun any idea what the DNF failures are? I get them often locally too, and it seems like either or both of (a) systemctl restart docker or (b) removing the previous image eventually work. Can we get better logs out of the docker build?

@dcbw
Copy link
Contributor

dcbw commented Dec 10, 2016

Jenkins failure is:

--> COPY openshift-generate-node-config.sh /usr/local/bin/
error: API error (500): {"message":"Error processing tar file(exit status 1): open /usr/local/bin/openshift-generate-node-config.sh: not a directory"}
[ERROR] PID 3189: hack/build-base-images.sh:11: `oc ex dockerbuild /data/src/github.com/openshift/origin/images/dind/node openshift/dind-node` exited with status 1.

@marun
Copy link
Contributor Author

marun commented Dec 10, 2016

@dcbw afaict the dnf failures are mirror related and I don't know how local configuration could affect that.

Where are you seeing that jenkins failure? I would expect that kind of problem to be deterministic, and I can't replicate that locally. Are you able to reproduce?

@marun marun force-pushed the add-dind-to-base-images branch 2 times, most recently from bff6d50 to 331215d Compare December 13, 2016 19:31
@marun
Copy link
Contributor Author

marun commented Dec 13, 2016

I'm assuming the failure around /usr/local/bin not being a directory was because /usr/local/bin did not exist and the first COPY in the dind Dockerfile didn't have a trailing slash. I've fixed that, let's see if it solves the problem.

@marun marun force-pushed the add-dind-to-base-images branch from 331215d to adf8aaf Compare December 13, 2016 21:15
@marun
Copy link
Contributor Author

marun commented Dec 13, 2016

Updated to reflect @smarterclayton's recent changes.

@marun marun force-pushed the add-dind-to-base-images branch from adf8aaf to 8a38db2 Compare December 13, 2016 21:16
@marun
Copy link
Contributor Author

marun commented Dec 13, 2016

@stevekuznetsov looks good!

@smarterclayton
Copy link
Contributor

I'm not in favor of building these on every build. Why do we need them?

@smarterclayton
Copy link
Contributor

I'd be open to building these via a different script in the base ami.

@marun
Copy link
Contributor Author

marun commented Dec 14, 2016

@smarterclayton: I'm not familiar with how the ami is built so I don't know how to accomplish what you're suggesting. Is it as simple as adding another script and modifying a jenkins job to target it?

@smarterclayton
Copy link
Contributor

Probably. I'd just prefer to keep anything except the truly core images out of base since that's always built. hack/build-dind-images.sh is fine for now until we sort out what our image build strategy will be (we're going to scrap most of the current dockerfiles and replace them with package installed images eventually).

Building the dind images as part of a ci job is a frequent source of
flakes caused by transient failures during package installation.  This
change ensures the dind images can be built via
hack/build-dind-images.sh to make it easier to bake them into the ami.
@marun marun force-pushed the add-dind-to-base-images branch from 8a38db2 to 365fce0 Compare December 14, 2016 03:41
@marun marun changed the title Ensure hack/build-base-images.sh builds dind images Add hack/build-dind-images.sh Dec 14, 2016
@marun
Copy link
Contributor Author

marun commented Dec 14, 2016

Ok, adding a new script instead of modifying build-base-images.sh.

@stevekuznetsov
Copy link
Contributor

@smarterclayton are you sure? AFAICT the build-base-images step is supposed to happen in AMI creation and build-images actually in the jobs. Is that no longer the case? If not, what's the point of build-base-images?

@smarterclayton
Copy link
Contributor

smarterclayton commented Dec 14, 2016 via email

@marun
Copy link
Contributor Author

marun commented Dec 14, 2016

flake #12072 re-[test]

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 365fce0

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/12387/) (Base Commit: e9e7618)

@stevekuznetsov
Copy link
Contributor

@marun You'll need to add a call of your script here to get the AMIs working with it.

[merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 365fce0

@openshift-bot
Copy link
Contributor

openshift-bot commented Dec 14, 2016

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/12387/) (Base Commit: 039e433) (Image: devenv-rhel7_5549)

@openshift-bot openshift-bot merged commit 1001d01 into openshift:master Dec 15, 2016
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.

5 participants