Merge pull request #11600 from hashicorp/f-remove-unused-version

core: remove all traces of unused protocol version
This commit is contained in:
Michael Schurter 2022-02-22 09:51:42 -08:00 committed by GitHub
commit 6ccdc6a022
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
25 changed files with 59 additions and 212 deletions

3
.changelog/11600.txt Normal file
View file

@ -0,0 +1,3 @@
```release-note:improvement
core: The unused protocol_version agent configuration value has been removed.
```

View file

@ -749,18 +749,6 @@ func (c *Client) secretNodeID() string {
return c.config.Node.SecretID
}
// RPCMajorVersion returns the structs.ApiMajorVersion supported by the
// client.
func (c *Client) RPCMajorVersion() int {
return structs.ApiMajorVersion
}
// RPCMinorVersion returns the structs.ApiMinorVersion supported by the
// client.
func (c *Client) RPCMinorVersion() int {
return structs.ApiMinorVersion
}
// Shutdown is used to tear down the client
func (c *Client) Shutdown() error {
c.shutdownLock.Lock()
@ -2773,7 +2761,7 @@ DISCOLOOP:
continue
}
var peers []string
if err := c.connPool.RPC(region, addr, c.RPCMajorVersion(), "Status.Peers", rpcargs, &peers); err != nil {
if err := c.connPool.RPC(region, addr, "Status.Peers", rpcargs, &peers); err != nil {
mErr.Errors = append(mErr.Errors, err)
continue
}

View file

@ -74,7 +74,7 @@ TRY:
}
// Make the request.
rpcErr := c.connPool.RPC(c.Region(), server.Addr, c.RPCMajorVersion(), method, args, reply)
rpcErr := c.connPool.RPC(c.Region(), server.Addr, method, args, reply)
if rpcErr == nil {
c.fireRpcRetryWatcher()
@ -427,7 +427,7 @@ func resolveServer(s string) (net.Addr, error) {
// a potential error.
func (c *Client) Ping(srv net.Addr) error {
var reply struct{}
err := c.connPool.RPC(c.Region(), srv, c.RPCMajorVersion(), "Status.Ping", struct{}{}, &reply)
err := c.connPool.RPC(c.Region(), srv, "Status.Ping", struct{}{}, &reply)
return err
}

View file

@ -191,9 +191,6 @@ func convertServerConfig(agentConfig *Config) (*nomad.Config, error) {
if agentConfig.Server.DataDir != "" {
conf.DataDir = agentConfig.Server.DataDir
}
if agentConfig.Server.ProtocolVersion != 0 {
conf.ProtocolVersion = uint8(agentConfig.Server.ProtocolVersion)
}
if agentConfig.Server.RaftProtocol != 0 {
conf.RaftConfig.ProtocolVersion = raft.ProtocolVersion(agentConfig.Server.RaftProtocol)
}

View file

@ -434,6 +434,12 @@ func (c *Command) IsValidConfig(config, cmdConfig *Config) bool {
}
}
// ProtocolVersion has never been used. Warn if it is set as someone
// has probably made a mistake.
if config.Server.ProtocolVersion != 0 {
c.agent.logger.Warn("Please remove deprecated protocol_version field from config.")
}
return true
}

View file

@ -367,7 +367,9 @@ type ServerConfig struct {
// ProtocolVersion is the protocol version to speak. This must be between
// ProtocolVersionMin and ProtocolVersionMax.
ProtocolVersion int `hcl:"protocol_version"`
//
// Deprecated: This has never been used and will emit a warning if nonzero.
ProtocolVersion int `hcl:"protocol_version" json:"-"`
// RaftProtocol is the Raft protocol version to speak. This must be from [1-3].
RaftProtocol int `hcl:"raft_protocol"`

View file

@ -95,7 +95,6 @@ var basicConfig = &Config{
AuthoritativeRegion: "foobar",
BootstrapExpect: 5,
DataDir: "/tmp/data",
ProtocolVersion: 3,
RaftProtocol: 3,
RaftMultiplier: helper.IntToPtr(4),
NumSchedulers: helper.IntToPtr(2),
@ -494,7 +493,6 @@ func TestConfig_Parse(t *testing.T) {
}
actual = oldDefault.Merge(actual)
//panic(fmt.Sprintf("first: %+v \n second: %+v", actual.TLSConfig, tc.Result.TLSConfig))
require.EqualValues(tc.Result, removeHelperAttributes(actual))
})
}

View file

@ -106,7 +106,6 @@ server {
authoritative_region = "foobar"
bootstrap_expect = 5
data_dir = "/tmp/data"
protocol_version = 3
raft_protocol = 3
num_schedulers = 2
enabled_schedulers = ["test"]

View file

@ -277,7 +277,6 @@
"node_gc_threshold": "12h",
"non_voting_server": true,
"num_schedulers": 2,
"protocol_version": 3,
"raft_protocol": 3,
"raft_multiplier": 4,
"redundancy_zone": "foo",

View file

@ -48,7 +48,6 @@ type Conn struct {
addr net.Addr
session *yamux.Session
lastUsed time.Time
version int
pool *ConnPool
@ -278,7 +277,7 @@ func (p *ConnPool) SetConnListener(l chan<- *Conn) {
// Acquire is used to get a connection that is
// pooled or to return a new connection
func (p *ConnPool) acquire(region string, addr net.Addr, version int) (*Conn, error) {
func (p *ConnPool) acquire(region string, addr net.Addr) (*Conn, error) {
// Check to see if there's a pooled connection available. This is up
// here since it should the vastly more common case than the rest
// of the code here.
@ -305,7 +304,7 @@ func (p *ConnPool) acquire(region string, addr net.Addr, version int) (*Conn, er
// If we are the lead thread, make the new connection and then wake
// everybody else up to see if we got it.
if isLeadThread {
c, err := p.getNewConn(region, addr, version)
c, err := p.getNewConn(region, addr)
p.Lock()
delete(p.limiter, addr.String())
close(wait)
@ -349,7 +348,7 @@ func (p *ConnPool) acquire(region string, addr net.Addr, version int) (*Conn, er
}
// getNewConn is used to return a new connection
func (p *ConnPool) getNewConn(region string, addr net.Addr, version int) (*Conn, error) {
func (p *ConnPool) getNewConn(region string, addr net.Addr) (*Conn, error) {
// Try to dial the conn
conn, err := net.DialTimeout("tcp", addr.String(), 10*time.Second)
if err != nil {
@ -404,7 +403,6 @@ func (p *ConnPool) getNewConn(region string, addr net.Addr, version int) (*Conn,
session: session,
clients: list.New(),
lastUsed: time.Now(),
version: version,
pool: p,
}
return c, nil
@ -429,12 +427,12 @@ func (p *ConnPool) clearConn(conn *Conn) {
}
}
// getClient is used to get a usable client for an address and protocol version
func (p *ConnPool) getRPCClient(region string, addr net.Addr, version int) (*Conn, *StreamClient, error) {
// getClient is used to get a usable client for an address
func (p *ConnPool) getRPCClient(region string, addr net.Addr) (*Conn, *StreamClient, error) {
retries := 0
START:
// Try to get a conn first
conn, err := p.acquire(region, addr, version)
conn, err := p.acquire(region, addr)
if err != nil {
return nil, nil, fmt.Errorf("failed to get conn: %v", err)
}
@ -457,8 +455,8 @@ START:
// StreamingRPC is used to make an streaming RPC call. Callers must
// close the connection when done.
func (p *ConnPool) StreamingRPC(region string, addr net.Addr, version int) (net.Conn, error) {
conn, err := p.acquire(region, addr, version)
func (p *ConnPool) StreamingRPC(region string, addr net.Addr) (net.Conn, error) {
conn, err := p.acquire(region, addr)
if err != nil {
return nil, fmt.Errorf("failed to get conn: %v", err)
}
@ -477,9 +475,9 @@ func (p *ConnPool) StreamingRPC(region string, addr net.Addr, version int) (net.
}
// RPC is used to make an RPC call to a remote host
func (p *ConnPool) RPC(region string, addr net.Addr, version int, method string, args interface{}, reply interface{}) error {
func (p *ConnPool) RPC(region string, addr net.Addr, method string, args interface{}, reply interface{}) error {
// Get a usable client
conn, sc, err := p.getRPCClient(region, addr, version)
conn, sc, err := p.getRPCClient(region, addr)
if err != nil {
return fmt.Errorf("rpc error: %w", err)
}

View file

@ -8,7 +8,6 @@ import (
"github.com/hashicorp/nomad/helper/freeport"
"github.com/hashicorp/nomad/helper/testlog"
"github.com/hashicorp/nomad/nomad/structs"
"github.com/stretchr/testify/require"
)
@ -50,7 +49,7 @@ func TestConnPool_ConnListener(t *testing.T) {
pool.SetConnListener(c)
// Make an RPC
_, err = pool.acquire("test", addr, structs.ApiMajorVersion)
_, err = pool.acquire("test", addr)
require.Nil(err)
// Assert we get a connection.

View file

@ -188,8 +188,7 @@ func (s *Server) serverWithNodeConn(nodeID, region string) (*serverParts, error)
// Make the RPC
var resp structs.NodeConnQueryResponse
err := s.connPool.RPC(s.config.Region, server.Addr, server.MajorVersion,
"Status.HasNodeConn", &req, &resp)
err := s.connPool.RPC(s.config.Region, server.Addr, "Status.HasNodeConn", &req, &resp)
if err != nil {
multierror.Append(&rpcErr, fmt.Errorf("failed querying server %q: %v", server.Addr.String(), err))
continue

View file

@ -1,7 +1,6 @@
package nomad
import (
"fmt"
"io"
"net"
"os"
@ -27,23 +26,6 @@ const (
DefaultSerfPort = 4648
)
// These are the protocol versions that Nomad can understand
const (
ProtocolVersionMin uint8 = 1
ProtocolVersionMax = 1
)
// ProtocolVersionMap is the mapping of Nomad protocol versions
// to Serf protocol versions. We mask the Serf protocols using
// our own protocol version.
var protocolVersionMap map[uint8]uint8
func init() {
protocolVersionMap = map[uint8]uint8{
1: 4,
}
}
func DefaultRPCAddr() *net.TCPAddr {
return &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 4647}
}
@ -93,10 +75,6 @@ type Config struct {
// Logger is the logger used by the server.
Logger log.InterceptLogger
// ProtocolVersion is the protocol version to speak. This must be between
// ProtocolVersionMin and ProtocolVersionMax.
ProtocolVersion uint8
// RPCAddr is the RPC address used by Nomad. This should be reachable
// by the other servers and clients
RPCAddr *net.TCPAddr
@ -370,18 +348,6 @@ type Config struct {
DeploymentQueryRateLimit float64
}
// CheckVersion is used to check if the ProtocolVersion is valid
func (c *Config) CheckVersion() error {
if c.ProtocolVersion < ProtocolVersionMin {
return fmt.Errorf("Protocol version '%d' too low. Must be in range: [%d, %d]",
c.ProtocolVersion, ProtocolVersionMin, ProtocolVersionMax)
} else if c.ProtocolVersion > ProtocolVersionMax {
return fmt.Errorf("Protocol version '%d' too high. Must be in range: [%d, %d]",
c.ProtocolVersion, ProtocolVersionMin, ProtocolVersionMax)
}
return nil
}
// DefaultConfig returns the default configuration. Only used as the basis for
// merging agent or test parameters.
func DefaultConfig() *Config {
@ -396,7 +362,6 @@ func DefaultConfig() *Config {
Datacenter: DefaultDC,
NodeName: hostname,
NodeID: uuid.Generate(),
ProtocolVersion: ProtocolVersionMax,
RaftConfig: raft.DefaultConfig(),
RaftTimeout: 10 * time.Second,
LogOutput: os.Stderr,

View file

@ -267,8 +267,6 @@ func (n *Node) constructNodeServerInfoResponse(snap *state.StateSnapshot, reply
reply.Servers = append(reply.Servers,
&structs.NodeServerInfo{
RPCAdvertiseAddr: v.RPCAddr.String(),
RPCMajorVersion: int32(v.MajorVersion),
RPCMinorVersion: int32(v.MinorVersion),
Datacenter: v.Datacenter,
})
}

View file

@ -644,7 +644,7 @@ func (r *rpcHandler) forwardLeader(server *serverParts, method string, args inte
if server == nil {
return structs.ErrNoLeader
}
return r.connPool.RPC(r.config.Region, server.Addr, server.MajorVersion, method, args, reply)
return r.connPool.RPC(r.config.Region, server.Addr, method, args, reply)
}
// forwardServer is used to forward an RPC call to a particular server
@ -653,7 +653,7 @@ func (r *rpcHandler) forwardServer(server *serverParts, method string, args inte
if server == nil {
return errors.New("must be given a valid server address")
}
return r.connPool.RPC(r.config.Region, server.Addr, server.MajorVersion, method, args, reply)
return r.connPool.RPC(r.config.Region, server.Addr, method, args, reply)
}
func (r *rpcHandler) findRegionServer(region string) (*serverParts, error) {
@ -680,7 +680,7 @@ func (r *rpcHandler) forwardRegion(region, method string, args interface{}, repl
// Forward to remote Nomad
metrics.IncrCounter([]string{"nomad", "rpc", "cross-region", region}, 1)
return r.connPool.RPC(region, server.Addr, server.MajorVersion, method, args, reply)
return r.connPool.RPC(region, server.Addr, method, args, reply)
}
func (r *rpcHandler) getServer(region, serverID string) (*serverParts, error) {
@ -708,7 +708,7 @@ func (r *rpcHandler) getServer(region, serverID string) (*serverParts, error) {
// initial handshake, returning the connection or an error. It is the callers
// responsibility to close the connection if there is no returned error.
func (r *rpcHandler) streamingRpc(server *serverParts, method string) (net.Conn, error) {
c, err := r.connPool.StreamingRPC(r.config.Region, server.Addr, server.MajorVersion)
c, err := r.connPool.StreamingRPC(r.config.Region, server.Addr)
if err != nil {
return nil, err
}

View file

@ -164,7 +164,7 @@ func (s *Server) maybeBootstrap() {
// Retry with exponential backoff to get peer status from this server
for attempt := uint(0); attempt < maxPeerRetries; attempt++ {
if err := s.connPool.RPC(s.config.Region, server.Addr, server.MajorVersion,
if err := s.connPool.RPC(s.config.Region, server.Addr,
"Status.Peers", req, &peers); err != nil {
nextRetry := (1 << attempt) * peerRetryBase
s.logger.Error("failed to confirm peer status", "peer", server.Name, "error", err, "retry", nextRetry)

View file

@ -291,10 +291,6 @@ type endpoints struct {
// NewServer is used to construct a new Nomad server from the
// configuration, potentially returning an error
func NewServer(config *Config, consulCatalog consul.CatalogAPI, consulConfigEntries consul.ConfigAPI, consulACLs consul.ACLsAPI) (*Server, error) {
// Check the protocol version
if err := config.CheckVersion(); err != nil {
return nil, err
}
// Create an eval broker
evalBroker, err := NewEvalBroker(
@ -1398,8 +1394,6 @@ func (s *Server) setupSerf(conf *serf.Config, ch chan serf.Event, path string) (
conf.Tags["role"] = "nomad"
conf.Tags["region"] = s.config.Region
conf.Tags["dc"] = s.config.Datacenter
conf.Tags["vsn"] = fmt.Sprintf("%d", structs.ApiMajorVersion)
conf.Tags["mvn"] = fmt.Sprintf("%d", structs.ApiMinorVersion)
conf.Tags["build"] = s.config.Build
conf.Tags["raft_vsn"] = fmt.Sprintf("%d", s.config.RaftConfig.ProtocolVersion)
conf.Tags["id"] = s.config.NodeID
@ -1433,7 +1427,6 @@ func (s *Server) setupSerf(conf *serf.Config, ch chan serf.Event, path string) (
return nil, err
}
}
conf.ProtocolVersion = protocolVersionMap[s.config.ProtocolVersion]
conf.RejoinAfterLeave = true
// LeavePropagateDelay is used to make sure broadcasted leave intents propagate
// This value was tuned using https://www.serf.io/docs/internals/simulator.html to

View file

@ -43,7 +43,7 @@ func NewStatsFetcher(logger log.Logger, pool *pool.ConnPool, region string) *Sta
func (f *StatsFetcher) fetch(server *serverParts, replyCh chan *autopilot.ServerStats) {
var args struct{}
var reply autopilot.ServerStats
err := f.pool.RPC(f.region, server.Addr, server.MajorVersion, "Status.RaftStats", &args, &reply)
err := f.pool.RPC(f.region, server.Addr, "Status.RaftStats", &args, &reply)
if err != nil {
f.logger.Warn("failed retrieving server health", "server", server.Name, "error", err)
} else {

View file

@ -17,23 +17,6 @@ type Status struct {
logger log.Logger
}
// Version is used to allow clients to determine the capabilities
// of the server
func (s *Status) Version(args *structs.GenericRequest, reply *structs.VersionResponse) error {
if done, err := s.srv.forward("Status.Version", args, args, reply); done {
return err
}
conf := s.srv.config
reply.Build = conf.Build
reply.Versions = map[string]int{
structs.ProtocolVersion: int(conf.ProtocolVersion),
structs.APIMajorVersion: structs.ApiMajorVersion,
structs.APIMinorVersion: structs.ApiMinorVersion,
}
return nil
}
// Ping is used to just check for connectivity
func (s *Status) Ping(args struct{}, reply *struct{}) error {
return nil

View file

@ -13,38 +13,6 @@ import (
"github.com/stretchr/testify/require"
)
func TestStatusVersion(t *testing.T) {
t.Parallel()
s1, cleanupS1 := TestServer(t, nil)
defer cleanupS1()
codec := rpcClient(t, s1)
arg := &structs.GenericRequest{
QueryOptions: structs.QueryOptions{
Region: "global",
AllowStale: true,
},
}
var out structs.VersionResponse
if err := msgpackrpc.CallWithCodec(codec, "Status.Version", arg, &out); err != nil {
t.Fatalf("err: %v", err)
}
if out.Build == "" {
t.Fatalf("bad: %#v", out)
}
if out.Versions[structs.ProtocolVersion] != ProtocolVersionMax {
t.Fatalf("bad: %#v", out)
}
if out.Versions[structs.APIMajorVersion] != structs.ApiMajorVersion {
t.Fatalf("bad: %#v", out)
}
if out.Versions[structs.APIMinorVersion] != structs.ApiMinorVersion {
t.Fatalf("bad: %#v", out)
}
}
func TestStatusPing(t *testing.T) {
t.Parallel()

View file

@ -123,21 +123,6 @@ const (
// methods directly that require an FSM MessageType
MsgTypeTestSetup MessageType = IgnoreUnknownTypeFlag
// ApiMajorVersion is returned as part of the Status.Version request.
// It should be incremented anytime the APIs are changed in a way
// that would break clients for sane client versioning.
ApiMajorVersion = 1
// ApiMinorVersion is returned as part of the Status.Version request.
// It should be incremented anytime the APIs are changed to allow
// for sane client versioning. Minor changes should be compatible
// within the major version.
ApiMinorVersion = 1
ProtocolVersion = "protocol"
APIMajorVersion = "api.major"
APIMinorVersion = "api.minor"
GetterModeAny = "any"
GetterModeFile = "file"
GetterModeDir = "dir"

View file

@ -30,21 +30,19 @@ func ensurePath(path string, dir bool) error {
// serverParts is used to return the parts of a server role
type serverParts struct {
Name string
ID string
Region string
Datacenter string
Port int
Bootstrap bool
Expect int
MajorVersion int
MinorVersion int
Build version.Version
RaftVersion int
Addr net.Addr
RPCAddr net.Addr
Status serf.MemberStatus
NonVoter bool
Name string
ID string
Region string
Datacenter string
Port int
Bootstrap bool
Expect int
Build version.Version
RaftVersion int
Addr net.Addr
RPCAddr net.Addr
Status serf.MemberStatus
NonVoter bool
}
func (s *serverParts) String() string {
@ -100,21 +98,6 @@ func isNomadServer(m serf.Member) (bool, *serverParts) {
return false, nil
}
// The "vsn" tag was Version, which is now the MajorVersion number.
majorVersionStr := m.Tags["vsn"]
majorVersion, err := strconv.Atoi(majorVersionStr)
if err != nil {
return false, nil
}
// To keep some semblance of convention, "mvn" is now the "Minor
// Version Number."
minorVersionStr := m.Tags["mvn"]
minorVersion, err := strconv.Atoi(minorVersionStr)
if err != nil {
minorVersion = 0
}
raftVsn := 0
raftVsnString, ok := m.Tags["raft_vsn"]
if ok {
@ -130,21 +113,19 @@ func isNomadServer(m serf.Member) (bool, *serverParts) {
addr := &net.TCPAddr{IP: m.Addr, Port: port}
rpcAddr := &net.TCPAddr{IP: rpcIP, Port: port}
parts := &serverParts{
Name: m.Name,
ID: id,
Region: region,
Datacenter: datacenter,
Port: port,
Bootstrap: bootstrap,
Expect: expect,
Addr: addr,
RPCAddr: rpcAddr,
MajorVersion: majorVersion,
MinorVersion: minorVersion,
Build: *buildVersion,
RaftVersion: raftVsn,
Status: m.Status,
NonVoter: nonVoter,
Name: m.Name,
ID: id,
Region: region,
Datacenter: datacenter,
Port: port,
Bootstrap: bootstrap,
Expect: expect,
Addr: addr,
RPCAddr: rpcAddr,
Build: *buildVersion,
RaftVersion: raftVsn,
Status: m.Status,
NonVoter: nonVoter,
}
return true, parts
}

View file

@ -23,7 +23,6 @@ func TestIsNomadServer(t *testing.T) {
"dc": "east-aws",
"rpc_addr": "1.1.1.1",
"port": "10000",
"vsn": "1",
"raft_vsn": "2",
"build": "0.7.0+ent",
"nonvoter": "1",
@ -69,9 +68,6 @@ func TestIsNomadServer(t *testing.T) {
if parts.Addr.String() != "127.0.0.1:10000" {
t.Fatalf("bad addr: %v", parts.Addr)
}
if parts.MajorVersion != 1 {
t.Fatalf("bad: %v", parts)
}
m.Tags["expect"] = "3"
delete(m.Tags, "bootstrap")
@ -204,7 +200,6 @@ func makeMember(version string, status serf.MemberStatus) serf.Member {
"dc": "east-aws",
"port": "10000",
"build": version,
"vsn": "1",
},
Status: status,
}

View file

@ -348,15 +348,11 @@ $ curl \
"commit_index": "144",
"term": "2",
"last_log_index": "144",
"protocol_version_max": "3",
"snapshot_version_max": "1",
"latest_configuration_index": "1",
"latest_configuration": "[{Suffrage:Voter ID:127.0.0.1:4647 Address:127.0.0.1:4647}]",
"last_contact": "never",
"applied_index": "144",
"protocol_version": "1",
"protocol_version_min": "0",
"snapshot_version_min": "0",
"state": "Leader",
"last_snapshot_term": "0"
},

View file

@ -156,11 +156,6 @@ server {
disallow this server from making any scheduling decisions. This defaults to
the number of CPU cores.
- `protocol_version` `(int: 1)` - Specifies the Nomad protocol version to use
when communicating with other Nomad servers. This value is typically not
required as the agent internally knows the latest version, but may be useful
in some upgrade scenarios.
- `raft_protocol` `(int: 3)` - Specifies the Raft protocol version to use when
communicating with other Nomad servers. This affects available Autopilot
features and is typically not required as the agent internally knows the