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

Use netip.AddrPort rather than string for node.ProcessContext #3449

Merged
merged 1 commit into from
Oct 7, 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
33 changes: 11 additions & 22 deletions node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ type Node struct {
// The staking address will optionally be written to a process context
// file to enable other nodes to be configured to use this node as a
// beacon.
stakingAddress string
stakingAddress netip.AddrPort

// tlsKeyLogWriterCloser is a debug file handle that writes all the TLS
// session keys. This value should only be non-nil during debugging.
Expand Down Expand Up @@ -440,15 +440,15 @@ func (n *Node) initNetworking(reg prometheus.Registerer) error {
listener = throttling.NewThrottledListener(listener, n.Config.NetworkConfig.ThrottlerConfig.MaxInboundConnsPerSec)

// Record the bound address to enable inclusion in process context file.
n.stakingAddress = listener.Addr().String()
stakingAddrPort, err := ips.ParseAddrPort(n.stakingAddress)
n.stakingAddress, err = ips.ParseAddrPort(listener.Addr().String())
if err != nil {
return err
}

var (
publicAddr netip.Addr
atomicIP *utils.Atomic[netip.AddrPort]
stakingPort = n.stakingAddress.Port()
publicAddr netip.Addr
atomicIP *utils.Atomic[netip.AddrPort]
)
switch {
case n.Config.PublicIP != "":
Expand All @@ -459,7 +459,7 @@ func (n *Node) initNetworking(reg prometheus.Registerer) error {
}
atomicIP = utils.NewAtomic(netip.AddrPortFrom(
publicAddr,
stakingAddrPort.Port(),
stakingPort,
))
n.ipUpdater = dynamicip.NewNoUpdater()
case n.Config.PublicIPResolutionService != "":
Expand All @@ -478,7 +478,7 @@ func (n *Node) initNetworking(reg prometheus.Registerer) error {
}
atomicIP = utils.NewAtomic(netip.AddrPortFrom(
publicAddr,
stakingAddrPort.Port(),
stakingPort,
))
n.ipUpdater = dynamicip.NewUpdater(atomicIP, resolver, n.Config.PublicIPResolutionFreq)
default:
Expand All @@ -488,7 +488,7 @@ func (n *Node) initNetworking(reg prometheus.Registerer) error {
}
atomicIP = utils.NewAtomic(netip.AddrPortFrom(
publicAddr,
stakingAddrPort.Port(),
stakingPort,
))
n.ipUpdater = dynamicip.NewNoUpdater()
}
Expand All @@ -501,8 +501,8 @@ func (n *Node) initNetworking(reg prometheus.Registerer) error {

// Regularly update our public IP and port mappings.
n.portMapper.Map(
stakingAddrPort.Port(),
stakingAddrPort.Port(),
stakingPort,
stakingPort,
stakingPortName,
atomicIP,
n.Config.PublicIPResolutionFreq,
Expand Down Expand Up @@ -644,24 +644,13 @@ func (n *Node) initNetworking(reg prometheus.Registerer) error {
return err
}

type NodeProcessContext struct {
// The process id of the node
PID int `json:"pid"`
// URI to access the node API
// Format: [https|http]://[host]:[port]
URI string `json:"uri"`
// Address other nodes can use to communicate with this node
// Format: [host]:[port]
StakingAddress string `json:"stakingAddress"`
}

// Write process context to the configured path. Supports the use of
// dynamically chosen network ports with local network orchestration.
func (n *Node) writeProcessContext() error {
n.Log.Info("writing process context", zap.String("path", n.Config.ProcessContextFilePath))

// Write the process context to disk
processContext := &NodeProcessContext{
processContext := &ProcessContext{
PID: os.Getpid(),
URI: n.apiURI,
StakingAddress: n.stakingAddress, // Set by network initialization
Expand Down
16 changes: 16 additions & 0 deletions node/process_context.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

package node

import "net/netip"

type ProcessContext struct {
// The process id of the node
PID int `json:"pid"`
// URI to access the node API
// Format: [https|http]://[host]:[port]
URI string `json:"uri"`
// Address other nodes can use to communicate with this node
StakingAddress netip.AddrPort `json:"stakingAddress"`
}
62 changes: 62 additions & 0 deletions node/process_context_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

package node

import (
"encoding/json"
"net/netip"
"testing"

"github.com/stretchr/testify/require"
)

func TestProcessContext(t *testing.T) {
tests := []struct {
name string
context ProcessContext
expected string
}{
{
name: "ipv4 loopback",
context: ProcessContext{
PID: 1,
URI: "http://localhost:9650",
StakingAddress: netip.AddrPortFrom(
netip.AddrFrom4([4]byte{127, 0, 0, 1}),
9651,
),
},
expected: `{
marun marked this conversation as resolved.
Show resolved Hide resolved
"pid": 1,
"uri": "http://localhost:9650",
"stakingAddress": "127.0.0.1:9651"
}`,
},
{
name: "ipv6 loopback",
context: ProcessContext{
PID: 1,
URI: "http://localhost:9650",
StakingAddress: netip.AddrPortFrom(
netip.IPv6Loopback(),
9651,
),
},
expected: `{
"pid": 1,
"uri": "http://localhost:9650",
"stakingAddress": "[::1]:9651"
}`,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
require := require.New(t)

contextJSON, err := json.MarshalIndent(test.context, "", "\t")
require.NoError(err)
require.Equal(test.expected, string(contextJSON))
})
}
}
5 changes: 3 additions & 2 deletions tests/fixture/tmpnet/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"errors"
"fmt"
"io"
"net/netip"
"os"
"os/exec"
"path/filepath"
Expand Down Expand Up @@ -855,12 +856,12 @@ func (n *Network) getBootstrapIPsAndIDs(skippedNode *Node) ([]string, []string,
continue
}

if len(node.StakingAddress) == 0 {
if node.StakingAddress == (netip.AddrPort{}) {
// Node is not running
continue
}

bootstrapIPs = append(bootstrapIPs, node.StakingAddress)
bootstrapIPs = append(bootstrapIPs, node.StakingAddress.String())
bootstrapIDs = append(bootstrapIDs, node.NodeID.String())
}

Expand Down
3 changes: 2 additions & 1 deletion tests/fixture/tmpnet/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"io"
"net"
"net/http"
"net/netip"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -79,7 +80,7 @@ type Node struct {

// Runtime state, intended to be set by NodeRuntime
URI string
StakingAddress string
StakingAddress netip.AddrPort

// Initialized on demand
runtime NodeRuntime
Expand Down
6 changes: 3 additions & 3 deletions tests/fixture/tmpnet/node_process.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type NodeProcess struct {
pid int
}

func (p *NodeProcess) setProcessContext(processContext node.NodeProcessContext) {
func (p *NodeProcess) setProcessContext(processContext node.ProcessContext) {
p.pid = processContext.PID
p.node.URI = processContext.URI
p.node.StakingAddress = processContext.StakingAddress
Expand All @@ -55,12 +55,12 @@ func (p *NodeProcess) readState() error {
bytes, err := os.ReadFile(path)
if errors.Is(err, fs.ErrNotExist) {
// The absence of the process context file indicates the node is not running
p.setProcessContext(node.NodeProcessContext{})
p.setProcessContext(node.ProcessContext{})
return nil
} else if err != nil {
return fmt.Errorf("failed to read node process context: %w", err)
}
processContext := node.NodeProcessContext{}
processContext := node.ProcessContext{}
if err := json.Unmarshal(bytes, &processContext); err != nil {
return fmt.Errorf("failed to unmarshal node process context: %w", err)
}
Expand Down
Loading