Skip to content
This repository has been archived by the owner on Oct 19, 2020. It is now read-only.

MGMT-1233 - support v4.6, use official openshift-installer extracted … #19

Merged

Conversation

yevgeny-shnaidman
Copy link
Contributor

…from release

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 23, 2020
@app-sre-bot
Copy link

Can one of the admins verify this patch?

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: yevgeny-shnaidman

The full list of commands accepted by this bot can be found here.

The pull request process is described 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-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 23, 2020
@yevgeny-shnaidman
Copy link
Contributor Author

/assign @romfreiman
/assign @ronniel1

4) INVENTORY_ENDPOINT - url that defines how python client connects to assisted-service.
5) S3_ENDPOINT_URL - S3 endpoint. results of the job will be uploaded to that S3
6) S3_BUCKET - the S3 bucket to upload to
7) aws_access_key_id - AWS access key id
Copy link
Contributor

Choose a reason for hiding this comment

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

why small case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will fix

Copy link
Contributor

Choose a reason for hiding this comment

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

?

@@ -69,5 +69,4 @@ platform:
bootMode: UEFI
hardwareProfile: unknown

pullSecret: '{"auths":{"cloud.openshift.com":{"auth":"b3BlbnNoaWZ0LXJlbGVhc2UtZGV2K3lzaG5haWRtMXkxdTZhempjaGJ0cm5hZDFkb3U1Z3FtcnliOlVBNjU0SVZFQzZPMFFBM0ZJTDA2WjY1WDRSSjM0WElNM0JYRVkzVFBCVU9IUkZESEhRSkkxMFFJNUI5MEY1Tks=","email":"yshnaidman@gmail.com"},"quay.io":{"auth":"b3BlbnNoaWZ0LXJlbGVhc2UtZGV2K3lzaG5haWRtMXkxdTZhempjaGJ0cm5hZDFkb3U1Z3FtcnliOlVBNjU0SVZFQzZPMFFBM0ZJTDA2WjY1WDRSSjM0WElNM0JYRVkzVFBCVU9IUkZESEhRSkkxMFFJNUI5MEY1Tks=","email":"yshnaidman@gmail.com"},"registry.connect.redhat.com":{"auth":"NTMxOTg0MDl8dWhjLTFZMXU2QXpqY0hidHJOYUQxZE91NUdxbVJZQjpleUpoYkdjaU9pSlNVelV4TWlKOS5leUp6ZFdJaU9pSmpPREUzWVRZMk1qQXhOR00wTlRFMFlqWXlPR1ppTkROaE9UazNNek0xTUNKOS5temRzdWFYTHY1VWMzQ0Y5OVRsT2lCNlJ1bnhOMmNlaTJGTmZsZ2RKTG5WWEdqQVVSMDlqd3FiNXlJXzV2blhYUjlydVAxem5MaEh3NU1acG1iMDA3cl9jVllNWk53WHVtZGZDLWQxdlVhY1NmMjR1Yl9yM0RRT04tQ3Q2U2FQWnc3a3ZtLTU3anlvNXU4YUhYSlNWZ0hrRlJielFFMF9oNHF3bVJBZ0R6RmRNQ2V5dU5ST2MxaGZmWHM2QzdYSjg4QVlFSlZPUi1xeVZIVXo5VTdCRl9Pcm1nX1hYajQyLVAzZUxZSkhqUk9FSEdfTDhxTEJsS2R3T0hvQ2owR2dIaXgxelotd0ExU0hiY25BNE5aWGNUa1dMUTFsM0tTVUM2ODVYbWFxbDBCT3ZEVXpaRnZramhPOVBVTXRZUUF4bXZZMG1oTGg1RzhhdXJkRi1QdjRRcXhtMkdHbEpEYmlITUZjbGxzamFtbnR5b0loaFZwcURMYzJxLTZ4UXVub0FiZno0Zk5uNWdEekRNemVZWVNEMjZUakVYWi1DN0lJZncwdE92b2tSZGk2QTB6LTE2cEc2MHh5ZFRkX1JISEZwM0sxeXJLdlBCZXZLRVRNalJPamxPbE1zYVJKM3lITkczWV9OYkRWSEVHWnRFYmtIS3Rfel9BeV9zSE1YYUx5b0FwUVMwNmNDUUc0VHU4S3Q2dHpOLXg0QnJ6bTJqcGhYU1Y5ZFpoTEhjTUpSNGRHTkVBQnIteG9BNFdadFlPQ3RvWEpRZEQzYUc4Y2l3R1ptN0VNb1lLR0ZWSUcycmhTQnZfeUxsQXRzbTVpRTdLaGhoQUtqak1TdE9BZlRnSEswWFVDczhGWE9PWGNrdEl4ZWhsVHgwVnIteUZoY0FGcmdhU1NBMWhycFdlTQ==","email":"yshnaidman@gmail.com"},"registry.redhat.io":{"auth":"NTMxOTg0MDl8dWhjLTFZMXU2QXpqY0hidHJOYUQxZE91NUdxbVJZQjpleUpoYkdjaU9pSlNVelV4TWlKOS5leUp6ZFdJaU9pSmpPREUzWVRZMk1qQXhOR00wTlRFMFlqWXlPR1ppTkROaE9UazNNek0xTUNKOS5temRzdWFYTHY1VWMzQ0Y5OVRsT2lCNlJ1bnhOMmNlaTJGTmZsZ2RKTG5WWEdqQVVSMDlqd3FiNXlJXzV2blhYUjlydVAxem5MaEh3NU1acG1iMDA3cl9jVllNWk53WHVtZGZDLWQxdlVhY1NmMjR1Yl9yM0RRT04tQ3Q2U2FQWnc3a3ZtLTU3anlvNXU4YUhYSlNWZ0hrRlJielFFMF9oNHF3bVJBZ0R6RmRNQ2V5dU5ST2MxaGZmWHM2QzdYSjg4QVlFSlZPUi1xeVZIVXo5VTdCRl9Pcm1nX1hYajQyLVAzZUxZSkhqUk9FSEdfTDhxTEJsS2R3T0hvQ2owR2dIaXgxelotd0ExU0hiY25BNE5aWGNUa1dMUTFsM0tTVUM2ODVYbWFxbDBCT3ZEVXpaRnZramhPOVBVTXRZUUF4bXZZMG1oTGg1RzhhdXJkRi1QdjRRcXhtMkdHbEpEYmlITUZjbGxzamFtbnR5b0loaFZwcURMYzJxLTZ4UXVub0FiZno0Zk5uNWdEekRNemVZWVNEMjZUakVYWi1DN0lJZncwdE92b2tSZGk2QTB6LTE2cEc2MHh5ZFRkX1JISEZwM0sxeXJLdlBCZXZLRVRNalJPamxPbE1zYVJKM3lITkczWV9OYkRWSEVHWnRFYmtIS3Rfel9BeV9zSE1YYUx5b0FwUVMwNmNDUUc0VHU4S3Q2dHpOLXg0QnJ6bTJqcGhYU1Y5ZFpoTEhjTUpSNGRHTkVBQnIteG9BNFdadFlPQ3RvWEpRZEQzYUc4Y2l3R1ptN0VNb1lLR0ZWSUcycmhTQnZfeUxsQXRzbTVpRTdLaGhoQUtqak1TdE9BZlRnSEswWFVDczhGWE9PWGNrdEl4ZWhsVHgwVnIteUZoY0FGcmdhU1NBMWhycFdlTQ==","email":"yshnaidman@gmail.com"}}}'
sshKey: 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQD14Gv4V5DVvyr7O6/44laYx52VYLe8yrEA3fOieWDmojRs3scqLnfeLHJWsfYA4QMjTuraLKhT8dhETSYiSR88RMM56+isLbcLshE6GkNkz3MBZE2hcdakqMDm6vucP3dJD6snuh5Hfpq7OWDaTcC0zCAzNECJv8F7LcWVa8TLpyRgpek4U022T5otE1ZVbNFqN9OrGHgyzVQLtC4xN1yT83ezo3r+OEdlSVDRQfsq73Zg26d4dyagb6lmrryUUAAbfmn/HalJTHB73LyjilKiPvJ+x2bG7AeiqyVHwtQSpt02FCdQGptmsSqqWF/b9botOO38eUsqPNppMn7LT5wzDZdDlfwTCBWkpqijPcdo/LTD9dJlNHjwXZtHETtiid6N3ZZWpA0/VKjqUeQdSnHqLEzTidswsnOjCIoIhmJFqczeP5kOty/MWdq1II/FX/EpYCJxoSWkT/hVwD6VOamGwJbLVw9LkEb0VVWFRJB5suT/T8DtPdPl+A0qUGiN4KM= oscohen@localhost.localdomain'
pullSecret: '{"auths": ...}'
Copy link
Contributor

Choose a reason for hiding this comment

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

why we have ... ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this file is in github (part of testing), so we cannot keep real pull secret there.

# set_pull_secret(config_dir)
# [TODO] - remove comment after fixing subsystem
# oc_utils.extract_baremetal_installer(work_dir, openshift_release_image)
# set pull secret in a file
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure why comments are needed, u just comment function name :)

# oc_utils.extract_baremetal_installer(work_dir, openshift_release_image)
# set pull secret in a file
registry_config_file = set_pull_secret(work_dir, config_dir)
# extract openshift-installer
Copy link
Contributor

Choose a reason for hiding this comment

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

same

Copy link
Contributor

Choose a reason for hiding this comment

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

i think better to comment on the function itself if it is complicated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in any case, once we move to 4.6, we will move the code to Golang in assisted-service... The same applies for comment above

@@ -201,7 +201,7 @@ def main():
bucket = os.environ.get('S3_BUCKET', args.s3_bucket)
aws_access_key_id = os.environ.get("AWS_ACCESS_KEY_ID", "accessKey1")
aws_secret_access_key = os.environ.get("AWS_SECRET_ACCESS_KEY", "verySecretKey1")
# openshift_release_image = os.environ.get("OPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDE")
openshift_release_image = os.environ.get("OPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDE")
Copy link
Contributor

Choose a reason for hiding this comment

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

and if not set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the job will fail on extracting openshift-installer, which is good

4) INVENTORY_ENDPOINT - url that defines how python client connects to assisted-service.
5) S3_ENDPOINT_URL - S3 endpoint. results of the job will be uploaded to that S3
6) S3_BUCKET - the S3 bucket to upload to
7) aws_access_key_id - AWS access key id
Copy link
Contributor

Choose a reason for hiding this comment

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

?

for file_data in storage_files:
# if file_data['path'] == '/etc/motd':
# storage_files.remove(file_data)
for file_data in storage_files[:]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that you remove a file you should iterate a copy

Copy link
Contributor

Choose a reason for hiding this comment

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

Also why do we need to remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. i am iterating a copy
  2. motd is actually an update to existing file, and somehow, ignition 3.1 fails on it. Since it is only motd, i choose to remove it, instead of trying to dig deeper into it

@yevgeny-shnaidman
Copy link
Contributor Author

/wip

@yevgeny-shnaidman
Copy link
Contributor Author

/unhold

@tsorya
Copy link
Contributor

tsorya commented Sep 6, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 6, 2020
@yevgeny-shnaidman yevgeny-shnaidman changed the title [WIP] MGMT-1233 - support v4.6, use official openshift-installer extracted … MGMT-1233 - support v4.6, use official openshift-installer extracted … Sep 6, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 6, 2020
@openshift-merge-robot openshift-merge-robot merged commit fa0e2d5 into openshift:master Sep 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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
Development

Successfully merging this pull request may close these issues.

None yet

8 participants