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.

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

e2e-metal: add pxe_kernel_args to tf vars #9960

Merged
merged 1 commit into from
Jul 9, 2020
Merged

e2e-metal: add pxe_kernel_args to tf vars #9960

merged 1 commit into from
Jul 9, 2020

Conversation

sdodson
Copy link
Member

@sdodson sdodson commented Jun 26, 2020 •
edited
Loading

This will allow us to ratchet openshift/installer changes necessary to stop relying on pxe_os_image_url, merge order as follows
1 - #9960
2 - openshift/installer#3865
3 - #10121

/cc @sohankunkerkar

@openshift-ci-robot
Copy link
Contributor

@sdodson: GitHub didn't allow me to request PR reviews from the following users: sohankunkerkar.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

rhcos 4.6 uses a new livefs set of images added here and adds support for 4k block sizes
https://github.com/openshift/installer/pull/3763/files#diff-356005118eaf2541a211c4f966231ddfR98

/cc @sohankunkerkar

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.

@sdodson
Copy link
Member Author

sdodson commented Jun 26, 2020

/hold

@openshift-ci-robot openshift-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 26, 2020
@openshift-ci-robot
Copy link
Contributor

@sohankunkerkar: The /retest command does not accept any targets.
The following commands are available to trigger jobs:

  • /test promrules
  • /test app-ci-config-dry
  • /test build-farm-consistency
  • /test build01-dry
  • /test build02-dry
  • /test ci-operator-config
  • /test ci-operator-config-metadata
  • /test ci-operator-registry
  • /test config
  • /test core-dry
  • /test core-valid
  • /test correctly-sharded-config
  • /test generated-config
  • /test generated-dashboards
  • /test ordered-prow-config
  • /test owners
  • /test pj-rehearse
  • /test prow-config
  • /test prow-config-filenames
  • /test prow-config-semantics
  • /test services-dry
  • /test services-valid
  • /test step-registry-shellcheck
  • /test pylint

Use /test all to run the following jobs:

  • pull-ci-openshift-release-master-app-ci-config-dry
  • pull-ci-openshift-release-master-build-farm-consistency
  • pull-ci-openshift-release-master-build01-dry
  • pull-ci-openshift-release-master-build02-dry
  • pull-ci-openshift-release-master-ci-operator-config
  • pull-ci-openshift-release-master-ci-operator-config-metadata
  • pull-ci-openshift-release-master-ci-operator-registry
  • pull-ci-openshift-release-master-config
  • pull-ci-openshift-release-master-core-dry
  • pull-ci-openshift-release-master-core-valid
  • pull-ci-openshift-release-master-correctly-sharded-config
  • pull-ci-openshift-release-master-generated-config
  • pull-ci-openshift-release-master-generated-dashboards
  • pull-ci-openshift-release-master-ordered-prow-config
  • pull-ci-openshift-release-master-owners
  • pull-ci-openshift-release-master-pj-rehearse
  • pull-ci-openshift-release-master-prow-config
  • pull-ci-openshift-release-master-prow-config-filenames
  • pull-ci-openshift-release-master-prow-config-semantics
  • pull-ci-openshift-release-master-services-dry
  • pull-ci-openshift-release-master-services-valid
  • pull-ci-openshift-release-master-step-registry-shellcheck

In response to this:

/retest ci/rehearse/openshift/installer/master/e2e-metal

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.

@sohankunkerkar
Copy link
Member

/retest

@sohankunkerkar
Copy link
Member

/test pj-rehearse

1 similar comment
@sohankunkerkar
Copy link
Member

/test pj-rehearse

@sohankunkerkar
Copy link
Member

/test e2e-metal

@openshift-ci-robot
Copy link
Contributor

@sohankunkerkar: The specified target(s) for /test were not found.
The following commands are available to trigger jobs:

  • /test promrules
  • /test app-ci-config-dry
  • /test build-farm-consistency
  • /test build01-dry
  • /test build02-dry
  • /test ci-operator-config
  • /test ci-operator-config-metadata
  • /test ci-operator-registry
  • /test config
  • /test core-dry
  • /test core-valid
  • /test correctly-sharded-config
  • /test generated-config
  • /test generated-dashboards
  • /test ordered-prow-config
  • /test owners
  • /test pj-rehearse
  • /test prow-config
  • /test prow-config-filenames
  • /test prow-config-semantics
  • /test services-dry
  • /test services-valid
  • /test step-registry-shellcheck
  • /test pylint

Use /test all to run the following jobs:

  • pull-ci-openshift-release-master-app-ci-config-dry
  • pull-ci-openshift-release-master-build-farm-consistency
  • pull-ci-openshift-release-master-build01-dry
  • pull-ci-openshift-release-master-build02-dry
  • pull-ci-openshift-release-master-ci-operator-config
  • pull-ci-openshift-release-master-ci-operator-config-metadata
  • pull-ci-openshift-release-master-ci-operator-registry
  • pull-ci-openshift-release-master-config
  • pull-ci-openshift-release-master-core-dry
  • pull-ci-openshift-release-master-core-valid
  • pull-ci-openshift-release-master-correctly-sharded-config
  • pull-ci-openshift-release-master-generated-config
  • pull-ci-openshift-release-master-generated-dashboards
  • pull-ci-openshift-release-master-ordered-prow-config
  • pull-ci-openshift-release-master-owners
  • pull-ci-openshift-release-master-pj-rehearse
  • pull-ci-openshift-release-master-prow-config
  • pull-ci-openshift-release-master-prow-config-filenames
  • pull-ci-openshift-release-master-prow-config-semantics
  • pull-ci-openshift-release-master-services-dry
  • pull-ci-openshift-release-master-services-valid
  • pull-ci-openshift-release-master-step-registry-shellcheck

In response to this:

/test e2e-metal

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.

@sdodson
Copy link
Member Author

sdodson commented Jun 29, 2020

/test pj-rehearse

@sdodson
Copy link
Member Author

sdodson commented Jun 29, 2020

@cgwalters @sohankunkerkar

iPXE is timing out after ~ 20 minutes while fetching the initramfs. It got to approximately 30% before timing out. The net effect of this patch is that it's going to prefer to use the live-kernel live-initramfs and metal4k kernel, initrd, and iso respectively. Are those correct? It never got to the ISO but I imagine that's probably unused?

@sdodson
Copy link
Member Author

sdodson commented Jun 29, 2020

/test pj-rehearse

@jlebon
Copy link
Member

jlebon commented Jun 29, 2020

Oh cool, thanks for tackling this!

iPXE is timing out after ~ 20 minutes while fetching the initramfs. It got to approximately 30% before timing out.

Hmm yeah, there's some UX issues around using iPXE for fetching the live media over HTTPS. See coreos/fedora-coreos-tracker#390. That said, AFAIK this is fetching from S3 as well (right?) just like FCOS, and we've definitely been able to test the live PXE FCOS media on Packet, though it was still on the order of 5 mins: coreos/fedora-coreos-tracker#390 (comment).

Is it timing out consistently?

The net effect of this patch is that it's going to prefer to use the live-kernel live-initramfs and metal4k kernel, initrd, and iso respectively. Are those correct? It never got to the ISO but I imagine that's probably unused?

Hmm, not sure if I'm parsing this right. The patch looks correct to me. You should be able to drop references to the metal image entirely with the new installer since the metal image is embedded into the live initramfs. Could do that as a follow-up though! We don't have to worry about metal4k until we do that; the installer will transparently use the right image for the target disk.

Re. the ISO, the Packet tests aren't using that right now, right?

@sohankunkerkar
Copy link
Member

/retest

@sohankunkerkar
Copy link
Member

Let's wait until openshift/installer#3844 gets merged

@sohankunkerkar
Copy link
Member

/retest

@sdodson
Copy link
Member Author

sdodson commented Jul 7, 2020

/test pj-rehearse

@sdodson
Copy link
Member Author

sdodson commented Jul 7, 2020

It's currently failing on

         Starting CoreOS Installer...
 #-#O=#   #                                                                                                                  
[  238.226127] coreos-installer-service[1283]: coreos-installer install /dev/sda --ignition /tmp/coreos-installer-3fcRSv --image-url https://releases-art-rhcos.svc.ci.openshift.org/art/storage/releases/rhcos-4.6/46.82.202007051540-0/x86_64/rhcos-46.82.202007051540-0-metal.x86_64.raw.gz
[  238.252220] coreos-installer-service[1283]: Downloading image from https://releases-art-rhcos.svc.ci.openshift.org/art/storage/releases/rhcos-4.6/46.82.202007051540-0/x86_64/rhcos-46.82.202007051540-0-metal.x86_64.raw.gz
[  238.275351] coreos-installer-service[1283]: Downloading signature from https://releases-art-rhcos.svc.ci.openshift.org/art/storage/releases/rhcos-4.6/46.82.202007051540-0/x86_64/rhcos-46.82.202007051540-0-metal.x86_64.raw.gz.sig
[  238.710271] coreos-installer-service[1283]: Failed to fetch signature: fetching signature URL
[  239.190103] coreos-installer-service[1283]: Error: --insecure not specified and signature not found
[FAILED] Failed to start CoreOS Installer.
See 'systemctl status coreos-installer.service' for details.
[DEPEND] Dependency failed for CoreOS Installer Target.

I'm going to see if I can update the job not to pass in the coreos.inst.image_url option which should no longer be necessary with the live image.

@sohankunkerkar
Copy link
Member

/retest

sohankunkerkar
sohankunkerkar reviewed Jul 8, 2020
View reviewed changes
Copy link
Member

@sohankunkerkar sohankunkerkar left a comment

Choose a reason for hiding this comment

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

Looks like you need to remove pxe_os_image_url = "${PXE_OS_IMAGE_URL}" too
https://deck-ci.apps.ci.l2s4.p1.openshiftapps.com/view/gcs/origin-ci-test/pr-logs/pull/openshift_release/9960/rehearse-9960-pull-ci-openshift-installer-master-e2e-metal/1280652055815393280

@sdodson sdodson changed the title metal-e2e: Add support for new liveos images WIP metal-e2e: Add support for new liveos images Jul 8, 2020
@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 Jul 8, 2020
@sdodson sdodson force-pushed the liveimage branch 2 times, most recently from 7974aa5 to 9288ae6 Compare July 8, 2020 18:21
@sdodson sdodson changed the title WIP metal-e2e: Add support for new liveos images e2e-metal: Add kernel_args to tf vars Jul 8, 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 Jul 8, 2020
@sdodson sdodson changed the title e2e-metal: Add kernel_args to tf vars e2e-metal: add pxe_kernel_args to tf vars Jul 8, 2020
This was referenced Jul 8, 2020
Merged
Merged
@sdodson
e2e-metal: add pxe_kernel_args to tf vars
b797585 
This will allow us to ratchet openshift/installer changes necessary to stop
relying on pxe_os_image_url
@sohankunkerkar
Copy link
Member

/retest

@sohankunkerkar
Copy link
Member

CI looks green! I will ask @cgwalters to add a /lgtm label
/approve

cgwalters
cgwalters reviewed Jul 9, 2020
View reviewed changes
ci-operator/templates/openshift/installer/cluster-launch-installer-metal-e2e.yaml
else
PXE_KERNEL_ARGS="coreos.inst.image_url=${PXE_OS_IMAGE_URL}"
fi
# 4.3 unified 'metal-bios' and 'metal-uefi' into just 'metal', unused in 4.6
Copy link
Member

Choose a reason for hiding this comment

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

Comment seems misplaced, I think you mean to change the one above? But not worth a respin.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I'll clean that up in #10121

@cgwalters
Copy link
Member

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 9, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, sdodson, sohankunkerkar

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

@sdodson
Copy link
Member Author

sdodson commented Jul 9, 2020

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 9, 2020
@openshift-merge-robot openshift-merge-robot merged commit 322e00d into openshift:master Jul 9, 2020
@openshift-ci-robot
Copy link
Contributor

@sdodson: Updated the following 4 configmaps:

  • prow-job-cluster-launch-installer-metal-e2e configmap in namespace ci at cluster api.ci using the following files:
    • key cluster-launch-installer-metal-e2e.yaml using file ci-operator/templates/openshift/installer/cluster-launch-installer-metal-e2e.yaml
  • prow-job-cluster-launch-installer-metal-e2e configmap in namespace ci at cluster app.ci using the following files:
    • key cluster-launch-installer-metal-e2e.yaml using file ci-operator/templates/openshift/installer/cluster-launch-installer-metal-e2e.yaml
  • prow-job-cluster-launch-installer-metal-e2e configmap in namespace ci at cluster build01 using the following files:
    • key cluster-launch-installer-metal-e2e.yaml using file ci-operator/templates/openshift/installer/cluster-launch-installer-metal-e2e.yaml
  • prow-job-cluster-launch-installer-metal-e2e configmap in namespace ci at cluster build02 using the following files:
    • key cluster-launch-installer-metal-e2e.yaml using file ci-operator/templates/openshift/installer/cluster-launch-installer-metal-e2e.yaml

In response to this:

This will allow us to ratchet openshift/installer changes necessary to stop relying on pxe_os_image_url, merge order as follows
1 - #9960
2 - openshift/installer#3865
3 - #10121

/cc @sohankunkerkar

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sohankunkerkar sohankunkerkar sohankunkerkar left review comments

@cgwalters cgwalters cgwalters left review comments

Assignees

@cgwalters cgwalters

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.
6 participants
@sdodson @openshift-ci-robot @sohankunkerkar @jlebon @cgwalters @openshift-merge-robot