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

Fix an issue when generating ExternalEntity from ExternalNode #4259

Merged
merged 1 commit into from
Oct 17, 2022

Conversation

wenyingd
Copy link
Contributor

  1. Use group/apiversion for OwnerReference when creating ExternalEntity from ExternalNode
  2. Add unit tests for ExternalNodeController

Signed-off-by: wenyingd wenyingd@vmware.com

@codecov
Copy link

codecov bot commented Sep 29, 2022

Codecov Report

Merging #4259 (944620d) into main (b2ba248) will decrease coverage by 1.68%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4259      +/-   ##
==========================================
- Coverage   61.30%   59.61%   -1.69%     
==========================================
  Files         388      378      -10     
  Lines       55145    54682     -463     
==========================================
- Hits        33806    32601    -1205     
- Misses      18799    19621     +822     
+ Partials     2540     2460      -80     
Flag Coverage Δ
e2e-tests 39.47% <0.00%> (?)
integration-tests 34.39% <ø> (-0.03%) ⬇️
kind-e2e-tests 47.56% <0.00%> (+4.92%) ⬆️
unit-tests 44.59% <100.00%> (-0.45%) ⬇️
Impacted Files Coverage Δ
pkg/controller/externalnode/controller.go 66.16% <100.00%> (+66.16%) ⬆️
pkg/ipfix/ipfix_intermediate.go 0.00% <0.00%> (-90.91%) ⬇️
pkg/ipfix/ipfix_collector.go 0.00% <0.00%> (-83.34%) ⬇️
...trollers/multicluster/label_identity_controller.go 0.00% <0.00%> (-81.73%) ⬇️
pkg/cni/client.go 0.00% <0.00%> (-77.78%) ⬇️
pkg/flowaggregator/certificate.go 0.00% <0.00%> (-75.53%) ⬇️
...rs/multicluster/commonarea/clusterinfo_importer.go 0.00% <0.00%> (-74.51%) ⬇️
...s/multicluster/label_identity_export_controller.go 0.00% <0.00%> (-72.93%) ⬇️
pkg/flowaggregator/exporter/utils.go 0.00% <0.00%> (-72.73%) ⬇️
...kg/agent/flowexporter/connections/conntrack_ovs.go 0.00% <0.00%> (-70.91%) ⬇️
... and 106 more

@wenyingd wenyingd force-pushed the externalentity_owner_apiversion branch from a43990b to 04e0766 Compare September 29, 2022 10:25
@wenyingd wenyingd mentioned this pull request Sep 30, 2022
37 tasks
@wenyingd wenyingd requested a review from tnqn September 30, 2022 09:52
pkg/controller/externalnode/controller.go Outdated Show resolved Hide resolved
pkg/controller/externalnode/controller.go Outdated Show resolved Hide resolved
pkg/controller/externalnode/controller_test.go Outdated Show resolved Hide resolved
pkg/controller/externalnode/controller_test.go Outdated Show resolved Hide resolved
pkg/controller/externalnode/controller_test.go Outdated Show resolved Hide resolved
pkg/controller/externalnode/controller_test.go Outdated Show resolved Hide resolved
pkg/controller/externalnode/controller_test.go Outdated Show resolved Hide resolved
pkg/controller/externalnode/controller_test.go Outdated Show resolved Hide resolved
@wenyingd wenyingd force-pushed the externalentity_owner_apiversion branch from 04e0766 to 8797d1f Compare October 6, 2022 02:07
@wenyingd wenyingd requested a review from tnqn October 6, 2022 02:07
@wenyingd wenyingd force-pushed the externalentity_owner_apiversion branch from 8797d1f to 08f3a80 Compare October 6, 2022 02:46
@tnqn tnqn added this to the Antrea v1.9 release milestone Oct 11, 2022
ExternalNode: "vm1",
},
}
testDeleteExternalNode(t, externalNode, expectedEntity)
Copy link
Member

Choose a reason for hiding this comment

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

same as testAddExternalNode, separting input and test code makes the test less readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

informerFactory.WaitForCacheSync(stopCh)
err := controller.reconcileExternalNodes()
require.NoError(t, err)
err = wait.PollImmediate(time.Millisecond*50, time.Second, func() (done bool, err error) {
Copy link
Member

Choose a reason for hiding this comment

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

Unlike other tests that rely on Run method to create/delete objects asynchronously, reconcileExternalNodes does everything synchronously, I think it should do the following check immediately and one-time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

@wenyingd wenyingd force-pushed the externalentity_owner_apiversion branch from 08f3a80 to 717d13b Compare October 13, 2022 06:16
@wenyingd wenyingd requested a review from tnqn October 13, 2022 06:16
@wenyingd wenyingd force-pushed the externalentity_owner_apiversion branch from 717d13b to 45548ac Compare October 17, 2022 06:10
1. Use group/apiversion for ownerreference when creating ExternalEntity
   from ExternalNode
2. Add unit tests for ExternalNodeController

Signed-off-by: wenyingd <wenyingd@vmware.com>
@wenyingd wenyingd force-pushed the externalentity_owner_apiversion branch from 45548ac to 944620d Compare October 17, 2022 06:14
@wenyingd
Copy link
Contributor Author

/test-all

Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM

@tnqn tnqn added the action/release-note Indicates a PR that should be included in release notes. label Oct 17, 2022
@tnqn
Copy link
Member

tnqn commented Oct 17, 2022

The unit test failure is a known issue on main branch: #4294, no need to rerun.

@tnqn tnqn merged commit 549e0fb into antrea-io:main Oct 17, 2022
heanlan pushed a commit to heanlan/antrea that referenced this pull request Mar 29, 2023
…-io#4259)

1. Use group/apiversion for ownerreference when creating ExternalEntity
   from ExternalNode
2. Add unit tests for ExternalNodeController

Signed-off-by: wenyingd <wenyingd@vmware.com>
@wenyingd wenyingd deleted the externalentity_owner_apiversion branch May 30, 2023 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/release-note Indicates a PR that should be included in release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants