Skip to content

Add support for copying yum repos from separate git repo#3036

Merged
jlebon merged 4 commits intocoreos:mainfrom
saqibali-2k:pr/yumrepos
Aug 25, 2022
Merged

Add support for copying yum repos from separate git repo#3036
jlebon merged 4 commits intocoreos:mainfrom
saqibali-2k:pr/yumrepos

Conversation

@saqibali-2k
Copy link
Copy Markdown
Contributor

@saqibali-2k saqibali-2k commented Aug 11, 2022

Add a new option --yumrepos option to cosa init to allow import of
yum repo files and content sets located outside of the source git.

Motivation: RHCOS needs yum repo definitions hidden behind a firewall.
Let's add a clean way to import them.

Co-authored-by: Jonathan Lebon jonathan@jlebon.com

@saqibali-2k saqibali-2k force-pushed the pr/yumrepos branch 2 times, most recently from 3daa435 to ffe6314 Compare August 15, 2022 13:17
Comment thread src/cmd-init Outdated
Comment thread src/cmd-init Outdated
Comment thread src/cmdlib.sh Outdated
Comment thread src/cmdlib.sh Outdated
Comment thread src/cmdlib.sh Outdated
@cgwalters
Copy link
Copy Markdown
Member

Hmm. I'm OK with this, but IMO it also cuts against the "elegance" of the current architecture. For example, suddenly pull requests don't quite work because there are two git repositories involved. We're also today injecting metadata about the config git repository into the base container image. But that loses some meaning with two git repositories.

Related to that topic, this PR isn't injecting any metadata from (e.g. exact git rev) the "extconfig" git repo into the cosa metadata.

@jlebon
Copy link
Copy Markdown
Member

jlebon commented Aug 15, 2022

Hmm. I'm OK with this, but IMO it also cuts against the "elegance" of the current architecture. For example, suddenly pull requests don't quite work because there are two git repositories involved. We're also today injecting metadata about the config git repository into the base container image. But that loses some meaning with two git repositories.

The use case for this is RHCOS, which sadly already suffers from this issue. The idea with this PR is essentially upstreaming and formalizing the cp hack that currently exists in the RHCOS pipeline into cosa.

Related to that topic, this PR isn't injecting any metadata from (e.g. exact git rev) the "extconfig" git repo into the cosa metadata.

👍 Yeah we should add that (and is actually another benefit of formalizing it in cosa directly).

@cgwalters
Copy link
Copy Markdown
Member

Note: if we have lockfiles, then this extconfig repo degrades basically to a mirrorlist.

Comment thread src/cmdlib.sh Outdated
@jlebon jlebon force-pushed the pr/yumrepos branch 2 times, most recently from e0c4e8b to ad651aa Compare August 23, 2022 21:49
@jlebon jlebon changed the title src/cmd-init: add a way to import protected .repo files and content sets Add support for copying yum repos from separate git repo Aug 23, 2022
@jlebon
Copy link
Copy Markdown
Member

jlebon commented Aug 23, 2022

Updated this now but still testing it locally!

@cgwalters
Copy link
Copy Markdown
Member

CI failure looks legit

@jlebon jlebon force-pushed the pr/yumrepos branch 2 times, most recently from 699515c to 536974d Compare August 24, 2022 16:10
@jlebon
Copy link
Copy Markdown
Member

jlebon commented Aug 24, 2022

Rebased and now tested locally!

jlebon added 4 commits August 24, 2022 14:09
Make the `tarball` argument last. Prep for making it optional.
For the upcoming yumrepos, we don't want to publish a tarball of it. We
still want the git rev information though. Make the tarball argument
optional.
Add a new option `--yumrepos` option to `cosa init` to allow import of
yum repo files and content sets located outside of the source git.

Motivation: RHCOS needs yum repo definitions hidden behind a firewall.
Let's add a clean way to import them.

Co-authored-by: Jonathan Lebon <jonathan@jlebon.com>
Comment thread src/cmdlib.sh
Comment thread src/cmd-build
Comment thread src/cmdlib.sh
@travier
Copy link
Copy Markdown
Member

travier commented Aug 25, 2022

I think we should coordinate the changes in this one with those from #2934

Comment thread src/cmdlib.sh
# each included dir.
# https://github.com/projectatomic/rpm-ostree/issues/1628
cp "${workdir}"/src/config/*.repo "${tmp_overridesdir}"/
find "${configdir}" -name '*.repo' -exec cp -t "${tmp_overridesdir}" {} +
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is great. That will make things easier for SCOS!

Copy link
Copy Markdown
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.

Thanks. This looks good to me. I would prefer another name for the option (repo, add(itional)-repo, private-repo, extra-repo?) but this is close to a bikeshed discussion so not blocking on that.

@travier
Copy link
Copy Markdown
Member

travier commented Aug 25, 2022

The only concern I could find was about the additional repo becoming stale on developer systems. This will not be an issue in CI as we clone from scratch every time.

@travier
Copy link
Copy Markdown
Member

travier commented Aug 25, 2022

One option to work around that is to include the minor RHEL version in the repo names thus the build would fail when the repos are missing.

@jlebon jlebon merged commit 772a966 into coreos:main Aug 25, 2022
@jlebon
Copy link
Copy Markdown
Member

jlebon commented Aug 25, 2022

In a follow-up, I'll add support for symlinking. This will allow devs to point to their existing checkouts just like for the config repo, which should help counter that a bit.

Done in #3045.

@travier
Copy link
Copy Markdown
Member

travier commented Aug 26, 2022

This likely broke RHCOS CI: openshift/os#959. Investigating:

+ cp src/config/repos/c9s.repo src/config/c9s.repo
+ curl -L http://base-4-12-rhel90.ocp.svc.cluster.local -o src/config/tmp.repo
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100   804  100   804    0     0   100k      0 --:--:-- --:--:-- --:--:--  112k
+ awk '/rhel-9-server-ose/,/^$/' src/config/tmp.repo
+ echo includepkgs=cri-o,cri-tools,openshift-clients,openshift-hyperkube
+ rm src/config/tmp.repo
+ cosa fetch
Config commit: v0.0.2-785-gf2b445d4302a85fc157526e766fe9c209158dbba
Using manifest: /tmp/cosa/src/config/manifest.yaml
ls: cannot access '/tmp/cosa/tmp/override/*.repo': No such file or directory
ERROR: no yum repo files were found
error: failed to execute cmd-fetch: exit status 1

@travier
Copy link
Copy Markdown
Member

travier commented Aug 26, 2022

And I've realized late that this will also not work when we have two minor RHEL minor versions at the same time in the repo (9.0 & 9.2) as the repo names will be the same but not the URLs.

We thus have to make repo names minor version specific (this should also work for content sets) or rework this one.

@travier
Copy link
Copy Markdown
Member

travier commented Aug 26, 2022

We also need to be able to specify a branch for the yumrepo git repo, unless we make all repo names explicitly include their version.

@travier
Copy link
Copy Markdown
Member

travier commented Aug 26, 2022

Related docs update in openshift/os#958

@cgwalters
Copy link
Copy Markdown
Member

We thus have to make repo names minor version specific (this should also work for content sets)

I'm in favor of this.

Comment thread src/cmdlib.sh
if [ -d "${workdir}/src/yumrepos" ]; then
find "${workdir}/src/yumrepos" -name '*.repo' -exec cp -t "${tmp_overridesdir}" {} +
fi
if ! ls "${tmp_overridesdir}"/*.repo; then
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the issue is here as this assumes that there is an overridesdir when there may not be

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Where this code is, we always create an overrides dir. ISTM like the issue OpenShift CI is hitting is the same that FCOS was hitting and should be fixed by 24ffe87 (#3045). Let's see what the CI rerun in openshift/os#959 says. :)

@jlebon
Copy link
Copy Markdown
Member

jlebon commented Aug 26, 2022

And I've realized late that this will also not work when we have two minor RHEL minor versions at the same time in the repo (9.0 & 9.2) as the repo names will be the same but not the URLs.

We thus have to make repo names minor version specific (this should also work for content sets) or rework this one.

Yes, if the repo contents are different, we shouldn't be using the same repo ID. Otherwise we'll be claiming we're using the wrong content set for one of them. This is what this comment is about.

And this jogs my memory now. I said higher up:

We can't change the repo names unfortunately because they're used in the content set mapping.

This is not true. I was thinking instead I think of the fact we have to match the repo names of the mirrored versions in OpenShift CI, which I think we have some control over.

@travier
Copy link
Copy Markdown
Member

travier commented Aug 26, 2022

Yes, we have control over the names in Prow CI. I'll make the PRs to change everything.

travier added a commit to travier/coreos-assembler that referenced this pull request Sep 7, 2022
Copy repo config files from config dir & yumrepos git repo.

Fixes: coreos#3074
Fixes: coreos#3036
Fixes: coreos#3045
travier added a commit that referenced this pull request Sep 12, 2022
Copy repo config files from config dir & yumrepos git repo.

Fixes: #3074
Fixes: #3036
Fixes: #3045
@travier travier mentioned this pull request Sep 14, 2022
28 tasks
jlebon pushed a commit to jlebon/coreos-assembler that referenced this pull request Nov 8, 2022
Copy repo config files from config dir & yumrepos git repo.

Fixes: coreos#3074
Fixes: coreos#3036
Fixes: coreos#3045
(cherry picked from commit 5c708b4)
jlebon pushed a commit to jlebon/coreos-assembler that referenced this pull request Nov 8, 2022
Copy repo config files from config dir & yumrepos git repo.

Fixes: coreos#3074
Fixes: coreos#3036
Fixes: coreos#3045
(cherry picked from commit 5c708b4)
(cherry picked from commit a82d230)
dustymabe pushed a commit that referenced this pull request Nov 9, 2022
Copy repo config files from config dir & yumrepos git repo.

Fixes: #3074
Fixes: #3036
Fixes: #3045
(cherry picked from commit 5c708b4)
dustymabe pushed a commit that referenced this pull request Nov 9, 2022
Copy repo config files from config dir & yumrepos git repo.

Fixes: #3074
Fixes: #3036
Fixes: #3045
(cherry picked from commit 5c708b4)
(cherry picked from commit a82d230)
jlebon pushed a commit to dustymabe/coreos-assembler that referenced this pull request Nov 10, 2022
Copy repo config files from config dir & yumrepos git repo.

Fixes: coreos#3074
Fixes: coreos#3036
Fixes: coreos#3045
(cherry picked from commit 5c708b4)
(cherry picked from commit a82d230)
(cherry picked from commit dd45d41)
jlebon pushed a commit to dustymabe/coreos-assembler that referenced this pull request Nov 10, 2022
Copy repo config files from config dir & yumrepos git repo.

Fixes: coreos#3074
Fixes: coreos#3036
Fixes: coreos#3045
(cherry picked from commit 5c708b4)
(cherry picked from commit a82d230)
(cherry picked from commit dd45d41)
jlebon pushed a commit to dustymabe/coreos-assembler that referenced this pull request Nov 10, 2022
Copy repo config files from config dir & yumrepos git repo.

Fixes: coreos#3074
Fixes: coreos#3036
Fixes: coreos#3045
(cherry picked from commit 5c708b4)
(cherry picked from commit a82d230)
(cherry picked from commit dd45d41)
dustymabe pushed a commit to dustymabe/coreos-assembler that referenced this pull request Nov 10, 2022
Copy repo config files from config dir & yumrepos git repo.

Fixes: coreos#3074
Fixes: coreos#3036
Fixes: coreos#3045
(cherry picked from commit 5c708b4)
(cherry picked from commit a82d230)
(cherry picked from commit dd45d41)
dustymabe pushed a commit to dustymabe/coreos-assembler that referenced this pull request Nov 13, 2022
Copy repo config files from config dir & yumrepos git repo.

Fixes: coreos#3074
Fixes: coreos#3036
Fixes: coreos#3045
(cherry picked from commit 5c708b4)
(cherry picked from commit a82d230)
(cherry picked from commit dd45d41)
dustymabe pushed a commit to dustymabe/coreos-assembler that referenced this pull request Nov 15, 2022
Copy repo config files from config dir & yumrepos git repo.

Fixes: coreos#3074
Fixes: coreos#3036
Fixes: coreos#3045
(cherry picked from commit 5c708b4)
(cherry picked from commit a82d230)
(cherry picked from commit dd45d41)
dustymabe pushed a commit that referenced this pull request Nov 15, 2022
Copy repo config files from config dir & yumrepos git repo.

Fixes: #3074
Fixes: #3036
Fixes: #3045
(cherry picked from commit 5c708b4)
(cherry picked from commit a82d230)
(cherry picked from commit dd45d41)
dustymabe pushed a commit to dustymabe/coreos-assembler that referenced this pull request Dec 1, 2022
Copy repo config files from config dir & yumrepos git repo.

Fixes: coreos#3074
Fixes: coreos#3036
Fixes: coreos#3045
(cherry picked from commit 5c708b4)
dustymabe pushed a commit that referenced this pull request Dec 2, 2022
Copy repo config files from config dir & yumrepos git repo.

Fixes: #3074
Fixes: #3036
Fixes: #3045
(cherry picked from commit 5c708b4)
jlebon pushed a commit that referenced this pull request Dec 2, 2022
Copy repo config files from config dir & yumrepos git repo.

Fixes: #3074
Fixes: #3036
Fixes: #3045
(cherry picked from commit 5c708b4)
(cherry picked from commit a82d230)
jlebon pushed a commit that referenced this pull request Dec 2, 2022
Copy repo config files from config dir & yumrepos git repo.

Fixes: #3074
Fixes: #3036
Fixes: #3045
(cherry picked from commit 5c708b4)
(cherry picked from commit a82d230)
(cherry picked from commit dd45d41)
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