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

Don't skip external net test in external net E2E #1686

Merged
merged 2 commits into from
Feb 16, 2022

Conversation

dfarrell07
Copy link
Member

The current Shipyard default is to skip tests labled external-dataplane.
Override this in tests that are meant to test external networks.

Signed-off-by: Daniel Farrell dfarrell@redhat.com

The current Shipyard default is to skip tests labled external-dataplane.
Override this in tests that are meant to test external networks.

Signed-off-by: Daniel Farrell <dfarrell@redhat.com>
@submariner-bot
Copy link
Contributor

🤖 Created branch: z_pr1686/dfarrell07/test_ext_net2
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

@dfarrell07
Copy link
Member Author

You can see this working here:

2022-02-03T22:54:33.5451280Z ##[group]Run make "e2e" using="external-net globalnet"
2022-02-03T22:54:33.5451622Z �[36;1mmake "e2e" using="external-net globalnet"�[0m
2022-02-03T22:54:33.5494145Z shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
2022-02-03T22:54:33.5494424Z env:
2022-02-03T22:54:33.5494632Z   CLUSTERS_ARGS: --k8s_version="1.23"
2022-02-03T22:54:33.5494855Z   FOCUS: \[.*\]
2022-02-03T22:54:33.5495043Z   SKIP: 
2022-02-03T22:54:33.5495211Z   PLUGIN: 
2022-02-03T22:54:33.5495409Z   E2E_TESTDIR: test/e2e
2022-02-03T22:54:33.5495600Z ##[endgroup]

dfarrell07#47

@dfarrell07
Copy link
Member Author

@mkimuram If you're okay with it, it would be great to have your review "officially" through "Files changed"->"Review changes". We really need more code reviewers around Submariner, and your review combined with Sridhar's as an Owner would allow this to be merged.

But I'm realizing we still need to get you added as a Member and I'm not sure if you have permission to review currently. Can you let us know if it's possible to review with your current permissions? And even better, we'd love to have you submit the Member Request Template. I'd be glad to be a sponsor, I'm sure @sridhargaddam and others would be happy to be a second one.

@mkimuram
Copy link
Contributor

mkimuram commented Feb 7, 2022

@dfarrell07 @sridhargaddam

I've opened a member request #1688 . Please approve it.

Copy link
Contributor

@mkimuram mkimuram left a comment

Choose a reason for hiding this comment

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

The change looks good to me (and let me try to approve).

@submariner-bot submariner-bot added the ready-to-test When a PR is ready for full E2E testing label Feb 7, 2022
@dfarrell07
Copy link
Member Author

@mkimuram Now that you're a Member (:partying_face:), I think if you use the UI to vote "approve" again you'll have a green check and will be able to merge the PR.

Copy link
Contributor

@mkimuram mkimuram left a comment

Choose a reason for hiding this comment

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

Approved.

@dfarrell07
Copy link
Member Author

@mkimuram Humm, it seems you're not in the member group yet. Do you have an invite in your email you need to accept?

@mkimuram mkimuram self-requested a review February 10, 2022 13:55
@mkimuram
Copy link
Contributor

@dfarrell07

I've accepted the invite and I should be member now. It seems that the code needs to be rebased.

@dfarrell07
Copy link
Member Author

This is currently failing with:

[Fail] [external-dataplane] Connectivity [It] should be able to connect from an external app to a pod in a cluster 
/go/src/github.com/submariner-io/submariner/vendor/github.com/submariner-io/shipyard/test/e2e/framework/network_pods.go:221

I think this is exposing an existing failure by fixing CI though, so we should go ahead and merge. There are other PRs in review to fix issues with external networks.

@dfarrell07
Copy link
Member Author

I think this is exposing an existing failure by fixing CI though, so we should go ahead and merge.

After 24h for comment, going to go ahead and assume this is okay and merge.

@dfarrell07 dfarrell07 merged commit 8d18f83 into submariner-io:devel Feb 16, 2022
@submariner-bot
Copy link
Contributor

🤖 Closed branches: [z_pr1686/dfarrell07/test_ext_net2]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation ready-to-test When a PR is ready for full E2E testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants