feat: P0-2b implement ControlHost via xcecli_hostctrl + fix allFlag race

- backend/clib.go: Replace ControlHost stub with real implementation
  using xcecli_hostctrl, following the same pattern as ControlQueue
- controllers/hosts.go: Remove package-level allFlag variable;
  preCheckHostControlRequest now returns (bool, error) so the allFlag
  is a local value in the caller, eliminating the data race
- controllers/queues.go: Same fix for preCheckQueueControlRequest
- Add unit tests for both preCheck functions verifying return values,
  error cases, and concurrent safety
- Add compile-time verification test for ControlHost stub signature
This commit is contained in:
Rongsong Shen 2026-04-17 08:17:56 +00:00
parent ff940223ec
commit 8124cda83e
6 changed files with 330 additions and 26 deletions

View file

@ -309,11 +309,21 @@ func ControlQueue(queuename string, action int) error {
return nil
}
// ControlHost is a placeholder; host control via xcecli is not yet exposed
// in client.h. Use the REST API workaround until the C library adds
// xcecli_hostctrl.
// ControlHost sends a host open/close action to the backend via xcecli_hostctrl.
func ControlHost(hostname string, action int) error {
return fmt.Errorf("host control not yet supported via C library (host: %s, action: %d)", hostname, action)
cHost := C.CString(hostname)
defer C.free(unsafe.Pointer(cHost))
ctrl := C.xce_hostctrl_t{
action: C.int(action),
hostname: cHost,
}
rc := C.xcecli_hostctrl(gCtx.ctx, &ctrl)
if rc < 0 {
return fmt.Errorf("xcecli_hostctrl failed: %d", int(rc))
}
return nil
}
// signalNumber maps a job action and optional signal name to a POSIX signal

View file

@ -0,0 +1,25 @@
package backend
import (
"testing"
)
// TestControlHost_StubReturnsError verifies that in the non-CGO build the
// ControlHost function is present, accepts the expected parameters, and
// returns a non-nil error (the errNoCGO sentinel).
func TestControlHost_StubReturnsError(t *testing.T) {
err := ControlHost("node01", 1)
if err == nil {
t.Fatal("expected non-nil error from ControlHost stub")
}
}
// TestControlHost_SignatureCompiles is a compile-time check that the function
// signature matches: func ControlHost(hostname string, action int) error.
// If the signature changes this test will not compile.
func TestControlHost_SignatureCompiles(t *testing.T) {
var fn func(string, int) error = ControlHost
if fn == nil {
t.Fatal("ControlHost must not be nil")
}
}

View file

@ -0,0 +1,139 @@
package controllers
import (
"apiserver/models"
"apiserver/store"
"apiserver/utils"
"sync"
"testing"
"go.uber.org/zap"
)
func resetHostControlState() {
store.InitStores()
utils.Log = zap.NewNop()
}
// TestPreCheckHostControlRequest_AllFlag verifies that when hostname is ["all"],
// the returned bool is true.
func TestPreCheckHostControlRequest_AllFlag(t *testing.T) {
resetHostControlState()
req := &models.HostControlRequest{
ControlAction: "close",
Hostname: []string{"all"},
}
allFlag, err := preCheckHostControlRequest(req)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if !allFlag {
t.Error("expected allFlag=true when hostname is [\"all\"]")
}
}
// TestPreCheckHostControlRequest_NotAll verifies that when hostname is a
// specific host, the returned bool is false.
func TestPreCheckHostControlRequest_NotAll(t *testing.T) {
resetHostControlState()
req := &models.HostControlRequest{
ControlAction: "open",
Hostname: []string{"node01"},
}
allFlag, err := preCheckHostControlRequest(req)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if allFlag {
t.Error("expected allFlag=false when hostname is [\"node01\"]")
}
}
// TestPreCheckHostControlRequest_NoRace verifies that concurrent calls to
// preCheckHostControlRequest do not share state via a package-level variable.
func TestPreCheckHostControlRequest_NoRace(t *testing.T) {
t.Parallel()
resetHostControlState()
var wg sync.WaitGroup
const goroutines = 50
results := make([]bool, goroutines)
errors := make([]error, goroutines)
for i := 0; i < goroutines; i++ {
wg.Add(1)
go func(idx int) {
defer wg.Done()
var req models.HostControlRequest
if idx%2 == 0 {
req = models.HostControlRequest{
ControlAction: "close",
Hostname: []string{"all"},
}
} else {
req = models.HostControlRequest{
ControlAction: "open",
Hostname: []string{"node01"},
}
}
results[idx], errors[idx] = preCheckHostControlRequest(&req)
}(i)
}
wg.Wait()
for i := 0; i < goroutines; i++ {
if errors[i] != nil {
t.Errorf("goroutine %d: unexpected error: %v", i, errors[i])
continue
}
if i%2 == 0 && !results[i] {
t.Errorf("goroutine %d: expected allFlag=true for [\"all\"]", i)
}
if i%2 != 0 && results[i] {
t.Errorf("goroutine %d: expected allFlag=false for [\"node01\"]", i)
}
}
}
// TestPreCheckHostControlRequest_InvalidAction verifies that an unsupported
// action returns an error.
func TestPreCheckHostControlRequest_InvalidAction(t *testing.T) {
resetHostControlState()
req := &models.HostControlRequest{
ControlAction: "invalid_action",
Hostname: []string{"node01"},
}
_, err := preCheckHostControlRequest(req)
if err == nil {
t.Error("expected error for invalid action, got nil")
}
}
// TestPreCheckHostControlRequest_CommentTooLong verifies that a comment
// exceeding 512 characters returns an error.
func TestPreCheckHostControlRequest_CommentTooLong(t *testing.T) {
resetHostControlState()
longComment := make([]byte, 513)
for i := range longComment {
longComment[i] = 'a'
}
req := &models.HostControlRequest{
ControlAction: "close",
Hostname: []string{"node01"},
Comment: string(longComment),
}
_, err := preCheckHostControlRequest(req)
if err == nil {
t.Error("expected error for comment exceeding 512 chars, got nil")
}
}

View file

@ -19,8 +19,6 @@ import (
type HostHandler struct {
}
var allFlag bool
// GetHosts godoc
// @Summary Get host information
// @Description Get host information
@ -392,7 +390,7 @@ func (h *HostHandler) ControlHost(ctx *gin.Context) {
return
}
err := preCheckHostControlRequest(&req)
allFlag, err := preCheckHostControlRequest(&req)
if err != nil {
logger.Error("Failed to send host control request", zap.Error(err))
ReturnError(ctx, common.ERROR_CODE, err.Error(), nil)
@ -551,26 +549,22 @@ func isHostControlActionAllowed(actionType int, status int) bool {
}
return false
}
func preCheckHostControlRequest(req *models.HostControlRequest) error {
func preCheckHostControlRequest(req *models.HostControlRequest) (bool, error) {
logger.Debug("Enter preCheckHostControlRequest")
if err := utils.ValidateControlAction(req.ControlAction, models.SupportedHostActions); err != nil {
return err
return false, err
}
req.ControlAction = strings.ToLower(strings.TrimSpace(req.ControlAction))
if req.Hostname != nil && len(req.Hostname) == 1 && req.Hostname[0] == "all" {
allFlag = true
} else {
allFlag = false
}
isAll := req.Hostname != nil && len(req.Hostname) == 1 && req.Hostname[0] == "all"
if req.Comment != "" {
if len(req.Comment) > 512 {
return errors.New("comment length exceeds the limit of 512 characters")
return false, errors.New("comment length exceeds the limit of 512 characters")
}
}
return nil
return isAll, nil
}
func processHostData(rows []utils.OrderMap, page_token string) utils.QueryResult {

View file

@ -0,0 +1,139 @@
package controllers
import (
"apiserver/models"
"apiserver/store"
"apiserver/utils"
"sync"
"testing"
"go.uber.org/zap"
)
func resetQueueControlState() {
store.InitStores()
utils.Log = zap.NewNop()
}
// TestPreCheckQueueControlRequest_AllFlag verifies that when queue_name is
// ["all"], the returned bool is true.
func TestPreCheckQueueControlRequest_AllFlag(t *testing.T) {
resetQueueControlState()
req := &models.QueueControlRequest{
ControlAction: "close",
QueueName: []string{"all"},
}
allFlag, err := preCheckQueueControlRequest(req)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if !allFlag {
t.Error("expected allFlag=true when queue_name is [\"all\"]")
}
}
// TestPreCheckQueueControlRequest_NotAll verifies that when queue_name is a
// specific queue, the returned bool is false.
func TestPreCheckQueueControlRequest_NotAll(t *testing.T) {
resetQueueControlState()
req := &models.QueueControlRequest{
ControlAction: "open",
QueueName: []string{"normal"},
}
allFlag, err := preCheckQueueControlRequest(req)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if allFlag {
t.Error("expected allFlag=false when queue_name is [\"normal\"]")
}
}
// TestPreCheckQueueControlRequest_NoRace verifies that concurrent calls to
// preCheckQueueControlRequest do not share state via a package-level variable.
func TestPreCheckQueueControlRequest_NoRace(t *testing.T) {
t.Parallel()
resetQueueControlState()
var wg sync.WaitGroup
const goroutines = 50
results := make([]bool, goroutines)
errors := make([]error, goroutines)
for i := 0; i < goroutines; i++ {
wg.Add(1)
go func(idx int) {
defer wg.Done()
var req models.QueueControlRequest
if idx%2 == 0 {
req = models.QueueControlRequest{
ControlAction: "close",
QueueName: []string{"all"},
}
} else {
req = models.QueueControlRequest{
ControlAction: "open",
QueueName: []string{"normal"},
}
}
results[idx], errors[idx] = preCheckQueueControlRequest(&req)
}(i)
}
wg.Wait()
for i := 0; i < goroutines; i++ {
if errors[i] != nil {
t.Errorf("goroutine %d: unexpected error: %v", i, errors[i])
continue
}
if i%2 == 0 && !results[i] {
t.Errorf("goroutine %d: expected allFlag=true for [\"all\"]", i)
}
if i%2 != 0 && results[i] {
t.Errorf("goroutine %d: expected allFlag=false for [\"normal\"]", i)
}
}
}
// TestPreCheckQueueControlRequest_InvalidAction verifies that an unsupported
// action returns an error.
func TestPreCheckQueueControlRequest_InvalidAction(t *testing.T) {
resetQueueControlState()
req := &models.QueueControlRequest{
ControlAction: "invalid_action",
QueueName: []string{"normal"},
}
_, err := preCheckQueueControlRequest(req)
if err == nil {
t.Error("expected error for invalid action, got nil")
}
}
// TestPreCheckQueueControlRequest_CommentTooLong verifies that a comment
// exceeding 512 characters returns an error.
func TestPreCheckQueueControlRequest_CommentTooLong(t *testing.T) {
resetQueueControlState()
longComment := make([]byte, 513)
for i := range longComment {
longComment[i] = 'a'
}
req := &models.QueueControlRequest{
ControlAction: "close",
QueueName: []string{"normal"},
Comment: string(longComment),
}
_, err := preCheckQueueControlRequest(req)
if err == nil {
t.Error("expected error for comment exceeding 512 chars, got nil")
}
}

View file

@ -437,7 +437,7 @@ func (q *QueueController) ControlQueue(ctx *gin.Context) {
return
}
err = preCheckQueueControlRequest(&req)
allFlag, err := preCheckQueueControlRequest(&req)
if err != nil {
logger.Error("Failed to send queue control request", zap.Error(err))
ReturnError(ctx, common.ERROR_CODE, err.Error(), nil)
@ -523,25 +523,22 @@ func (q *QueueController) ControlQueue(ctx *gin.Context) {
}
func preCheckQueueControlRequest(req *models.QueueControlRequest) error {
func preCheckQueueControlRequest(req *models.QueueControlRequest) (bool, error) {
logger.Debug("Enter preCheckQueueControlRequest")
if err := utils.ValidateControlAction(req.ControlAction, models.SupportedQueueActions); err != nil {
return err
return false, err
}
req.ControlAction = strings.ToLower(strings.TrimSpace(req.ControlAction))
if req.QueueName != nil && len(req.QueueName) == 1 && req.QueueName[0] == "all" {
allFlag = true
} else {
allFlag = false
}
isAll := req.QueueName != nil && len(req.QueueName) == 1 && req.QueueName[0] == "all"
if req.Comment != "" {
if len(req.Comment) > 512 {
return errors.New("comment length exceeds the limit of 512 characters")
return false, errors.New("comment length exceeds the limit of 512 characters")
}
}
return nil
return isAll, nil
}
func validateQueue(control int, queuenames []string) (successQueues []string, failedQueues map[string]string) {