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 openstack beta repos mirror #4835

Merged

Conversation

elfosardo
Copy link
Contributor

In order to be able to properly test latest features of ironic
and related services, we need to enable the beta openstack
repos with the latest content available.

In order to be able to properly test latest features of ironic
and related services, we need to enable the beta openstack
repos with the latest content available.
@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 23, 2019
Copy link
Contributor

@juliakreger juliakreger left a comment

Choose a reason for hiding this comment

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

LGTM, and aligns both image build processes. Thanks!

@juliakreger
Copy link
Contributor

@smarterclayton Any chance we can get eyes on this? This merging would at least allow CI images to be testable fully with the correct versions matching downstream.

Makefile Outdated
@@ -179,6 +179,10 @@ prow-ocp-rpm-secrets:
--from-file=cluster/test-deploy/gcp/ops-mirror.pem \
--from-file=ci-operator/infra/openshift/release-controller/repos/ocp-4.2-openstack.repo \
-o yaml --dry-run | oc apply -n ocp -f -
oc create secret generic openstack-beta-rhel8-4-2-repos \
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not consistent with our naming system. You would call this openstack-beta or openstack-next. We are trying to avoid endless variants with inscrutable names for the repos - your name should capture the reason the mirrors exist, not what content they contain. Are you trying to test these within 4.2 (which is now code freeze)? Are these going to be used by optional 4.2 or 4.3 jobs? Mixing "beta" and "4.2" doesn't really make sense for default jobs.

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like the naming standard can be debated, after seeing some of the other repository configurations. Regardless, I agree openstack-beta makes the most sense. Riccardo went ahead and gave me access to edit his branch so I should be able to revise this without any issues.

Ultimately, the desire is to be able to build CI jobs and people do testing with the upstream CI images, at least as I understand it.

The side-effect of this is that no 4.2 images will
ever be built with the correct RPMs as the the original
configuration that was supplied in the upstream image
build process pointed to the prior release's software.

Performing as a separate change so the prior configuration
can be recovered and reviewed as opposed to force-push
overwrite.
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 28, 2019
@elfosardo
Copy link
Contributor Author

@smarterclayton hi! Anything we could do to move forward with this ?

@smarterclayton
Copy link
Contributor

/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: elfosardo, juliakreger, smarterclayton

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

The pull request process is described here

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-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 9, 2019
@openshift-merge-robot openshift-merge-robot merged commit 6a65f1c into openshift:master Sep 9, 2019
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/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants