Skip to content

Commit

Permalink
fix(run): --storage and --tenant had no effect
Browse files Browse the repository at this point in the history
  • Loading branch information
Jorres committed Apr 15, 2024
1 parent 382d9b1 commit d781c31
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 26 deletions.
2 changes: 1 addition & 1 deletion pkg/options/restart.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func (o *RestartOptions) DefineFlags(fs *pflag.FlagSet) {
fs.BoolVar(&o.Tenant, "tenant", false, `Only include tenant nodes. Otherwise, include all nodes by default`)

fs.StringSliceVar(&o.TenantList, "tenant-list", []string{}, `Comma-delimited list of tenant names to restart.
E.g.:'--tenant-list name1,name2,name3'`)
E.g.:'--tenant-list=name1,name2,name3'`)

fs.StringVar(&o.CustomSystemdUnitName, "systemd-unit", "", "Specify custom systemd unit name to restart")

Expand Down
31 changes: 30 additions & 1 deletion pkg/rolling/restarters/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (

"github.com/ydb-platform/ydb-go-genproto/draft/protos/Ydb_Maintenance"
"go.uber.org/zap"

"github.com/ydb-platform/ydbops/pkg/options"
)

const (
Expand Down Expand Up @@ -47,8 +49,35 @@ func NewRunRestarter(logger *zap.SugaredLogger) *RunRestarter {
}
}

func determineRunScope(spec FilterNodeParams, cluster ClusterNodesInfo) []*Ydb_Maintenance.Node {
// Abstraction leaks here:
restartOpts := options.RestartOptionsInstance

if !restartOpts.Tenant && !restartOpts.Storage {
return cluster.AllNodes
}

result := []*Ydb_Maintenance.Node{}
if restartOpts.Storage {
storageNodes := FilterStorageNodes(cluster.AllNodes)
result = append(result, storageNodes...)
}

if restartOpts.Tenant {
tenantNodes := FilterTenantNodes(cluster.AllNodes)

selectedByTenantName := PopulateByTenantNames(tenantNodes, spec.SelectedTenants, cluster.TenantToNodeIds)

result = append(result, selectedByTenantName...)
}

return result
}

func (r RunRestarter) Filter(spec FilterNodeParams, cluster ClusterNodesInfo) []*Ydb_Maintenance.Node {
preSelectedNodes := PopulateByCommonFields(cluster.AllNodes, spec)
runScopeNodes := determineRunScope(spec, cluster)

preSelectedNodes := PopulateByCommonFields(runScopeNodes, spec)

filteredNodes := ExcludeByCommonFields(preSelectedNodes, spec)

Expand Down
41 changes: 22 additions & 19 deletions pkg/rolling/rolling.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (

"github.com/google/uuid"
"github.com/ydb-platform/ydb-go-genproto/draft/protos/Ydb_Maintenance"
"github.com/ydb-platform/ydb-go-genproto/protos/Ydb_Discovery"
"go.uber.org/zap"

"github.com/ydb-platform/ydbops/internal/collections"
Expand Down Expand Up @@ -314,31 +313,35 @@ func (r *Rolling) atomicRememberComplete(m *sync.Mutex, actionUID *Ydb_Maintenan
r.completedActions = append(r.completedActions, actionUID)
}

func (r *Rolling) populateTenantToNodesMapping(nodes []*Ydb_Maintenance.Node) map[string][]uint32 {
tenantNameToNodeIds := make(map[string][]uint32)
for _, node := range nodes {
dynamicNode := node.GetDynamic()
if dynamicNode != nil {
tenantNameToNodeIds[dynamicNode.GetTenant()] = append(
tenantNameToNodeIds[dynamicNode.GetTenant()],
node.NodeId,
)
}
}

return tenantNameToNodeIds
}

func (r *Rolling) prepareState() (*state, error) {
nodes, err := r.cms.Nodes()
if err != nil {
return nil, fmt.Errorf("failed to list available nodes: %w", err)
}

tenants, err := r.cms.Tenants()
if err != nil {
return nil, fmt.Errorf("failed to list available tenants: %w", err)
}

tenantNameToNodeIds := make(map[string][]uint32)

for _, tenant := range r.opts.TenantList {
if collections.Contains(r.opts.TenantList, tenant) {
if !collections.Contains(tenants, tenant) {
return nil, fmt.Errorf("tenant %s is not found in tenant list of this cluster", tenant)
}

endpoints, err := r.discovery.ListEndpoints(tenant)
if err != nil {
return nil, fmt.Errorf("failed to get the list of endpoints for the tenant %s", tenant)
}
tenantNameToNodeIds[tenant] = collections.Convert(endpoints, func(endpoint *Ydb_Discovery.EndpointInfo) uint32 {
return endpoint.NodeId
})
}

nodes, err := r.cms.Nodes()
if err != nil {
return nil, fmt.Errorf("failed to list available nodes: %w", err)
}

userSID, err := r.discovery.WhoAmI()
Expand All @@ -347,7 +350,7 @@ func (r *Rolling) prepareState() (*state, error) {
}

return &state{
tenantNameToNodeIds: tenantNameToNodeIds,
tenantNameToNodeIds: r.populateTenantToNodesMapping(nodes),
tenants: tenants,
userSID: userSID,
nodes: collections.ToMap(nodes, func(n *Ydb_Maintenance.Node) uint32 { return n.NodeId }),
Expand Down
12 changes: 11 additions & 1 deletion tests/mock/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/anypb"
"google.golang.org/protobuf/types/known/timestamppb"

"github.com/ydb-platform/ydbops/internal/collections"
)

var (
Expand Down Expand Up @@ -205,8 +207,16 @@ func (s *YdbMock) Login(ctx context.Context, req *Ydb_Auth.LoginRequest) (*Ydb_A

func (s *YdbMock) ListDatabases(ctx context.Context, req *Ydb_Cms.ListDatabasesRequest) (*Ydb_Cms.ListDatabasesResponse, error) {
s.RequestLog = append(s.RequestLog, req)

paths := make(map[string]bool)
for _, node := range s.nodes {
if node.GetDynamic() != nil {
paths[node.GetDynamic().GetTenant()] = true
}
}

result := &Ydb_Cms.ListDatabasesResult{
Paths: []string{},
Paths: collections.Keys(paths),
}

return &Ydb_Cms.ListDatabasesResponse{Operation: wrapIntoOperation(result)}, nil
Expand Down
82 changes: 78 additions & 4 deletions tests/rolling_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,8 @@ var _ = Describe("Test Rolling", func() {
User: mock.TestUser,
Password: mock.TestPassword,
},
&Ydb_Cms.ListDatabasesRequest{},
&Ydb_Maintenance.ListClusterNodesRequest{},
&Ydb_Cms.ListDatabasesRequest{},
&Ydb_Discovery.WhoAmIRequest{
IncludeGroups: false,
},
Expand Down Expand Up @@ -213,8 +213,8 @@ var _ = Describe("Test Rolling", func() {
User: mock.TestUser,
Password: mock.TestPassword,
},
&Ydb_Cms.ListDatabasesRequest{},
&Ydb_Maintenance.ListClusterNodesRequest{},
&Ydb_Cms.ListDatabasesRequest{},
&Ydb_Discovery.WhoAmIRequest{},
&Ydb_Maintenance.ListMaintenanceTasksRequest{
User: &mock.TestUser,
Expand Down Expand Up @@ -283,8 +283,8 @@ var _ = Describe("Test Rolling", func() {
User: mock.TestUser,
Password: mock.TestPassword,
},
&Ydb_Cms.ListDatabasesRequest{},
&Ydb_Maintenance.ListClusterNodesRequest{},
&Ydb_Cms.ListDatabasesRequest{},
&Ydb_Discovery.WhoAmIRequest{},
&Ydb_Maintenance.ListMaintenanceTasksRequest{
User: &mock.TestUser,
Expand Down Expand Up @@ -364,8 +364,8 @@ var _ = Describe("Test Rolling", func() {
User: mock.TestUser,
Password: mock.TestPassword,
},
&Ydb_Cms.ListDatabasesRequest{},
&Ydb_Maintenance.ListClusterNodesRequest{},
&Ydb_Cms.ListDatabasesRequest{},
&Ydb_Discovery.WhoAmIRequest{},
&Ydb_Maintenance.ListMaintenanceTasksRequest{
User: &mock.TestUser,
Expand Down Expand Up @@ -402,5 +402,79 @@ var _ = Describe("Test Rolling", func() {
},
},
),
Entry("happy path, restart dynnodes by tenant name", testCase{
nodeConfiguration: [][]uint32{
{1, 2, 3, 4, 5, 6, 7, 8},
{9, 10, 11},
},
nodeInfoMap: map[uint32]mock.TestNodeInfo{
9: {
IsDynnode: true,
TenantName: "fakeTenant1",
},
10: {
IsDynnode: true,
TenantName: "fakeTenant2",
},
11: {
IsDynnode: true,
TenantName: "fakeTenant2",
},
},
ydbopsInvocation: []string{
"--endpoint", "grpcs://localhost:2135",
"--verbose",
"--availability-mode", "strong",
"--user", mock.TestUser,
"--cms-query-interval", "1",
"--tenant",
"--tenant-list=fakeTenant2",
"run",
"--payload", filepath.Join(".", "mock", "noop-payload.sh"),
"--ca-file", filepath.Join(".", "test-data", "ssl-data", "ca.crt"),
},
expectedRequests: []proto.Message{
&Ydb_Auth.LoginRequest{
User: mock.TestUser,
Password: mock.TestPassword,
},
&Ydb_Maintenance.ListClusterNodesRequest{},
&Ydb_Cms.ListDatabasesRequest{},
&Ydb_Discovery.WhoAmIRequest{},
&Ydb_Maintenance.ListMaintenanceTasksRequest{
User: &mock.TestUser,
},
&Ydb_Maintenance.CreateMaintenanceTaskRequest{
TaskOptions: &Ydb_Maintenance.MaintenanceTaskOptions{
TaskUid: "task-UUID-1",
Description: "Rolling restart maintenance task",
AvailabilityMode: Ydb_Maintenance.AvailabilityMode_AVAILABILITY_MODE_STRONG,
},
ActionGroups: mock.MakeActionGroups(10, 11),
},
&Ydb_Maintenance.CompleteActionRequest{
ActionUids: []*Ydb_Maintenance.ActionUid{
{
TaskUid: "task-UUID-1",
GroupId: "group-UUID-1",
ActionId: "action-UUID-1",
},
},
},
&Ydb_Maintenance.RefreshMaintenanceTaskRequest{
TaskUid: "task-UUID-1",
},
&Ydb_Maintenance.CompleteActionRequest{
ActionUids: []*Ydb_Maintenance.ActionUid{
{
TaskUid: "task-UUID-1",
GroupId: "group-UUID-2",
ActionId: "action-UUID-2",
},
},
},
},
},
),
)
})

0 comments on commit d781c31

Please sign in to comment.