Avoid endless loop in DPCManager.runVerify when we get RemoteTemporaryFailure

This commit fixes a bug in DpcManager.runVerify that would cause an infinite
loop when a RemoteTemporaryFailure was returned by the ConnectivityTester.

The issue was that DPCManager didn’t flag the DPC as tested when a RemoteTemporaryFailure
was received. Such a DPC was then mistakenly evaluated as a new DPC received during
testing, and the entire testing procedure was repeated—resulting in an endless cycle.

This commit fixes the issue by flagging the DPC as tested, and actually as
successful, reporting the RemoteTemporaryFailure only as a warning,
since connectivity itself is working.

There is no need to handle testing retries for RemoteTemporaryFailure explicitly,
as they are already performed periodically every 5 minutes (timer.port.testinterval),
even when the latest DPC is marked as successful.

Signed-off-by: Milan Lenco <[email protected]>
(cherry picked from commit 07215ab)

Author: milan-zededa

pkg/pillar/dpcmanager/dpcmanager_test.go

@@ -1823,3 +1823,59 @@ func TestOldDPC(test *testing.T) {
 	eth0Dhcpcd := dg.Reference(generic.Dhcpcd{AdapterIfName: "eth0"})
 	t.Expect(itemIsCreated(eth0Dhcpcd)).To(BeTrue())
 }
+
+func TestRemoteTemporaryFailure(test *testing.T) {
+	t := initTest(test)
+
+	// Prepare simulated network stack.
+	eth0 := mockEth0()
+	networkMonitor.AddOrUpdateInterface(eth0)
+
+	// Single interface configured for mgmt.
+	aa := makeAA(selectedIntfs{eth0: true})
+	dpcManager.UpdateAA(aa)
+
+	// Apply global config.
+	dpcManager.UpdateGCP(globalConfig())
+
+	// Apply initial "bootstrap" DPC with single ethernet port.
+	timePrio1 := time.Now()
+	dpc := makeDPC("bootstrap", timePrio1, selectedIntfs{eth0: true})
+	dpcManager.AddDPC(dpc)
+	t.Eventually(testingInProgressCb()).Should(BeTrue())
+	t.Eventually(testingInProgressCb()).Should(BeFalse())
+	t.Eventually(dpcIdxCb()).Should(Equal(0))
+	t.Eventually(dnsKeyCb()).Should(Equal("bootstrap"))
+	t.Eventually(dpcStateCb(0)).Should(Equal(types.DPCStateSuccess))
+
+	// Apply "zedagent" DPC with single ethernet port.
+	timePrio2 := time.Now()
+	dpc = makeDPC("zedagent", timePrio2, selectedIntfs{eth0: true})
+
+	// Simulate certificate error, which is reported from ConnectivityTester to DpcManager
+	// as a RemoteTemporaryFailure.
+	connTester.SetConnectivityError("zedagent", "eth0",
+		&conntester.RemoteTemporaryFailure{
+			Endpoint:   "simulated-controller",
+			WrappedErr: errors.New("certificate error"),
+		})
+
+	// Even though we are getting error from the simulated controller,
+	// the connectivity is working and DPC should be marked as working
+	// and replace the "bootstrap" DPC.
+	// Previously, we had a bug that would result in DPCManager being
+	// stuck in a loop inside runVerify, re-running verification
+	// for the same DPC and constantly getting RemoteTemporaryFailure.
+	dpcManager.AddDPC(dpc)
+	t.Eventually(testingInProgressCb()).Should(BeTrue())
+	t.Eventually(testingInProgressCb()).Should(BeFalse())
+	t.Eventually(dpcListLenCb()).Should(Equal(1)) // "bootstrap" compressed out
+	t.Eventually(dpcIdxCb()).Should(Equal(0))
+	t.Eventually(dnsKeyCb()).Should(Equal("zedagent"))
+	t.Eventually(dpcStateCb(0)).Should(Equal(types.DPCStateRemoteWait))
+	dpc = getDPC(0)
+	t.Expect(dpc.HasError()).To(BeFalse())
+	t.Expect(dpc.HasWarning()).To(BeTrue())
+	t.Expect(dpc.LastWarning).To(Equal(
+		"Remote temporary failure (endpoint: simulated-controller): certificate error"))
+}

pkg/pillar/dpcmanager/verify.go

@@ -251,7 +251,7 @@ func (m *DpcManager) verifyDPC(ctx context.Context) (status types.DPCState) {
 			switch dpc.State {
 			case types.DPCStateFail, types.DPCStateFailWithIPAndDNS:
 				cloudConnWorks = false
-			case types.DPCStateSuccess:
+			case types.DPCStateSuccess, types.DPCStateRemoteWait:
 				cloudConnWorks = true
 			default:
 				// DpcManager is waiting for something (IP address, DNS server, etc.)
@@ -277,8 +277,9 @@ func (m *DpcManager) verifyDPC(ctx context.Context) (status types.DPCState) {
 	_, rtf := err.(*conntester.RemoteTemporaryFailure)
 	if rtf {
 		m.Log.Errorf("DPC verify: remoteTemporaryFailure: %v", err)
-		// NOTE: We retry until the certificate or ECONNREFUSED is fixed
-		// on the server side.
+		// We treat RemoteTemporaryFailure as a success because the connectivity is working.
+		// Save and publish the server-side error only as a warning.
+		dpc.RecordSuccessWithWarning(err.Error())
 		status = types.DPCStateRemoteWait
 		dpc.State = status
 		return status