Skip to content

roachprod: fix roachprod start ignoring --binary flag#72432

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
healthy-pod:roachprod-fix-binary-flag
Nov 4, 2021
Merged

roachprod: fix roachprod start ignoring --binary flag#72432
craig[bot] merged 1 commit intocockroachdb:masterfrom
healthy-pod:roachprod-fix-binary-flag

Conversation

@healthy-pod
Copy link
Copy Markdown
Contributor

@healthy-pod healthy-pod commented Nov 4, 2021 •
edited
Loading

Merging #71660 introduced a bug where roachprod ignores --binary
flag when running roachprod start.

This patch reverts to the old way of setting config.Binary as a quick solution to the bug.

Release note: None

Fixes #72425 #72420 #72373 #72372

@healthy-pod healthy-pod requested a review from a team as a code owner November 4, 2021 14:56
@cockroach-teamcity
Copy link
Copy Markdown
Member

cockroach-teamcity commented Nov 4, 2021

This change is Reviewable

@healthy-pod healthy-pod requested review from rail and tbg November 4, 2021 14:59
rail
rail approved these changes Nov 4, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@rail rail left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @tbg)

@tbg
Copy link
Copy Markdown
Member

tbg commented Nov 4, 2021

Thanks! Don't know this code well (🧑‍🔬 🐶) so Rail's approval is the one that matters.

Please always mention related issues, for example via

Fixes #72425

in the PR or commit message. This really helps keep context together.

You should also in a similar vein have this PR fix the test failures linked into #72425 and I would recommend checking that they are actually fixed if you haven't already (maybe while bors is doing its job to avoid losing time).

@healthy-pod healthy-pod force-pushed the roachprod-fix-binary-flag branch from 5b79e78 to 4bee29f Compare November 4, 2021 15:45
@healthy-pod
Copy link
Copy Markdown
Contributor Author

healthy-pod commented Nov 4, 2021

@tbg thanks for letting me know! I tested roachprod start --binary before and after the change to make sure the behaviour is as expected. I will run the linked roachtests in some time to make sure they are fixed (assuming it's safe to bors this PR since it's fixing a bug either ways).

@tbg
Copy link
Copy Markdown
Member

tbg commented Nov 4, 2021

I am pretty sure that the change fixes it. If you've verified that --binary is now taken into account I'm fine adding the Fixes: lines for the test failures to this PR. See if you can get it merged before EOD so that the nightlies don't incur another round of failures, thanks!

@healthy-pod
Copy link
Copy Markdown
Contributor Author

healthy-pod commented Nov 4, 2021

bors r=rail

@ZhouXing19 ZhouXing19 mentioned this pull request Nov 4, 2021
Closed
@healthy-pod healthy-pod force-pushed the roachprod-fix-binary-flag branch from 4bee29f to 283ecd5 Compare November 4, 2021 18:48
@craig
Copy link
Copy Markdown
Contributor

craig bot commented Nov 4, 2021

Canceled.

roachprod: fix roachprod start ignoring --binary flag
87811bc 
Merging cockroachdb#71660 introduced a bug where roachprod ignores --binary
flag when running `roachprod start`.

This patch reverts to the old way of setting config.Binary.

Release note: None

Fixes cockroachdb#72425 cockroachdb#72420 cockroachdb#72373 cockroachdb#72372
@healthy-pod healthy-pod force-pushed the roachprod-fix-binary-flag branch from 283ecd5 to 87811bc Compare November 4, 2021 19:05
This was linked to issues Nov 4, 2021
roachtest: acceptance/version-upgrade failed [caused by #72425] #72372
Closed
roachtest: decommission/mixed-versions failed [caused by #72425] #72373
Closed
roachtest: multitenant-upgrade failed [caused by #72425] #72420
Closed
@healthy-pod
Copy link
Copy Markdown
Contributor Author

healthy-pod commented Nov 4, 2021

bors r=[rail,tbg]

@healthy-pod
Copy link
Copy Markdown
Contributor Author

healthy-pod commented Nov 4, 2021

@RaduBerinde just want to make sure you are aware of this short but important PR!

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Nov 4, 2021

Build succeeded:

@craig craig bot merged commit ed35bbe into cockroachdb:master Nov 4, 2021
@tbg tbg mentioned this pull request Nov 5, 2021
Closed
This was referenced Nov 9, 2021
Closed
Closed
@healthy-pod healthy-pod deleted the roachprod-fix-binary-flag branch September 20, 2022 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rail rail rail approved these changes

@tbg tbg tbg approved these changes

Assignees

No one assigned

Labels

A-roachprod

Projects

None yet

Milestone

No milestone

4 participants

@healthy-pod @cockroach-teamcity @tbg @rail