OCPBUGS-52363: ci/get-ocp-repo.sh: Fixes for scos to accomodate building images in CI#1757
Conversation
6cccb01 to
57cefb4
Compare
| # on SCOS, we need to add the GPG keys of the various SIGs we need - same as what is done for extensions | ||
| RUN if rpm -q centos-stream-release && ! rpm -q centos-release-cloud; then dnf install -y centos-release-{cloud,nfv,virt}-common; fi | ||
| RUN mkdir -p /usr/share/distribution-gpg-keys/centos | ||
| RUN ln -s /etc/pki/rpm-gpg/RPM-GPG-KEY-centosofficial /usr/share/distribution-gpg-keys/centos/RPM-GPG-KEY-CentOS-Official | ||
| RUN ln -s {/etc/pki/rpm-gpg,/usr/share/distribution-gpg-keys/centos}/RPM-GPG-KEY-CentOS-SIG-Cloud | ||
| RUN ln -s {/etc/pki/rpm-gpg,/usr/share/distribution-gpg-keys/centos}/RPM-GPG-KEY-CentOS-SIG-Extras-SHA512 | ||
| RUN ln -s {/etc/pki/rpm-gpg,/usr/share/distribution-gpg-keys/centos}/RPM-GPG-KEY-CentOS-SIG-NFV | ||
| RUN ln -s {/etc/pki/rpm-gpg,/usr/share/distribution-gpg-keys/centos}/RPM-GPG-KEY-CentOS-SIG-Virtualization |
There was a problem hiding this comment.
Hmm, I think this hints at a larger issue though, which is that we don't actually want the repo definitions fetched by get-ocp-repo.sh to stay in the final node image. For RHCOS/OCP, it doesn't matter too much because get-ocp-repo.sh is only used in the CI path. That's not the case for OKD AIUI. (And even for RHCOS, ideally we do clean it up.)
So I think we need to rework this so that we only use those repos for the dnf install, and then remove them. I guess we should probably also uninstall the SIG packages too? If the repos don't actually ship by default, then it seems odd to still ship the keys for it.
OK how about:
- enhance
get-ocp-repo.shto install the centos-release-* packages and do the symlink hack - remove these
RUNcommands here and in the extensions build since it's already done inget-ocp-repo.sh, which they both call - add a e.g.
get-ocp-repo.sh --cleanup, which will:- remove any repo files it added
- remove any packages it installed
- call
get-ocp-repo.sh --cleanupafterapply-manifestbut before thefindto call. we don't have to do this in the extensions build, since it's a multi-stage build anyway
There was a problem hiding this comment.
That's not the case for OKD AIUI.
Ah right. I was doing it locally with the --secret id=yumrepos which was why it was getting cleaned up. That's not the case when building with prow
I guess we should probably also uninstall the SIG packages too? If the repos don't actually ship by default, then it seems odd to still ship the keys for it.
Yes, we could uninstall it and keep it clean, but I thought having those was benign, and anyway, they are public.
I like the suggestion of pulling these into get-ocp-repo though . I'll work on those changes.
There was a problem hiding this comment.
(And even for RHCOS, ideally we do clean it up.)
Actually, we really want it there too. Someone just filed https://issues.redhat.com/browse/OCPBUGS-52363 which this now fixes! (I think the related issue there is that our sed isn't handling space before/after the = as it occurs in the mirror repo definitions, but we can fix that separately.)
There was a problem hiding this comment.
fixed the sed line in a separate commit. the PR is ready if you want to give it a look!
9965ca7 to
326db48
Compare
|
@Prashanth684: This pull request references Jira Issue OCPBUGS-52363, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@jlebon: This pull request references Jira Issue OCPBUGS-52363, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
jlebon
left a comment
There was a problem hiding this comment.
Some comments, but LGTM overall. Thanks for working on this!
| info "Neutering RHEL repos for SCOS" | ||
| awk '/server-ose/,/^$/' "$repo_path" > "$repo_path.tmp" | ||
| # only pull in certain Openshift packages as the rest come from the c9s repo | ||
| sed -i '/^baseurl = /a includepkgs=openshift-* ose-aws-ecr-* ose-azure-acr-* ose-gcp-gcr-*' "$repo_path.tmp" |
There was a problem hiding this comment.
Do we need this? What else comes from the plashet without it?
Just want to avoid having to maintain a list here.
There was a problem hiding this comment.
yes - we want cri-o, cri-tools, conmon, openvswitch to come from the sig-cloud and the Centos Stream openvswitch repos and not from the plashet.
There was a problem hiding this comment.
in the longterm we want to target even building the kubelet, openshift-clients and the platform related packages through sig-cloud so we can eliminate the need for the plashet completely.
There was a problem hiding this comment.
in the longterm we want to target even building the kubelet, openshift-clients and the platform related packages through sig-cloud so we can eliminate the need for the plashet completely.
Yes, agree.
But I mean more, are those other packages frequently higher versioned in the plashet than in the SIG repos?
There was a problem hiding this comment.
the versions differ, but not by much, except for openvswitch and ovn - we cannot use the one from plashets (they come from FDP repos)
There was a problem hiding this comment.
I think ideally this list would live elsewhere. E.g. as another repo definition in openshift/release that's really just the same as the plashet, but has the includepkgs= inlined. We do something similar also for the kernel: openshift/release#61361
The advantage of that is that anything that wants to reuse these repo definitions doesn't have to redefine this stuff.
There was a problem hiding this comment.
oh yeah that's an idea..we could put the repo there and have a service to access similar to the rhel ones. especially once we start building the images through prow. Although every release we would need to bump the sig-cloud repos, which is fine as we have to do that in this repo too anyway
| if [ -n "$ocp_manifest" ]; then | ||
| workdir=$(dirname "$ocp_manifest") | ||
| fi | ||
| cat "$workdir/c9s.repo" >> "$repo_path.tmp" |
There was a problem hiding this comment.
This is OK, though I think at least baseos and appstream will be duplicates with the builtin ones.
Actually, is there a package we could install that has the SIG repo definitions?
You can imagine once this repo is split in two and the base image is defined elsewhere, we wouldn't necessarily have the c9s.repo file here.
There was a problem hiding this comment.
i'd have to look into that - you are correct that the base and appstream are present, but nothing else. installing these repos: centos-release-{cloud,nfv,virt}-common, do add some of the nfv and rt repos, but I didn't find an equivalent for sig-cloud or openvswitch.
- For scos, we need the rhel-9.x-server-ose repo to get the Openshift releated binaries (kubelet, oc..) - Append the c9s.repo so the packages for the rest of the images come from the CentOS Stream repos. - Install centos stream packages so GPG keys are available when accessing the CentOS Stream repos
|
@Prashanth684: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
|
/approve Thanks! |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jlebon, Prashanth684 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@Prashanth684: Jira Issue OCPBUGS-52363: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-52363 has been moved to the MODIFIED state. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
Once we have this merged, I am going to try to have config changes similar to the rhel-coreos config to build and promote scos images. This will be the substitute for the mass open cloud pipeline through which we build our images today and will eliminate having us maintain and run it.