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

Bug 1759146: data/rhcos: fix base url to use correct baseURI #2462

Merged
merged 1 commit into from
Oct 7, 2019

Conversation

stbenjam
Copy link
Member

@stbenjam stbenjam commented Oct 7, 2019

The current baseURI is pointing at
releases-rhcos-art.cloud.privileged.psi.redhat.com, which is an internal
URL and signed by the corporate CA. This is breaking any 4.2 install that relies on downloading RHCOS (e.g. some UPI installs, baremetal IPI, etc), and the user is not on the VPN or is on the VPN and the particular server doesn't trust RH's corporate CA.

The current baseURI is pointing at
releases-rhcos-art.cloud.privileged.psi.redhat.com, which is an internal
URL and signed by the corporate CA. This is breaking most 4.2 installs.
@openshift-ci-robot
Copy link
Contributor

@stbenjam: This pull request references Bugzilla bug 1759146, which is invalid:

  • expected the bug to target the "4.2.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1759146: data/rhcos: fix base url to use correct baseURI

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 7, 2019
@stbenjam
Copy link
Member Author

stbenjam commented Oct 7, 2019

/bugzilla refresh

@openshift-ci-robot openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Oct 7, 2019
@openshift-ci-robot
Copy link
Contributor

@stbenjam: This pull request references Bugzilla bug 1759146, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Oct 7, 2019
@patrickdillon
Copy link
Contributor

/lgtm

needs an approver

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 7, 2019
@sdodson
Copy link
Member

sdodson commented Oct 7, 2019

/approve

@wking
Copy link
Member

wking commented Oct 7, 2019

Huh, looks like I picked a private URI for #2454, but got a public one for #2455. Not sure how that happened. We need blocking CI coverage to keep this from happening again.

cgwalters added a commit to cgwalters/installer that referenced this pull request Oct 7, 2019
Don't allow people to point to e.g. an RHT-internal endpoint.

See: openshift#2462
@cgwalters
Copy link
Member

We need blocking CI coverage to keep this from happening again.

#2463
isn't CI but should stop this from happening again.

@cgwalters
Copy link
Member

This is breaking most 4.2 installs.

How is that?

@openshift-ci-robot
Copy link
Contributor

@stbenjam: This pull request references Bugzilla bug 1759146, which is valid.

In response to this:

Bug 1759146: data/rhcos: fix base url to use correct baseURI

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@stbenjam
Copy link
Member Author

stbenjam commented Oct 7, 2019

This is breaking most 4.2 installs.

How is that?

I updated it to be be more specific: This is breaking any 4.2 install that relies on downloading RHCOS (e.g. UPI, baremetal IPI), and the user is not on the VPN or is on the VPN and the particular server doesn't trust RH's corporate CA. Any cloud providers we upload the images to in advance wouldn't be affected, so it's not really most installs.

Copy link
Member

@ashcrow ashcrow 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
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ashcrow, patrickdillon, sdodson, stbenjam

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

@stbenjam
Copy link
Member Author

stbenjam commented Oct 7, 2019

/test e2e-azure

@stbenjam
Copy link
Member Author

stbenjam commented Oct 7, 2019

@sdodson Tide is reporting that it also "Needs cherry-pick-approved label."

@stbenjam
Copy link
Member Author

stbenjam commented Oct 7, 2019

/test e2e-metal

@sdodson
Copy link
Member

sdodson commented Oct 7, 2019

/test e2e-vsphere-upi

@cgwalters
Copy link
Member

This is breaking any 4.2 install that relies on downloading RHCOS (e.g. UPI, baremetal IPI),

Not even that right, once https://mirror.openshift.com/pub/openshift-v4/dependencies/rhcos/pre-release/latest/ is updated?

@wking
Copy link
Member

wking commented Oct 7, 2019

/test e2e-openstack

OpenStack uses the base URI too.

@wking
Copy link
Member

wking commented Oct 7, 2019

But e2e-openstack is only in our master presubmits as well. Sigh...

@stbenjam
Copy link
Member Author

stbenjam commented Oct 7, 2019

This is breaking any 4.2 install that relies on downloading RHCOS (e.g. UPI, baremetal IPI),

Not even that right, once https://mirror.openshift.com/pub/openshift-v4/dependencies/rhcos/pre-release/latest/ is updated?

Not sure I understand, what is not right? We've always used releases-art-rhcos.svc.ci.openshift.org as baseURI in rhcos.json, even the release-4.1 branch is pointing there. In any case, releases-rhcos-art.cloud.privileged.psi.redhat.com is certainly incorrect.

For releases, something else must change rhcos.json to point to the production location on openshift.com?

@cgwalters
Copy link
Member

For releases, something else must change rhcos.json to point to the production location on openshift.com?

That doesn't exist today; see #1399 etc.

Not sure I understand, what is not right?

The PR is fine. What I am trying to say is that not all UPI installs depend on the metadata here. Clearly OpenStack is scripting against it today, but e.g. one can download the metal bootimages from mirror.openshift.com for UPI metal, etc.

Or to phrase it differently: This fixes OpenStack IPI (and possibly metalkube).

@openshift-ci-robot
Copy link
Contributor

@stbenjam: This pull request references Bugzilla bug 1759146, which is valid.

In response to this:

Bug 1759146: data/rhcos: fix base url to use correct baseURI

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@stbenjam
Copy link
Member Author

stbenjam commented Oct 7, 2019

But e2e-openstack is only in our master presubmits as well. Sigh...

We can trigger metal3 CI, at least. That's broken and should be fixed by this PR.

@stbenjam
Copy link
Member Author

stbenjam commented Oct 7, 2019

/label platform/baremetal

(Temporarily labeling this to trigger metal3 CI so we can see this fixes one of the broken platforms)

@openshift-ci-robot
Copy link
Contributor

@stbenjam: The label(s) /label platform/baremetal cannot be applied. These labels are supported: platform/aws, platform/azure, platform/baremetal, platform/google, platform/libvirt, platform/openstack, ga

In response to this:

/label platform/baremetal

(Temporarily labeling this to trigger metal3 CI so we can see this fixes one of the broken platforms)

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@stbenjam
Copy link
Member Author

stbenjam commented Oct 7, 2019

Well, that didn't work. I know @sdodson kicked off a cluster-bot install with vmware, we might have to rely on that.

@sdodson sdodson added the platform/baremetal IPI bare metal hosts platform label Oct 7, 2019
@stbenjam
Copy link
Member Author

stbenjam commented Oct 7, 2019

The PR is fine. What I am trying to say is that not all UPI installs depend on the metadata here. Clearly OpenStack is scripting against it today, but e.g. one can download the metal bootimages from mirror.openshift.com for UPI metal, etc.

Or to phrase it differently: This fixes OpenStack IPI (and possibly metalkube).

Ok got it, thanks. Based on what I saw from Slack, this affected at least baremetal UPI, baremetal IPI (metalkube), openstack IPI, and vmware UPI.

@openshift-ci-robot
Copy link
Contributor

@stbenjam: This pull request references Bugzilla bug 1759146, which is valid.

In response to this:

Bug 1759146: data/rhcos: fix base url to use correct baseURI

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@sdodson
Copy link
Member

sdodson commented Oct 7, 2019

/test e2e-vsphere

@stbenjam
Copy link
Member Author

stbenjam commented Oct 7, 2019

Metal3 CI is kicked off, but it's going to fail shortly. It always tries to apply a PR to master instead of the actual target (in this case release-4.2). I filed openshift-metal3/dev-scripts#822

@metal3ci
Copy link

metal3ci commented Oct 7, 2019

Build FAILURE, see build http://10.8.144.11:8080/job/dev-tools/1199/

@jwforres jwforres added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Oct 7, 2019
@openshift-merge-robot openshift-merge-robot merged commit b5aebb5 into openshift:release-4.2 Oct 7, 2019
@openshift-ci-robot
Copy link
Contributor

@stbenjam: All pull requests linked via external trackers have merged. Bugzilla bug 1759146 has been moved to the MODIFIED state.

In response to this:

Bug 1759146: data/rhcos: fix base url to use correct baseURI

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@stbenjam stbenjam deleted the fix-rhcos branch October 7, 2019 19:06
jhixson74 pushed a commit to jhixson74/installer that referenced this pull request Dec 6, 2019
Don't allow people to point to e.g. an RHT-internal endpoint.

See: openshift#2462
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. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. lgtm Indicates that a PR is ready to be merged. platform/baremetal IPI bare metal hosts platform 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.

10 participants