Skip to content

AGENT-263: Get images from release payload #6100

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 1 commit into from
Jul 12, 2022
Merged

AGENT-263: Get images from release payload #6100

openshift-ci merged 1 commit into from
Jul 12, 2022

Conversation

@bfournie
Copy link
Contributor

@bfournie bfournie commented Jul 8, 2022 •
edited
Loading

Get the images used to start all services from the release payload.

This works in a connected environment, however in order to fully complete a disconnected install, an assisted-service change to use an icsp file when running 'oc adm release extract' is required as described in https://issues.redhat.com/browse/MGMT-10209.

The disconnected environment can be tested in dev-scripts with openshift-metal3/dev-scripts#1424 using
export MIRROR_IMAGES=true

@openshift-ci openshift-ci bot requested review from andfasano and dhellmann July 8, 2022 02:23
@bfournie bfournie force-pushed the images-from-payload branch 5 times, most recently from 56fa1a2 to 88e4812 Compare July 8, 2022 15:03
andfasano
andfasano reviewed Jul 8, 2022
View reviewed changes
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 8, 2022
zaneb
zaneb reviewed Jul 9, 2022
View reviewed changes
Copy link
Member

@zaneb zaneb left a comment

Choose a reason for hiding this comment

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

This looks good but it is a lot of changes to go in a single commit. I think it could be split up into at least:

  • Use payload containers for agent/assisted-service pods and as env vars (actually I don't see code for that part at all) for agent/assisted-installer/csr-approver.
  • Use assisted-service container to run DB
  • Drop UI
  • Add a 4.12 image to OS_IMAGES

possibly the first could be split even further.

@bfournie bfournie force-pushed the images-from-payload branch from 88e4812 to 04af13b Compare July 9, 2022 11:25
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 9, 2022
@bfournie bfournie force-pushed the images-from-payload branch 3 times, most recently from 2d78cd6 to 8373af5 Compare July 10, 2022 16:56
@bfournie
Copy link
Contributor Author

bfournie commented Jul 10, 2022

This looks good but it is a lot of changes to go in a single commit. I think it could be split up into at least:

  • Use payload containers for agent/assisted-service pods and as env vars (actually I don't see code for that part at all) for agent/assisted-installer/csr-approver.
  • Use assisted-service container to run DB
  • Drop UI
  • Add a 4.12 image to OS_IMAGES

possibly the first could be split even further.

  • Moved the UI removal to AGENT-268: Remove assisted-service UI container #6106
  • With a recent backport to 4.11, the change to add 4.12 images to OS_IMAGES isn't necessary so removed that here, will create a separate PR for it
  • The change to use the assisted-service to run DB also has the change to use the new container images so will keep that here

@bfournie bfournie force-pushed the images-from-payload branch 2 times, most recently from 69a5a71 to 6f2f9a3 Compare July 11, 2022 10:52
zaneb
zaneb reviewed Jul 11, 2022
View reviewed changes
Copy link
Member

@zaneb zaneb left a comment

Choose a reason for hiding this comment

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

Some nits inline but this looks good.

data/data/agent/files/usr/local/bin/get-container-images.sh.template Outdated
Copy link
Member

@zaneb zaneb Jul 11, 2022

Choose a reason for hiding this comment

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

Should this be > instead of >>? There is no grep test on this one.

Copy link
Contributor Author

@bfournie bfournie Jul 12, 2022

Choose a reason for hiding this comment

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

Yes true, fixed

@zaneb
Copy link
Member

zaneb commented Jul 11, 2022

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 11, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zaneb

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 11, 2022
@bfournie bfournie force-pushed the images-from-payload branch from 6f2f9a3 to 877f4a6 Compare July 12, 2022 17:41
@bfournie
AGENT-263: Get images from release payload
0bfaa1d 
Get the images used to start all services from the release payload which may be a
mirror. Since we do not build a version of the UI image, that has been removed. Also
for the assisted-service-db the postgresql image is no longer used, instead the
assisted-service image is used to start the database.
@bfournie bfournie force-pushed the images-from-payload branch from 877f4a6 to 0bfaa1d Compare July 12, 2022 19:07
@pawanpinjarkar
Copy link
Contributor

pawanpinjarkar commented Jul 12, 2022

/lgtm

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

openshift-ci bot commented Jul 12, 2022

@bfournie: 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 b68d3de into openshift:agent-installer Jul 12, 2022
@bfournie bfournie deleted the images-from-payload branch July 13, 2022 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zaneb zaneb zaneb left review comments

@dhellmann dhellmann Awaiting requested review from dhellmann

+1 more reviewer

@andfasano andfasano andfasano left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@pawanpinjarkar pawanpinjarkar

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.

4 participants

@bfournie @zaneb @pawanpinjarkar @andfasano