Skip to content

Commit 0d1c64b

Browse files
Create SLURM associations only when the cluster account is ready (#544)
* Reconcile Slurm associations instead of polling for provisioning Fixes #538 * Write only the Slurm associations that differ from the cluster * Split gres/gpu into type and name when writing Slurm limits
1 parent e1c69ae commit 0d1c64b

22 files changed

Lines changed: 954 additions & 169 deletions

config/custos.yaml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,15 @@ connectors:
3636
version: "${SLURM_API_VERSION}"
3737
username: "${SLURM_API_USERNAME}"
3838
token: "${SLURM_TOKEN}"
39+
# How often the sweep re-declares associations for provisioned members.
40+
# This is what guarantees an association exists, so it also decides how
41+
# long a member waits when an event is lost or the account provisions late.
42+
association_reconcile_interval: "5m"
43+
# How long to hold a just-provisioned account back before writing its
44+
# association. Covers the directory export and the cluster's name cache,
45+
# which Custos cannot observe. Raise it if users hit "Invalid account"
46+
# right after being added.
47+
association_provision_grace: "30s"
3948

4049
slurm-usage-monitor:
4150
type: "slurm-usage-monitor"

connectors/COmanage/Identity-Provisioner/internal/operations/ensure_posix_account.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,9 @@ func (o *Orchestrator) ensurePOSIXAccountImpl(ctx context.Context, cu *models.Co
132132
}
133133
log.Info("comanage: UnixClusterAccount attached", "username", cu.LocalUsername, "uid", uidInt, "co_group_id", coGroupID)
134134
o.audit(ctx, cu, "ComanageClusterAccountAttached", fmt.Sprintf("comanage_id=%s username=%s uid=%d", personID, cu.LocalUsername, uidInt))
135+
if err := o.core.MarkComputeClusterUserProvisioned(ctx, cu.ID); err != nil {
136+
log.Warn("comanage: failed to mark cluster user provisioned", "err", err)
137+
}
135138
return nil
136139
}
137140

connectors/COmanage/Identity-Provisioner/internal/operations/orchestrator.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ type CoreService interface {
3838
ListUserIdentitiesForUser(ctx context.Context, userID string) ([]models.UserIdentity, error)
3939
CreateUserIdentity(ctx context.Context, ui *models.UserIdentity) (*models.UserIdentity, error)
4040
CreateAuditEvent(ctx context.Context, e *models.AuditEvent) (*models.AuditEvent, error)
41+
MarkComputeClusterUserProvisioned(ctx context.Context, id string) error
4142
}
4243

4344
type Orchestrator struct {

connectors/COmanage/Identity-Provisioner/internal/operations/orchestrator_test.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,11 @@ func installRecorder(t *testing.T) *recordingProcessor {
7777
// fakeCore is an in-memory CoreService stub that records audit-event writes
7878
// and returns canned user/identity data.
7979
type fakeCore struct {
80-
user *models.User
81-
identities []models.UserIdentity
82-
auditEvents []models.AuditEvent
83-
createdIdentity *models.UserIdentity
80+
user *models.User
81+
identities []models.UserIdentity
82+
auditEvents []models.AuditEvent
83+
createdIdentity *models.UserIdentity
84+
markedProvisioned []string
8485
}
8586

8687
func (f *fakeCore) GetUser(_ context.Context, _ string) (*models.User, error) {
@@ -98,6 +99,11 @@ func (f *fakeCore) CreateUserIdentity(_ context.Context, ui *models.UserIdentity
9899
return ui, nil
99100
}
100101

102+
func (f *fakeCore) MarkComputeClusterUserProvisioned(_ context.Context, id string) error {
103+
f.markedProvisioned = append(f.markedProvisioned, id)
104+
return nil
105+
}
106+
101107
func (f *fakeCore) CreateAuditEvent(ctx context.Context, e *models.AuditEvent) (*models.AuditEvent, error) {
102108
tracing.PopulateAuditIDs(ctx, &e.TraceID, &e.SpanID, &e.ParentSpanID)
103109
if e.ID == "" {
@@ -211,6 +217,10 @@ func TestEnsurePOSIXAccount_EmitsComanageSpanTree(t *testing.T) {
211217
t.Errorf("missing span %q. got=%v", want, got)
212218
}
213219
}
220+
221+
if len(core.markedProvisioned) != 1 || core.markedProvisioned[0] != "ccu-1" {
222+
t.Errorf("expected cluster user ccu-1 marked provisioned, got %v", core.markedProvisioned)
223+
}
214224
}
215225

216226
func TestEnsurePOSIXAccount_DlqAuditCarriesTraceID(t *testing.T) {

connectors/SLURM/Association-Mapper/internal/subscribers/account.go

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -157,10 +157,7 @@ func (a *AssociationSubscriber) SubscribeToComputeAllocationResourceMappingCreat
157157
return
158158
}
159159

160-
grpTres = append(grpTres, client.TRES{
161-
Type: resource.ResourceType,
162-
Count: mapping.ResourceAmount,
163-
})
160+
grpTres = append(grpTres, tresFor(resource.ResourceType, mapping.ResourceAmount))
164161
}
165162

166163
grpTresMins := []client.TRES{}
@@ -173,10 +170,7 @@ func (a *AssociationSubscriber) SubscribeToComputeAllocationResourceMappingCreat
173170
return
174171
}
175172

176-
grpTresMins = append(grpTresMins, client.TRES{
177-
Type: resource.ResourceType,
178-
Count: mapping.ResourceTime,
179-
})
173+
grpTresMins = append(grpTresMins, tresFor(resource.ResourceType, mapping.ResourceTime))
180174
}
181175

182176
limits := client.AssocLimits{

connectors/SLURM/Association-Mapper/internal/subscribers/accountsub_integration_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func TestSubscribeToComputeAllocationCreation(t *testing.T) {
6161
},
6262
}
6363

64-
associationSubscriber := NewAssociationSubscriber(client, nil, mockCoreService)
64+
associationSubscriber := NewAssociationSubscriber(client, nil, mockCoreService, 0, 0)
6565

6666
computeAccount := models.ComputeAllocation{
6767
ID: "test_compute_account_id",
@@ -124,7 +124,7 @@ func TestSubscribeToComputeAllocationCreationWrongCluster(t *testing.T) {
124124
},
125125
}
126126

127-
associationSubscriber := NewAssociationSubscriber(client, nil, mockCoreService)
127+
associationSubscriber := NewAssociationSubscriber(client, nil, mockCoreService, 0, 0)
128128

129129
computeAccount := models.ComputeAllocation{
130130
ID: "test_compute_account_id",
@@ -157,7 +157,7 @@ func createAllocationMapping(client *operations.Client, mockCoreService *service
157157
projectID string, allocationMappingId string,
158158
allocationResourceId string, partitionName string) error {
159159

160-
associationSubscriber := NewAssociationSubscriber(client, nil, mockCoreService)
160+
associationSubscriber := NewAssociationSubscriber(client, nil, mockCoreService, 0, 0)
161161

162162
computeAllocation := models.ComputeAllocation{
163163
ID: computeAllocationID,
Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package subscribers
19+
20+
import (
21+
"context"
22+
"errors"
23+
"fmt"
24+
"log/slog"
25+
"strings"
26+
27+
"github.com/apache/airavata-custos/connectors/SLURM/Rest-Client/pkg/client"
28+
"github.com/apache/airavata-custos/pkg/models"
29+
)
30+
31+
// tresFor builds a SLURM TRES from a stored resource type and count. A GRES
32+
// resource is stored joined as "gres/gpu" but SLURM wants it split into type
33+
// and name; a plain resource like "cpu" has no name.
34+
func tresFor(resourceType string, count int64) client.TRES {
35+
if typ, name, ok := strings.Cut(resourceType, "/"); ok {
36+
return client.TRES{Type: typ, Name: name, Count: count}
37+
}
38+
return client.TRES{Type: resourceType, Count: count}
39+
}
40+
41+
// errNotProvisioned means the account does not exist on the cluster yet, so
42+
// writing the association now would make slurmctld cache a failed uid lookup
43+
// and reject the user's jobs. The reconciler retries these.
44+
var errNotProvisioned = errors.New("cluster account not provisioned yet")
45+
46+
// assocKey identifies an association the way Slurm does. Cluster is not part
47+
// of it because callers scope their lookups to one cluster already.
48+
type assocKey struct {
49+
account string
50+
user string
51+
partition string
52+
}
53+
54+
func keyOf(a client.Association) assocKey {
55+
return assocKey{account: a.Account, user: a.User, partition: a.Partition}
56+
}
57+
58+
// sameLimits reports whether two associations already carry the same limits.
59+
// Only the limits are compared: the key fields match by construction.
60+
func sameLimits(want, got client.Association) bool {
61+
return sameTRES(want.Limits.GrpTRES, got.Limits.GrpTRES) &&
62+
sameTRES(want.Limits.GrpTRESMins, got.Limits.GrpTRESMins)
63+
}
64+
65+
func sameTRES(want, got []client.TRES) bool {
66+
if len(want) != len(got) {
67+
return false
68+
}
69+
counts := make(map[string]int64, len(want))
70+
for _, t := range want {
71+
counts[t.Type] = t.Count
72+
}
73+
for _, t := range got {
74+
if c, ok := counts[t.Type]; !ok || c != t.Count {
75+
return false
76+
}
77+
}
78+
return true
79+
}
80+
81+
// upsertAssociationsForMembership writes the association for every resource on
82+
// the membership's allocation, without checking what the cluster already has.
83+
// The event paths use this: an event fires because something changed.
84+
func (a *AssociationSubscriber) upsertAssociationsForMembership(ctx context.Context, membership models.ComputeAllocationMembership) error {
85+
return a.syncAssociationsForMembership(ctx, membership, nil)
86+
}
87+
88+
// syncAssociationsForMembership builds the association for each of the
89+
// allocation's resources and writes the ones that differ from existing, the
90+
// state already on the cluster. A nil existing writes all of them.
91+
//
92+
// Every caller goes through here on purpose. Slurm keys an association by
93+
// (cluster, account, user, partition) and its upsert is last-write-wins, so a
94+
// caller that built the record without limits would wipe limits another caller
95+
// had set. Folding the per-member overrides in here keeps all writers
96+
// producing the same record for the same key.
97+
func (a *AssociationSubscriber) syncAssociationsForMembership(ctx context.Context, membership models.ComputeAllocationMembership, existing map[assocKey]client.Association) error {
98+
allocation, err := a.coreService.GetComputeAllocation(ctx, membership.ComputeAllocationID)
99+
if err != nil {
100+
return fmt.Errorf("get compute allocation: %w", err)
101+
}
102+
cluster, err := a.coreService.GetComputeCluster(ctx, allocation.ComputeClusterID)
103+
if err != nil {
104+
return fmt.Errorf("get compute cluster: %w", err)
105+
}
106+
csu, err := a.coreService.GetComputeClusterUserByPair(ctx, cluster.ID, membership.UserID)
107+
if err != nil {
108+
return fmt.Errorf("get compute cluster user: %w", err)
109+
}
110+
if csu.ProvisionedAt == nil {
111+
return errNotProvisioned
112+
}
113+
114+
resources, err := a.coreService.ListResourcesForAllocation(ctx, allocation.ID)
115+
if err != nil {
116+
return fmt.Errorf("list resources for allocation: %w", err)
117+
}
118+
if len(resources) == 0 {
119+
// Nothing to map onto a partition. Skipping beats guessing a
120+
// partition name the cluster may not have.
121+
slog.Warn("Allocation has no resources, no association written",
122+
"allocation_id", allocation.ID, "user_id", membership.UserID)
123+
return nil
124+
}
125+
126+
overrides, err := a.coreService.ListOverridesForMembership(ctx, membership.ID)
127+
if err != nil {
128+
return fmt.Errorf("list overrides for membership: %w", err)
129+
}
130+
overrideByResource := make(map[string]models.ComputeAllocationMembershipResourceOverride, len(overrides))
131+
for _, o := range overrides {
132+
overrideByResource[o.ComputeAllocationResourceID] = o
133+
}
134+
135+
for _, resource := range resources {
136+
association := client.Association{
137+
Account: allocation.Name,
138+
Cluster: cluster.Name,
139+
User: csu.LocalUsername,
140+
Partition: resource.Name,
141+
QoS: []string{"normal"},
142+
Limits: limitsFor(resource, overrideByResource[resource.ID]),
143+
}
144+
if got, ok := existing[keyOf(association)]; ok && sameLimits(association, got) {
145+
continue
146+
}
147+
if err := a.slurmClient.UpsertAssociation(association); err != nil {
148+
return fmt.Errorf("upsert association for partition %s: %w", resource.Name, err)
149+
}
150+
slog.Info("Upserted association", "association", association)
151+
}
152+
return nil
153+
}
154+
155+
// limitsFor turns a per-member override into association limits. A zero
156+
// override means the member draws on the allocation-wide limits, so the
157+
// association carries none of its own.
158+
func limitsFor(resource models.ComputeAllocationResource, override models.ComputeAllocationMembershipResourceOverride) client.AssocLimits {
159+
var limits client.AssocLimits
160+
if override.OverrideResourceAmount > 0 {
161+
limits.GrpTRES = []client.TRES{tresFor(resource.ResourceType, override.OverrideResourceAmount)}
162+
}
163+
if override.OverrideResourceTime > 0 {
164+
limits.GrpTRESMins = []client.TRES{tresFor(resource.ResourceType, override.OverrideResourceTime)}
165+
}
166+
return limits
167+
}

0 commit comments

Comments
 (0)