Skip to content

Conversation

@tnozicka
Copy link
Contributor

@tnozicka tnozicka commented Aug 7, 2019

Those RPMs have moved into oc repo. We are now trying to switch ART team to use them but we shouldn't build from 2 places at once.

hyperkube needs to stay for providing kubelet to the base image. (

yumdownloader --enablerepo=built --destdir=/tmp/rpms openshift-hyperkube openshift-clients && \
)

/cc @smarterclayton @eparis @deads2k @soltysh

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 7, 2019
@tnozicka
Copy link
Contributor Author

tnozicka commented Aug 7, 2019

/hold
(for a sync with https://jira.coreos.com/browse/ART-844) when this is tagged)

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 7, 2019
@tnozicka
Copy link
Contributor Author

tnozicka commented Aug 7, 2019

/retest

1 similar comment
@tnozicka
Copy link
Contributor Author

tnozicka commented Aug 8, 2019

/retest

@tnozicka
Copy link
Contributor Author

tnozicka commented Aug 8, 2019

rpm repos timeouts for other packages
/retest

@tnozicka
Copy link
Contributor Author

tnozicka commented Aug 8, 2019

/retest

2 similar comments
@tnozicka
Copy link
Contributor Author

tnozicka commented Aug 8, 2019

/retest

@tnozicka
Copy link
Contributor Author

tnozicka commented Aug 9, 2019

/retest

@tnozicka
Copy link
Contributor Author

tnozicka commented Aug 9, 2019

https://mirror.umd.edu/fedora/linux/updates/29/Modular/x86_64/repodata/5ddd47c03de6cd5e4b1d102c96f23f7e794c404611c49820745047dfbfbd4320-primary.xml.gz: [Errno 14] HTTPS Error 404 - Not Found
Trying other mirror.
No Match for argument openshift-clients
++ find /tmp/rpms/ -name 'openshift-*' -iname '*.rpm'

sounds like we don't support cross repo rpms

@tnozicka tnozicka changed the title Remove oc rpms Bug 1739445: Remove oc rpms Aug 9, 2019
@openshift-ci-robot
Copy link

@tnozicka: This pull request references a valid Bugzilla bug. 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.

Details

In response to this:

Bug 1739445: Remove oc rpms

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 the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Aug 9, 2019
@tnozicka
Copy link
Contributor Author

tnozicka commented Aug 9, 2019

It is failing on missing openshift-clients package - not sure what to do to make it pull a promoted RPM from oc repo like images do... @smarterclayton ideas?

@tnozicka
Copy link
Contributor Author

tnozicka commented Aug 9, 2019

/cc @stevekuznetsov

Copy link
Contributor

@stevekuznetsov stevekuznetsov left a comment

Choose a reason for hiding this comment

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

What is the chain of dependencies here? What is the base image? Which repo are you trying to use?

mkdir /tmp/working && cd /tmp/working && \
yumdownloader --enablerepo=built --destdir=/tmp/rpms openshift-hyperkube openshift-clients && \
PACKAGES=(openshift-hyperkube openshift-clients) && \
yumdownloader --enablerepo=built --destdir=/tmp/rpms ${PACKAGES[@]} && \
Copy link
Contributor

Choose a reason for hiding this comment

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

If you are using [@] you must use double quotes or it's useless.

@stevekuznetsov
Copy link
Contributor

If you're trying to install RPMs built in a different repo from the ephemeral RPM server, then no, that does not work.

@tnozicka
Copy link
Contributor Author

If you're trying to install RPMs built in a different repo from the ephemeral RPM server, then no, that does not work.

Sounds like a blocker for 4.2; given oc lives in openshift/oc and it needs to be installed into machine-os image build from origin, we need a way to promote those as we do for images. Can DPTP team help support promoting RPMs?

@stevekuznetsov
Copy link
Contributor

Can you build machine-os-content from openshift/oc?

@stevekuznetsov
Copy link
Contributor

Or, we could move openshift/oc back into this repo.

@deads2k
Copy link
Contributor

deads2k commented Aug 19, 2019

Or, we could move openshift/oc back into this repo.

No. That is an anti-goal. The two are separate and upstream is pushing them even further apart.

Can you build machine-os-content from openshift/oc?

What does machine-os-content mean?

How about we simply eliminate the Requires of clients? There's not actual need for a dependency between them.

@tnozicka
Copy link
Contributor Author

/retest

@tnozicka
Copy link
Contributor Author

/test images

@tnozicka
Copy link
Contributor Author

/retest

@tnozicka
Copy link
Contributor Author

talked to @smarterclayton and we can drop the oc from the machine-os-image as nothing in CI uses it and ART will install oc there

Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 22, 2019
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Aug 22, 2019
Copy link
Contributor

@soltysh soltysh 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 Aug 22, 2019
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: soltysh, tnozicka

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

The pull request process is described here

Details 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

@tnozicka
Copy link
Contributor Author

/test e2e-aws

@tnozicka
Copy link
Contributor Author

test-cmd should be fixed by #23649
/test e2e-cmd

@soltysh
Copy link
Contributor

soltysh commented Aug 22, 2019

/retest

@sosiouxme
Copy link
Member

At this point, if an image needs an oc client, we would prefer copying it over from ose-cli image rather than picking it up as an RPM.

That doesn't really matter here, though, as we still need to produce a client RPM, it just needs to come from a different location.

@soltysh
Copy link
Contributor

soltysh commented Aug 22, 2019

That doesn't really matter here, though, as we still need to produce a client RPM, it just needs to come from a different location.

@sosiouxme not quite, aside from the binary RPM must deliver man pages, I don't think we put those in the image, although maybe we should. @tnozicka can you check if we do and update our image to have it? That'd be
a) handy for users to have man there
b) handy for art

@sosiouxme
Copy link
Member

We actually had a bug open about oc not having man pages - I think we passed it on, but it is certainly desirable for oc to have man pages wherever it might be used by a human.

@tnozicka
Copy link
Contributor Author

/retest

@tnozicka
Copy link
Contributor Author

I am not sure about copying it out from the image. What if the man pages or some of the shared libraries differ? We link differently for RHEL, more dynamically and with special libraries. If we need to install those into both RHEL 7 and 8 those libraries and man pages could differ and we have the CLI image build just for one of them, so copying it out might not be the safest option.

@tnozicka
Copy link
Contributor Author

/retest

@tnozicka
Copy link
Contributor Author

ok, this is ready to be unhold when ART switches the oc RPM to be built from oc repo

@tnozicka
Copy link
Contributor Author

https://jira.coreos.com/browse/ART-844 is done
/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 28, 2019
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-merge-robot openshift-merge-robot merged commit b9b9b93 into openshift:master Aug 29, 2019
@openshift-ci-robot
Copy link

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

Details

In response to this:

Bug 1739445: Remove oc rpms

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.

@tnozicka tnozicka deleted the remove-oc-rpms branch August 29, 2019 05:43
@tnozicka tnozicka mentioned this pull request Aug 29, 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. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. 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.

8 participants