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.

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

Rework integer type selection #3144

Merged
merged 2 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion pkg/apis/submariner.io/v1/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ func parsePort(port string) (int32, error) {
return -1, errors.Errorf("port %s is > 65535", port)
}

//nolint:gosec // We can safely ignore integer conversion error
return int32(portInt), nil
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/cableengine/healthchecker/healthchecker.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ type Config struct {
WatcherConfig *watcher.Config
EndpointNamespace string
ClusterID string
PingInterval uint
MaxPacketLossCount uint
PingInterval int
MaxPacketLossCount int
NewPinger func(pinger.Config) pinger.Interface
}

Expand Down Expand Up @@ -147,7 +147,7 @@ func (h *controller) endpointCreatedOrUpdated(obj runtime.Object, _ int) bool {
}

if h.config.PingInterval != 0 {
pingerConfig.Interval = time.Second * time.Duration(h.config.PingInterval) //nolint:gosec // We can safely ignore integer conversion error
pingerConfig.Interval = time.Second * time.Duration(h.config.PingInterval)
}

newPingerFunc := h.config.NewPinger
Expand Down
6 changes: 3 additions & 3 deletions pkg/natdiscovery/proto/natdiscovery.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/natdiscovery/proto/natdiscovery.proto
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ message SubmarinerNATDiscoveryResponse {

message IPPortPair {
string IP = 1;
uint32 port = 2;
int32 port = 2;
}

message EndpointDetails {
Expand Down
2 changes: 1 addition & 1 deletion pkg/natdiscovery/request_handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (nd *natDiscovery) handleRequestFromAddress(req *proto.SubmarinerNATDiscove
},
Receiver: req.Sender,
ReceivedSrc: &proto.IPPortPair{
Port: uint32(addr.Port), //nolint:gosec // We can safely ignore integer conversion error
Port: int32(addr.Port), //nolint:gosec // We can safely ignore integer conversion error
IP: addr.IP.String(),
},
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/natdiscovery/request_handle_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,11 +174,11 @@ func createMalformedRequest(mangleFunction func(*natproto.SubmarinerNATDiscovery
},
UsingSrc: &natproto.IPPortPair{
IP: testRemotePrivateIP,
Port: uint32(natproto.DefaultPort),
Port: natproto.DefaultPort,
},
UsingDst: &natproto.IPPortPair{
IP: testLocalPrivateIP,
Port: uint32(natproto.DefaultPort),
Port: natproto.DefaultPort,
},
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/natdiscovery/request_send.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,11 @@ func (nd *natDiscovery) sendCheckRequestToTargetIP(remoteNAT *remoteEndpointNAT,
},
UsingSrc: &natproto.IPPortPair{
IP: sourceIP,
Port: uint32(nd.serverPort),
Port: nd.serverPort,
},
UsingDst: &natproto.IPPortPair{
IP: targetIP,
Port: uint32(targetPort),
Port: targetPort,
},
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/natdiscovery/request_send_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,12 @@ var _ = When("a request is sent", func() {
It("should set the using source fields correctly", func() {
Expect(request.UsingSrc).NotTo(BeNil())
Expect(request.UsingSrc.IP).To(Equal(testLocalPrivateIP))
Expect(request.UsingSrc.Port).To(Equal(uint32(testLocalNATPort)))
Expect(request.UsingSrc.Port).To(Equal(testLocalNATPort))
})

It("should set the using destination fields correctly", func() {
Expect(request.UsingDst).NotTo(BeNil())
Expect(request.UsingDst.Port).To(Equal(uint32(testRemoteNATPort)))
Expect(request.UsingDst.Port).To(Equal(testRemoteNATPort))
Expect(request.UsingDst.IP).To(Equal(srcIP))
})

Expand Down
18 changes: 9 additions & 9 deletions pkg/pinger/pinger.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,14 @@ type LatencyInfo struct {
}

var (
defaultMaxPacketLossCount uint = 5
defaultMaxPacketLossCount = 5

defaultPingInterval = 1 * time.Second

// The RTT will be stored and will be used to calculate the statistics until
// the size is reached. Once the size is reached the array will be reset and
// the last elements will be added to the array for statistics.
size uint64 = 1000
size int64 = 1000

// Even though we set up the pinger to run continuously, we still have to give it a non-zero timeout else it will
// fail so set a really long one.
Expand All @@ -78,15 +78,15 @@ type Config struct {
IP string
Interval time.Duration
Timeout time.Duration
MaxPacketLossCount uint
MaxPacketLossCount int
}

type pingerImpl struct {
sync.Mutex
ip string
pingInterval time.Duration
pingTimeout time.Duration
maxPacketLossCount uint
maxPacketLossCount int
statistics statistics
failureMsg string
connectionStatus ConnectionStatus
Expand All @@ -101,7 +101,7 @@ func NewPinger(config Config) Interface {
maxPacketLossCount: config.MaxPacketLossCount,
statistics: statistics{
size: size,
previousRtts: make([]uint64, size),
previousRtts: make([]int64, size),
},
stopCh: make(chan struct{}),
}
Expand Down Expand Up @@ -172,7 +172,7 @@ func (p *pingerImpl) doPing() error {
}

// Pinger will mark a connection as an error if the packet loss reaches the threshold
if uint(pinger.PacketsSent-pinger.PacketsRecv) > p.maxPacketLossCount {
if pinger.PacketsSent-pinger.PacketsRecv > p.maxPacketLossCount {
p.Lock()
defer p.Unlock()

Expand All @@ -198,7 +198,7 @@ func (p *pingerImpl) doPing() error {

p.connectionStatus = Connected
p.failureMsg = ""
p.statistics.update(uint64(packet.Rtt.Nanoseconds()))
p.statistics.update(packet.Rtt.Nanoseconds())

pinger.PacketsSent = 0
pinger.PacketsRecv = 0
Expand All @@ -223,8 +223,8 @@ func (p *pingerImpl) GetLatencyInfo() *LatencyInfo {
p.Lock()
defer p.Unlock()

toDurationString := func(v uint64) string {
return time.Duration(v).String() //nolint:gosec // We can safely ignore integer conversion error
toDurationString := func(v int64) string {
return time.Duration(v).String()
}

return &LatencyInfo{
Expand Down
32 changes: 15 additions & 17 deletions pkg/pinger/statistics.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,19 @@ import (
)

type statistics struct {
previousRtts []uint64
sum uint64
mean uint64
stdDev uint64
lastRtt uint64
minRtt uint64
maxRtt uint64
sqrDiff uint64
index uint64
size uint64
previousRtts []int64
sum int64
mean int64
stdDev int64
lastRtt int64
minRtt int64
maxRtt int64
sqrDiff int64
index int64
size int64
}

func (s *statistics) update(rtt uint64) {
func (s *statistics) update(rtt int64) {
s.lastRtt = rtt

if s.index == s.size {
Expand All @@ -47,9 +47,8 @@ func (s *statistics) update(rtt uint64) {
s.sum = s.previousRtts[0] + s.previousRtts[1]
s.mean = s.sum / 2

//nolint:gosec // Ignore "integer overflow conversion uint64 -> int64" - we subtract uint64's and want to preserve sign.
s.sqrDiff = uint64(int64(s.previousRtts[0]-s.mean)*int64(s.previousRtts[0]-s.mean) +
int64(s.previousRtts[1]-s.mean)*int64(s.previousRtts[1]-s.mean))
s.sqrDiff = (s.previousRtts[0]-s.mean)*(s.previousRtts[0]-s.mean) +
(s.previousRtts[1]-s.mean)*(s.previousRtts[1]-s.mean)
}

if s.index+1 > 1 {
Expand All @@ -66,9 +65,8 @@ func (s *statistics) update(rtt uint64) {
oldMean := s.mean
s.mean = s.sum / (s.index + 1)

//nolint:gosec // Ignore "integer overflow conversion uint64 -> int64" - we subtract uint64's and want to preserve sign.
s.sqrDiff += uint64(int64(rtt-oldMean) * int64(rtt-s.mean))
s.stdDev = uint64(math.Sqrt(float64(s.sqrDiff / (s.index + 1))))
s.sqrDiff += (rtt - oldMean) * (rtt - s.mean)
s.stdDev = int64(math.Sqrt(float64(s.sqrDiff / (s.index + 1))))
} else {
s.sum = rtt
s.sqrDiff = 0
Expand Down
46 changes: 23 additions & 23 deletions pkg/pinger/statistics_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,48 +25,48 @@ import (

var _ = Describe("Statistics", func() {
const (
testMinRTT = 404351
testMaxRTT = 1048263
testLastRTT = 1044609
testNewMinRTT = 404300
testNewMaxRTT = 1048264
testNewLastRTT = 609555
testMinRTT int64 = 404351
testMaxRTT int64 = 1048263
testLastRTT int64 = 1044609
testNewMinRTT int64 = 404300
testNewMaxRTT int64 = 1048264
testNewLastRTT int64 = 609555
)

When("update is called with a sample space", func() {
It("should correctly compute the statistics", func() {
size := 10
statistics := &statistics{
size: uint64(size),
previousRtts: make([]uint64, size),
size: int64(size),
previousRtts: make([]int64, size),
}

sampleSpace := [10]uint64{testMinRTT, 490406, 530333, 609556, 609650, 685106, 726265, 785707, testMaxRTT, testLastRTT}
expectedMean := 693424
expectedSD := 205994
sampleSpace := [10]int64{testMinRTT, 490406, 530333, 609556, 609650, 685106, 726265, 785707, testMaxRTT, testLastRTT}
expectedMean := int64(693424)
expectedSD := int64(205994)

for _, v := range sampleSpace {
statistics.update(v)
}

Expect(statistics.maxRtt).To(Equal(uint64(testMaxRTT)))
Expect(statistics.minRtt).To(Equal(uint64(testMinRTT)))
Expect(statistics.lastRtt).To(Equal(uint64(testLastRTT)))
Expect(statistics.mean).To(Equal(uint64(expectedMean)))
Expect(statistics.stdDev).To(Equal(uint64(expectedSD)))
Expect(statistics.maxRtt).To(Equal(testMaxRTT))
Expect(statistics.minRtt).To(Equal(testMinRTT))
Expect(statistics.lastRtt).To(Equal(testLastRTT))
Expect(statistics.mean).To(Equal(expectedMean))
Expect(statistics.stdDev).To(Equal(expectedSD))

statistics.update(testNewMinRTT)
statistics.update(testNewMaxRTT)
statistics.update(testNewLastRTT)

newExpectedMean := 830998
newExpectedSD := 272450
newExpectedMean := int64(830998)
newExpectedSD := int64(272450)

Expect(statistics.maxRtt).To(Equal(uint64(testNewMaxRTT)))
Expect(statistics.minRtt).To(Equal(uint64(testNewMinRTT)))
Expect(statistics.lastRtt).To(Equal(uint64(testNewLastRTT)))
Expect(statistics.mean).To(Equal(uint64(newExpectedMean)))
Expect(statistics.stdDev).To(Equal(uint64(newExpectedSD)))
Expect(statistics.maxRtt).To(Equal(testNewMaxRTT))
Expect(statistics.minRtt).To(Equal(testNewMinRTT))
Expect(statistics.lastRtt).To(Equal(testNewLastRTT))
Expect(statistics.mean).To(Equal(newExpectedMean))
Expect(statistics.stdDev).To(Equal(newExpectedSD))
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ import (
)

type Config struct {
PingInterval uint
MaxPacketLossCount uint
PingInterval int
MaxPacketLossCount int
HealthCheckerEnabled bool
RouteAgentUpdateInterval time.Duration
NewPinger func(pinger.Config) pinger.Interface
Expand Down Expand Up @@ -147,7 +147,7 @@ func (h *controller) processEndpointCreatedOrUpdated(endpoint *submarinerv1.Endp
}

if h.config.PingInterval != 0 {
pingerConfig.Interval = time.Second * time.Duration(h.config.PingInterval) //nolint:gosec // We can safely ignore integer conversion error
pingerConfig.Interval = time.Second * time.Duration(h.config.PingInterval)
}

if h.config.MaxPacketLossCount != 0 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,7 @@ func newTestDriver() *testDriver {

config.NewPinger = func(pingerCfg pinger.Config) pinger.Interface {
defer GinkgoRecover()
Expect(pingerCfg.Interval).To(Equal(time.Second *
time.Duration(config.PingInterval))) //nolint:gosec // We can safely ignore integer conversion error
Expect(pingerCfg.Interval).To(Equal(time.Second * time.Duration(config.PingInterval)))
Expect(pingerCfg.MaxPacketLossCount).To(Equal(config.MaxPacketLossCount))

p, ok := t.pingerMap[pingerCfg.IP]
Expand Down
4 changes: 2 additions & 2 deletions pkg/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ type SubmarinerSpecification struct {
HealthCheckEnabled bool `default:"true"`
Uninstall bool
HaltOnCertError bool `split_words:"true"`
HealthCheckInterval uint
HealthCheckMaxPacketLossCount uint
HealthCheckInterval int
HealthCheckMaxPacketLossCount int
MetricsPort int `default:"32780"`
}
16 changes: 9 additions & 7 deletions test/e2e/framework/dataplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ package framework
import (
"context"
"fmt"
"strconv"

. "github.com/onsi/gomega"
resourceUtil "github.com/submariner-io/admiral/pkg/resource"
Expand Down Expand Up @@ -157,12 +156,15 @@ func verifyGlobalnetDatapathConnectivity(p tcp.ConnectivityTestParams, gn Global
}

verifyConnectivity := func(listener *framework.NetworkPod, connector *framework.NetworkPod) {
cmd := []string{"sh", "-c", "for j in $(seq 1 " + strconv.FormatUint(uint64(connector.Config.NumOfDataBufs), 10) + "); do echo" +
" [dataplane] connector says " + connector.Config.Data + "; done" +
" | for i in $(seq " + strconv.FormatUint(uint64(listener.Config.ConnectionAttempts), 10) + ");" +
" do if nc -v " + remoteIP + " " + strconv.FormatUint(uint64(connector.Config.Port), 10) + " -w " +
strconv.FormatUint(uint64(listener.Config.ConnectionTimeout), 10) + ";" +
" then break; else sleep " + strconv.FormatUint(uint64(listener.Config.ConnectionTimeout/2), 10) + "; fi; done"}
cmd := []string{
"sh",
"-c",
fmt.Sprintf("for j in $(seq 1 %d); do echo [dataplane] connector says %s; done"+
" | for i in $(seq %d); do if nc -v %s %d -w %d; then break; else sleep %d; fi; done",
connector.Config.NumOfDataBufs, connector.Config.Data, listener.Config.ConnectionAttempts, remoteIP, connector.Config.Port,
listener.Config.ConnectionTimeout, listener.Config.ConnectionTimeout/2,
),
}

stdOut, _, err := p.Framework.ExecWithOptions(context.TODO(), &framework.ExecOptions{
Command: cmd,
Expand Down
Loading