Skip to content

WIP Run BYOR install on installer presubmits #2481

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
Closed
vrutkovs wants to merge 1 commit into from
Closed

WIP Run BYOR install on installer presubmits #2481

vrutkovs wants to merge 1 commit into from

Conversation

@vrutkovs
Copy link
Contributor

@vrutkovs vrutkovs commented Jan 2, 2019

This PR would ensure openshift-ansible is not broken by installer changes.
always_run is set to False, so it would be started by request for now.

PTAL @wking @crawford

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 2, 2019
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jan 2, 2019

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: vrutkovs
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: steveej

If they are not already assigned, you can assign the PR to them by writing /assign @steveej in a comment when ready.

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

wking
wking reviewed Jan 2, 2019
View reviewed changes
Copy link
Member

@wking wking 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 reasonable to me, although I'm not comfortable enough with these configs to want to claim that "looks reasonable" means "likely to work" ;). Still, with always_run: false, I'm fine landing this even if it turns out to be broken (it may be fine, I dunno).

Do we also want to set optional: true so folks who do run this manually can still merge the PR even if the tests fail? Without optional, it's not clear to me how we get over installer changes that do require Ansible changes. Wouldn't the openshift-ansible PR block on e2e-aws failures while the installer PR blocked on e2e-byor failures?

ci-operator/config/openshift/installer/openshift-installer-master.yaml Outdated
Copy link
Member

@wking wking Jan 2, 2019

Choose a reason for hiding this comment

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

Looks like you're missing a trailing newline here.

ci-operator/config/openshift/installer/openshift-installer-master.yaml Outdated
Copy link
Member

@wking wking Jan 2, 2019

Choose a reason for hiding this comment

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

Looks like these run-tests doublings were typos, see #2278.

ci-operator/jobs/openshift/installer/openshift-installer-master-presubmits.yaml Outdated
Copy link
Member

@wking wking Jan 2, 2019

Choose a reason for hiding this comment

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

Here's another run-tests doubling.

@stevekuznetsov
Copy link
Contributor

stevekuznetsov commented Jan 2, 2019

Why not test these using the local testing tools before merge? :)

@vrutkovs vrutkovs changed the title Run BYOR install on installer presubmits WIP Run BYOR install on installer presubmits Jan 2, 2019
@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 Jan 2, 2019
@vrutkovs
Copy link
Contributor Author

vrutkovs commented Jan 2, 2019

I'll mark this is as WIP as it would currently always fail due to changes in openshift/installer#955

@vrutkovs vrutkovs force-pushed the byor-4.0-installer-cli-cmds branch from 34545e7 to 98e96f4 Compare January 4, 2019 18:59
Run BYOR install on installer presubmits
1bade7f 
This PR would ensure openshift-ansible is not broken by installer changes.
`always_run` is set to False, so it would be started by request for now.

PTAL @wking @crawford
@vrutkovs vrutkovs force-pushed the byor-4.0-installer-cli-cmds branch from 98e96f4 to 1bade7f Compare January 4, 2019 19:01
@vrutkovs
Copy link
Contributor Author

vrutkovs commented Jan 4, 2019 •
edited
Loading

Fixed syntax error, duplicated run-tests and added optional.

Two more PRs are required to be merged for this test to pass:

@vrutkovs
Copy link
Contributor Author

vrutkovs commented Feb 20, 2019

This is no longer planned

@vrutkovs vrutkovs closed this Feb 20, 2019
@vrutkovs vrutkovs deleted the byor-4.0-installer-cli-cmds branch February 20, 2019 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@russellb russellb Awaiting requested review from russellb

1 more reviewer

@wking wking wking left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vrutkovs @openshift-ci-robot @stevekuznetsov @wking