Wait for OVS bridge datapath ID to be available after creating br-int (…

…antrea-io#6472)

We wait (for a maximum of 5s) for the datapath_id of the br-int OVS
bridge to be reported in OVSDB, after creating the bridge and before
checking supported datapath features. This prevents errors when querying
the supported features before the ofproto-dpif provider has been
initialized.

Fixes antrea-io#6471

Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>

pkg/agent/agent.go

@@ -212,6 +212,12 @@ func (i *Initializer) setupOVSBridge() error {
 		return err
 	}
 
+	// Wait for the datapath ID for the bridge to be available, as it indicates that the bridge
+	// has been configured and that we should be able to query supported datapath features.
+	if _, err := i.ovsBridgeClient.WaitForDatapathID(5 * time.Second); err != nil {
+		return fmt.Errorf("error when waiting for OVS bridge datapath ID: %w", err)
+	}
+
 	if err := i.validateSupportedDPFeatures(); err != nil {
 		return err
 	}

pkg/ovs/ovsconfig/interfaces.go

@@ -16,6 +16,7 @@ package ovsconfig
 
 import (
 	"net"
+	"time"
 )
 
 type TunnelType string
@@ -54,6 +55,7 @@ type OVSBridgeClient interface {
 	GetExternalIDs() (map[string]string, Error)
 	SetExternalIDs(externalIDs map[string]interface{}) Error
 	GetDatapathID() (string, Error)
+	WaitForDatapathID(timeout time.Duration) (string, Error)
 	SetDatapathID(datapathID string) Error
 	GetInterfaceOptions(name string) (map[string]string, Error)
 	SetInterfaceOptions(name string, options map[string]interface{}) Error

pkg/ovs/ovsconfig/ovs_client.go

@@ -317,6 +317,45 @@ func (br *OVSBridge) GetDatapathID() (string, Error) {
 	}
 }
 
+func (br *OVSBridge) WaitForDatapathID(timeout time.Duration) (string, Error) {
+	tx := br.ovsdb.Transaction(openvSwitchSchema)
+	tx.Wait(dbtransaction.Wait{
+		Table:   "Bridge",
+		Timeout: uint64(timeout.Milliseconds()),
+		Columns: []string{"datapath_id"},
+		Until:   "!=",
+		Rows: []interface{}{
+			map[string]interface{}{
+				"datapath_id": helpers.MakeOVSDBSet(map[string]interface{}{}),
+			},
+		},
+		Where: [][]interface{}{{"name", "==", br.name}},
+	})
+	tx.Select(dbtransaction.Select{
+		Table:   "Bridge",
+		Columns: []string{"datapath_id"},
+		Where:   [][]interface{}{{"name", "==", br.name}},
+	})
+
+	res, err, temporary := tx.Commit()
+	if err != nil {
+		klog.Error("Transaction failed: ", err)
+		return "", NewTransactionError(err, temporary)
+	}
+
+	if len(res) < 2 || len(res[1].Rows) == 0 {
+		return "", NewTransactionError(fmt.Errorf("bridge %s not found", br.name), false)
+	}
+
+	datapathID := res[1].Rows[0].(map[string]interface{})["datapath_id"]
+	switch datapathID.(type) {
+	case string:
+		return datapathID.(string), nil
+	default:
+		return "", nil
+	}
+}
+
 // GetPortUUIDList returns UUIDs of all ports on the bridge.
 func (br *OVSBridge) GetPortUUIDList() ([]string, Error) {
 	tx := br.ovsdb.Transaction(openvSwitchSchema)
@@ -658,7 +697,7 @@ func (br *OVSBridge) GetOFPort(ifName string, waitUntilValid bool) (int32, Error
 	}
 	tx.Wait(dbtransaction.Wait{
 		Table:   "Interface",
-		Timeout: uint64(defaultGetPortTimeout / time.Millisecond), // The unit of timeout is millisecond
+		Timeout: uint64(defaultGetPortTimeout.Milliseconds()),
 		Columns: []string{"ofport"},
 		Until:   "!=",
 		Rows:    []interface{}{invalidRow},

test/integration/ovs/ovs_client_test.go

@@ -96,28 +96,33 @@ func TestOVSBridge(t *testing.T) {
 	data.setup(t)
 	defer data.teardown(t)
 
-	checkPorts := func(expectedCount int) {
-		portList, err := data.br.GetPortUUIDList()
-		require.Nil(t, err, "Error when retrieving port list")
-		assert.Equal(t, expectedCount, len(portList))
-	}
+	var err error
+
+	start := time.Now()
+	datapathID, err := data.br.WaitForDatapathID(3 * time.Second)
+	end := time.Now()
+	require.NoError(t, err)
+	require.NotEmpty(t, datapathID)
+	t.Logf("Waited %v for datapath ID to be assigned", end.Sub(start))
 
 	// Test set fixed datapath ID
 	expectedDatapathID, err := randomDatapathID()
 	require.Nilf(t, err, "Failed to generate datapath ID: %s", err)
 	err = data.br.SetDatapathID(expectedDatapathID)
 	require.Nilf(t, err, "Set datapath id failed: %s", err)
-	var datapathID string
-	for retry := 0; retry < 3; retry++ {
-		datapathID, _ = data.br.GetDatapathID()
-		if datapathID == expectedDatapathID {
-			break
-		}
-		time.Sleep(time.Second)
-	}
-	assert.Equal(t, expectedDatapathID, datapathID)
+	assert.Eventually(t, func() bool {
+		datapathID, _ := data.br.GetDatapathID()
+		return datapathID == expectedDatapathID
+	}, 5*time.Second, 1*time.Second)
+
 	vlanID := uint16(100)
 
+	checkPorts := func(expectedCount int) {
+		portList, err := data.br.GetPortUUIDList()
+		require.Nil(t, err, "Error when retrieving port list")
+		assert.Equal(t, expectedCount, len(portList))
+	}
+
 	deleteAllPorts(t, data.br)
 	checkPorts(0)