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

c9s: Add distribution-gpg-keys rpm from EPEL #1032

Closed
wants to merge 1 commit into from

Conversation

LorbusChris
Copy link
Member

This is required for the extensions build running inside the base OS container via COSA as otherwise the GPG key files are not available there.

This is required for the extensions build running inside the base OS
container via COSA as otherwise the GPG key files are not available there.
LorbusChris added a commit to LorbusChris/okd-cosa-pipeline that referenced this pull request Oct 21, 2022
With the inclusion of the `distribution-gpg-keys` rpm package in SCOS
in openshift/os#1032,
it is no longer required to fetch them from the internet.
LorbusChris added a commit to LorbusChris/okd-cosa-pipeline that referenced this pull request Oct 21, 2022
With the inclusion of the `distribution-gpg-keys` rpm package in SCOS
in openshift/os#1032,
it is no longer required to fetch them from the internet.
Copy link
Member

@travier travier 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 openshift-ci bot 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 Oct 21, 2022
@travier
Copy link
Member

travier commented Oct 21, 2022

But I'm wondering, which package from EPEL are we using? Aren't those key available in the C9S repos?

@LorbusChris
Copy link
Member Author

The distribution-gpg-keys rpm is not available in C9S itself unfortunately (https://kojihub.stream.centos.org/koji/search?match=glob&type=package&terms=distribution-gpg-keys). We only use the EPEL repo to get that one package.

We currently have this workaround in place: https://github.com/okd-project/okd-coreos-pipeline/blob/581420489cb09616c5b2225869d6e96995712c75/manifests/tekton/tasks/base/cosa-init.yaml#L64-L65

See also #1006

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD de8da21 and 2 for PR HEAD 0d88252 in total

@cgwalters
Copy link
Member

If we were to do this only in RHCOS/C9S, it'd create another divergence from FCOS - i.e. we should consider this also for FCOS if we were to do this. Oh wait, we're actually doing this only for C9S, which means just that stream diverges a bit.

On one hand, the idea of adding distribution-gpg-keys actually aligns with the conceptual shift of *CoreOS to be a base image. It's certainly convenient to have these keys there.

It's a bit unfortunate that this package is in EPEL, because enabling the EPEL repos by default is a whole other world of stuff.

All this said, which actual keys are you needing? And for what packages? I think we talked about this before, but can you remind me? Do you have a link to a PR/code that needs this?

@cgwalters
Copy link
Member

Oh I see, sorry you did link it here. OK, right and I'm paging back in the context from #1006

(Ohhh wow and I see https://github.com/okd-project/okd-coreos-pipeline/blob/581420489cb09616c5b2225869d6e96995712c75/manifests/tekton/tasks/base/cosa-init.yaml#L80 - so it begins...

I think honestly we are all under a bit of confusion as to who builds scos and how/where and with what config. okd-project/okd-coreos-pipeline@e7fc6d8 seems exactly like the kind of thing that conceptually should have been in this repository right? )

Anyways um...but wait is the problem that you're getting the c9s.repo file in this git repo into the extensions container build? Why isn't it using the .repo files from centos-release?

@LorbusChris
Copy link
Member Author

(Ohhh wow and I see https://github.com/okd-project/okd-coreos-pipeline/blob/581420489cb09616c5b2225869d6e96995712c75/manifests/tekton/tasks/base/cosa-init.yaml#L80 - so it begins...

indeed 😂 😭

I think honestly we are all under a bit of confusion as to who builds scos and how/where and with what config. okd-project/okd-coreos-pipeline@e7fc6d8 seems exactly like the kind of thing that conceptually should have been in this repository right? )

The plan has always been to involve the CoreOS team as much as possible, but it was made quite clear that the there are limited resources to work on the OKD pipeline and that the team had to focus on building RHCOS 9. Also we needed (and still need) something that does not require an /ok-to-test for external folks. Having the pipeline independent from the manifests is good I think, even though it'd probably be better to pin packages in a manifest properly (could be in a fork, which is then PR'd here). The repo to build from is actually a parameter of the pipeline, so building from a forked repo is trivial.

Anyways um...but wait is the problem that you're getting the c9s.repo file in this git repo into the extensions container build? Why isn't it using the .repo files from centos-release?

Yes that's exactly the problem. Everything is copied into the container to build the extensions, which is required to get the manifests etc in there (

ADD . .
). Is there even a way to tell rpm-ostree compose to not use the .repo file(s) in the dir it's invoked from?

@LorbusChris
Copy link
Member Author

On a side note: The extensions build regularly breaks for us for so far unknown reasons. I think it would be clearer to do the extensions build in the same env as the base build instead of in a nested container.

@LorbusChris
Copy link
Member Author

/retest

@LorbusChris
Copy link
Member Author

/test scos-9-build-test-qemu

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 21, 2022

@LorbusChris: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/scos-9-build-test-qemu 0d88252 link true /test scos-9-build-test-qemu
ci/prow/scos-9-build-test-metal 0d88252 link true /test scos-9-build-test-metal

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

@travier
Copy link
Member

travier commented Oct 27, 2022

Is this because the builds run in RHCOS? https://github.com/openshift/os/blob/master/extensions/Dockerfile#L2
You should override the FROM line with the SCOS OCI image instead.

Edit: Re-reading #1006, and I'm confused. Can we set some time to talk about this synchronously?

@travier
Copy link
Member

travier commented Oct 27, 2022

/lgtm cancel

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 27, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 27, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: LorbusChris
Once this PR has been reviewed and has the lgtm label, please ask for approval from travier by writing /assign @travier in a comment. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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 openshift-ci bot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 27, 2022
@cgwalters
Copy link
Member

Is this because the builds run in RHCOS? https://github.com/openshift/os/blob/master/extensions/Dockerfile#L2
You should override the FROM line with the SCOS OCI image instead.

Yeah, cosa build-extensions-container does this automatically. But the messy thing here is that cosa is all about generating base images, whereas in the new world we want OKD to derive from FCOS. (And we also want OCP-RHCOS to drive from pure RHEL I think)

Though at at even higher level, to me again the extensions container is just a crutch - anyone who wants to inject code into the host can do it on their own in a fully "container native" way, although it does take over OS updates.

@LorbusChris
Copy link
Member Author

LorbusChris commented Oct 27, 2022

The problem is that the extensions build runs inside an SCOS base container (with the FROM directive being replaced automatically by COSA as Colin mentioned), and that very SCOS base does not have the GPG keys at the locations specified in c9s.repo. The distribution-gpg-keys RPM does ship them at those locations, so adding it to the base SCOS compose solves this problem.

I agree that the extensions container is a crutch (and the nested way in which its currently built, too). We're not currently shipping it with OKD/SCOS. However if we wanted to do that, we'd need the metadata that cosa build-extensions-container generates, so we couldn't just do the extensions container build with podman/without COSA.

Happy to talk about this synchronously, however next week I'll only be available on Wednesday or Friday (due to a public holiday on Monday, travels on Tuesday, and the Mini DevConf.cz in Brno on Thursday 🙂 ).

Edit: Also, I don't think the Prow SCOS CI build failures that we are seeing on this PR are caused by this RPM addition.

@travier
Copy link
Member

travier commented Oct 28, 2022

Ah OK, now I think I understand. I don't really like adding a dependency on EPEL. Could the SCOS pipeline do a sed on the c9s repo config files to fix the path before starting the build-extensions-container step? We could also ship symlinks in the image which is also fine for me.

@LorbusChris
Copy link
Member Author

@travier we currently hack around this with sed in the pipeline, which we'd like to avoid.
I think including this RPM is the most straightforward way to solve this.
EPEL isn't an ideal source, but we can work on getting distribution-gpg-keys packaged for c9s properly as a follow-up.

LorbusChris added a commit to LorbusChris/okd-cosa-pipeline that referenced this pull request Nov 17, 2022
With the inclusion of the `distribution-gpg-keys` rpm package in SCOS
in openshift/os#1032,
it is no longer required to fetch them from the internet.
@LorbusChris
Copy link
Member Author

@travier please see the comment above. Can we merge this?

@cgwalters
Copy link
Member

One thing I could imagine doing here is teaching the dnf stack to automatically look in /etc/pki/rpm-gpg/$x if the repo file references /usr/share/distribution-gpg-keys/$x and it doesn't exist.

This issue is annoying I agree because otherwise I think pretty much everything else is set up to work in both src != target mode (how we build the "base image" with cosa) and src == target (extensions container).

Perhaps for now, we could carry the sed invocation inside the extensions container build, or do it automatically in coreos-assembler?

@cgwalters
Copy link
Member

It's worth noting here that this isn't biting FCOS as it doesn't ship an extensions container today.

@travier
Copy link
Member

travier commented Jan 25, 2023

Alternative in #1116

travier added a commit to travier/coreos-assembler that referenced this pull request Jan 25, 2023
Make a "local" copy of distribution GPG keys from COSA to use them
inside the container build as we may not have all keys there or they may
not be in the same place.

See: openshift/os#1032
See: openshift/os#1116
@travier
Copy link
Member

travier commented Jan 25, 2023

Better option in #1118

@travier
Copy link
Member

travier commented Jan 25, 2023

/close

@openshift-ci openshift-ci bot closed this Jan 25, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 25, 2023

@travier: Closed this PR.

In response to this:

/close

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.

LorbusChris added a commit to LorbusChris/okd-cosa-pipeline that referenced this pull request Feb 20, 2023
With the inclusion of the `distribution-gpg-keys` rpm package in SCOS
in openshift/os#1032,
it is no longer required to fetch them from the internet.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants