Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

roachprod: remove ClusterImpl interface #72887

Merged
merged 1 commit into from
Nov 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions pkg/cmd/roachprod/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,18 @@ var (
"COCKROACH_ENABLE_RPC_COMPRESSION=false",
"COCKROACH_UI_RELEASE_NOTES_SIGNUP_DISMISSED=true",
}
tag string
external = false
certsDir string
adminurlOpen = false
adminurlPath = ""
adminurlIPs = false
useTreeDist = true
quiet = false
sig = 9
waitFlag = false
createVMOpts = vm.DefaultCreateOpts()
startOpts = install.StartOpts{
tag string
external = false
pgurlCertsDir string
adminurlOpen = false
adminurlPath = ""
adminurlIPs = false
useTreeDist = true
quiet = false
sig = 9
waitFlag = false
createVMOpts = vm.DefaultCreateOpts()
startOpts = install.StartOpts{
Encrypt: false,
Sequential: true,
SkipInit: false,
Expand Down Expand Up @@ -152,7 +152,7 @@ func initFlags() {

pgurlCmd.Flags().BoolVar(&external,
"external", false, "return pgurls for external connections")
pgurlCmd.Flags().StringVar(&certsDir,
pgurlCmd.Flags().StringVar(&pgurlCertsDir,
"certs-dir", "./certs", "cert dir to use for secure connections")

pprofCmd.Flags().DurationVar(&pprofOptions.duration,
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/roachprod/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func wrap(f func(cmd *cobra.Command, args []string) error) func(cmd *cobra.Comma
func clusterOpts() install.ClusterSettings {
return install.ClusterSettings{
Tag: tag,
CertsDir: certsDir,
PGUrlCertsDir: pgurlCertsDir,
Secure: secure,
Quiet: quiet || !term.IsTerminal(int(os.Stdout.Fd())),
UseTreeDist: useTreeDist,
Expand Down
70 changes: 24 additions & 46 deletions pkg/roachprod/install/cluster_synced.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,21 +45,10 @@ import (
"golang.org/x/sync/errgroup"
)

// ClusterImpl TODO(peter): document
type ClusterImpl interface {
Start(c *SyncedCluster, opts StartOpts) error
CertsDir(c *SyncedCluster, index int) string
NodeDir(c *SyncedCluster, index, storeIndex int) string
LogDir(c *SyncedCluster, index int) string
NodeURL(c *SyncedCluster, host string, port int) string
NodePort(c *SyncedCluster, index int) int
NodeUIPort(c *SyncedCluster, index int) int
}

// ClusterSettings contains various knobs that affect operations on a cluster.
type ClusterSettings struct {
Secure bool
CertsDir string
PGUrlCertsDir string
Env []string
Tag string
UseTreeDist bool
Expand All @@ -73,11 +62,11 @@ type ClusterSettings struct {
// DefaultClusterSettings returns the default settings.
func DefaultClusterSettings() ClusterSettings {
return ClusterSettings{
Tag: "",
CertsDir: "./certs",
Secure: false,
Quiet: false,
UseTreeDist: true,
Tag: "",
PGUrlCertsDir: "./certs",
Secure: false,
Quiet: false,
UseTreeDist: true,
Env: []string{
"COCKROACH_ENABLE_RPC_COMPRESSION=false",
"COCKROACH_UI_RELEASE_NOTES_SIGNUP_DISMISSED=true",
Expand All @@ -102,8 +91,6 @@ type SyncedCluster struct {

ClusterSettings

Impl ClusterImpl

Localities []string

// AuthorizedKeys is used by SetupSSH to add additional authorized keys.
Expand All @@ -120,7 +107,6 @@ func NewSyncedCluster(
c := &SyncedCluster{
Cluster: *metadata,
ClusterSettings: settings,
Impl: Cockroach{},
}
c.Localities = make([]string, len(c.VMs))
for i := range c.VMs {
Expand Down Expand Up @@ -166,14 +152,14 @@ func (c *SyncedCluster) localVMDir(nodeIdx int) string {
return local.VMDir(c.Name, nodeIdx)
}

// ServerNodes is the fully expanded, ordered list of nodes that any given
// TargetNodes is the fully expanded, ordered list of nodes that any given
// roachprod command is intending to target.
//
// $ roachprod create local -n 4
// $ roachprod start local # [1, 2, 3, 4]
// $ roachprod start local:2-4 # [2, 3, 4]
// $ roachprod start local:2,1,4 # [1, 2, 4]
func (c *SyncedCluster) ServerNodes() []int {
func (c *SyncedCluster) TargetNodes() []int {
return append([]int{}, c.Nodes...)
}

Expand Down Expand Up @@ -262,11 +248,6 @@ type StartOpts struct {
ExtraArgs []string
}

// Start TODO(peter): document
func (c *SyncedCluster) Start(startOpts StartOpts) error {
return c.Impl.Start(c, startOpts)
}

func (c *SyncedCluster) newSession(i int) (session, error) {
if c.IsLocal() {
return newLocalSession(), nil
Expand Down Expand Up @@ -306,7 +287,7 @@ func (c *SyncedCluster) Stop(sig int, wait bool) error {
done
echo "${pid}: dead" >> %[1]s/roachprod.log
done`,
c.Impl.LogDir(c, c.Nodes[i]), // [1]
c.LogDir(c.Nodes[i]), // [1]
)
}

Expand All @@ -323,7 +304,7 @@ if [ -n "${pids}" ]; then
kill -%[3]d ${pids}
%[4]s
fi`,
c.Impl.LogDir(c, c.Nodes[i]), // [1]
c.LogDir(c.Nodes[i]), // [1]
c.roachprodEnvRegex(c.Nodes[i]), // [2]
sig, // [3]
waitCmd, // [4]
Expand Down Expand Up @@ -442,7 +423,7 @@ type NodeMonitorInfo struct {
// be emitted for them.
func (c *SyncedCluster) Monitor(ignoreEmptyNodes bool, oneShot bool) chan NodeMonitorInfo {
ch := make(chan NodeMonitorInfo)
nodes := c.ServerNodes()
nodes := c.TargetNodes()
var wg sync.WaitGroup

for i := range nodes {
Expand Down Expand Up @@ -474,8 +455,8 @@ func (c *SyncedCluster) Monitor(ignoreEmptyNodes bool, oneShot bool) chan NodeMo
}{
OneShot: oneShot,
IgnoreEmpty: ignoreEmptyNodes,
Store: Cockroach{}.NodeDir(c, nodes[i], 1 /* storeIndex */),
Port: Cockroach{}.NodePort(c, nodes[i]),
Store: c.NodeDir(nodes[i], 1 /* storeIndex */),
Port: c.NodePort(nodes[i]),
Local: c.IsLocal(),
}

Expand Down Expand Up @@ -1377,7 +1358,7 @@ func (c *SyncedCluster) Logs(
if c.IsLocal() {
// This here is a bit of a hack to guess that the parent of the log dir is
// the "home" for the local node and that the srcBase is relative to that.
localHome := filepath.Dir(c.Impl.LogDir(c, idx))
localHome := filepath.Dir(c.LogDir(idx))
remote = filepath.Join(localHome, src) + "/"
} else {
logDir := src
Expand Down Expand Up @@ -1706,7 +1687,7 @@ func (c *SyncedCluster) pgurls(nodes []int) (map[int]string, error) {
}
m := make(map[int]string, len(hosts))
for node, host := range hosts {
m[node] = c.Impl.NodeURL(c, host, c.Impl.NodePort(c, node))
m[node] = c.NodeURL(host, c.NodePort(node))
}
return m, nil
}
Expand Down Expand Up @@ -1942,19 +1923,16 @@ func (c *SyncedCluster) ParallelE(
return nil, nil
}

// Init initializes the cluster. It does it through node 1 (as per ServerNodes)
// Init initializes the cluster. It does it through node 1 (as per TargetNodes)
// to maintain parity with auto-init behavior of `roachprod start` (when
// --skip-init) is not specified. The implementation should be kept in
// sync with Cockroach.Start.
// sync with Start().
func (c *SyncedCluster) Init() error {
r := c.Impl.(Cockroach)
h := &crdbInstallHelper{c: c, r: r}

// See (Cockroach).Start. We reserve a few special operations for the first
// node, so we strive to maintain the same here for interoperability.
// See Start(). We reserve a few special operations for the first node, so we
// strive to maintain the same here for interoperability.
const firstNodeIdx = 0

vers, err := getCockroachVersion(c, c.ServerNodes()[firstNodeIdx])
vers, err := getCockroachVersion(c, c.TargetNodes()[firstNodeIdx])
if err != nil {
return errors.WithDetail(err, "install.Init() failed: unable to retrieve cockroach version.")
}
Expand All @@ -1963,17 +1941,17 @@ func (c *SyncedCluster) Init() error {
return errors.New("install.Init() failed: `roachprod init` only supported for v20.1 and beyond")
}

fmt.Printf("%s: initializing cluster\n", h.c.Name)
initOut, err := h.initializeCluster(firstNodeIdx)
fmt.Printf("%s: initializing cluster\n", c.Name)
initOut, err := c.initializeCluster(firstNodeIdx)
if err != nil {
return errors.WithDetail(err, "install.Init() failed: unable to initialize cluster.")
}
if initOut != "" {
fmt.Println(initOut)
}

fmt.Printf("%s: setting cluster settings\n", h.c.Name)
clusterSettingsOut, err := h.setClusterSettings(firstNodeIdx)
fmt.Printf("%s: setting cluster settings\n", c.Name)
clusterSettingsOut, err := c.setClusterSettings(firstNodeIdx)
if err != nil {
return errors.WithDetail(err, "install.Init() failed: unable to set cluster settings.")
}
Expand Down
Loading