Skip to content

roachprod: fixup roachprod --sequential#51893

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
irfansharif:200724.roachprod-sequential
Jul 25, 2020
Merged

roachprod: fixup roachprod --sequential#51893
craig[bot] merged 1 commit intocockroachdb:masterfrom
irfansharif:200724.roachprod-sequential

Conversation

@irfansharif
Copy link
Copy Markdown
Contributor

@irfansharif irfansharif commented Jul 24, 2020

..and the setting of cluster settings for single node clusters.
roachprod start --sequential was broken in #51329, and the broken-ness
outlined in TODOs in #51790. This PR just addresses those TODOs.

Fixes #51497
Fixes #51721
Fixes #51738
Fixes #51768
Fixes #51769
Fixes #51776

Release note: None

@irfansharif irfansharif requested review from nvb and petermattis July 24, 2020 22:16
@cockroach-teamcity
Copy link
Copy Markdown
Member

cockroach-teamcity commented Jul 24, 2020

This change is Reviewable

nvb
nvb approved these changes Jul 24, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@nvb nvb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @irfansharif, @nvanbenschoten, and @petermattis)

pkg/cmd/roachprod/install/cockroach.go, line 145 at r1 (raw file):

		// We reserve a few special operations like initializing the cluster,
		// and setting cluster settings, for node 1.

nit: no comma.

pkg/cmd/roachprod/install/cockroach.go, line 475 at r1 (raw file):

	//    which prompts CRDB to auto-initialize. For nodes running >=20.1, we
	//    need to explicitly initialize.
	return !h.useStartSingleNode(vers) && nodes[nodeIdx] == 1 && vers.AtLeast(version.MustParse("v20.1.0"))

Do we want/need the nodes[nodeIdx] == 1 check?

@irfansharif irfansharif force-pushed the 200724.roachprod-sequential branch from bf77241 to b7889b0 Compare July 24, 2020 23:30
irfansharif
irfansharif commented Jul 24, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@irfansharif irfansharif left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the quick review!

bors r+

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @petermattis)

pkg/cmd/roachprod/install/cockroach.go, line 145 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

nit: no comma.

Done.

pkg/cmd/roachprod/install/cockroach.go, line 475 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

Do we want/need the nodes[nodeIdx] == 1 check?

Done. Inlined at caller.

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jul 25, 2020

Build failed:

@irfansharif
Copy link
Copy Markdown
Contributor Author

irfansharif commented Jul 25, 2020 

✖ 1 test failed
FAILED TESTS:
  <TimeScaleDropdown>
    ✖ getTimeRangeTitle must return custom Title
      HeadlessChrome 83.0.4103 (Linux 0.0.0)
    AssertionError: expected 'Jul 24, 11:52PM -  12:02AM' to equal ' 11:52PM -  12:02AM'
        at Context.eval (webpack-internal:///./src/views/cluster/containers/timescale/timescale.spec.tsx:105:50)

UI test flake?

@irfansharif
roachprod: fixup roachprod --sequential
6d6706b 
..and the setting of cluster settings for single node clusters.
`roachprod start --sequential` was broken in cockroachdb#51329, and the broken-ness
outlined in TODOs in cockroachdb#51790. This PR just addresses those TODOs.

Fixes cockroachdb#51497
Fixes cockroachdb#51721
Fixes cockroachdb#51738
Fixes cockroachdb#51768
Fixes cockroachdb#51769
Fixes cockroachdb#51776

Release note: None
@irfansharif irfansharif force-pushed the 200724.roachprod-sequential branch from b7889b0 to 6d6706b Compare July 25, 2020 02:05
@irfansharif
Copy link
Copy Markdown
Contributor Author

irfansharif commented Jul 25, 2020

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jul 25, 2020

Build succeeded:

@craig craig bot merged commit a16eb55 into cockroachdb:master Jul 25, 2020
@irfansharif irfansharif deleted the 200724.roachprod-sequential branch July 25, 2020 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@petermattis petermattis Awaiting requested review from petermattis

1 more reviewer

@nvb nvb nvb approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

3 participants

@irfansharif @cockroach-teamcity @nvb