Skip to content

Commit d2e4eba

Browse files
sunya-chsunya-ch
committed
add NetStatusUpdated/CheckDefChanged
Signed-off-by: Sunyanan Choochotkaew <sunyanan.choochotkaew1@ibm.com>
1 parent 2bbb4ec commit d2e4eba

File tree

3 files changed

+258
-34
lines changed

3 files changed

+258
-34
lines changed

‎controllers/multinicnetwork_handler.go

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,9 @@ func (h *MultiNicNetworkHandler) GetNetwork(name string) (*multinicv1.MultiNicNe
4242
Name: name,
4343
Namespace: metav1.NamespaceAll,
4444
}
45-
err := h.Client.Get(context.TODO(), namespacedName, instance)
45+
ctx, cancel := context.WithTimeout(context.Background(), vars.ContextTimeout)
46+
defer cancel()
47+
err := h.Client.Get(ctx, namespacedName, instance)
4648
return instance, err
4749
}
4850

@@ -111,8 +113,16 @@ func (h *MultiNicNetworkHandler) updateStatus(instance *multinicv1.MultiNicNetwo
111113
RouteStatus: status,
112114
}
113115

116+
if !NetStatusUpdated(instance, netStatus) {
117+
vars.NetworkLog.V(2).Info(fmt.Sprintf("No status update %s", instance.Name))
118+
return netStatus, nil
119+
}
120+
121+
vars.NetworkLog.V(2).Info(fmt.Sprintf("Update %s status", instance.Name))
114122
instance.Status = netStatus
115-
err := h.Client.Status().Update(context.Background(), instance)
123+
ctx, cancel := context.WithTimeout(context.Background(), vars.ContextTimeout)
124+
defer cancel()
125+
err := h.Client.Status().Update(ctx, instance)
116126
if err != nil {
117127
vars.NetworkLog.V(2).Info(fmt.Sprintf("Failed to update %s status: %v", instance.Name, err))
118128
} else {
@@ -121,6 +131,28 @@ func (h *MultiNicNetworkHandler) updateStatus(instance *multinicv1.MultiNicNetwo
121131
return netStatus, err
122132
}
123133

134+
func NetStatusUpdated(instance *multinicv1.MultiNicNetwork, newStatus multinicv1.MultiNicNetworkStatus) bool {
135+
prevStatus := instance.Status
136+
if prevStatus.Message != newStatus.Message || prevStatus.RouteStatus != newStatus.RouteStatus || prevStatus.NetConfigStatus != newStatus.NetConfigStatus || prevStatus.DiscoverStatus != newStatus.DiscoverStatus {
137+
return true
138+
}
139+
if len(prevStatus.ComputeResults) != len(newStatus.ComputeResults) {
140+
return true
141+
}
142+
prevComputeMap := make(map[string]int)
143+
for _, status := range prevStatus.ComputeResults {
144+
prevComputeMap[status.NetAddress] = status.NumOfHost
145+
}
146+
for _, status := range newStatus.ComputeResults {
147+
if numOfHost, found := prevComputeMap[status.NetAddress]; !found {
148+
return true
149+
} else if numOfHost != status.NumOfHost {
150+
return true
151+
}
152+
}
153+
return false
154+
}
155+
124156
func (h *MultiNicNetworkHandler) UpdateNetConfigStatus(instance *multinicv1.MultiNicNetwork, netConfigStatus multinicv1.NetConfigStatus, message string) error {
125157
if message != "" {
126158
instance.Status.Message = message
@@ -129,7 +161,9 @@ func (h *MultiNicNetworkHandler) UpdateNetConfigStatus(instance *multinicv1.Mult
129161
instance.Status.ComputeResults = []multinicv1.NicNetworkResult{}
130162
}
131163
instance.Status.NetConfigStatus = netConfigStatus
132-
err := h.Client.Status().Update(context.Background(), instance)
164+
ctx, cancel := context.WithTimeout(context.Background(), vars.ContextTimeout)
165+
defer cancel()
166+
err := h.Client.Status().Update(ctx, instance)
133167
if err != nil {
134168
vars.NetworkLog.V(2).Info(fmt.Sprintf("Failed to update %s status: %v", instance.Name, err))
135169
} else {

‎plugin/net_attach_def.go

Lines changed: 56 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,18 @@ func GetNetAttachDefHandler(config *rest.Config) (*NetAttachDefHandler, error) {
108108
}, nil
109109
}
110110

111+
func CheckDefChanged(def, existingDef *NetworkAttachmentDefinition) bool {
112+
if def.Spec.Config != existingDef.Spec.Config || len(def.Annotations) != len(existingDef.Annotations) {
113+
return true
114+
}
115+
for k, v := range existingDef.Annotations {
116+
if def.Annotations[k] != v {
117+
return true
118+
}
119+
}
120+
return false
121+
}
122+
111123
// CreateOrUpdate creates new NetworkAttachmentDefinition resource if not exists, otherwise update
112124
func (h *NetAttachDefHandler) CreateOrUpdate(net *multinicv1.MultiNicNetwork, pluginStr string, annotations map[string]string) error {
113125
defs, err := h.generate(net, pluginStr, annotations)
@@ -122,9 +134,14 @@ func (h *NetAttachDefHandler) CreateOrUpdate(net *multinicv1.MultiNicNetwork, pl
122134
if h.IsExist(name, namespace) {
123135
existingDef, _ := h.Get(name, namespace)
124136
def.ObjectMeta = existingDef.ObjectMeta
125-
err := h.DynamicHandler.Update(namespace, def, result)
126-
if err != nil {
127-
errMsg = fmt.Sprintf("%s\n%s: %v", errMsg, namespace, err)
137+
if CheckDefChanged(def, existingDef) {
138+
if namespace == "default" {
139+
vars.NetworkLog.V(2).Info(fmt.Sprintf("Update net-attach-def %s", def.Name))
140+
}
141+
err := h.DynamicHandler.Update(namespace, def, result)
142+
if err != nil {
143+
errMsg = fmt.Sprintf("%s\n%s: %v", errMsg, namespace, err)
144+
}
128145
}
129146
} else {
130147
err := h.DynamicHandler.Create(namespace, def, result)
@@ -159,6 +176,40 @@ func (h *NetAttachDefHandler) getNamespace(net *multinicv1.MultiNicNetwork) ([]s
159176
return namespaces, nil
160177
}
161178

179+
// NetToDef generates net-attach-def from multinicnetwork on specific namespace called by generate function
180+
func NetToDef(namespace string, net *multinicv1.MultiNicNetwork, pluginStr string, annotations map[string]string) (*NetworkAttachmentDefinition, error) {
181+
name := net.GetName()
182+
config := &NetConf{
183+
NetConf: types.NetConf{
184+
CNIVersion: CNI_VERSION,
185+
Name: name,
186+
Type: vars.TargetCNI,
187+
},
188+
Subnet: net.Spec.Subnet,
189+
MasterNetAddrs: net.Spec.MasterNetAddrs,
190+
IsMultiNICIPAM: net.Spec.IsMultiNICIPAM,
191+
DaemonPort: vars.DaemonPort,
192+
}
193+
var ipamObj map[string]interface{}
194+
configBytes, _ := json.Marshal(config)
195+
configStr := string(configBytes)
196+
err := json.Unmarshal([]byte(net.Spec.IPAM), &ipamObj)
197+
if err != nil {
198+
return nil, err
199+
}
200+
ipamBytes, _ := json.Marshal(ipamObj)
201+
pluginValue := fmt.Sprintf("\"plugin\":%s", pluginStr)
202+
ipamValue := fmt.Sprintf("\"ipam\":%s", string(ipamBytes))
203+
configStr = strings.ReplaceAll(configStr, "\"ipam\":{}", ipamValue)
204+
configStr = strings.ReplaceAll(configStr, "\"plugin\":null", pluginValue)
205+
metaObj := GetMetaObject(name, namespace, annotations)
206+
spec := NetworkAttachmentDefinitionSpec{
207+
Config: configStr,
208+
}
209+
netattachdef := NewNetworkAttachmentDefinition(metaObj, spec)
210+
return &netattachdef, nil
211+
}
212+
162213
// generate initializes NetworkAttachmentDefinition objects from MultiNicNetwork and unmarshal plugin
163214
func (h *NetAttachDefHandler) generate(net *multinicv1.MultiNicNetwork, pluginStr string, annotations map[string]string) ([]*NetworkAttachmentDefinition, error) {
164215
defs := []*NetworkAttachmentDefinition{}
@@ -168,37 +219,11 @@ func (h *NetAttachDefHandler) generate(net *multinicv1.MultiNicNetwork, pluginSt
168219
}
169220
vars.NetworkLog.V(2).Info(fmt.Sprintf("generate net-attach-def config on %d namespaces", len(namespaces)))
170221
for _, ns := range namespaces {
171-
name := net.GetName()
172-
namespace := ns
173-
config := &NetConf{
174-
NetConf: types.NetConf{
175-
CNIVersion: CNI_VERSION,
176-
Name: name,
177-
Type: vars.TargetCNI,
178-
},
179-
Subnet: net.Spec.Subnet,
180-
MasterNetAddrs: net.Spec.MasterNetAddrs,
181-
IsMultiNICIPAM: net.Spec.IsMultiNICIPAM,
182-
DaemonPort: vars.DaemonPort,
183-
}
184-
var ipamObj map[string]interface{}
185-
configBytes, _ := json.Marshal(config)
186-
configStr := string(configBytes)
187-
err := json.Unmarshal([]byte(net.Spec.IPAM), &ipamObj)
222+
def, err := NetToDef(ns, net, pluginStr, annotations)
188223
if err != nil {
189224
return defs, err
190225
}
191-
ipamBytes, _ := json.Marshal(ipamObj)
192-
pluginValue := fmt.Sprintf("\"plugin\":%s", pluginStr)
193-
ipamValue := fmt.Sprintf("\"ipam\":%s", string(ipamBytes))
194-
configStr = strings.ReplaceAll(configStr, "\"ipam\":{}", ipamValue)
195-
configStr = strings.ReplaceAll(configStr, "\"plugin\":null", pluginValue)
196-
metaObj := GetMetaObject(name, namespace, annotations)
197-
spec := NetworkAttachmentDefinitionSpec{
198-
Config: configStr,
199-
}
200-
netattachdef := NewNetworkAttachmentDefinition(metaObj, spec)
201-
defs = append(defs, &netattachdef)
226+
defs = append(defs, def)
202227
}
203228
return defs, nil
204229
}

‎unit-test/multinicnetwork_test.go

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,12 @@ package controllers
88
import (
99
"fmt"
1010

11+
multinicv1 "github.com/foundation-model-stack/multi-nic-cni/api/v1"
12+
"github.com/foundation-model-stack/multi-nic-cni/controllers"
13+
"github.com/foundation-model-stack/multi-nic-cni/plugin"
1114
. "github.com/onsi/ginkgo"
1215
. "github.com/onsi/gomega"
16+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1317
//+kubebuilder:scaffold:imports
1418
)
1519

@@ -33,3 +37,164 @@ var _ = Describe("Test deploying MultiNicNetwork", func() {
3337
Expect(err).NotTo(HaveOccurred())
3438
})
3539
})
40+
41+
var _ = Describe("Test definition changes check", func() {
42+
cniVersion := "0.3.0"
43+
cniType := "ipvlan"
44+
mode := "l2"
45+
mtu := 1500
46+
cniArgs := make(map[string]string)
47+
cniArgs["mode"] = mode
48+
cniArgs["mtu"] = fmt.Sprintf("%d", mtu)
49+
50+
multinicnetwork := getMultiNicCNINetwork("test-mn", cniVersion, cniType, cniArgs)
51+
It("detect no change", func() {
52+
mainPlugin, annotations, err := multinicnetworkReconciler.GetMainPluginConf(multinicnetwork)
53+
Expect(err).NotTo(HaveOccurred())
54+
def, err := plugin.NetToDef("", multinicnetwork, mainPlugin, annotations)
55+
Expect(err).NotTo(HaveOccurred())
56+
defCopy, err := plugin.NetToDef("", multinicnetwork, mainPlugin, annotations)
57+
Expect(err).NotTo(HaveOccurred())
58+
changed := plugin.CheckDefChanged(defCopy, def)
59+
Expect(changed).To(BeFalse())
60+
})
61+
62+
It("detect annotation change", func() {
63+
mainPlugin, annotations, err := multinicnetworkReconciler.GetMainPluginConf(multinicnetwork)
64+
Expect(err).NotTo(HaveOccurred())
65+
def, err := plugin.NetToDef("", multinicnetwork, mainPlugin, annotations)
66+
Expect(err).NotTo(HaveOccurred())
67+
68+
newAnnotations := map[string]string{"resource": "new"}
69+
defWithNewAnnotation, err := plugin.NetToDef("", multinicnetwork, mainPlugin, newAnnotations)
70+
Expect(err).NotTo(HaveOccurred())
71+
changed := plugin.CheckDefChanged(defWithNewAnnotation, def)
72+
Expect(changed).To(BeTrue())
73+
})
74+
75+
It("detect config change", func() {
76+
mainPlugin, annotations, err := multinicnetworkReconciler.GetMainPluginConf(multinicnetwork)
77+
Expect(err).NotTo(HaveOccurred())
78+
def, err := plugin.NetToDef("", multinicnetwork, mainPlugin, annotations)
79+
Expect(err).NotTo(HaveOccurred())
80+
81+
newCniArgs := make(map[string]string)
82+
newCniArgs["mode"] = "l3"
83+
newCniArgs["mtu"] = fmt.Sprintf("%d", mtu)
84+
changedArgsNetwork := getMultiNicCNINetwork("test-mn", cniVersion, cniType, newCniArgs)
85+
newMainPlugin, annotations, err := multinicnetworkReconciler.GetMainPluginConf(changedArgsNetwork)
86+
Expect(err).NotTo(HaveOccurred())
87+
defWithNewArgs, err := plugin.NetToDef("", changedArgsNetwork, newMainPlugin, annotations)
88+
Expect(err).NotTo(HaveOccurred())
89+
changed := plugin.CheckDefChanged(defWithNewArgs, def)
90+
Expect(changed).To(BeTrue())
91+
})
92+
})
93+
94+
func getNetStatus(computeResults []multinicv1.NicNetworkResult, discoverStatus multinicv1.DiscoverStatus, netConfigStatus multinicv1.NetConfigStatus, routeStatus multinicv1.RouteStatus) multinicv1.MultiNicNetworkStatus {
95+
return multinicv1.MultiNicNetworkStatus{
96+
ComputeResults: computeResults,
97+
DiscoverStatus: discoverStatus,
98+
NetConfigStatus: netConfigStatus,
99+
RouteStatus: routeStatus,
100+
Message: "",
101+
LastSyncTime: metav1.Now(),
102+
}
103+
}
104+
105+
func testNewNetStatus(multinicnetwork *multinicv1.MultiNicNetwork, newStatus multinicv1.MultiNicNetworkStatus, expectedChange bool) *multinicv1.MultiNicNetwork {
106+
if expectedChange {
107+
updated := controllers.NetStatusUpdated(multinicnetwork, newStatus)
108+
// check new update
109+
Expect(updated).To(Equal(expectedChange))
110+
// update status
111+
multinicnetwork.Status = newStatus
112+
}
113+
updated := controllers.NetStatusUpdated(multinicnetwork, newStatus)
114+
// expect no update
115+
Expect(updated).To(BeFalse())
116+
return multinicnetwork
117+
}
118+
119+
var _ = Describe("Test multinicnetwork status change check", func() {
120+
cniVersion := "0.3.0"
121+
cniType := "ipvlan"
122+
mode := "l3"
123+
mtu := 1500
124+
cniArgs := make(map[string]string)
125+
cniArgs["mode"] = mode
126+
cniArgs["mtu"] = fmt.Sprintf("%d", mtu)
127+
128+
It("detect change from no status", func() {
129+
multinicnetwork := getMultiNicCNINetwork("test-mn", cniVersion, cniType, cniArgs)
130+
initStatus := getNetStatus([]multinicv1.NicNetworkResult{}, multinicv1.DiscoverStatus{}, multinicv1.WaitForConfig, multinicv1.ApplyingRoute)
131+
updated := controllers.NetStatusUpdated(multinicnetwork, initStatus)
132+
Expect(updated).To(BeTrue())
133+
})
134+
135+
It("detect change on compute results", func() {
136+
multinicnetwork := getMultiNicCNINetwork("test-mn", cniVersion, cniType, cniArgs)
137+
multinicnetwork.Status = getNetStatus([]multinicv1.NicNetworkResult{}, multinicv1.DiscoverStatus{}, multinicv1.WaitForConfig, multinicv1.ApplyingRoute)
138+
139+
net1 := multinicv1.NicNetworkResult{
140+
NetAddress: "192.168.0.0/24",
141+
NumOfHost: 1,
142+
}
143+
net2 := multinicv1.NicNetworkResult{
144+
NetAddress: "192.168.1.0/24",
145+
NumOfHost: 2,
146+
}
147+
148+
computeResults := []multinicv1.NicNetworkResult{net1}
149+
newStatus := getNetStatus(computeResults, multinicv1.DiscoverStatus{}, multinicv1.WaitForConfig, multinicv1.ApplyingRoute)
150+
expectedChange := true
151+
multinicnetwork = testNewNetStatus(multinicnetwork, newStatus, expectedChange)
152+
153+
// add new compute result
154+
computeResults = []multinicv1.NicNetworkResult{net1, net2}
155+
newStatus = getNetStatus(computeResults, multinicv1.DiscoverStatus{}, multinicv1.WaitForConfig, multinicv1.ApplyingRoute)
156+
expectedChange = true
157+
multinicnetwork = testNewNetStatus(multinicnetwork, newStatus, expectedChange)
158+
159+
// change order
160+
computeResults = []multinicv1.NicNetworkResult{net2, net1}
161+
newStatus = getNetStatus(computeResults, multinicv1.DiscoverStatus{}, multinicv1.WaitForConfig, multinicv1.ApplyingRoute)
162+
expectedChange = false
163+
multinicnetwork = testNewNetStatus(multinicnetwork, newStatus, expectedChange)
164+
165+
// change values
166+
net1.NetAddress = "192.168.0.2/24"
167+
computeResults = []multinicv1.NicNetworkResult{net1, net2}
168+
newStatus = getNetStatus(computeResults, multinicv1.DiscoverStatus{}, multinicv1.WaitForConfig, multinicv1.ApplyingRoute)
169+
expectedChange = true
170+
testNewNetStatus(multinicnetwork, newStatus, expectedChange)
171+
net1.NumOfHost = 3
172+
computeResults = []multinicv1.NicNetworkResult{net1, net2}
173+
newStatus = getNetStatus(computeResults, multinicv1.DiscoverStatus{}, multinicv1.WaitForConfig, multinicv1.ApplyingRoute)
174+
expectedChange = true
175+
testNewNetStatus(multinicnetwork, newStatus, expectedChange)
176+
})
177+
178+
It("detect change on simple status", func() {
179+
multinicnetwork := getMultiNicCNINetwork("test-mn", cniVersion, cniType, cniArgs)
180+
multinicnetwork.Status = getNetStatus([]multinicv1.NicNetworkResult{}, multinicv1.DiscoverStatus{}, multinicv1.WaitForConfig, multinicv1.ApplyingRoute)
181+
182+
// change discover status
183+
discoverStatus := multinicv1.DiscoverStatus{
184+
ExistDaemon: 10,
185+
}
186+
newStatus := getNetStatus([]multinicv1.NicNetworkResult{}, discoverStatus, multinicv1.WaitForConfig, multinicv1.ApplyingRoute)
187+
expectedChange := true
188+
testNewNetStatus(multinicnetwork, newStatus, expectedChange)
189+
190+
// change config status
191+
newStatus = getNetStatus([]multinicv1.NicNetworkResult{}, discoverStatus, multinicv1.ConfigComplete, multinicv1.ApplyingRoute)
192+
expectedChange = true
193+
testNewNetStatus(multinicnetwork, newStatus, expectedChange)
194+
195+
// change route status
196+
newStatus = getNetStatus([]multinicv1.NicNetworkResult{}, discoverStatus, multinicv1.ConfigComplete, multinicv1.AllRouteApplied)
197+
expectedChange = true
198+
testNewNetStatus(multinicnetwork, newStatus, expectedChange)
199+
})
200+
})

0 commit comments

Comments
 (0)