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

dind: fix token race, enable GENEVE UDP port, and update OVN repo #16311

Merged
merged 1 commit into from
Sep 15, 2017

Conversation

dcbw
Copy link
Contributor

@dcbw dcbw commented Sep 12, 2017

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Sep 12, 2017
@openshift-merge-robot openshift-merge-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 12, 2017
Copy link
Contributor

@rajatchopra rajatchopra left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 12, 2017
@bparees
Copy link
Contributor

bparees commented Sep 12, 2017

/unassign

@@ -31,7 +31,7 @@ RUN dnf -y update && dnf -y install\
# with the newer release. (This can go away when the base image is upgraded to
# OVS 2.8 prerelease or release versions and include OVN sub-packages)
RUN dnf -y install dnf-plugins-core &&\
dnf -y copr enable leifmadsen/ovs-master &&\
dnf -y copr enable danw/origin-dind-ovs-master &&\
Copy link
Contributor

Choose a reason for hiding this comment

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

What OVS do you need that's not in the CentOS PaaS SIG repos we're using elsewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something relatively recent; OVS 2.8 (for some conntrack stuff) + OVN sub-packages (for OVN obviously). We need to wait for F26 to get moved to OVN 2.8, and then we'll get both 2.8 and the OVN sub-packages when we switch the DIND images to F26 base.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, and the DIND images are based on Fedora, not CentOS, so...

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, let me move the build from danw/origin-dind-ovs-master over to danw/origin-dind-ovs and we can use that. I don't plan to keep that COPR up to date with OVS master so it's just confusing to have "master" in the name.

We need to wait for F26 to get moved to OVS 2.8

I'm not sure there's any plan to do that... I think we're waiting for F27 at this point. (This OVS build is a rebuild of the current rawhide RPM.)

Oh, and the DIND images are based on Fedora, not CentOS, so...

FWIW I have a branch that switches dind over to CentOS (a spinoff of the old ansible-install-into-dind work). It makes things slightly less convenient since some packages you might want for debugging aren't available on CentOS (and even with EPEL you're still getting much older versions of things). But maybe it's a net improvement because it's more like real OCP environments?

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, sounds good. We have had stability issues with COPRs in the past, so I just want to make sure we are not inviting flakiness in with this approach.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, let me move the build from danw/origin-dind-ovs-master over to danw/origin-dind-ovs and we can use that.

(this is done)

@dcbw
Copy link
Contributor Author

dcbw commented Sep 12, 2017

/test integration issue #16312

@dcbw
Copy link
Contributor Author

dcbw commented Sep 12, 2017

yay imagestream breakage! /test extended_conformance_gce

@openshift-merge-robot openshift-merge-robot removed the lgtm Indicates that a PR is ready to be merged. label Sep 13, 2017
@@ -31,7 +31,7 @@ RUN dnf -y update && dnf -y install\
# with the newer release. (This can go away when the base image is upgraded to
# OVS 2.8 prerelease or release versions and include OVN sub-packages)
RUN dnf -y install dnf-plugins-core &&\
dnf -y copr enable leifmadsen/ovs-master &&\
dnf -y copr enable danw/origin-dind-ovs-master &&\
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, let me move the build from danw/origin-dind-ovs-master over to danw/origin-dind-ovs and we can use that. I don't plan to keep that COPR up to date with OVS master so it's just confusing to have "master" in the name.

We need to wait for F26 to get moved to OVS 2.8

I'm not sure there's any plan to do that... I think we're waiting for F27 at this point. (This OVS build is a rebuild of the current rawhide RPM.)

Oh, and the DIND images are based on Fedora, not CentOS, so...

FWIW I have a branch that switches dind over to CentOS (a spinoff of the old ansible-install-into-dind work). It makes things slightly less convenient since some packages you might want for debugging aren't available on CentOS (and even with EPEL you're still getting much older versions of things). But maybe it's a net improvement because it's more like real OCP environments?

@dcbw
Copy link
Contributor Author

dcbw commented Sep 13, 2017

@danwinship updated the COPR repo path

@danwinship
Copy link
Contributor

/lgtm
/retest

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 14, 2017
@openshift-merge-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danwinship, dcbw, rajatchopra

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

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@rajatchopra
Copy link
Contributor

/test extended_conformance_install_update

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot
Copy link
Contributor

Automatic merge from submit-queue

@openshift-merge-robot openshift-merge-robot merged commit 87ceff2 into openshift:master Sep 15, 2017
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.

9 participants