From 2476df9d58a3ba3de92f99c0dbd8f92fc0320c79 Mon Sep 17 00:00:00 2001 From: Milan Lenco Date: Tue, 26 Sep 2023 12:28:49 +0200 Subject: [PATCH] Use 'defer' to simplify handling of AppNetworkStatus pending flags Currently, we have to remember to set PendingAdd/PendingModify inside AppNetworkStatus to false just before the corresponding handler returns. However, there are few error branches where we forgot to clear the pending flag. This causes zedmanager to just wait and not report the error published inside the AppNetworkStatus. Instead of fixing this case-by-case, let's take advantage of 'defer' to make sure that we will never forget to clear the Pending flag, even if a new branch with return statement is added in the future. Signed-off-by: Milan Lenco (cherry picked from commit fbacc17bd315b668174cc2c065f2ece24c5168a9) --- pkg/pillar/cmd/zedrouter/appnetwork.go | 5 ---- pkg/pillar/cmd/zedrouter/pubsubhandlers.go | 29 +++++++++------------- 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/pkg/pillar/cmd/zedrouter/appnetwork.go b/pkg/pillar/cmd/zedrouter/appnetwork.go index 2d7ac10762..29ece4d953 100644 --- a/pkg/pillar/cmd/zedrouter/appnetwork.go +++ b/pkg/pillar/cmd/zedrouter/appnetwork.go @@ -140,8 +140,6 @@ func (z *zedrouter) doActivateAppNetwork(config types.AppNetworkConfig, // Update AppNetwork and NetworkInstance status. status.Activated = true - status.PendingAdd = false - status.PendingModify = false z.publishAppNetworkStatus(status) z.updateNIStatusAfterAppNetworkActivate(status) @@ -307,7 +305,6 @@ func (z *zedrouter) doUpdateActivatedAppNetwork(oldConfig, newConfig types.AppNe // Update app network status as well as status of connected network instances. z.processAppConnReconcileStatus(appConnRecStatus, status) z.reloadStatusOfAssignedIPs(status) - status.PendingModify = false z.publishAppNetworkStatus(status) z.updateNIStatusAfterAppNetworkActivate(status) } @@ -335,8 +332,6 @@ func (z *zedrouter) doInactivateAppNetwork(config types.AppNetworkConfig, // Update AppNetwork and NetworkInstance status. status.Activated = false - status.PendingModify = false - status.PendingDelete = false z.updateNIStatusAfterAppNetworkInactivate(status) z.removeAssignedIPsFromAppNetStatus(status) z.publishAppNetworkStatus(status) diff --git a/pkg/pillar/cmd/zedrouter/pubsubhandlers.go b/pkg/pillar/cmd/zedrouter/pubsubhandlers.go index ffbd14028c..b25957c2eb 100644 --- a/pkg/pillar/cmd/zedrouter/pubsubhandlers.go +++ b/pkg/pillar/cmd/zedrouter/pubsubhandlers.go @@ -439,15 +439,19 @@ func (z *zedrouter) handleAppNetworkCreate(ctxArg interface{}, key string, // Start by marking with PendingAdd status := types.AppNetworkStatus{ UUIDandVersion: config.UUIDandVersion, - PendingAdd: true, DisplayName: config.DisplayName, } z.doCopyAppNetworkConfigToStatus(config, &status) + status.PendingAdd = true + z.publishAppNetworkStatus(&status) + defer func() { + status.PendingAdd = false + z.publishAppNetworkStatus(&status) + }() if err := z.validateAppNetworkConfig(config); err != nil { z.log.Errorf("handleAppNetworkCreate(%v): validation failed: %v", config.UUIDandVersion.UUID, err) - status.PendingAdd = false z.addAppNetworkError(&status, "handleAppNetworkCreate", err) return } @@ -460,7 +464,6 @@ func (z *zedrouter) handleAppNetworkCreate(ctxArg interface{}, key string, err = fmt.Errorf("failed to allocate appNum for %s/%s: %v", config.UUIDandVersion.UUID, config.DisplayName, err) z.log.Errorf("handleAppNetworkCreate(%v): %v", config.UUIDandVersion.UUID, err) - status.PendingAdd = false z.addAppNetworkError(&status, "handleAppNetworkCreate", err) return } @@ -474,7 +477,6 @@ func (z *zedrouter) handleAppNetworkCreate(ctxArg interface{}, key string, err = fmt.Errorf("failed to allocate numbers for VIFs of the app %s/%s: %v", config.UUIDandVersion.UUID, config.DisplayName, err) z.log.Errorf("handleAppNetworkCreate(%v): %v", config.UUIDandVersion.UUID, err) - status.PendingAdd = false z.addAppNetworkError(&status, "handleAppNetworkCreate", err) return } @@ -485,20 +487,14 @@ func (z *zedrouter) handleAppNetworkCreate(ctxArg interface{}, key string, if err != nil { z.log.Errorf("handleAppNetworkCreate(%v): %v", config.UUIDandVersion.UUID, err) status.AwaitNetworkInstance = true - status.PendingAdd = false if netInErrState { z.addAppNetworkError(&status, "handleAppNetworkCreate", err) - } else { - z.publishAppNetworkStatus(&status) } return } if config.Activate { z.doActivateAppNetwork(config, &status) - } else { - status.PendingAdd = false - z.publishAppNetworkStatus(&status) } z.maybeScheduleRetry() @@ -519,12 +515,15 @@ func (z *zedrouter) handleAppNetworkModify(ctxArg interface{}, key string, status.ClearError() status.PendingModify = true z.publishAppNetworkStatus(status) + defer func() { + status.PendingModify = false + z.publishAppNetworkStatus(status) + }() // Check for unsupported/invalid changes. if err := z.validateAppNetworkConfigForModify(newConfig, oldConfig); err != nil { z.log.Errorf("handleAppNetworkModify(%v): validation failed: %v", newConfig.UUIDandVersion.UUID, err) - status.PendingModify = false z.addAppNetworkError(status, "handleAppNetworkModify", err) return } @@ -538,11 +537,8 @@ func (z *zedrouter) handleAppNetworkModify(ctxArg interface{}, key string, if err != nil { z.log.Errorf("handleAppNetworkModify(%v): %v", newConfig.UUIDandVersion.UUID, err) status.AwaitNetworkInstance = true - status.PendingModify = false if netInErrState { z.addAppNetworkError(status, "handleAppNetworkModify", err) - } else { - z.publishAppNetworkStatus(status) } return } @@ -550,15 +546,12 @@ func (z *zedrouter) handleAppNetworkModify(ctxArg interface{}, key string, if !newConfig.Activate && status.Activated { z.doInactivateAppNetwork(newConfig, status) z.doCopyAppNetworkConfigToStatus(newConfig, status) - z.publishAppNetworkStatus(status) } else if newConfig.Activate && !status.Activated { z.doCopyAppNetworkConfigToStatus(newConfig, status) z.doActivateAppNetwork(newConfig, status) } else if !status.Activated { // Just copy in newConfig z.doCopyAppNetworkConfigToStatus(newConfig, status) - status.PendingModify = false - z.publishAppNetworkStatus(status) } else { // Config change while application network is active. z.doUpdateActivatedAppNetwork(oldConfig, newConfig, status) } @@ -584,6 +577,8 @@ func (z *zedrouter) handleAppNetworkDelete(ctxArg interface{}, key string, // Deactivate app network if it is currently activated. if status.Activated { + // No need to clear PendingDelete later. Instead, we un-publish + // the status completely few lines below. status.PendingDelete = true z.publishAppNetworkStatus(status) z.doInactivateAppNetwork(config, status)