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

Dual Stack OVN-Kubernetes job #1520

Merged
merged 1 commit into from
Jul 27, 2020
Merged

Dual Stack OVN-Kubernetes job #1520

merged 1 commit into from
Jul 27, 2020

Conversation

aojea
Copy link
Contributor

@aojea aojea commented Jul 21, 2020

- What this PR does and why is it needed

Set up a periodic job to test dual stack

- Special notes for reviewers

This does not affect merging, since the job added runes periodically and is only informative

- How to verify it

You can see an example of how it runs here
https://github.com/aojea/ovn-kubernetes/actions/runs/184296553

- Description for the changelog

@coveralls
Copy link

coveralls commented Jul 21, 2020

Coverage Status

Coverage decreased (-0.1%) to 58.54% when pulling 2b4568b on aojea:dual into fe9a632 on ovn-org:master.

@aojea
Copy link
Contributor Author

aojea commented Jul 22, 2020

Results here, it is working
https://github.com/aojea/ovn-kubernetes/runs/899312656

@aojea
Copy link
Contributor Author

aojea commented Jul 22, 2020

/assign @dcbw

Results here, it is working
https://github.com/aojea/ovn-kubernetes/runs/899312656

It's working the job , not all the tests 🙃

@aojea
Copy link
Contributor Author

aojea commented Jul 23, 2020

/retest

4 similar comments
@aojea
Copy link
Contributor Author

aojea commented Jul 23, 2020

/retest

@aojea
Copy link
Contributor Author

aojea commented Jul 23, 2020

/retest

@aojea
Copy link
Contributor Author

aojea commented Jul 27, 2020

/retest

@aojea
Copy link
Contributor Author

aojea commented Jul 27, 2020

/retest

Add a dual stack job that runs periodically.

It's also possible to run the job on demand.

This is just needed to give signal, since some of the
tests are known to fail.
Once it is stable we should add a premerge job.

Signed-off-by: Antonio Ojea <aojea@redhat.com>
@dcbw
Copy link
Contributor

dcbw commented Jul 27, 2020

/lgtm

@danwinship
Copy link
Contributor

yeah lgtm too. I don't really know the CI setup stuff though

@Billy99
Copy link
Contributor

Billy99 commented Jul 27, 2020

So this just runs periodic job on master? Why would we limit it to just dual-stack? For the IPv6 CI PR, I limited the test matrix just to reduce the test time for each PR. The periodic run could test those additional matrix items in the background for sanity.

@aojea
Copy link
Contributor Author

aojea commented Jul 27, 2020

So this just runs periodic job on master? Why would we limit it to just dual-stack? For the IPv6 CI PR, I limited the test matrix just to reduce the test time for each PR. The periodic run could test those additional matrix items in the background for sanity.

1 step at a time 😄

we can add more periodic jobs later, we just need dual stack ASAP

also, @Billy99 , if we merge this #1500, we can enable more combinations on presubmit

Copy link
Contributor

@Billy99 Billy99 left a comment

Choose a reason for hiding this comment

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

/lgtm
Just a few questions.

run: |
docker load --input image.tar

- name: kind setup
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this be replaced with install-kind.sh at some time in the future? Assume you need additional patches to KIND for dual-stack (https://github.com/aojea/kind/releases/download/dualstack/kind).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, but right now is a chicken an egg problem, we need to stabilize dual-stack first.
This way we avoid any risk of affecting other things in the repo, and allows us to have signal on master

@@ -0,0 +1,226 @@
name: ovn-ci-periodic
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this file have to always be separate from test.yml? Always get nervous when there is a lot the same or real similar logic in two places and trying to make sure they stay in synch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same comment as in here #1520 (comment)

until it is stable, I expect lot of churn in this file, we can do any changes here without impacting the rest of the jobs

@dcbw
Copy link
Contributor

dcbw commented Jul 27, 2020

@aojea I'll merge becuase we have a bunch of LGTMs, but can you answer Billy's questions too? Thanks!

@dcbw dcbw merged commit f592712 into ovn-kubernetes:master Jul 27, 2020
andreaskaris pushed a commit to andreaskaris/ovn-kubernetes that referenced this pull request Jul 18, 2023
…e-snat-4.12

OCPBUGS-7317: [release-4.12] Delete stale egress ip snat entries by node
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants