Skip to content

Commit

Permalink
[commands] finish refactoring
Browse files Browse the repository at this point in the history
  • Loading branch information
shmel1k committed Jul 4, 2024
1 parent 4bf4835 commit c0debac
Show file tree
Hide file tree
Showing 14 changed files with 70 additions and 94 deletions.
2 changes: 2 additions & 0 deletions cmd/maintenance/complete/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ type Options struct {
func (o *Options) DefineFlags(fs *pflag.FlagSet) {
fs.StringSliceVar(&o.HostFQDNs, "hosts", []string{},
"FQDNs of hosts with completed maintenance")
fs.StringVar(&o.TaskID, "task-id", "",
"ID of your maintenance task (result of `ydbops maintenance host`)")
}

func (o *Options) Validate() error {
Expand Down
14 changes: 10 additions & 4 deletions cmd/maintenance/create/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,22 @@ package create

import (
"fmt"
"time"

"github.com/google/uuid"
"github.com/spf13/cobra"
"google.golang.org/protobuf/types/known/durationpb"

"github.com/ydb-platform/ydbops/pkg/cli"
"github.com/ydb-platform/ydbops/pkg/client/cms"
"github.com/ydb-platform/ydbops/pkg/cmdutil"
"github.com/ydb-platform/ydbops/pkg/rolling"
)

func New(f cmdutil.Factory) *cobra.Command {
opts := &Options{}
opts := &Options{
RestartOptions: &rolling.RestartOptions{},
}

cmd := cli.SetDefaultsOn(&cobra.Command{
Use: "create",
Expand All @@ -24,10 +29,11 @@ func New(f cmdutil.Factory) *cobra.Command {
),
RunE: func(cmd *cobra.Command, args []string) error {
taskUID := cms.TaskUuidPrefix + uuid.New().String()
duration := time.Duration(opts.RestartOptions.RestartDuration) * time.Minute
taskId, err := f.GetCMSClient().CreateMaintenanceTask(cms.MaintenanceTaskParams{
Hosts: opts.HostFQDNs,
Duration: opts.GetMaintenanceDuration(),
AvailabilityMode: opts.GetAvailabilityMode(),
Hosts: opts.RestartOptions.Hosts,
Duration: durationpb.New(duration),
AvailabilityMode: opts.RestartOptions.GetAvailabilityMode(),
ScopeType: cms.HostScope,
TaskUID: taskUID,
})
Expand Down
52 changes: 4 additions & 48 deletions cmd/maintenance/create/options.go
Original file line number Diff line number Diff line change
@@ -1,62 +1,18 @@
package create

import (
"fmt"
"strings"
"time"

"github.com/spf13/pflag"
"github.com/ydb-platform/ydb-go-genproto/draft/protos/Ydb_Maintenance"
"github.com/ydb-platform/ydbops/internal/collections"
"github.com/ydb-platform/ydbops/pkg/options"
"google.golang.org/protobuf/types/known/durationpb"
"github.com/ydb-platform/ydbops/pkg/rolling"
)

type Options struct {
HostFQDNs []string
MaintenanceDurationSeconds int
AvailabilityMode string
}

func (o *Options) GetAvailabilityMode() Ydb_Maintenance.AvailabilityMode {
title := strings.ToUpper(fmt.Sprintf("availability_mode_%s", o.AvailabilityMode))
value := Ydb_Maintenance.AvailabilityMode_value[title]

return Ydb_Maintenance.AvailabilityMode(value)
}

func (o *Options) GetMaintenanceDuration() *durationpb.Duration {
return durationpb.New(time.Second * time.Duration(o.MaintenanceDurationSeconds))
*rolling.RestartOptions
}

const (
DefaultMaintenanceDurationSeconds = 3600
)

func (o *Options) DefineFlags(fs *pflag.FlagSet) {
// TODO(shmel1k@): move to 'WithAvailbilityModes' helper.
fs.StringSliceVar(&o.HostFQDNs, "hosts", []string{},
"Request the hosts with these FQDNs from the cluster")

fs.StringVar(&o.AvailabilityMode, "availability-mode", "strong",
fmt.Sprintf("Availability mode. Available choices: %s", strings.Join(options.AvailabilityModes, ", ")))

fs.IntVar(&o.MaintenanceDurationSeconds, "duration", DefaultMaintenanceDurationSeconds,
fmt.Sprintf("How much time do you need for maintenance, in seconds. Default: %v",
DefaultMaintenanceDurationSeconds))
o.RestartOptions.DefineFlags(fs)
}

func (o *Options) Validate() error {
if len(o.HostFQDNs) == 0 {
return fmt.Errorf("--hosts unspecified")
}

if !collections.Contains(options.AvailabilityModes, o.AvailabilityMode) {
return fmt.Errorf("specified a non-existing availability mode: %s", o.AvailabilityMode)
}

if o.MaintenanceDurationSeconds <= 0 {
return fmt.Errorf("specified invalid maintenance duration seconds: %d. Must be positive", o.MaintenanceDurationSeconds)
}
return nil
return o.RestartOptions.Validate()
}
4 changes: 1 addition & 3 deletions cmd/maintenance/list/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@ import (
)

func New(f cmdutil.Factory) *cobra.Command {
_ = &Options{
BaseOptions: f.GetBaseOptions(),
}
_ = &Options{}

cmd := cli.SetDefaultsOn(&cobra.Command{
Use: "list",
Expand Down
6 changes: 1 addition & 5 deletions cmd/maintenance/list/options.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
package list

import "github.com/ydb-platform/ydbops/pkg/command"

type Options struct {
*command.BaseOptions
}
type Options struct{}
1 change: 1 addition & 0 deletions cmd/run/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ func New(
return err
},
}

opts.DefineFlags(cmd.Flags())
return cmd
}
6 changes: 3 additions & 3 deletions pkg/client/cms/cms.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ const (
defaultRetryCount = 5
)

type CMSClient interface {
type CMS interface {
Tenants() ([]string, error)
Nodes() ([]*Ydb_Maintenance.Node, error)
}

type Client interface {
CMSClient
MaintenanceClient
CMS
Maintenance

Close() error
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/client/cms/maintenance.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ type CreateTaskParams struct {
AvailabilityMode string
}

type MaintenanceClient interface {
type Maintenance interface {
CompleteAction([]*Ydb_Maintenance.ActionUid) (*Ydb_Maintenance.ManageActionResult, error)
CompleteActions(string, []string) (*Ydb_Maintenance.ManageActionResult, error)
CreateMaintenanceTask(MaintenanceTaskParams) (MaintenanceTask, error)
Expand Down
26 changes: 21 additions & 5 deletions pkg/command/command.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package command

import (
"errors"
"os"
"path/filepath"

"github.com/spf13/pflag"
"github.com/ydb-platform/ydbops/pkg/options"
)
Expand Down Expand Up @@ -34,16 +38,28 @@ func (o *BaseOptions) DefineFlags(fs *pflag.FlagSet) {
o.GRPC.DefineFlags(fs)
o.Auth.DefineFlags(fs)

fs.StringVar(
&o.ProfileFile, "config-file",
"",
"Path to config file with profile data in yaml format")

fs.StringVar(
&o.ActiveProfile, "profile",
"",
"Override currently set profile name from --config-file")

defaultProfileLocation := ""
if home, present := os.LookupEnv("HOME"); present {
defaultProfileLocation = filepath.Join(home, "ydb", "ydbops", "config", "config.yaml")
}

_, err := os.Stat(defaultProfileLocation)
if errors.Is(err, os.ErrNotExist) {
// it is of course allowed, user does not have the default config,
// "" will be treated as unspecified in profile code later
defaultProfileLocation = ""
}

fs.StringVar(
&o.ProfileFile, "profile-file",
defaultProfileLocation,
"Path to config file with profile data in yaml format. Default: $HOME/ydb/ydbops/config/config.yaml")

fs.BoolVar(&o.Verbose, "verbose", false, "Switches log level from INFO to DEBUG")
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/rolling/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ but you can not mix host FQDNs and node ids in this option. The list is comma-de
fs.StringVar(&o.AvailabilityMode, "availability-mode", "strong",
fmt.Sprintf("Availability mode. Available choices: %s", strings.Join(options.AvailabilityModes, ", ")))

fs.IntVar(&o.RestartDuration, "restart-duration", DefaultRestartDurationSeconds,
`CMS will release the node for maintenance for restart-duration * restart-retry-number seconds. Any maintenance
fs.IntVar(&o.RestartDuration, "duration", DefaultRestartDurationSeconds,
`CMS will release the node for maintenance for duration * restart-retry-number seconds. Any maintenance
after that would be considered a regular cluster failure`)

fs.IntVar(&o.RestartRetryNumber, "restart-retry-number", DefaultRetryCount,
Expand All @@ -196,7 +196,7 @@ for this invocation must be the same as for the previous invocation, and this ca
the ydbops utility is stateless. Use at your own risk.`)

fs.IntVar(&o.MaxStaticNodeId, "max-static-node-id", DefaultMaxStaticNodeId,
`This argument is used to help ydbops distinguish storage and dynamic nodes.
`This argument is used to help ydbops distinguish storage and dynamic nodes.
Nodes with this nodeId or less will be considered storage.`)

profile.PopulateFromProfileLater(
Expand Down
5 changes: 3 additions & 2 deletions tests/filtering_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"go.uber.org/zap"

"github.com/ydb-platform/ydbops/pkg/options"
"github.com/ydb-platform/ydbops/pkg/rolling"
"github.com/ydb-platform/ydbops/pkg/rolling/restarters"
"github.com/ydb-platform/ydbops/tests/mock"
)
Expand Down Expand Up @@ -40,7 +41,7 @@ var _ = Describe("Test storage Filter", func() {
nodes := mock.CreateNodesFromShortConfig(nodeGroups, nodeInfoMap)

filterSpec := restarters.FilterNodeParams{
MaxStaticNodeId: options.DefaultMaxStaticNodeId,
MaxStaticNodeId: rolling.DefaultMaxStaticNodeId,
StartedTime: &options.StartedTime{
Direction: '<',
Timestamp: fiveMinutesAgoTimestamp,
Expand Down Expand Up @@ -91,7 +92,7 @@ var _ = Describe("Test storage Filter", func() {
nodes := mock.CreateNodesFromShortConfig(nodeGroups, nodeInfoMap)

filterSpec := restarters.FilterNodeParams{
MaxStaticNodeId: options.DefaultMaxStaticNodeId,
MaxStaticNodeId: rolling.DefaultMaxStaticNodeId,
}

clusterInfo := restarters.ClusterNodesInfo{
Expand Down
32 changes: 16 additions & 16 deletions tests/maintenance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"github.com/ydb-platform/ydb-go-genproto/protos/Ydb_Discovery"
"google.golang.org/protobuf/proto"

"github.com/ydb-platform/ydbops/pkg/maintenance"
"github.com/ydb-platform/ydbops/pkg/client/cms"
"github.com/ydb-platform/ydbops/tests/mock"
)

Expand All @@ -29,12 +29,12 @@ var _ = Describe("Test Maintenance", func() {
ydbopsInvocation: Command{
"--endpoint", "grpcs://localhost:2135",
"--verbose",
"--availability-mode", "strong",
"--user", mock.TestUser,
"--cms-query-interval", "1",
"--ca-file", filepath.Join(".", "test-data", "ssl-data", "ca.crt"),
"maintenance",
"create",
"--availability-mode", "strong",
"--cms-query-interval", "1",
"--hosts=ydb-1.ydb.tech,ydb-2.ydb.tech",
},
expectedRequests: []proto.Message{
Expand All @@ -53,19 +53,19 @@ var _ = Describe("Test Maintenance", func() {
},
expectedOutputRegexps: []string{
// Your task id is:\n\n<uuid>\n\nPlease write it down for refreshing and completing the task later.\n
fmt.Sprintf("Your task id is:\n\n%s%s\n\n", maintenance.TaskUuidPrefix, uuidRegexpString),
fmt.Sprintf("Your task id is:\n\n%s%s\n\n", cms.TaskUuidPrefix, uuidRegexpString),
},
},
{
ydbopsInvocation: Command{
"--endpoint", "grpcs://localhost:2135",
"--verbose",
"--availability-mode", "strong",
"--user", mock.TestUser,
"--cms-query-interval", "1",
"--ca-file", filepath.Join(".", "test-data", "ssl-data", "ca.crt"),
"maintenance",
"list",
// "--availability-mode", "strong",
// "--cms-query-interval", "1",
},
expectedRequests: []proto.Message{
&Ydb_Auth.LoginRequest{
Expand All @@ -81,7 +81,7 @@ var _ = Describe("Test Maintenance", func() {
},
},
expectedOutputRegexps: []string{
fmt.Sprintf("Uid: %s%s\n", maintenance.TaskUuidPrefix, uuidRegexpString),
fmt.Sprintf("Uid: %s%s\n", cms.TaskUuidPrefix, uuidRegexpString),
" Lock on host ydb-1.ydb.tech",
"PERFORMED",
" Lock on host ydb-2.ydb.tech",
Expand All @@ -92,15 +92,15 @@ var _ = Describe("Test Maintenance", func() {
ydbopsInvocation: Command{
"--endpoint", "grpcs://localhost:2135",
"--verbose",
"--availability-mode", "strong",
"--user", mock.TestUser,
"--cms-query-interval", "1",
"--ca-file", filepath.Join(".", "test-data", "ssl-data", "ca.crt"),
"maintenance",
"complete",
"--task-id",
testWillInsertTaskUuid,
"--hosts=ydb-1.ydb.tech",
// "--availability-mode", "strong",
// "--cms-query-interval", "1",
},
expectedRequests: []proto.Message{
&Ydb_Auth.LoginRequest{
Expand Down Expand Up @@ -128,14 +128,14 @@ var _ = Describe("Test Maintenance", func() {
ydbopsInvocation: Command{
"--endpoint", "grpcs://localhost:2135",
"--verbose",
"--availability-mode", "strong",
"--user", mock.TestUser,
"--cms-query-interval", "1",
"--ca-file", filepath.Join(".", "test-data", "ssl-data", "ca.crt"),
"maintenance",
"refresh",
"--task-id",
testWillInsertTaskUuid,
// "--availability-mode", "strong",
// "--cms-query-interval", "1",
},
expectedRequests: []proto.Message{
&Ydb_Auth.LoginRequest{
Expand All @@ -147,7 +147,7 @@ var _ = Describe("Test Maintenance", func() {
},
},
expectedOutputRegexps: []string{
fmt.Sprintf("Uid: %s%s\n", maintenance.TaskUuidPrefix, uuidRegexpString),
fmt.Sprintf("Uid: %s%s\n", cms.TaskUuidPrefix, uuidRegexpString),
" Lock on host ydb-2.ydb.tech",
"PERFORMED",
},
Expand All @@ -156,15 +156,15 @@ var _ = Describe("Test Maintenance", func() {
ydbopsInvocation: Command{
"--endpoint", "grpcs://localhost:2135",
"--verbose",
"--availability-mode", "strong",
"--user", mock.TestUser,
"--cms-query-interval", "1",
"--ca-file", filepath.Join(".", "test-data", "ssl-data", "ca.crt"),
"maintenance",
"complete",
"--task-id",
testWillInsertTaskUuid,
"--hosts=ydb-2.ydb.tech",
// "--availability-mode", "strong",
// "--cms-query-interval", "1",
},
expectedRequests: []proto.Message{
&Ydb_Auth.LoginRequest{
Expand Down Expand Up @@ -192,12 +192,12 @@ var _ = Describe("Test Maintenance", func() {
ydbopsInvocation: Command{
"--endpoint", "grpcs://localhost:2135",
"--verbose",
"--availability-mode", "strong",
"--user", mock.TestUser,
"--cms-query-interval", "1",
"--ca-file", filepath.Join(".", "test-data", "ssl-data", "ca.crt"),
"maintenance",
"list",
// "--availability-mode", "strong",
// "--cms-query-interval", "1",
},
expectedRequests: []proto.Message{
&Ydb_Auth.LoginRequest{
Expand Down
Loading

0 comments on commit c0debac

Please sign in to comment.