-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Adding template for ipi e2e job #5016
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.
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
Conversation
|
/assign @smarterclayton |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming here is wrong. This really upi for packet, so update the naming to be consistent (cluster-launch-installer-upi-packet-e2e).
What's more concerning is that this template is inconsistent in structure, naming, and behavior from the other templates. This template really needs to look like cluster-launch-installer-e2e with a few small tweaks (such as your custom terraform setup) or be part of that template with a cluster type (packet). You can omit some of that to get started, but the end result needs to be more consistent with the rest or this will forever atrophy when someone needs to refactor all of them and yours is unmaintainable.
Why is the libvirt image needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming here is wrong. This really upi for packet, so update the naming to be consistent (cluster-launch-installer-upi-packet-e2e).
What's more concerning is that this template is inconsistent in structure, naming, and behavior from the other templates. This template really needs to look like cluster-launch-installer-e2e with a few small tweaks (such as your custom terraform setup) or be part of that template with a cluster type (packet). You can omit some of that to get started, but the end result needs to be more consistent with the rest or this will forever atrophy when someone needs to refactor all of them and yours is unmaintainable.
I'm not sure I understand this, we're not using UPI, this is IPI, I don't see it easily fitting into any of the other templates as the flow is different, for the moment we just want to spin up a baremetal centos server and run the installer on it. But I can look at adding the cluster type to cluster-launch-installer-e2e
Why is the libvirt image needed?
I'm using it because it was the only image I found that had the libnss_wrapper so I could ssh to packet.net servers, incidentally the reason for using the UPI image is because it had terraform.
e190946 to
5159175
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: derekhiggins The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
5159175 to
839d445
Compare
3238ee7 to
ebc782f
Compare
|
@smarterclayton I've updated this into the cluster-launch-installer-e2e.yaml template, is this what you wanted? I'm still not sure about your naming comment, AIUI this is baremetal ipi? |
ebc782f to
b49cd8d
Compare
b49cd8d to
d2a4456
Compare
Updating cluster-launch-installer-e2e template to for jobs that test Bare Metal provided Installer Provisioned Infrastructure. Use packet.net to spin up a single baremetal server and test the ipi installer on it.
d2a4456 to
538ba0d
Compare
|
@derekhiggins: PR needs rebase. DetailsInstructions 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. |
1 similar comment
|
@derekhiggins: PR needs rebase. DetailsInstructions 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. |
|
@derekhiggins: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions 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. |
|
Stale PR, please adapt and reopen if still needed. /close |
|
@petr-muller: Closed this PR. DetailsIn response to this:
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. |
Adding a template to for jobs that test Bare Metal
Installer Provisioned Infrastructure. This template uses
packet.net to spin up a single baremetal server and test
the ipi installer on it.
Due to openshift/ci-tools#136 I'll add a seperate PR to use it in a presubmit job
Update:
updates cluster-launch-installer-e2e, No longer adds a template