Skip to content

Commit 838c7a1

Browse files
authored
Improve network policy comparison function to account for cases where network policies would do no-op updates when unnecessary (#510)
1 parent 528a0d9 commit 838c7a1

File tree

5 files changed

+456
-20
lines changed

5 files changed

+456
-20
lines changed

src/operator/controllers/intents_controller.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,9 +155,12 @@ func (r *IntentsReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
155155
if err := r.client.Status().Patch(ctx, intentsCopy, client.MergeFrom(intents)); err != nil {
156156
return ctrl.Result{}, errors.Wrap(err)
157157
}
158-
health.UpdateLastReconcileEndTime()
159158
}
160159

160+
// Only consider reconcile ended if no error and no requeue.
161+
if result.IsZero() {
162+
health.UpdateLastReconcileEndTime()
163+
}
161164
return result, nil
162165
}
163166

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
package networkpolicy
2+
3+
import (
4+
"github.com/otterize/intents-operator/src/shared/errors"
5+
networkingv1 "k8s.io/api/networking/v1"
6+
"reflect"
7+
"sort"
8+
)
9+
10+
// PAY ATTENTION: deepEqual is sensitive the differance between nil and empty slice
11+
// therefore, we marshal and unmarshal to nullify empty slices of the new policy
12+
func marshalUnmarshalNetpol(netpol *networkingv1.NetworkPolicy) (networkingv1.NetworkPolicy, error) {
13+
data, err := netpol.Marshal()
14+
if err != nil {
15+
return networkingv1.NetworkPolicy{}, errors.Wrap(err)
16+
}
17+
newNetpol := networkingv1.NetworkPolicy{}
18+
err = newNetpol.Unmarshal(data)
19+
if err != nil {
20+
return networkingv1.NetworkPolicy{}, errors.Wrap(err)
21+
}
22+
return newNetpol, nil
23+
}
24+
25+
// isNetworkPolicySpecEqual compares two NetworkPolicySpec structs, ignoring the order of items in nested slices.
26+
func isNetworkPolicySpecEqual(spec1, spec2 networkingv1.NetworkPolicySpec) bool {
27+
28+
// Compare PodSelector
29+
if !reflect.DeepEqual(spec1.PodSelector, spec2.PodSelector) {
30+
return false
31+
}
32+
33+
// Sort and compare Ingress rules
34+
sortIngressRules(spec1.Ingress)
35+
sortIngressRules(spec2.Ingress)
36+
if !reflect.DeepEqual(spec1.Ingress, spec2.Ingress) {
37+
return false
38+
}
39+
40+
// Sort and compare Egress rules
41+
sortEgressRules(spec1.Egress)
42+
sortEgressRules(spec2.Egress)
43+
if !reflect.DeepEqual(spec1.Egress, spec2.Egress) {
44+
return false
45+
}
46+
47+
// Sort and compare PolicyTypes
48+
sortPolicyTypes(spec1.PolicyTypes)
49+
sortPolicyTypes(spec2.PolicyTypes)
50+
if !reflect.DeepEqual(spec1.PolicyTypes, spec2.PolicyTypes) {
51+
return false
52+
}
53+
54+
return true
55+
}
56+
57+
// Helper function to sort Ingress rules by all relevant fields deterministically
58+
func sortIngressRules(rules []networkingv1.NetworkPolicyIngressRule) {
59+
for i := range rules {
60+
sortNetworkPolicyPorts(rules[i].Ports)
61+
sortNetworkPolicyPeers(rules[i].From)
62+
}
63+
sort.SliceStable(rules, func(i, j int) bool {
64+
return reflect.DeepEqual(rules[i], rules[j])
65+
})
66+
}
67+
68+
// Helper function to sort Egress rules by all relevant fields deterministically
69+
func sortEgressRules(rules []networkingv1.NetworkPolicyEgressRule) {
70+
for i := range rules {
71+
sortNetworkPolicyPorts(rules[i].Ports)
72+
sortNetworkPolicyPeers(rules[i].To)
73+
}
74+
sort.SliceStable(rules, func(i, j int) bool {
75+
return reflect.DeepEqual(rules[i], rules[j])
76+
})
77+
}
78+
79+
// Helper function to sort NetworkPolicyPorts by all fields deterministically
80+
func sortNetworkPolicyPorts(ports []networkingv1.NetworkPolicyPort) {
81+
sort.SliceStable(ports, func(i, j int) bool {
82+
if ports[i].Protocol != nil && ports[j].Protocol != nil {
83+
if *ports[i].Protocol != *ports[j].Protocol {
84+
return *ports[i].Protocol < *ports[j].Protocol
85+
}
86+
} else if ports[i].Protocol != ports[j].Protocol {
87+
return ports[i].Protocol != nil
88+
}
89+
90+
if ports[i].Port != nil && ports[j].Port != nil {
91+
if ports[i].Port.IntVal != ports[j].Port.IntVal {
92+
return ports[i].Port.IntVal < ports[j].Port.IntVal
93+
}
94+
} else if ports[i].Port != ports[j].Port {
95+
return ports[i].Port != nil
96+
}
97+
98+
if ports[i].EndPort != nil && ports[j].EndPort != nil {
99+
return *ports[i].EndPort < *ports[j].EndPort
100+
}
101+
return ports[i].EndPort != nil
102+
})
103+
}
104+
105+
// Helper function to sort NetworkPolicyPeers by a deterministic order
106+
func sortNetworkPolicyPeers(peers []networkingv1.NetworkPolicyPeer) {
107+
sort.SliceStable(peers, func(i, j int) bool {
108+
if peers[i].PodSelector != nil && peers[j].PodSelector != nil {
109+
if !reflect.DeepEqual(peers[i].PodSelector, peers[j].PodSelector) {
110+
return reflect.DeepEqual(peers[i].PodSelector, peers[j].PodSelector)
111+
}
112+
} else if peers[i].PodSelector != peers[j].PodSelector {
113+
return peers[i].PodSelector != nil
114+
}
115+
116+
if peers[i].NamespaceSelector != nil && peers[j].NamespaceSelector != nil {
117+
if !reflect.DeepEqual(peers[i].NamespaceSelector, peers[j].NamespaceSelector) {
118+
return reflect.DeepEqual(peers[i].NamespaceSelector, peers[j].NamespaceSelector)
119+
}
120+
} else if peers[i].NamespaceSelector != peers[j].NamespaceSelector {
121+
return peers[i].NamespaceSelector != nil
122+
}
123+
124+
if peers[i].IPBlock != nil && peers[j].IPBlock != nil {
125+
if peers[i].IPBlock.CIDR != peers[j].IPBlock.CIDR {
126+
return peers[i].IPBlock.CIDR < peers[j].IPBlock.CIDR
127+
}
128+
129+
sort.Strings(peers[i].IPBlock.Except)
130+
sort.Strings(peers[j].IPBlock.Except)
131+
return reflect.DeepEqual(peers[i].IPBlock.Except, peers[j].IPBlock.Except)
132+
}
133+
return peers[i].IPBlock != nil
134+
})
135+
}
136+
137+
// Helper function to sort PolicyTypes
138+
func sortPolicyTypes(types []networkingv1.PolicyType) {
139+
sort.SliceStable(types, func(i, j int) bool {
140+
return types[i] < types[j]
141+
})
142+
}

0 commit comments

Comments
 (0)