Skip to content

Commit

Permalink
Merge pull request #5725 from oasisprotocol/kostko/fix/observer-compu…
Browse files Browse the repository at this point in the history
…te-role

go/worker/storage: Do not add any particular roles
  • Loading branch information
kostko authored Jun 25, 2024
2 parents 7743160 + 7caaced commit 7e58b09
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 3 deletions.
5 changes: 5 additions & 0 deletions .changelog/5725.bugfix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
go/worker/storage: Do not add any particular roles

Since the storage node is always coupled with another role, make sure
to not add any particular role as otherwise this could cause observer
nodes to also register as compute nodes and then misbehave.
7 changes: 7 additions & 0 deletions go/common/node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,8 @@ func (sw SoftwareVersion) ValidateBasic() error {
type RolesMask uint32

const (
// RoleEmpty is the roles bitmask that specifies no roles.
RoleEmpty RolesMask = 0
// RoleComputeWorker is the compute worker role.
RoleComputeWorker RolesMask = 1 << 0
// RoleObserver is the observer role.
Expand Down Expand Up @@ -216,6 +218,11 @@ func Roles() (roles []RolesMask) {
}
}

// IsEmptyRole returns true if RolesMask encodes no roles (e.g. is equal to RoleEmpty).
func (m RolesMask) IsEmptyRole() bool {
return m == RoleEmpty
}

// IsSingleRole returns true if RolesMask encodes a single valid role.
func (m RolesMask) IsSingleRole() bool {
// Ensures exactly one bit is set, and the set bit is a valid role.
Expand Down
12 changes: 10 additions & 2 deletions go/worker/registration/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -769,8 +769,8 @@ func (w *Worker) newRoleProvider(role node.RolesMask, runtimeID *common.Namespac
"id", runtimeID,
"role", role,
)
if !role.IsSingleRole() {
return nil, fmt.Errorf("RegisterRole: registration role mask does not encode a single role. RoleMask: '%s'", role)
if !role.IsEmptyRole() && !role.IsSingleRole() {
return nil, fmt.Errorf("registration role mask is non-empty and does not encode a single role: '%s'", role)
}
if err := w.runtimeRegistry.AddRoles(role, runtimeID); err != nil {
w.logger.Info("runtime doesn't exist, roles not registered",
Expand Down Expand Up @@ -887,6 +887,14 @@ func (w *Worker) registerNode(epoch beacon.EpochTime, hook RegisterNodeHook) (er
return err
}

// Make sure there is at least one role to register for.
if nodeDesc.Roles.IsEmptyRole() {
w.logger.Error("not registering: no roles to register for",
"node_descriptor", nodeDesc,
)
return fmt.Errorf("registration: no roles to register for")
}

// Sanity check to prevent an invalid registration when no role provider added any runtimes but
// runtimes are required due to the specified role.
if nodeDesc.HasRoles(registry.RuntimesRequiredRoles) && len(nodeDesc.Runtimes) == 0 {
Expand Down
5 changes: 4 additions & 1 deletion go/worker/storage/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,10 @@ func (w *Worker) registerRuntime(commonNode *committeeCommon.Node) error {
"runtime_id", id,
)

rp, err := w.registration.NewRuntimeRoleProvider(node.RoleComputeWorker, id)
// Since the storage node is always coupled with another role, make sure to not add any
// particular role here. Instead this only serves to prevent registration until the storage node
// is synced by making the role provider unavailable.
rp, err := w.registration.NewRuntimeRoleProvider(node.RoleEmpty, id)
if err != nil {
return fmt.Errorf("failed to create role provider: %w", err)
}
Expand Down

0 comments on commit 7e58b09

Please sign in to comment.