update

Signed-off-by: Hang Yan <[email protected]>
antrea-io · Nov 9, 2024 · fd77ff5 · fd77ff5
1 parent f674f00
commit fd77ff5
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 39 deletions.
diff --git a/pkg/agent/packetcapture/packetcapture_controller.go b/pkg/agent/packetcapture/packetcapture_controller.go
@@ -127,7 +127,7 @@ type Controller struct {
 	sftpUploader          sftp.Uploader
 	captureInterface      PacketCapturer
 	mutex                 sync.Mutex
-	// A name-phase mapping for all PacketCapture CRs.
+	// A name-state mapping for all PacketCapture CRs.
 	captures           map[string]*packetCaptureState
 	numRunningCaptures int
 }
@@ -270,7 +270,7 @@ func (c *Controller) syncPacketCapture(pcName string) error {
 
 	if err := c.validatePacketCapture(&pc.Spec); err != nil {
 		klog.ErrorS(err, "Invalid PacketCapture", "name", pc.Name)
-		if updateErr := c.updateStatus(context.Background(), pcName, &packetCaptureState{err: err}); updateErr != nil {
+		if updateErr := c.updateStatus(context.Background(), pcName, packetCaptureState{err: err}); updateErr != nil {
 			klog.ErrorS(err, "Failed to update PacketCapture status", "name", pc.Name)
 		}
 		cleanupStatus()
@@ -311,7 +311,7 @@ func (c *Controller) syncPacketCapture(pcName string) error {
 		return *state
 	}()
 
-	if updateErr := c.updateStatus(context.Background(), pcName, &state); updateErr != nil {
+	if updateErr := c.updateStatus(context.Background(), pcName, state); updateErr != nil {
 		return fmt.Errorf("error when patching status: %w", updateErr)
 	}
 	return err
@@ -423,7 +423,7 @@ func (c *Controller) performCapture(
 		klog.ErrorS(err, "Failed to start capture")
 		return err
 	}
-	klog.InfoS("Starting capture packets", "name", pc.Name, "device", device)
+	klog.InfoS("Starting packet capture", "name", pc.Name, "device", device)
 	for {
 		select {
 		case packet := <-packets:
@@ -567,7 +567,7 @@ func (c *Controller) uploadPackets(ctx context.Context, pc *crdv1alpha1.PacketCa
 	return uploader.Upload(pc.Spec.FileServer.URL, c.generatePacketsPathForServer(pc.Name), cfg, outputFile)
 }
 
-func (c *Controller) updateStatus(ctx context.Context, name string, state *packetCaptureState) error {
+func (c *Controller) updateStatus(ctx context.Context, name string, state packetCaptureState) error {
 	toUpdate, getErr := c.packetCaptureLister.Get(name)
 	if getErr != nil {
 		klog.InfoS("Didn't find the original PacketCapture, skip updating status", "name", name)
@@ -582,32 +582,30 @@ func (c *Controller) updateStatus(ctx context.Context, name string, state *packe
 
 	if state.err != nil {
 		updatedStatus.FilePath = ""
-		conditions = append(conditions, crdv1alpha1.PacketCaptureCondition{
-			Type:               crdv1alpha1.PacketCaptureComplete,
-			Status:             metav1.ConditionStatus(v1.ConditionFalse),
-			LastTransitionTime: metav1.Now(),
-			Reason:             "CaptureFailed",
-			Message:            state.err.Error(),
-		})
-
 		if errors.Is(state.err, context.DeadlineExceeded) {
-			conditions = []crdv1alpha1.PacketCaptureCondition{
-				{
-					Type:               crdv1alpha1.PacketCaptureComplete,
-					Status:             metav1.ConditionStatus(v1.ConditionTrue),
-					LastTransitionTime: t,
-					Reason:             "Timeout",
-				},
-			}
+			conditions = append(conditions, crdv1alpha1.PacketCaptureCondition{
+				Type:               crdv1alpha1.PacketCaptureComplete,
+				Status:             metav1.ConditionStatus(v1.ConditionTrue),
+				LastTransitionTime: t,
+				Reason:             "Timeout",
+			})
+
 		} else if state.isCaptureSuccessful() {
-			conditions = []crdv1alpha1.PacketCaptureCondition{
-				{
-					Type:               crdv1alpha1.PacketCaptureComplete,
-					Status:             metav1.ConditionStatus(v1.ConditionTrue),
-					LastTransitionTime: t,
-					Reason:             "Succeed",
-				},
-			}
+			// most likely failed to upload after capture succeed.
+			conditions = append(conditions, crdv1alpha1.PacketCaptureCondition{
+				Type:               crdv1alpha1.PacketCaptureComplete,
+				Status:             metav1.ConditionStatus(v1.ConditionTrue),
+				LastTransitionTime: t,
+				Reason:             "Succeed",
+			})
+		} else {
+			conditions = append(conditions, crdv1alpha1.PacketCaptureCondition{
+				Type:               crdv1alpha1.PacketCaptureComplete,
+				Status:             metav1.ConditionStatus(v1.ConditionFalse),
+				LastTransitionTime: metav1.Now(),
+				Reason:             "CaptureFailed",
+				Message:            state.err.Error(),
+			})
 		}
 		if toUpdate.Spec.FileServer != nil && state.filePath != "" {
 			conditions = append(conditions, crdv1alpha1.PacketCaptureCondition{

diff --git a/pkg/agent/packetcapture/packetcapture_controller_test.go b/pkg/agent/packetcapture/packetcapture_controller_test.go
@@ -58,7 +58,7 @@ var (
 	pod2MAC, _               = net.ParseMAC("aa:bb:cc:dd:ee:00")
 	ofPortPod1               = uint32(1)
 	ofPortPod2               = uint32(2)
-	testCaptureTimeout       = uint32(1)
+	testCaptureTimeout       = int32(1)
 	testCaptureNum     int32 = 15
 
 	icmpProto    = intstr.FromString("ICMP")
@@ -272,8 +272,9 @@ func addPodInterface(ifaceStore interfacestore.InterfaceStore, podNamespace, pod
 
 func TestMultiplePacketCaptures(t *testing.T) {
 	defaultFS = afero.NewMemMapFs()
-	packetsDir := "/tmp/antrea/packetcapture/packets"
-	defaultFS.MkdirAll(packetsDir, 0755)
+	defer func() {
+		defaultFS = afero.NewOsFs()
+	}()
 	nameFunc := func(id int) string {
 		return fmt.Sprintf("pc-%d", id)
 	}
@@ -326,8 +327,7 @@ func TestMultiplePacketCaptures(t *testing.T) {
 
 }
 
-// TestPacketCaptureControllerRun was used to validate the whole run process is working. It doesn't wait for
-// the testing pc to finish. on sandbox env, no good solution to open raw socket.
+// TestPacketCaptureControllerRun was used to validate the whole run process is working.
 func TestPacketCaptureControllerRun(t *testing.T) {
 	pcs := []struct {
 		name                 string

diff --git a/pkg/apis/crd/v1alpha1/types.go b/pkg/apis/crd/v1alpha1/types.go
@@ -450,7 +450,7 @@ type PacketCaptureFileServer struct {
 
 type PacketCaptureSpec struct {
 	// Timeout is the timeout for this capture session. If not specified, defaults to 60s.
-	Timeout       *uint32       `json:"timeout,omitempty"`
+	Timeout       *int32        `json:"timeout,omitempty"`
 	CaptureConfig CaptureConfig `json:"captureConfig"`
 	// Source is the traffic source we want to perform capture on. Both `Source` and `Destination` is required
 	// for a capture session, and at least one `Pod` should be present either in the source or the destination.

diff --git a/pkg/apis/crd/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/crd/v1alpha1/zz_generated.deepcopy.go
diff --git a/test/e2e/packetcapture_test.go b/test/e2e/packetcapture_test.go
@@ -165,7 +165,7 @@ func testPacketCaptureBasic(t *testing.T, data *TestData) {
 	udpProto := intstr.FromString("UDP")
 	tcpProto := intstr.FromString("TCP")
 	testServerPort := int32(80)
-	pcShortTimeout := uint32(5)
+	pcShortTimeout := int32(5)
 	nonExistPodName := "non-existing-pod"
 	testNonExistPort := int32(8085)
 
@@ -543,13 +543,13 @@ func runPacketCaptureTest(t *testing.T, data *TestData, tc pcTestCase) {
 
 	timeout := tc.pc.Spec.Timeout
 	if timeout == nil {
-		tv := uint32(15)
+		tv := int32(15)
 		timeout = &tv
 	}
 
 	if strings.Contains(tc.name, "timeout") {
 		// wait more for status update.
-		tv := *timeout + uint32(10)
+		tv := *timeout + int32(10)
 		timeout = &tv
 	}