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.

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

Add an e2e-openstack job #1824

Merged
merged 1 commit into from
Oct 26, 2018
Merged

Conversation

flaper87
Copy link
Contributor

@flaper87 flaper87 commented Oct 5, 2018

This is far from done but I thought I'd throw it out there to get some feedback and guidance

@openshift-ci-robot openshift-ci-robot added 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. labels Oct 5, 2018
@flaper87 flaper87 force-pushed the openstack branch 2 times, most recently from ba55d30 to 341c711 Compare October 11, 2018 10:34
name: pull-ci-openshift-installer-master-e2e-openstack
optional: true
rerun_command: /test e2e-openstack
run_if_changed: ^([^d]|d(d|o(d|cd))*([^do]|o([^cd]|c[^ds])))*(d(d|o(d|cd))*(oc?)?)?$
Copy link
Contributor

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copied it from the aws one. Let me ask you a couple of questions:

  • Should run_if_changed be set?
  • What should it be set to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed this line for now. I'd appreciate some info about how to use it properly, tho. Thanks a bunch

Copy link
Contributor

Choose a reason for hiding this comment

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

You shouldn't need it

skip_cloning: true
spec:
containers:
- command:
Copy link
Contributor

Choose a reason for hiding this comment

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

Use

- command:
  - ci-operator
- args:
  - --give...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks

name: cluster-profile
- mountPath: /usr/local/e2e-openstack
name: job-definition
subPath: cluster-launch-installer-e2e.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

@bbguimaraes do we generate this one?

Copy link
Contributor

Choose a reason for hiding this comment

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

The template, yes, but not the profile (which doesn't seem to exist).

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, can you help @flaper87 use the generator here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stevekuznetsov any points on how to use the generator? As I mentioned in the PR description, I'll need some guidance as I'm new to the community and the infrastructure.

thanks a bunch for your reviews! 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ran ./hack/update-generated-config.sh

@flaper87 flaper87 changed the title WIP: Add an e2e-openstack job Add an e2e-openstack job Oct 12, 2018
@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 Oct 12, 2018
@flaper87
Copy link
Contributor Author

Since the job is optional, I think it's fine to merge it and then improve it in subsequent patches as needed. I still have to upload the credentials for it to be able to run loads on an OpenStack cloud. I'm figuring that out in parallel.

@bbguimaraes
Copy link
Contributor

@flaper87: we can already generate jobs for this template, but the openstack cluster profile is still not in the generator's list. Regardless of auto-generation, you will need to get the cluster profile in the repository (here) to be able to use it in E2E tests.

After that, if the profile is meant to be used by other jobs and you want to be able to auto-generate jobs, you can take a look at this PR where new cluster profiles are being added to the generator.

@flaper87
Copy link
Contributor Author

@flaper87: we can already generate jobs for this template, but the openstack cluster profile is still not in the generator's list. Regardless of auto-generation, you will need to get the cluster profile in the repository (here) to be able to use it in E2E tests.

After that, if the profile is meant to be used by other jobs and you want to be able to auto-generate jobs, you can take a look at this PR where new cluster profiles are being added to the generator.

@wking can you provide some guidance here?

I looked into the existing profiles but it seems they serve a different purpose. Does the existing e2e-aws test for the installer use any of these profiles?

@flaper87
Copy link
Contributor Author

@wking @bbguimaraes can you help me understand what the next step on this PR is? I'm a bit confused on whether a profile is actually needed and how that should be done if so.

thanks 😄

@wking
Copy link
Member

wking commented Oct 24, 2018

You pobably need an openstack case over here too. @sallyom also has some notes for testing the templates locally.

@flaper87
Copy link
Contributor Author

You pobably need an openstack case over here too. @sallyom also has some notes for testing the templates locally.

Hey, thanks a lot! I'll add the missing case.

I've tested this job with @sallyom's tool already and it seemed to get as far as I expected it to get. I'll do more tests today and report back.

Thanks a bunch!

@flaper87
Copy link
Contributor Author

@wking I've pushed a new version and added the missing case and a few other env vars that are required.

I've tested this job with @sallyom installer-e2e and it starts successfully. I've also added an OpenStack cluster profile to ci-operator openshift/ci-operator#191

One thing that is missing are the OpenStack credentials. Who should I talk to to upload those credentials to api.ci ? @bbguimaraes @stevekuznetsov ?

@droslean
Copy link
Member

/test generated-config

1 similar comment
@flaper87
Copy link
Contributor Author

/test generated-config

@flaper87
Copy link
Contributor Author

Added the openstack profile to openshift/ci-operator-prowgen#40

@flaper87
Copy link
Contributor Author

/test generated-config

@droslean
Copy link
Member

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 26, 2018
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: droslean, flaper87

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 Oct 26, 2018
@openshift-merge-robot openshift-merge-robot merged commit 17ffc43 into openshift:master Oct 26, 2018
@openshift-ci-robot
Copy link
Contributor

@flaper87: Updated the following 3 configmaps:

  • prow-job-cluster-launch-installer-e2e configmap using the following files:
    • key cluster-launch-installer-e2e.yaml using file ci-operator/templates/openshift/installer/cluster-launch-installer-e2e.yaml
  • ci-operator-configs configmap using the following files:
    • key openshift-installer-master.yaml using file ci-operator/config/openshift/installer/openshift-installer-master.yaml
  • job-config configmap using the following files:
    • key openshift-installer-master-presubmits.yaml using file ci-operator/jobs/openshift/installer/openshift-installer-master-presubmits.yaml

In response to this:

This is far from done but I thought I'd throw it out there to get some feedback and guidance

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
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. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants