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: add install cmd for side-eye #123974

Merged
merged 1 commit into from
May 13, 2024
Merged

roachprod: add install cmd for side-eye #123974

merged 1 commit into from
May 13, 2024

Conversation

dt
Copy link
Member

@dt dt commented May 10, 2024

This also required a new optional extra map of local -- to the client running roachprod -- commands that can be run to replace arguments in the install cmd, so that we can get the key without checking it in.

Alternateively we could special-case the side-eye install in go code but this felt more generalized.

Release note: none.
Epic: none.

@dt dt requested review from herkolategan and msbutler May 10, 2024 21:49
@dt dt requested a review from a team as a code owner May 10, 2024 21:49
@dt dt requested review from vidit-bhat and removed request for a team May 10, 2024 21:49
@cockroach-teamcity
Copy link
Member

This change is Reviewable

This also required a new optional extra map of local -- to the client running roachprod --
commands that can be run to replace arguments in the install cmd, so that we can get the
key without checking it in.

Alternateively we could special-case the side-eye install in go code but this felt more
generalized.

Release note: none.
Epic: none.
@@ -93,6 +97,19 @@ echo "deb [signed-by=/etc/apt/keyrings/fluent-bit.gpg] https://packages.fluentbi
sudo apt-get update;
sudo apt-get install -y fluent-bit;
`,

"side-eye": `
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you write a docstring here that lays out how to install side eye? this could also go in the commit message. i.e. i think it is something like, but it's not totally clear from the code:

roachprod install $CLUSTER side-eye --secret $API_KEY

Copy link
Member Author

Choose a reason for hiding this comment

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

It is just roachprod install TARGET side-eye same as all the others, eg roachprod install TARGET haproxy. Given it is consistent with all the other install commands it seems weird to give it special documentation?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, so the user needs to set the SIDE_EYE_API_TOKEN env var. I think that's worth documenting.

Copy link
Collaborator

@msbutler msbutler May 13, 2024

Choose a reason for hiding this comment

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

i'm just thinking through ways to ease the onboarding process for any CRL engineer.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, so the user needs to set the SIDE_EYE_API_TOKEN env var. I think that's worth documenting.

No, the key is fetched from gcloud (which you already need to have configured to use roachprod); the user with keys on keyboard never touches the key.

Copy link
Member Author

Choose a reason for hiding this comment

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

i'm just thinking through ways to ease the onboarding process for any CRL engineer.

Again, it is identical to all the other install commands, which seems like it should be as easy as we can do

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see, sorry, was not reading the code as closely as I could have.

@dt
Copy link
Member Author

dt commented May 13, 2024

TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented May 13, 2024

Build failed:

@dt
Copy link
Member Author

dt commented May 13, 2024

brutal preemptions

bors r+

@craig craig bot merged commit d8999f7 into cockroachdb:master May 13, 2024
22 checks passed
@dt dt deleted the side-eye branch May 13, 2024 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants