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.

Sign up for GitHub

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

Jump to bottom

Add an AWS EC2 instance fetcher #13886

Merged
merged 4 commits into from
Aug 19, 2022
Merged

Add an AWS EC2 instance fetcher #13886

merged 4 commits into from
Aug 19, 2022

Conversation

lxea
Copy link
Contributor

@lxea lxea commented Jun 27, 2022

This adds a fetcher for ec2 instances as well as a watcher to periodically fetch them.

Requires IAM permissions: iam:DescribeInstances

Depends on #13787
Part of #12410

@lxea lxea force-pushed the lxea/aws-ec2-discovery-lib branch 2 times, most recently from 7f384b2 to 817844e Compare July 4, 2022 07:05
@lxea lxea force-pushed the lxea/aws-ec2-discovery-fetcher branch from ef84f69 to 58ee986 Compare July 5, 2022 12:45
@lxea lxea force-pushed the lxea/aws-ec2-discovery-lib branch from 817844e to aaafbd8 Compare July 5, 2022 12:48
@lxea lxea force-pushed the lxea/aws-ec2-discovery-fetcher branch from 58ee986 to 5cac73a Compare July 5, 2022 12:48
@lxea lxea mentioned this pull request Jul 5, 2022
Merged
@lxea lxea force-pushed the lxea/aws-ec2-discovery-fetcher branch from 5cac73a to e3b10fa Compare July 5, 2022 14:00
@lxea lxea marked this pull request as ready for review July 5, 2022 14:03
@github-actions github-actions bot added the database-access Database access related issues and PRs label Jul 5, 2022
@github-actions github-actions bot requested review from marcoandredinis and zmb3 July 5, 2022 14:04
@r0mant r0mant requested a review from tigrato July 5, 2022 15:42
tigrato
tigrato reviewed Jul 6, 2022
View reviewed changes
lib/srv/db/common/cloud.go Show resolved Hide resolved
lib/srv/regular/sshserver.go Outdated Show resolved Hide resolved
lib/srv/server/cloudwatcher.go Show resolved Hide resolved
lib/srv/regular/sshserver.go Outdated Show resolved Hide resolved
lib/srv/server/cloudwatcher.go Show resolved Hide resolved
@lxea lxea force-pushed the lxea/aws-ec2-discovery-fetcher branch 2 times, most recently from af2cc44 to d5648db Compare July 7, 2022 13:06
@lxea lxea force-pushed the lxea/aws-ec2-discovery-lib branch from aaafbd8 to 44fc15a Compare July 8, 2022 07:28
Base automatically changed from lxea/aws-ec2-discovery-lib to master July 8, 2022 09:25
@lxea lxea force-pushed the lxea/aws-ec2-discovery-fetcher branch 5 times, most recently from 18375a3 to 13f9318 Compare July 11, 2022 15:05
@lxea lxea mentioned this pull request Jul 11, 2022
Closed
zmb3
zmb3 previously requested changes Jul 12, 2022
View reviewed changes
Copy link
Collaborator

@zmb3 zmb3 left a comment

Choose a reason for hiding this comment

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

In addition to the review comments, when you add new code that requires new AWS permissions, please don't forget to document the new permission, update helm/terraform/etc.

lib/cloud/clients/clients.go Outdated Show resolved Hide resolved
lib/cloud/watchers/ec2.go Outdated Show resolved Hide resolved
lib/cloud/watchers/ec2.go Outdated Show resolved Hide resolved
lib/events/s3sessions/s3handler_test.go Outdated Show resolved Hide resolved
lib/cloud/watchers/watcher.go Outdated Show resolved Hide resolved
@github-actions github-actions bot requested review from greedy52 and zmb3 July 12, 2022 14:22
@github-actions github-actions bot requested review from Joerger and nklaassen July 12, 2022 14:36
@lxea lxea force-pushed the lxea/aws-ec2-discovery-feat-install-script-resource branch 3 times, most recently from a9d143a to 98c18e5 Compare August 15, 2022 10:06
Base automatically changed from lxea/aws-ec2-discovery-feat-install-script-resource to master August 15, 2022 11:18
@lxea lxea force-pushed the lxea/aws-ec2-discovery-fetcher branch from 35a8c75 to f94cf08 Compare August 15, 2022 12:59
@lxea lxea requested a review from greedy52 August 15, 2022 13:02
@lxea lxea force-pushed the lxea/aws-ec2-discovery-fetcher branch from eedfb9b to 877154b Compare August 15, 2022 14:27
@lxea
Copy link
Contributor Author

lxea commented Aug 16, 2022

@zmb3 do you mind taking a quick look? changes requested is still in place

@lxea lxea force-pushed the lxea/aws-ec2-discovery-fetcher branch from 877154b to d042b2a Compare August 16, 2022 09:33
@r0mant r0mant dismissed zmb3’s stale review August 17, 2022 16:37

Checked with Zac offline that he doesn't need to look again.

Alex McGrath added 4 commits August 19, 2022 11:29
Add an AWS EC2 instance fetcher
366ca39
Resolve comments
2343ce7 
- Remove leftover VPCID stuff
- Document public functions
- Return a notfound when there are no nodes fetched instead of nil nil
Resolve comments
a130a19
Fix fileconf_test.go
8ab8520
@lxea lxea force-pushed the lxea/aws-ec2-discovery-fetcher branch from d042b2a to 8f2689c Compare August 19, 2022 10:30
@lxea lxea enabled auto-merge (squash) August 19, 2022 10:31
@lxea lxea force-pushed the lxea/aws-ec2-discovery-fetcher branch from 8f2689c to 8ab8520 Compare August 19, 2022 10:31
@lxea lxea merged commit 122bda9 into master Aug 19, 2022
@lxea lxea deleted the lxea/aws-ec2-discovery-fetcher branch August 19, 2022 11:10
@lxea lxea mentioned this pull request Aug 31, 2022
Merged
lxea pushed a commit that referenced this pull request Sep 1, 2022
Add an AWS EC2 instance fetcher (#13886)
811a8d0 
* Add an AWS EC2 instance fetcher

* Resolve comments

- Remove leftover VPCID stuff
- Document public functions
- Return a notfound when there are no nodes fetched instead of nil nil

* Resolve comments

* Fix fileconf_test.go

Add AWS matchers to the ssh_service section of the fileconf (#13787)

Update file conf with AWS discovery on SSH nodes
lxea pushed a commit that referenced this pull request Sep 12, 2022
Add an AWS EC2 instance fetcher (#13886)
c9018f5 
* Add an AWS EC2 instance fetcher

* Resolve comments

- Remove leftover VPCID stuff
- Document public functions
- Return a notfound when there are no nodes fetched instead of nil nil

* Resolve comments

* Fix fileconf_test.go

Add AWS matchers to the ssh_service section of the fileconf (#13787)

Update file conf with AWS discovery on SSH nodes
lxea pushed a commit that referenced this pull request Sep 22, 2022
Add an AWS EC2 instance fetcher (#13886)
0cfb676 
* Add an AWS EC2 instance fetcher

* Resolve comments

- Remove leftover VPCID stuff
- Document public functions
- Return a notfound when there are no nodes fetched instead of nil nil

* Resolve comments

* Fix fileconf_test.go

Add AWS matchers to the ssh_service section of the fileconf (#13787)

Update file conf with AWS discovery on SSH nodes
lxea pushed a commit that referenced this pull request Sep 22, 2022
Backport V10: Add an AWS EC2 instance fetcher (#13886) (#16006)
9e719f1 
* Add an AWS EC2 instance fetcher (#13886)

* Add an AWS EC2 instance fetcher

* Resolve comments

- Remove leftover VPCID stuff
- Document public functions
- Return a notfound when there are no nodes fetched instead of nil nil

* Resolve comments

* Fix fileconf_test.go

Add AWS matchers to the ssh_service section of the fileconf (#13787)

Update file conf with AWS discovery on SSH nodes

* Fix another bad merge

* V10: Backport #13146 & #15682 :  (#16009)

* Add an installer script resource type and public HTTP endpoint (#13146)

* Add an installer script resource type and HTTP endpoint

* Add default installer scripts

* Resolve comments

- run `shfmt` over the installer script
- add MustNewInstaller
- move default installer logic to GetInstaller
- only include PublicProxyAddr in server.Config

* Resolve comments

- move from auth_with_roles -> auth/grpcserver
- document `MustNewInstallerV1`

* Allow for multiple installer scripts instead of a singleton (#15682)

* Allow for multiple installer scripts instead of using a singleton

* Resolve comments

- require name be provided to installer
- MetaNameClusterInstallerScript -> defaults.InstallerScriptName
- Require VerbList for GetInstallers

* Resolve comments

- Include the default installer in GetInstallers output
- Update how the scripts are prefixed

* resolve conflicts

* make the linters happy

* Update api/go.mod

* Fix bad merges
@espadolini espadolini mentioned this pull request Oct 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@r0mant r0mant r0mant approved these changes

@tigrato tigrato tigrato approved these changes

@greedy52 greedy52 greedy52 approved these changes

@zmb3 zmb3 Awaiting requested review from zmb3

Assignees
No one assigned
Labels
server-access
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@lxea @r0mant @tigrato @zmb3 @greedy52