Skip to content

AGENT-280: Generate mirror config from install-config #6098

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.

Sign up for GitHub

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

Jump to bottom
Merged
openshift-ci merged 8 commits into from
Jul 14, 2022
Merged

AGENT-280: Generate mirror config from install-config #6098

openshift-ci merged 8 commits into from
Jul 14, 2022

Conversation

@zaneb
Copy link
Member

@zaneb zaneb commented Jul 7, 2022

Generate the mirror configuration (mirror/registries.conf and mirror/ca-bundle.crt) from the install-config if it is supplied.

(Note: depends on #6087)

@zaneb zaneb requested a review from bfournie July 7, 2022 16:38
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 7, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 7, 2022

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@zaneb
Copy link
Member Author

zaneb commented Jul 7, 2022

/test all

@openshift-ci openshift-ci bot requested review from kirankt and rwsu July 7, 2022 16:43
@zaneb zaneb changed the title AGENT-137: Generate mirror config from install-config AGENT-280: Generate mirror config from install-config Jul 8, 2022
@zaneb zaneb force-pushed the mirror-from-install-config branch from 1da8ce2 to ea138ad Compare July 9, 2022 02:38
@zaneb zaneb marked this pull request as ready for review July 9, 2022 02:38
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 9, 2022
@openshift-ci openshift-ci bot requested review from andfasano and jstuever July 9, 2022 02:39
celebdor
celebdor reviewed Jul 11, 2022
View reviewed changes
@zaneb zaneb force-pushed the mirror-from-install-config branch from ea138ad to 9359be5 Compare July 11, 2022 13:24
rwsu
rwsu suggested changes Jul 11, 2022
View reviewed changes
@zaneb zaneb force-pushed the mirror-from-install-config branch from 9359be5 to 6289a47 Compare July 11, 2022 19:27
rwsu
rwsu suggested changes Jul 12, 2022
View reviewed changes
@zaneb zaneb force-pushed the mirror-from-install-config branch from 6289a47 to 7f3893c Compare July 12, 2022 20:58
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 12, 2022
zaneb added 5 commits July 12, 2022 17:16
@zaneb
Generate CA Bundle from InstallConfig if available
1db8802
@zaneb
Validate ca-bundle.crt when loaded from disk
a981f6d
@zaneb
Generate a default registries.conf file
04acaba 
Use the podman default, with no unqualified-search-registries.
@zaneb
Generate mirror registries.conf from install-config if available
d823e9a
@zaneb
Generate mirror configuration with cluster-manifests
0a3243c 
When the user runs the "agent create cluster-manifests" command, also
generate the mirror configuration from the install-config if it is
available.
@zaneb zaneb force-pushed the mirror-from-install-config branch from 7f3893c to fe23a37 Compare July 12, 2022 21:54
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 12, 2022
@zaneb
Make mirror config mounts less conditional
15db1cb 
Ideally we would just always mount the same /etc/containers/ config
(including registries.conf) and CA certificates into the
assisted-service container as we have in the live ISO. Both are pulling
images from the same location (since the assisted-service image itself
will come from the release payload).

However, the assisted-service code fails if there are no registry
mirrors configured in registries.conf, so continue to make just this
part conditional on there being some mirroring settings.
@zaneb zaneb force-pushed the mirror-from-install-config branch 2 times, most recently from ca08f38 to d31b43a Compare July 13, 2022 03:53
@zaneb
Eliminate unneeded asset AgentMirror
90af460 
Unlike with 'cluster-manifests', we'll never need to read user-supplied
files with unknown names from the 'mirror' directory, so there is no
point having an extra asset to collect data from the two mirror config
assets together.
@zaneb
Generate empty ca-bundle.crt file
c5fb531 
It's not always obvious to users what files they can provide, so
generate an empty ca-bundle.crt file in the "agent create
cluster-manifests" command if there is no additionalTrustBundle
specified in the install-config.
@zaneb zaneb force-pushed the mirror-from-install-config branch from d31b43a to c5fb531 Compare July 13, 2022 14:49
@bfournie
Copy link
Contributor

bfournie commented Jul 13, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 13, 2022
@bfournie
Copy link
Contributor

bfournie commented Jul 13, 2022

I tested this with openshift-metal3/dev-scripts#1424 with MIRROR_IMAGES=true and verified it works as expected, up to the point where we hit the assisted-service ICSP issue (https://issues.redhat.com/browse/MGMT-10209)

rwsu
rwsu approved these changes Jul 14, 2022
View reviewed changes
Copy link
Contributor

@rwsu rwsu left a comment

Choose a reason for hiding this comment

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

Verified that the issues have been fixed.

/approve

@rwsu
Copy link
Contributor

rwsu commented Jul 14, 2022

/assign @jhixson74

@patrickdillon
Copy link
Contributor

patrickdillon commented Jul 14, 2022

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 14, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: patrickdillon, rwsu

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 14, 2022
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jul 14, 2022

/retest-required

Remaining retests: 2 against base HEAD bad19ba and 8 for PR HEAD c5fb531 in total

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jul 14, 2022

/retest-required

Remaining retests: 1 against base HEAD bad19ba and 7 for PR HEAD c5fb531 in total

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jul 14, 2022

/retest-required

Remaining retests: 0 against base HEAD bad19ba and 6 for PR HEAD c5fb531 in total

@zaneb
Copy link
Member Author

zaneb commented Jul 14, 2022

Need openshift/release#30488 to fix the tests.

@zaneb
Copy link
Member Author

zaneb commented Jul 14, 2022

/test e2e-agent-compact

@zaneb
Copy link
Member Author

zaneb commented Jul 14, 2022

/test verify-vendor

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 14, 2022

@zaneb: all tests passed!

Full PR test history. Your PR dashboard.

Details

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.

@openshift-ci openshift-ci bot merged commit 9a50daf into openshift:agent-installer Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bfournie bfournie Awaiting requested review from bfournie

@kirankt kirankt Awaiting requested review from kirankt

@andfasano andfasano Awaiting requested review from andfasano

@jstuever jstuever Awaiting requested review from jstuever

2 more reviewers

@celebdor celebdor celebdor left review comments

@rwsu rwsu rwsu approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jhixson74 jhixson74

@bfournie bfournie

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@zaneb @bfournie @rwsu @patrickdillon @openshift-ci-robot @celebdor @jhixson74