Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Go updates for the test directory #658

Merged
merged 19 commits into from
Jan 21, 2024
Merged

Go updates for the test directory #658

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
e9df6a5
Remove deprecated `io/ioutil` package
gab-arrobo Dec 28, 2022
c4deb4a
No need of `break` in switch statement
gab-arrobo Dec 28, 2022
f5dddd4
Remove unused functions, variables and values
gab-arrobo Dec 28, 2022
0b99915
Due to `panic`, function never reaches `return` statement
gab-arrobo Dec 28, 2022
61a00cf
Use `time.NewTicker` instead of `time.Tick`, which leaks the underlyi…
gab-arrobo Dec 28, 2022
0394ef4
Merge branch 'master' into test-go-updates
gab-arrobo Jan 14, 2023
2ebaa22
Merge branch 'omec-project:master' into test-go-updates
gab-arrobo Jan 14, 2023
4c5cfd3
Merge branch 'omec-project:master' into test-go-updates
gab-arrobo Jan 17, 2023
e1605af
Merge branch 'omec-project:master' into test-go-updates
gab-arrobo Jan 18, 2023
6d19274
Merge branch 'omec-project:master' into test-go-updates
gab-arrobo Feb 22, 2023
4f386fd
Remove deprecated `io/ioutil` package
gab-arrobo Feb 22, 2023
601f09b
Merge branch 'omec-project:master' into test-go-updates
gab-arrobo Mar 28, 2023
13744c6
Merge branch 'master' into test-go-updates
gab-arrobo Mar 29, 2023
1669b8b
Merge branch 'omec-project:master' into test-go-updates
gab-arrobo Apr 6, 2023
5b2a6dc
Merge branch 'omec-project:master' into test-go-updates
gab-arrobo Apr 17, 2023
a06b76f
Merge branch 'master' into test-go-updates
gab-arrobo Jun 18, 2023
77576bf
Merge branch 'omec-project:master' into test-go-updates
gab-arrobo Oct 16, 2023
a2fb993
Merge branch 'omec-project:master' into test-go-updates
gab-arrobo Oct 19, 2023
1241092
Merge branch 'master' into test-go-updates
gab-arrobo Jan 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion test/integration/basic_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ func testUEAttach(t *testing.T, testcase *testCase) {

verifyEntries(t, testcase.input, testcase.expected, UEStateAttaching)

err = pfcpClient.ModifySession(sess, nil, []*ie.IE{
pfcpClient.ModifySession(sess, nil, []*ie.IE{
session.NewFARBuilder().
WithMethod(session.Update).WithID(2).
WithAction(ActionForward).WithDstInterface(ie.DstInterfaceAccess).
Expand Down
2 changes: 0 additions & 2 deletions test/integration/conf.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,6 @@ func GetConfig(datapath string, configType uint32) pfcpiface.Conf {
}

panic("Wrong datapath or config type provided")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we return some error instead than panic?

Copy link
Collaborator Author

@gab-arrobo gab-arrobo Jan 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the panic was originally added to stop the test and avoid it to keep running. Below is a way to avoid the panic. What do you think?

diff --git a/test/integration/conf.go b/test/integration/conf.go
index a6c0d62..e6d3165 100644
--- a/test/integration/conf.go
+++ b/test/integration/conf.go
@@ -6,6 +6,7 @@ package integration
 import (
        "bytes"
        "encoding/json"
+       "fmt"
        "github.com/omec-project/upf-epc/pfcpiface"
        "github.com/sirupsen/logrus"
        "net/http"
@@ -106,27 +107,27 @@ func UP4ConfigWipeOutOnUP4Restart() pfcpiface.Conf {
        return config
 }
 
-func GetConfig(datapath string, configType uint32) pfcpiface.Conf {
+func GetConfig(datapath string, configType uint32) (pfcpiface.Conf, error) {
        switch datapath {
        case DatapathUP4:
                switch configType {
                case ConfigDefault:
-                       return UP4ConfigDefault()
+                       return UP4ConfigDefault(), nil
                case ConfigUPFBasedIPAllocation:
-                       return UP4ConfigUPFBasedIPAllocation()
+                       return UP4ConfigUPFBasedIPAllocation(), nil
                case ConfigWipeOutOnUP4Restart:
-                       return UP4ConfigWipeOutOnUP4Restart()
+                       return UP4ConfigWipeOutOnUP4Restart(), nil
                }
        case DatapathBESS:
                switch configType {
                case ConfigDefault:
-                       return BESSConfigDefault()
+                       return BESSConfigDefault(), nil
                case ConfigUPFBasedIPAllocation:
-                       return BESSConfigUPFBasedIPAllocation()
+                       return BESSConfigUPFBasedIPAllocation(), nil
                }
        }
 
-       panic("Wrong datapath or config type provided")
+       return pfcpiface.Conf{}, fmt.Errorf("wrong datapath or config type provided")
 }
 
 func PushSliceMeterConfig(sliceConfig pfcpiface.NetworkSlice) error {
diff --git a/test/integration/framework.go b/test/integration/framework.go
index ebaef89..bc6f48e 100644
--- a/test/integration/framework.go
+++ b/test/integration/framework.go
@@ -327,21 +327,26 @@ func setup(t *testing.T, configType uint32) {
                MustStartMockUP4()
        }
 
+       getConfig, err := GetConfig(os.Getenv(EnvDatapath), configType)
+       if err != nil {
+               t.Fatal("Wrong datapath or config type provided")
+       }
+
        switch os.Getenv(EnvMode) {
        case ModeDocker:
-               jsonConf, _ := json.Marshal(GetConfig(os.Getenv(EnvDatapath), configType))
+               jsonConf, _ := json.Marshal(getConfig)
                err := os.WriteFile(ConfigPath, jsonConf, os.ModePerm)
                require.NoError(t, err)
                MustStartPFCPAgent()
        case ModeNative:
-               pfcpAgent = pfcpiface.NewPFCPIface(GetConfig(os.Getenv(EnvDatapath), configType))
+               pfcpAgent = pfcpiface.NewPFCPIface(getConfig)
                go pfcpAgent.Run()
        default:
                t.Fatal("Unexpected test mode")
        }
 
        pfcpClient = pfcpsim.NewPFCPClient("127.0.0.1")
-       err := pfcpClient.ConnectN4("127.0.0.1")
+       err = pfcpClient.ConnectN4("127.0.0.1")
        require.NoErrorf(t, err, "failed to connect to UPF")
 
        // wait for PFCP Agent to initialize, blocking

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@badhrinathpa, any comment on this?


return pfcpiface.Conf{}
}

func PushSliceMeterConfig(sliceConfig pfcpiface.NetworkSlice) error {
Expand Down
30 changes: 5 additions & 25 deletions test/integration/framework.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"
"github.com/wmnsk/go-pfcp/ie"
"io/ioutil"
"net"
"os"
"testing"
Expand All @@ -42,8 +41,6 @@ const (
ModeDocker = "docker"
ModeNative = "native"

defaultSliceID = 0

defaultSDFFilter = "permit out udp from any to assigned 80-80"

ueAddress = "17.0.0.1"
Expand Down Expand Up @@ -167,11 +164,6 @@ func init() {
providers.MustCreateNetworkIfNotExists(DockerTestNetwork)
}

func (af appFilter) isEmpty() bool {
return af.proto == 0 && len(af.appIP) == 0 &&
af.appPort.low == 0 && af.appPort.high == 0
}

func IsConnectionOpen(network string, host string, port string) bool {
target := net.JoinHostPort(host, port)

Expand Down Expand Up @@ -199,14 +191,14 @@ func IsConnectionOpen(network string, host string, port string) bool {

func waitForPortOpen(net string, host string, port string) error {
timeout := time.After(5 * time.Second)
ticker := time.Tick(500 * time.Millisecond)
ticker := time.NewTicker(500 * time.Millisecond)

// Keep trying until we're timed out or get a result/error
for {
select {
case <-timeout:
return errors.New("timed out")
case <-ticker:
case <-ticker.C:
if IsConnectionOpen(net, host, port) {
return nil
}
Expand All @@ -218,7 +210,7 @@ func waitForPortOpen(net string, host string, port string) error {
// It retries every 1.5 seconds (0.5 seconds of interval between tries + 1 seconds that PFCP Client waits for response).
func waitForPFCPAssociationSetup(pfcpClient *pfcpsim.PFCPClient) error {
timeout := time.After(30 * time.Second)
ticker := time.Tick(500 * time.Millisecond)
ticker := time.NewTicker(500 * time.Millisecond)

// Decrease timeout to wait for PFCP responses.
// This decreases time to wait for PFCP Agent to start.
Expand All @@ -229,7 +221,7 @@ func waitForPFCPAssociationSetup(pfcpClient *pfcpsim.PFCPClient) error {
select {
case <-timeout:
return errors.New("timed out")
case <-ticker:
case <-ticker.C:
// each test case requires PFCP Association, so we don't teardown it once we notice it's established.
if err := pfcpClient.SetupAssociation(); err == nil {
return nil
Expand All @@ -246,22 +238,10 @@ func waitForBESSFakeToStart() error {
return waitForPortOpen("tcp", "127.0.0.1", "10514")
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what were these functions used for ?

Copy link
Collaborator Author

@gab-arrobo gab-arrobo Jan 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not know if these functions were used at any point in the past. Currently, they are not use anywhere in the code. So, I do not know what they were used for. What do you think about removing them?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@badhrinathpa, or would you prefer that the unused code stays in the repo? If so, I can add a "nolint:unused" comment?

func isModeNative() bool {
return os.Getenv(EnvMode) == ModeNative
}

func isModeDocker() bool {
return os.Getenv(EnvMode) == ModeDocker
}

func isDatapathUP4() bool {
return os.Getenv(EnvDatapath) == DatapathUP4
}

func isDatapathBESS() bool {
return os.Getenv(EnvDatapath) == DatapathBESS
}

func initForwardingPipelineConfig() {
p4rtClient, err := providers.ConnectP4rt("127.0.0.1:50001", true)
if err != nil {
Expand Down Expand Up @@ -350,7 +330,7 @@ func setup(t *testing.T, configType uint32) {
switch os.Getenv(EnvMode) {
case ModeDocker:
jsonConf, _ := json.Marshal(GetConfig(os.Getenv(EnvDatapath), configType))
err := ioutil.WriteFile(ConfigPath, jsonConf, os.ModePerm)
err := os.WriteFile(ConfigPath, jsonConf, os.ModePerm)
require.NoError(t, err)
MustStartPFCPAgent()
case ModeNative:
Expand Down
8 changes: 4 additions & 4 deletions test/integration/providers/docker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
"github.com/docker/docker/client"
"github.com/docker/go-connections/nat"
"github.com/sirupsen/logrus"
"io/ioutil"
"io"
"log"
"os/exec"
"strings"
Expand Down Expand Up @@ -75,8 +75,8 @@ func RunDockerExecCommand(container string, cmd string) (
return 0, "", "", fmt.Errorf("error when starting command: %v", err)
}

stdoutBytes, _ := ioutil.ReadAll(stdoutPipe)
stderrBytes, _ := ioutil.ReadAll(stderrPipe)
stdoutBytes, _ := io.ReadAll(stdoutPipe)
stderrBytes, _ := io.ReadAll(stderrPipe)

if err := dockerCmd.Wait(); err != nil {
if e, ok := err.(*exec.ExitError); ok {
Expand Down Expand Up @@ -258,5 +258,5 @@ func MustPullDockerImage(image string) {
}

// waits for image to be pulled
ioutil.ReadAll(resp)
io.ReadAll(resp)
}
2 changes: 0 additions & 2 deletions test/integration/verify_up4.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,11 +172,9 @@ func buildExpectedSessionsDownlinkEntry(client *p4rtc.Client, expected p4RtValue
case UEStateAttaching, UEStateAttached:
action = ActSetDownlinkSession
actionParams = [][]byte{dummyTunnelPeerID, dummySessMeterIndex}
break
case UEStateBuffering:
action = ActSetDownlinkSessionBuff
actionParams = [][]byte{dummySessMeterIndex}
break
}

return client.NewTableEntry(TableDownlinkSessions, []p4rtc.MatchInterface{
Expand Down