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: fixup roachprod --sequential #51893

Merged
merged 1 commit into from
Jul 25, 2020

Conversation

irfansharif
Copy link
Contributor

..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

@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Member

@nvanbenschoten nvanbenschoten 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
Copy link
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
Contributor

craig bot commented Jul 25, 2020

Build failed:

@irfansharif
Copy link
Contributor Author

✖ 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?

..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
Contributor Author

bors r+

@craig
Copy link
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
Labels
None yet
Projects
None yet
3 participants