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

E2e tests check stateful sets #206

Conversation

maiqueb
Copy link
Collaborator

@maiqueb maiqueb commented Mar 8, 2022

No description provided.

maiqueb added 2 commits March 2, 2022 00:33
The IP reconcile loop also requires the IP ranges in a normalized
format; as such, we export it into a function, which will be used in a
follow-up commit.

Signed-off-by: Miguel Duarte Barroso <[email protected]>
@maiqueb
Copy link
Collaborator Author

maiqueb commented Mar 8, 2022

Hi @xagent003 . I'm trying to come up with e2e test to reproduce the issues you've reported about the leaking IP addresses.

Would you take a look at the top 2 commits in this PR to see if it makes sense to you ?

I have unfortunately not been able to reproduce them so far; would appreciate your help to figure out what I'm doing wrong.

@maiqueb
Copy link
Collaborator Author

maiqueb commented Mar 8, 2022

/cc @nicklesimba

@coveralls
Copy link

coveralls commented Mar 8, 2022

Pull Request Test Coverage Report for Build 1962247774

  • 16 of 22 (72.73%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.3%) to 40.811%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/api/whereabouts.cni.cncf.io/v1alpha1/register.go 16 22 72.73%
Totals Coverage Status
Change from base Build 1917045460: 0.3%
Covered Lines: 453
Relevant Lines: 1110

💛 - Coveralls

@maiqueb maiqueb force-pushed the e2e-tests-check-stateful-sets branch from cdd890a to 1485df3 Compare March 9, 2022 09:06
e2e/e2e_test.go Outdated Show resolved Hide resolved
return false, err
}

return isStatefulSetEmpty(statefulSet) && areAssociatedPodsGone(associatedPods), nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

Garbage collection (foreground cascading deletion) won't work for cleaning up pods, I guess?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It does, I just haven't used it yet.

e2e/e2e_test.go Outdated
"range": "10.10.0.0/16",
"log_level": "debug",
"log_file": "/tmp/wb"
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand this change, can you explain it please?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

4f2a691

I was wrong in adapting this in this PR, it is not needed, since whereabouts is only receiving the configuration for a single plugin at a time. On the other hand, it is required for the ip reconciliation loop.

If you try the test I've added in that commit without the associated code changes, you'll see it fails.

maiqueb added 3 commits March 10, 2022 10:26
These ugly tests do not cleanup after themselves; this way, the golang
based tests (which **do** cleanup after themselves) will not be impacted by
these left-overs.

Signed-off-by: Miguel Duarte Barroso <[email protected]>
Signed-off-by: Miguel Duarte Barroso <[email protected]>
@maiqueb maiqueb force-pushed the e2e-tests-check-stateful-sets branch from 1485df3 to 7360c64 Compare March 10, 2022 09:26
@maiqueb
Copy link
Collaborator Author

maiqueb commented Apr 18, 2022

Abandoning since this change is featured in (already merged PR #185 ).

@maiqueb maiqueb closed this Apr 18, 2022
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.

3 participants