You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airavata.apache.org by ya...@apache.org on 2026/10/03 21:00:40 UTC
(airavata-custos) branch slurm-default-account created (now dbbce78ee)
This is an automated email from the ASF dual-hosted git repository.
yasithdev pushed a change to branch slurm-default-account
in repository https://gitbox.apache.org/repos/asf/airavata-custos.git
at dbbce78ee delete a slurm user whose last associations are revoked
This branch includes the following new commits:
new dbbce78ee delete a slurm user whose last associations are revoked
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(airavata-custos) 01/01: delete a slurm user whose last associations are revoked
Posted by ya...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
yasithdev pushed a commit to branch slurm-default-account
in repository https://gitbox.apache.org/repos/asf/airavata-custos.git
commit dbbce78ee36650f7af675452ec34944222bd3abf
Author: Yasith Jayawardana <ya...@icloud.com>
AuthorDate: Sat Oct 3 16:43:15 2026 -0400
delete a slurm user whose last associations are revoked
---
.../internal/subscribers/deprovision.go | 37 ++++++++++++++++++--
.../internal/subscribers/deprovision_test.go | 26 ++++++++++++++-
.../internal/subscribers/members_test.go | 20 +++++++++--
.../internal/subscribers/reconciler.go | 39 +++++++++++++---------
.../internal/subscribers/subscriber.go | 1 +
.../SLURM/Rest-Client/pkg/client/associations.go | 31 +++++++++++++++++
.../Rest-Client/pkg/client/associations_test.go | 14 ++++++++
7 files changed, 148 insertions(+), 20 deletions(-)
diff --git a/connectors/SLURM/Association-Mapper/internal/subscribers/deprovision.go b/connectors/SLURM/Association-Mapper/internal/subscribers/deprovision.go
index 23ddaf124..c69e3c050 100644
--- a/connectors/SLURM/Association-Mapper/internal/subscribers/deprovision.go
+++ b/connectors/SLURM/Association-Mapper/internal/subscribers/deprovision.go
@@ -54,7 +54,7 @@ func (a *AssociationSubscriber) removeAssociationsForMembership(ctx context.Cont
Account: allocation.Name,
User: csu.LocalUsername,
}
- if err := a.slurmClient.DeleteAssociation(filter); err != nil {
+ if err := a.revoke(filter); err != nil {
return fmt.Errorf("delete associations for %s on %s: %w", csu.LocalUsername, allocation.Name, err)
}
slog.Info("Removed associations for membership",
@@ -73,7 +73,7 @@ func (a *AssociationSubscriber) removeAssociationsForAllocation(ctx context.Cont
Cluster: cluster.Name,
Account: allocation.Name,
}
- if err := a.slurmClient.DeleteAssociation(filter); err != nil {
+ if err := a.revoke(filter); err != nil {
return fmt.Errorf("delete associations for allocation %s: %w", allocation.Name, err)
}
slog.Info("Removed associations for allocation",
@@ -81,6 +81,39 @@ func (a *AssociationSubscriber) removeAssociationsForAllocation(ctx context.Cont
return nil
}
+// revoke removes the associations f matches. Slurm refuses to remove the last
+// associations of a user's default account, so a user left with none is deleted.
+func (a *AssociationSubscriber) revoke(f client.AssocFilter) error {
+ matched, err := a.slurmClient.ListAssociations(f)
+ if err != nil {
+ return err
+ }
+ perUser := map[string]int{}
+ for _, m := range matched {
+ if m.User != "" {
+ perUser[m.User]++
+ }
+ }
+ deleted := 0
+ for user, n := range perUser {
+ all, err := a.slurmClient.ListAssociations(client.AssocFilter{User: user})
+ if err != nil {
+ return err
+ }
+ if len(all) != n {
+ continue
+ }
+ if err := a.slurmClient.DeleteUser(user); err != nil {
+ return fmt.Errorf("delete user %s: %w", user, err)
+ }
+ deleted += n
+ }
+ if len(matched) > 0 && deleted == len(matched) {
+ return nil
+ }
+ return a.slurmClient.DeleteAssociation(f)
+}
+
// activeAllocation reports whether an allocation should still grant access.
func activeAllocation(allocation models.ComputeAllocation) bool {
return allocation.Status == models.ACTIVE
diff --git a/connectors/SLURM/Association-Mapper/internal/subscribers/deprovision_test.go b/connectors/SLURM/Association-Mapper/internal/subscribers/deprovision_test.go
index 6c6a2c54f..e74cafd5b 100644
--- a/connectors/SLURM/Association-Mapper/internal/subscribers/deprovision_test.go
+++ b/connectors/SLURM/Association-Mapper/internal/subscribers/deprovision_test.go
@@ -167,6 +167,7 @@ func TestReconcilerRemovesStaleAssociation(t *testing.T) {
{Account: "test-alloc", Cluster: "testcluster", User: "testuser", Partition: "compute"},
// left behind by a membership that is gone
{Account: "test-alloc", Cluster: "testcluster", User: "ghost", Partition: "compute"},
+ {Account: "other-account", Cluster: "testcluster", User: "ghost", Partition: "compute"},
}}
NewAssociationSubscriber(slurm, nil, core, 0, 0).reconcile(context.Background())
@@ -174,9 +175,32 @@ func TestReconcilerRemovesStaleAssociation(t *testing.T) {
if len(got) != 1 {
t.Fatalf("expected the stale association to be removed, got %d deletes", len(got))
}
- if got[0].User != "ghost" || got[0].Partition != "compute" {
+ if got[0].User != "ghost" || got[0].Account != "test-alloc" {
t.Errorf("removed the wrong association: %+v", got[0])
}
+ if len(slurm.deletedUsers) != 0 {
+ t.Errorf("a user with other access must be kept, deleted %v", slurm.deletedUsers)
+ }
+}
+
+func TestReconcilerDeletesUserWhoseOnlyAccessIsStale(t *testing.T) {
+ core := coreMock(mockOpts{
+ provisionedAt: ago(time.Minute),
+ memberships: []models.ComputeAllocationMembership{testMembership()},
+ })
+ slurm := &fakeSlurmClient{existing: []client.Association{
+ {Account: "test-alloc", Cluster: "testcluster", User: "testuser", Partition: "compute"},
+ {Account: "test-alloc", Cluster: "testcluster", User: "ghost", Partition: "compute"},
+ {Account: "test-alloc", Cluster: "testcluster", User: "ghost", Partition: "gpu"},
+ }}
+ NewAssociationSubscriber(slurm, nil, core, 0, 0).reconcile(context.Background())
+
+ if len(slurm.deletedUsers) != 1 || slurm.deletedUsers[0] != "ghost" {
+ t.Fatalf("expected ghost to be deleted, got %v", slurm.deletedUsers)
+ }
+ if got := slurm.allDeletes(); len(got) != 0 {
+ t.Errorf("no association delete should follow the user delete, got %+v", got)
+ }
}
// Guard: an empty desired set almost always means a failed lookup, so the
diff --git a/connectors/SLURM/Association-Mapper/internal/subscribers/members_test.go b/connectors/SLURM/Association-Mapper/internal/subscribers/members_test.go
index d3be7f56d..acf60c003 100644
--- a/connectors/SLURM/Association-Mapper/internal/subscribers/members_test.go
+++ b/connectors/SLURM/Association-Mapper/internal/subscribers/members_test.go
@@ -35,6 +35,15 @@ type fakeSlurmClient struct {
upserts []client.Association
deletes []client.AssocFilter
existing []client.Association
+
+ deletedUsers []string
+}
+
+func (f *fakeSlurmClient) DeleteUser(name string) error {
+ f.mu.Lock()
+ defer f.mu.Unlock()
+ f.deletedUsers = append(f.deletedUsers, name)
+ return nil
}
func (f *fakeSlurmClient) DeleteAssociation(filter client.AssocFilter) error {
@@ -50,8 +59,15 @@ func (f *fakeSlurmClient) allDeletes() []client.AssocFilter {
return append([]client.AssocFilter(nil), f.deletes...)
}
-func (f *fakeSlurmClient) ListAssociations(client.AssocFilter) ([]client.Association, error) {
- return f.existing, nil
+func (f *fakeSlurmClient) ListAssociations(q client.AssocFilter) ([]client.Association, error) {
+ var out []client.Association
+ for _, a := range f.existing {
+ if (q.Account == "" || q.Account == a.Account) && (q.User == "" || q.User == a.User) &&
+ (q.Cluster == "" || q.Cluster == a.Cluster) && (q.Partition == "" || q.Partition == a.Partition) {
+ out = append(out, a)
+ }
+ }
+ return out, nil
}
func (f *fakeSlurmClient) CreateAccount(client.Account, string) error { return nil }
diff --git a/connectors/SLURM/Association-Mapper/internal/subscribers/reconciler.go b/connectors/SLURM/Association-Mapper/internal/subscribers/reconciler.go
index c761a4bac..a817ac4f8 100644
--- a/connectors/SLURM/Association-Mapper/internal/subscribers/reconciler.go
+++ b/connectors/SLURM/Association-Mapper/internal/subscribers/reconciler.go
@@ -179,7 +179,11 @@ func (a *AssociationSubscriber) pruneStaleAssociations(ctx context.Context, clus
return 0
}
- removed := 0
+ // A member whose every partition on an account is stale is revoked in one call,
+ // since Slurm refuses partition-by-partition removal of a default account.
+ type member struct{ account, user string }
+ held := map[member]int{}
+ stale := map[member][]string{}
for key, assoc := range existing {
if assoc.User == "" {
continue
@@ -187,23 +191,28 @@ func (a *AssociationSubscriber) pruneStaleAssociations(ctx context.Context, clus
if _, ok := managed[assoc.Account]; !ok {
continue
}
- if _, ok := desired[key]; ok {
- continue
+ m := member{assoc.Account, assoc.User}
+ held[m]++
+ if _, ok := desired[key]; !ok {
+ stale[m] = append(stale[m], assoc.Partition)
}
- filter := client.AssocFilter{
- Cluster: cluster.Name,
- Account: assoc.Account,
- User: assoc.User,
- Partition: assoc.Partition,
+ }
+
+ removed := 0
+ for m, partitions := range stale {
+ if len(partitions) == held[m] {
+ partitions = []string{""}
}
- if err := a.slurmClient.DeleteAssociation(filter); err != nil {
- slog.Error("Association reconciler: failed to remove stale association",
- "account", assoc.Account, "user", assoc.User, "partition", assoc.Partition, "error", err)
- continue
+ for _, partition := range partitions {
+ filter := client.AssocFilter{Cluster: cluster.Name, Account: m.account, User: m.user, Partition: partition}
+ if err := a.revoke(filter); err != nil {
+ slog.Error("Association reconciler: failed to remove stale association",
+ "account", m.account, "user", m.user, "partition", partition, "error", err)
+ continue
+ }
+ slog.Info("Removed stale association", "account", m.account, "user", m.user, "partition", partition)
+ removed++
}
- slog.Info("Removed stale association",
- "account", assoc.Account, "user", assoc.User, "partition", assoc.Partition)
- removed++
}
return removed
}
diff --git a/connectors/SLURM/Association-Mapper/internal/subscribers/subscriber.go b/connectors/SLURM/Association-Mapper/internal/subscribers/subscriber.go
index 63af184f9..6f3870ef9 100644
--- a/connectors/SLURM/Association-Mapper/internal/subscribers/subscriber.go
+++ b/connectors/SLURM/Association-Mapper/internal/subscribers/subscriber.go
@@ -33,6 +33,7 @@ type SlurmClient interface {
UpsertAssociation(a client.Association) error
ListAssociations(f client.AssocFilter) ([]client.Association, error)
DeleteAssociation(f client.AssocFilter) error
+ DeleteUser(name string) error
}
type AssociationSubscriber struct {
diff --git a/connectors/SLURM/Rest-Client/pkg/client/associations.go b/connectors/SLURM/Rest-Client/pkg/client/associations.go
index b1eca11be..ee3fc1779 100644
--- a/connectors/SLURM/Rest-Client/pkg/client/associations.go
+++ b/connectors/SLURM/Rest-Client/pkg/client/associations.go
@@ -20,6 +20,7 @@ package client
import (
"errors"
+ "fmt"
"log"
"log/slog"
"net/url"
@@ -76,6 +77,11 @@ func (c *Client) UpsertAssociation(a Association) error {
log.Printf("Failed to upsert association: %v", err)
return err
}
+ if a.User != "" {
+ if err := c.ensureUser(a.User, a.Account); err != nil {
+ return fmt.Errorf("ensure user %s: %w", a.User, err)
+ }
+ }
filter := AssocFilter{
Account: a.Account,
@@ -137,6 +143,31 @@ func tresSliceEqualUnordered(a, b []TRES) bool {
return true
}
+// ensureUser creates a missing user record defaulting to account, which writing
+// an association does not, and without which Slurm cannot later remove it.
+func (c *Client) ensureUser(name, account string) error {
+ var out struct {
+ Users []struct {
+ Name string `json:"name"`
+ } `json:"users"`
+ }
+ if _, err := c.do("GET", "/slurmdb/v0.0."+c.apiVersion+"/user/"+url.PathEscape(name), nil, &out); err != nil {
+ return err
+ }
+ if len(out.Users) > 0 {
+ return nil
+ }
+ body := map[string]any{"users": []map[string]any{{"name": name, "default": map[string]any{"account": account}}}}
+ _, err := c.do("POST", "/slurmdb/v0.0."+c.apiVersion+"/users", body, nil)
+ return err
+}
+
+// DeleteUser removes a user and every association it holds.
+func (c *Client) DeleteUser(name string) error {
+ _, err := c.do("DELETE", "/slurmdb/v0.0."+c.apiVersion+"/user/"+url.PathEscape(name), nil, nil)
+ return err
+}
+
func (c *Client) DeleteAssociation(f AssocFilter) error {
path := "/slurmdb/v0.0." + c.apiVersion + "/association"
if q := f.query(); q != "" {
diff --git a/connectors/SLURM/Rest-Client/pkg/client/associations_test.go b/connectors/SLURM/Rest-Client/pkg/client/associations_test.go
index bd9bd3b49..9bc57302d 100644
--- a/connectors/SLURM/Rest-Client/pkg/client/associations_test.go
+++ b/connectors/SLURM/Rest-Client/pkg/client/associations_test.go
@@ -49,6 +49,7 @@ func TestListAssociationsByAccount(t *testing.T) {
}
func TestCreateAssociation(t *testing.T) {
+ var createdUser string
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Support to list associations so we can verify the association was created successfully
@@ -63,6 +64,16 @@ func TestCreateAssociation(t *testing.T) {
return
}
+ if r.Method == "GET" && r.URL.Path == "/slurmdb/v0.0.41/user/alice" {
+ _, _ = w.Write([]byte(`{"users":[]}`))
+ return
+ }
+ if r.Method == "POST" && r.URL.Path == "/slurmdb/v0.0.41/users" {
+ b, _ := io.ReadAll(r.Body)
+ createdUser = string(b)
+ _, _ = w.Write([]byte(`{}`))
+ return
+ }
if r.Method != "POST" || r.URL.Path != "/slurmdb/v0.0.41/associations" {
t.Fatalf("unexpected %s %s", r.Method, r.URL.Path)
}
@@ -83,6 +94,9 @@ func TestCreateAssociation(t *testing.T) {
if err != nil {
t.Fatal(err)
}
+ if createdUser != `{"users":[{"default":{"account":"eng"},"name":"alice"}]}` {
+ t.Errorf("a user with no record must be created defaulting to the account, got %s", createdUser)
+ }
}
func TestDeleteAssociation(t *testing.T) {