Skip to content

e2e: Prepare to support new openshift-tests binary in 4.0 jobs#2108

Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
smarterclayton:launch_aws
Nov 13, 2018
Merged

e2e: Prepare to support new openshift-tests binary in 4.0 jobs#2108
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
smarterclayton:launch_aws

Conversation

@smarterclayton
Copy link
Copy Markdown
Contributor

The new binary is simpler and easier to reason about.

Also add a launch-aws test for installer to allow bringing a cluster up.

Testing with openshift/installer#406 and openshift/origin#21459

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: smarterclayton

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-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. sig/azure Categorizes item related to Azure jobs labels Nov 12, 2018
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this just while you're testing? Won't we want the function and script to return the openshift-tests exit status?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it should already be set -e

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it should already be set -e

Oh, right. exit 0 is fine then, although I'd prefer a bare exit to avoid confusing folks like me who forget about the set -e ;).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the rest of the function avoids bare exits to be clear. In the long run, this would simply be a call to Openshift-tests, but until we remove the other sections it's better to be consistent.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need to sleep? If we just want to exercise the setup and teardown containers, couldn't this command just be exit 0?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the point of launch is to hold the cluster open so you can use it, that's why it's sleeping

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

basically you run /test launch-aws and then you have a cluster you can use for X minutes.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

basically you run /test launch-aws and then you have a cluster you can use for X minutes.

Ah, this makes sense. Can we put a comment about that into the commit message in case I don't remember later? A comment in the YAML would be fine too, but we sometimes mechanically rewrite YAML and dump any comments ;).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, launch is a fairly general convention we have. It'll be part of the general CI docs.

@smarterclayton smarterclayton force-pushed the launch_aws branch 3 times, most recently from f22f078 to d2963da Compare November 13, 2018 02:32
The new binary is simpler and easier to reason about.

Also add a `launch-aws` test for installer that brings a cluster up for
~2 hours to allow debugging failures in a realistic environment. Like
older e2e-gcp-launch jobs, this allows you to access the cluster via
the console to get the secret.

Make test trigger bypass 'all' for jobs that don't run all the time.
@smarterclayton
Copy link
Copy Markdown
Contributor Author

Tested in runs, merging.

@smarterclayton smarterclayton added the lgtm Indicates that a PR is ready to be merged. label Nov 13, 2018
@openshift-merge-robot openshift-merge-robot merged commit 563f21d into openshift:master Nov 13, 2018
wking added a commit to wking/openshift-release that referenced this pull request Feb 7, 2019
The job was first added in b9c3617 (e2e: Prepare to support new
openshift-tests binary in 4.0 jobs, 2018-11-12, openshift#2108) to provide a
scratch cluster for manually poking around.  But we don't actually
need it to gate merges, so make it optional.
@smarterclayton smarterclayton deleted the launch_aws branch April 24, 2019 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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. sig/azure Categorizes item related to Azure jobs size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants