Skip to content

Add e2e-aws-ovn-kubernetes tests for ovn-kubernetes and cluster-network-operator #4295

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
Merged
openshift-merge-robot merged 5 commits into from
Aug 19, 2019
Merged

Add e2e-aws-ovn-kubernetes tests for ovn-kubernetes and cluster-network-operator #4295

openshift-merge-robot merged 5 commits into from
Aug 19, 2019

Conversation

@dcbw
Copy link
Contributor

@dcbw dcbw commented Jul 5, 2019

We're going to add (optional) e2e-aws tests for ovn-kubernetes.

@rcarrillocruz @danwinship @wking

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 5, 2019
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 5, 2019
@dcbw dcbw changed the title templates/openshift/installer: allow overriding network type for e2e_aws Add e2e-aws-ovn-kubernetes tests for ovn-kubernetes and cluster-network-operator Jul 5, 2019
danwinship
danwinship suggested changes Jul 8, 2019
View reviewed changes
@dcbw dcbw force-pushed the override-network-type branch from 42fe59a to bbe8384 Compare July 17, 2019 02:40
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 17, 2019
@dcbw dcbw force-pushed the override-network-type branch from bbe8384 to b9f2abd Compare July 17, 2019 02:52
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 2, 2019
@dcbw dcbw force-pushed the override-network-type branch from b9f2abd to 69720e8 Compare August 6, 2019 18:53
@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 6, 2019
@dcbw dcbw force-pushed the override-network-type branch from 69720e8 to f24ca53 Compare August 6, 2019 19:09
@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. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 6, 2019
@dcbw dcbw force-pushed the override-network-type branch 8 times, most recently from 3a54aac to 5b83678 Compare August 6, 2019 20:56
@dcbw dcbw force-pushed the override-network-type branch 2 times, most recently from 9f74c2c to 2e185d4 Compare August 7, 2019 16:26
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 11, 2019
@danwinship danwinship force-pushed the override-network-type branch from 2e185d4 to 44ef9fc Compare August 13, 2019 12:17
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 13, 2019
@danwinship
Copy link
Contributor

danwinship commented Aug 13, 2019

why did this change

The stuff that looked "changed" was because the commit was simultaneously removing the old openshift-sdn-based e2e-aws and adding a new e2e-aws-ovn-kubernetes in the same place. I split that into two commits to make it clearer that we're not changing the existing test, we're replacing it.

Also made the test always_run: false even in openshift/ovn-kubernetes for now, until we're sure it's ready for prime time, removed some crufty bits, and fixed the CLUSTER_NETWORK_MANIFEST to include the full object.

danwinship and others added 4 commits August 13, 2019 10:07
@danwinship
templates: allow overriding networkType
ade608f 
The CLUSTER_NETWORK_MANIFEST override that was added for
e2e-aws-multitenant doesn't work for e2e-aws-ovn-kubernetes because
the network type needs to be overridden in install-config.yaml before
the manifests are created.
@dcbw @danwinship
cluster-network-operator: add e2e-aws test for OVNKubernetes
f53bd9c
@dcbw @danwinship
ovn-kubernetes: add e2e-aws test for OVNKubernetes
1591c18
@danwinship
ovn-kubernetes: drop (openshift-sdn-based) e2e-aws
ad63424 
It doesn't make any sense to run the openshift-sdn-based e2e-aws from
the ovn-kubernetes repo.
@danwinship danwinship force-pushed the override-network-type branch from 44ef9fc to ad63424 Compare August 13, 2019 14:07
@openshift-ci-robot openshift-ci-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 13, 2019
@danwinship
Copy link
Contributor

danwinship commented Aug 13, 2019

ok, the rehearsals ran the test suite correctly.

I needed to make another change to cluster-launch-installer-e2e.yaml, because you can't override networking.networkType by editing the network.operator.io config, because it has already been copied into the network.config.io config at that point, which will override it. So we need to be able to set it in the install-config. Yes, it's annoying that we ended up with 2 separate networking config mechanisms, and we should come up with something better later, but for now we really need to get ovn-kubernetes CI working.

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Aug 13, 2019

@dcbw: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/rehearse/openshift/ovn-kubernetes/master/e2e-aws 42fe59aaf3533861fb2df37b683a92b6fa66acfe link /test pj-rehearse
ci/rehearse/openshift/ovn-kubernetes/master/e2e-aws-ovn-kubernetes ad63424 link /test pj-rehearse
ci/rehearse/openshift/cluster-network-operator/master/e2e-aws-ovn-kubernetes ad63424 link /test pj-rehearse
ci/rehearse/openshift/cluster-api-provider-azure/master/e2e-azure ad63424 link /test pj-rehearse
ci/rehearse/openshift/installer/master/e2e-gcp ad63424 link /test pj-rehearse
ci/prow/pj-rehearse ad63424 link /test pj-rehearse

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.

Details

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. I understand the commands that are listed here.

@dcbw
Copy link
Contributor Author

dcbw commented Aug 13, 2019 •
edited
Loading

/lgtm

@danwinship haha, I guess you can LGTM this and then @stevekuznetsov can approve.

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Aug 13, 2019

@dcbw: you cannot LGTM your own PR.

Details

In response to this:

/lgtm

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.

@danwinship
Copy link
Contributor

danwinship commented Aug 13, 2019

need someone to approve the installer bits
/assign @wking

@danwinship
Copy link
Contributor

danwinship commented Aug 19, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 19, 2019
@wking
Copy link
Member

wking commented Aug 19, 2019

I don't really get the changed vs. replacing distinction; you're still running the same test suite, just against a different networking provider, right? But I don't have a stake in how you name your per-repo tests, and the cluster-launch-installer-e2e.yaml change looks fine, so:

/approve

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Aug 19, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danwinship, dcbw, wking

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 the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 19, 2019
@openshift-merge-robot openshift-merge-robot merged commit 1ec935e into openshift:master Aug 19, 2019
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Aug 19, 2019

@dcbw: Updated the following 5 configmaps:

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

In response to this:

We're going to add (optional) e2e-aws tests for ovn-kubernetes.

@rcarrillocruz @danwinship @wking

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.

@ravisantoshgudimetla ravisantoshgudimetla mentioned this pull request Oct 25, 2019
Merged
wking added a commit to wking/openshift-release that referenced this pull request Mar 12, 2020
@wking
ci-operator: Drop CLUSTER_NETWORK_TYPE (in favor of CLUSTER_VARIANT)
2f442c8 
CLUSTER_NETWORK_TYPE is from ade608f (templates: allow overriding
networkType, 2019-08-13, openshift#4295).  But we've had ovn in CLUSTER_VARIANT
since 8176a42 (jobs: Unify all options under CLUSTER_VARIANT,
2019-11-25, openshift#6077).  Drop CLUSTER_NETWORK_TYPE (which was only set by
e2e-gcp-ovn-upgrade jobs) and use CLUSTER_VARIANT instead.
@wking wking mentioned this pull request Mar 12, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hardys hardys Awaiting requested review from hardys

@staebler staebler Awaiting requested review from staebler

2 more reviewers

@stevekuznetsov stevekuznetsov stevekuznetsov left review comments

@danwinship danwinship danwinship requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

@danwinship danwinship

@wking wking

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/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@dcbw @danwinship @openshift-ci-robot @wking @stevekuznetsov @openshift-merge-robot