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 a race condition in networkpolicy_test.go #312

Merged
merged 1 commit into from Jun 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/network/node/networkpolicy.go
Expand Up @@ -44,7 +44,7 @@ type networkPolicyPlugin struct {
// namespaces includes only the namespaces that we have a VNID for, and is used
// for all flow-generating methods
namespaces map[uint32]*npNamespace
// nsMatchCache caches matches for namespaceSelectors; see selectNamespaceInternal
// nsMatchCache caches matches for namespaceSelectors; see selectNamespacesInternal
nsMatchCache map[string]*npCacheEntry
}

Expand Down
39 changes: 21 additions & 18 deletions pkg/network/node/networkpolicy_test.go
Expand Up @@ -163,7 +163,10 @@ func uid(npns *npNamespace, name string) ktypes.UID {
// watches/flows. It does not require that matches lists every policy in npns; any extra
// policies in npns that aren't in matches will just be ignored (other than the fact that
// nPolicies must still be correct).
func assertPolicies(npns *npNamespace, nPolicies int, matches map[string]*npPolicy) error {
func assertPolicies(np *networkPolicyPlugin, npns *npNamespace, nPolicies int, matches map[string]*npPolicy) error {
np.lock.Lock()
defer np.lock.Unlock()

var matched []string
for _, npp := range npns.policies {
match := matches[npp.policy.Name]
Expand Down Expand Up @@ -352,7 +355,7 @@ func TestNetworkPolicy(t *testing.T) {

// Each namespace should now have 2 policies, each with a single flow
for _, npns := range np.namespaces {
err := assertPolicies(npns, 2, map[string]*npPolicy{
err := assertPolicies(np, npns, 2, map[string]*npPolicy{
"allow-from-self": {
watchesNamespaces: false,
watchesAllPods: false,
Expand Down Expand Up @@ -384,7 +387,7 @@ func TestNetworkPolicy(t *testing.T) {
addPods(np, npns)

// There are no pod-selecting policies yet, so nothing should have changed
err := assertPolicies(npns, 2, nil)
err := assertPolicies(np, npns, 2, nil)
if err != nil {
t.Error(err.Error())
}
Expand Down Expand Up @@ -416,7 +419,7 @@ func TestNetworkPolicy(t *testing.T) {
})
waitForSync(np, synced, "networkpolicy sync")

err = assertPolicies(npns, 3, map[string]*npPolicy{
err = assertPolicies(np, npns, 3, map[string]*npPolicy{
"allow-client-to-server": {
watchesNamespaces: false,
watchesAllPods: false,
Expand Down Expand Up @@ -458,7 +461,7 @@ func TestNetworkPolicy(t *testing.T) {
})
waitForSync(np, synced, "networkpolicy sync")

err := assertPolicies(npns1, 4, map[string]*npPolicy{
err := assertPolicies(np, npns1, 4, map[string]*npPolicy{
"allow-from-even": {
watchesNamespaces: true,
watchesAllPods: false,
Expand Down Expand Up @@ -507,7 +510,7 @@ func TestNetworkPolicy(t *testing.T) {
})
waitForSync(np, synced, "networkpolicy sync")

err = assertPolicies(npns1, 5, map[string]*npPolicy{
err = assertPolicies(np, npns1, 5, map[string]*npPolicy{
"allow-from-odd-primes": {
watchesNamespaces: true,
watchesAllPods: true,
Expand Down Expand Up @@ -551,7 +554,7 @@ func TestNetworkPolicy(t *testing.T) {
})
waitForSync(np, synced, "networkpolicy sync")

err = assertPolicies(npns2, 4, map[string]*npPolicy{
err = assertPolicies(np, npns2, 4, map[string]*npPolicy{
"allow-from-all-clients": {
watchesNamespaces: true,
watchesAllPods: true,
Expand Down Expand Up @@ -593,7 +596,7 @@ func TestNetworkPolicy(t *testing.T) {
for vnid, npns := range np.namespaces {
switch vnid {
case 0:
err := assertPolicies(npns, 2, map[string]*npPolicy{
err := assertPolicies(np, npns, 2, map[string]*npPolicy{
"allow-from-self": {
watchesNamespaces: false,
watchesAllPods: false,
Expand All @@ -616,7 +619,7 @@ func TestNetworkPolicy(t *testing.T) {
}

case 1:
err := assertPolicies(npns, 5, map[string]*npPolicy{
err := assertPolicies(np, npns, 5, map[string]*npPolicy{
"allow-from-self": {
watchesNamespaces: false,
watchesAllPods: false,
Expand Down Expand Up @@ -669,7 +672,7 @@ func TestNetworkPolicy(t *testing.T) {
}

case 2:
err := assertPolicies(npns, 4, map[string]*npPolicy{
err := assertPolicies(np, npns, 4, map[string]*npPolicy{
"allow-from-self": {
watchesNamespaces: false,
watchesAllPods: false,
Expand Down Expand Up @@ -716,7 +719,7 @@ func TestNetworkPolicy(t *testing.T) {
}

case 3, 4, 5:
err := assertPolicies(npns, 3, map[string]*npPolicy{
err := assertPolicies(np, npns, 3, map[string]*npPolicy{
"allow-from-self": {
watchesNamespaces: false,
watchesAllPods: false,
Expand Down Expand Up @@ -747,7 +750,7 @@ func TestNetworkPolicy(t *testing.T) {
}

case 6, 7, 8, 9:
err := assertPolicies(npns, 0, nil)
err := assertPolicies(np, npns, 0, nil)
if err != nil {
t.Error(err.Error())
}
Expand All @@ -760,7 +763,7 @@ func TestNetworkPolicy(t *testing.T) {
// If we delete a namespace, then stale policies may be left behind...
forceSync(np, synced)
delNamespace(np, "two", 2)
err = assertPolicies(npns1, 5, map[string]*npPolicy{
err = assertPolicies(np, npns1, 5, map[string]*npPolicy{
"allow-from-even": {
watchesNamespaces: true,
watchesAllPods: false,
Expand All @@ -781,7 +784,7 @@ func TestNetworkPolicy(t *testing.T) {
synced.Store(false)
addNamespace(np, "unrelated", 100, nil)
waitForSync(np, synced, "namespace addition")
err = assertPolicies(npns1, 5, map[string]*npPolicy{
err = assertPolicies(np, npns1, 5, map[string]*npPolicy{
"allow-from-even": {
watchesNamespaces: true,
watchesAllPods: false,
Expand Down Expand Up @@ -822,7 +825,7 @@ func TestNetworkPolicy(t *testing.T) {
})
waitForSync(np, synced, "namespace deletion")

err = assertPolicies(npns4, 2, map[string]*npPolicy{
err = assertPolicies(np, npns4, 2, map[string]*npPolicy{
"allow-from-self": {
watchesNamespaces: false,
watchesAllPods: false,
Expand All @@ -844,7 +847,7 @@ func TestNetworkPolicy(t *testing.T) {
t.Error(err.Error())
}

err = assertPolicies(npns1, 5, map[string]*npPolicy{
err = assertPolicies(np, npns1, 5, map[string]*npPolicy{
"allow-from-default": {
watchesNamespaces: true,
watchesAllPods: false,
Expand Down Expand Up @@ -887,7 +890,7 @@ func TestNetworkPolicy(t *testing.T) {
waitForSync(np, synced, "host-network NP addition")

// make sure we add the right flows
err = assertPolicies(npns, 1, map[string]*npPolicy{
err = assertPolicies(np, npns, 1, map[string]*npPolicy{
"allow-from-host-network-ns": {
watchesNamespaces: true,
watchesAllPods: false,
Expand Down Expand Up @@ -1119,7 +1122,7 @@ func _TestNetworkPolicy_MultiplePoliciesOneNamespace(t *testing.T) {
addPods(np, npns)
waitForSync(np, synced, "pod addition")
// both policies should be updated
err := assertPolicies(npns, 2, map[string]*npPolicy{
err := assertPolicies(np, npns, 2, map[string]*npPolicy{
"allow-client-to-server-1": {
watchesNamespaces: false,
watchesAllPods: false,
Expand Down