myxce/src/lib/group.c
Rongsong Shen 22cfe61642
fix(scheduler): fix limit-plugin usergroup/scope/error-code cascade (gitea#46)
While live-verifying gitea#46, found and fixed a cascade of 5 previously
undetected bugs that together made every usergroup-scoped limit in
limits.yaml silently non-functional:

- Named usergroups/hostgroups (users.yaml/hosts.yaml) were never
  registered into the scheduler's group managers (groupmgr_add() had zero
  production callers), so any limit referencing one by name could never
  resolve real membership.
- An unconfigured limit scope (e.g. no "projects:") arrived as an
  empty-but-present capnp group, not NULL, so build_consumer() built a
  real empty group (matching no one) instead of falling back to "all".
- Three separate call sites conflated key_find()'s negative "not found"
  return with a real error, short-circuiting before ever reaching the
  "instantiate on first use" logic right below them -- the same defect
  class already fixed once before in this codebase (bug-1495).
- Two call sites passed &(sched->core.resmgr) where a plain handle was
  expected, one level of indirection too many, causing RTTI's key_find()
  to reject it as an invalid handle.

All five are live-verified: fresh cluster deploys now correctly match and
instantiate usage tracking for all 7 limits in limits.yaml on every job
submission, reproduced across multiple independent from-scratch redeploys.

A 6th, architecturally distinct issue remains open (bug-1789, logged, not
fixed): sched_fill_limit_requests() computes a job's consumption of a
slots-scoped global limit by reading the wrong request vector, so the
numeric cap itself still never binds. This needs a design decision on
intended cross-scope semantics, not a mechanical fix, so it's left for a
follow-up.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hb6Edtj4KbS8CrgEWYdA2E
2026-09-07 12:44:16 +00:00

503 lines
11 KiB
C

#include <errno.h>
#include <rtti.h>
#include <stdio.h>
#include <stdlib.h>
#include "group.h"
#include "kvmgr.h"
#include "utils.h"
#include "xce/types.h"
#include "xstring.h"
typedef struct {
} group_private_t;
static void group_killer(xce_group_t* g)
{
int i;
if (g == NULL) {
return;
}
for (i = 0; i < g->members.count; i++) {
xce_g_member_t* m = (xce_g_member_t*)xlist_get(&(g->members), i);
if (!m->is_group) {
dereference(m->u.name);
}
FREEUP(m);
}
for (i = 0; i < g->ex_members.count; i++) {
xce_g_member_t* m = (xce_g_member_t*)xlist_get(&(g->ex_members), i);
if (!m->is_group) {
dereference(m->u.name);
}
FREEUP(m);
}
for (i = 0; i < g->affecteds.count; i++) {
xce_group_t* g1 = (xce_group_t*)xlist_get(&(g->affecteds), i);
xlist_del(&(g1->members), g);
xlist_del(&(g1->ex_members), g);
}
xlist_fini(&(g->members));
xlist_fini(&(g->ex_members));
xlist_fini(&(g->affecteds));
dereference(g->name);
}
xce_g_member_t* make_g_member(var mgr, char* name)
{
xce_g_member_t* m = NULL;
xce_group_t* g = NULL;
if ((mgr == NULL) || (name == NULL)) {
errno = EINVAL;
return NULL;
}
m = (xce_g_member_t*)calloc(1, sizeof(*m));
if (m == NULL) {
return NULL;
}
g = get_group(mgr, name);
if (g != NULL) {
m->is_group = 1;
m->u.g = g;
} else {
m->is_group = 0;
m->u.name = makeString(name);
}
return m;
}
static int update_group_dirty(xce_group_t* g)
{
int i;
int count = 0;
for (i = 0; i < g->affecteds.count; i++) {
xce_group_t* g1 = (xce_group_t*)xlist_get(&(g->affecteds), i);
if (g1 == NULL) {
continue;
}
g1->dirty = 1;
count++;
count += update_group_dirty(g1);
}
return count;
}
static int init_group_by_config(xce_group_t* g, var mgr,
xce_group_config_t* g_config)
{
int i;
for (i = 0; i < g_config->members.count; i++) {
char* g_name = g_config->members.data[i];
xce_g_member_t* m = make_g_member(mgr, g_name);
if (m == NULL) {
continue;
}
xlist_store(&(g->members), m);
if (m->is_group) {
xlist_store(&(m->u.g->affecteds), g);
}
}
for (i = 0; i < g_config->ex_members.count; i++) {
char* g_name = g_config->ex_members.data[i];
xce_g_member_t* m = make_g_member(mgr, g_name);
if (m == NULL) {
continue;
}
xlist_store(&(g->ex_members), m);
if (m->is_group) {
xlist_store(&(m->u.g->affecteds), g);
}
}
return 0;
}
static size_t group_datasize(va_list ap)
{
size_t sz = sizeof(xce_group_t);
return sz;
}
static int group_init(meta_obj_t* obj, va_list ap)
{
xce_group_t* g = RTTI_OBJ_TO_DATA(xce_group_t, obj);
var mgr = va_arg(ap, var);
xce_group_config_t* config = va_arg(ap, xce_group_config_t*);
char* g_name = va_arg(ap, char*);
int rc;
if ((mgr == NULL) || (config == NULL)) {
return -EINVAL;
}
lock(mgr, RTTI_LOCK_WRITE);
rc = key_add(mgr, (void*)g_name, (void*)g);
unlock(mgr);
if (rc < 0) {
return rc;
}
xlist_init(&(g->affecteds), 4);
xlist_init(&(g->members), 4);
xlist_init(&(g->ex_members), 4);
g->name = makeString(g_name);
g->dirty = 0;
init_group_by_config(g, mgr, config);
return 1;
}
static void group_destroy(meta_obj_t* obj)
{
xce_group_t* g = RTTI_OBJ_TO_DATA(xce_group_t, obj);
group_killer(g);
}
static typeinfo_t group_type = { .name = "group",
.privateSize = sizeof(group_private_t),
.dataSize = (datasize_fn_t)group_datasize,
.init = (obj_init_fn_t)group_init,
.destroy = (obj_destroy_fn_t)group_destroy,
.interfaces = {} };
xce_group_t* makeGroup(var mgr, xce_group_config_t* config,
char* default_g_name)
{
xce_group_t* g = NULL;
char* g_name = NULL;
int rc = 0;
if ((mgr == NULL) || (config == NULL)) {
errno = EINVAL;
return NULL;
}
g_name = config->name;
if ((g_name == NULL) || (strlen(g_name) == 0)) {
g_name = default_g_name;
}
if (g_name == NULL) {
errno = EINVAL;
return NULL;
}
lock(mgr, RTTI_LOCK_READ);
rc = key_find(mgr, (void*)g_name, (void**)&g);
unlock(mgr);
if ((rc < 0) && (rc != -ENOENT)) {
errno = -rc;
return NULL;
}
if (g != NULL) {
int i;
for (i = 0; i < g->members.count; i++) {
xce_g_member_t* m = (xce_g_member_t*)xlist_get(&(g->members), i);
if (m == NULL) {
continue;
}
if (m->is_group == 0) {
dereference(m->u.name);
}
FREEUP(m);
}
g->members.count = 0;
for (i = 0; i < g->ex_members.count; i++) {
xce_g_member_t* m = (xce_g_member_t*)xlist_get(&(g->ex_members), i);
if (m == NULL) {
continue;
}
if (m->is_group == 0) {
dereference(m->u.name);
}
FREEUP(m);
}
g->ex_members.count = 0;
g->dirty = 1;
init_group_by_config(g, mgr, config);
return g;
}
g = makeInstance(&group_type, mgr, config, g_name);
return g;
}
int groupmgr_init(var* mgr, int nslots, int delta)
{
xce_group_config_t all = { .name = "all" };
xce_group_config_t empty = { .name = "empty" };
xce_group_t* g = NULL;
kvmgr_params_t params = { .key_type = KVMGR_KEY_STRING,
.nslots = nslots,
.delta = delta,
.datakiller = destroy };
*mgr = makeInstance(&xkvmgr_type, &params);
if ((*mgr) == NULL) {
return -errno;
}
/* create builtin all group */
g = makeGroup(*mgr, &all, NULL);
if (g == NULL) {
return -errno;
}
/* create builtin empty group */
g = makeGroup(*mgr, &empty, NULL);
if (g == NULL) {
return -ENOMEM;
}
return 1;
}
int groupmgr_add(var mgr, xce_group_t* g)
{
int rc = 0;
lock(mgr, RTTI_LOCK_WRITE);
rc = key_add(mgr, (void*)g->name, (void*)g);
unlock(mgr);
return rc;
}
/* bug-1784: registers a cluster config's NAMED group list (e.g.
* xce_conf4sched_t.ugroups/hgroups, sourced from users.yaml's top-level
* "usergroups:"/hosts.yaml's "hostgroups:") into mgr by name, via the
* existing makeGroup(). Without this, a group config's own name is never
* added to mgr's kvtab under that name, so any OTHER group config that
* references it as a member (e.g. a limit's per-limit consumer group,
* plugins/limit.c's build_consumer()) can never resolve it: make_g_member()
* falls back to treating the name as a literal, ungrouped member that will
* never match a real username/hostname. Callers (e.g. scheduler.c's
* sched_init_config()) must call this BEFORE building anything that may
* reference these groups by name. */
int groupmgr_load_configs(var mgr, int count, xce_group_config_t** configs)
{
int i;
if (mgr == NULL) {
return -EINVAL;
}
for (i = 0; i < count; i++) {
xce_group_config_t* config = configs[i];
xce_group_t* g;
if ((config == NULL) || (config->name == NULL)
|| (strlen(config->name) == 0)) {
continue;
}
g = makeGroup(mgr, config, config->name);
if (g == NULL) {
return -errno;
}
}
return 0;
}
xce_group_t* groupmgr_lock(var mgr, char* name, int op)
{
int rc;
xce_group_t* g = NULL;
lock(mgr, RTTI_LOCK_READ);
rc = key_find(mgr, (void*)name, (void**)&g);
unlock(mgr);
if (rc <= 0) {
return NULL;
}
return g;
}
void groupmgr_del(var mgr, xce_group_t* g)
{
lock(mgr, RTTI_LOCK_WRITE);
key_del(mgr, (void*)g->name);
unlock(mgr);
return;
}
xce_group_t* get_group(var mgr, char* name)
{
xce_group_t* g = NULL;
int rc = 0;
if ((mgr == NULL) || (name == NULL)) {
return NULL;
}
lock(mgr, RTTI_LOCK_READ);
rc = key_find(mgr, (void*)name, (void**)&g);
unlock(mgr);
if (rc <= 0) {
return NULL;
}
return g;
}
int fmt_group(char* buf, int len, xce_group_t* g)
{
int i, pos;
if (strcmp(g->name, "all") == 0) {
return snprintf(buf, len, "all");
}
if (strcmp(g->name, "empty") == 0) {
return snprintf(buf, len, "[]");
}
snprintf(buf, len, "[");
pos = strlen(buf);
for (i = 0; i < g->members.count; i++) {
xce_g_member_t* m = (xce_g_member_t*)xlist_get(&(g->members), i);
if (m == NULL) {
continue;
}
snprintf(buf + pos, len - pos, "%s ",
(m->is_group) ? m->u.g->name : m->u.name);
pos = strlen(buf);
}
for (i = 0; i < g->ex_members.count; i++) {
xce_g_member_t* m = (xce_g_member_t*)xlist_get(&(g->ex_members), i);
if (m == NULL) {
continue;
}
snprintf(buf + pos, len - pos, "~%s ",
(m->is_group) ? m->u.g->name : m->u.name);
pos = strlen(buf);
}
while ((pos > 0) && (buf[pos - 1] == ' ')) {
pos--;
}
snprintf(buf + pos, len - pos, "]");
return strlen(buf);
}
static int check_in_member(xce_g_member_t* m, va_list ap)
{
char* name = va_arg(ap, char*);
if ((m == NULL) || (name == NULL)) {
return 0;
}
if (m->is_group) {
if (is_in_group(m->u.g, name)) {
return 1;
}
return 0;
}
if (strcmp(m->u.name, name) == 0) {
return 1;
}
return 0;
}
int is_in_group(xce_group_t* g, char* name)
{
if ((g == NULL) || (name == NULL)) {
return -EINVAL;
}
/* everything should be in all group */
if (strcmp(g->name, "all") == 0) {
return 1;
}
/* nothing is in empty group */
if (strcmp(g->name, "empty") == 0) {
return 0;
}
if (strcmp(g->name, name) == 0) {
return 1;
}
if (xlist_find(&(g->ex_members), (xlist_find_fn_t)check_in_member, name)
!= NULL) {
return 0;
}
if (xlist_find(&(g->members), (xlist_find_fn_t)check_in_member, name)
!= NULL) {
return 1;
}
return 0;
}
int is_group_all(xce_group_t* g)
{
if (g == NULL) {
return 0;
}
if (g->name == NULL) {
return 0;
}
return (strcmp(g->name, "all") == 0);
}