google: add compute instance #56
Conversation
tormath1
force-pushed
the
google-compute-instance
branch
2 times, most recently
from
37cdfdd to
d022b70
Compare
There was a problem hiding this comment.
Ok a few comments on the code, but the commits need to be cleaner (you are changing and changing code from the same PR in commits).
We want to try the syntax as close as possible to TF+Provider as possible, this goes for the Tags and Labels, IIRC Google uses Label instead of tags, so we should expose the name label instead of tags but internally use the Tag abstraction.
Also about zones IDK if it's the regions we have on AWS
I think this is all though :)
google/reader.go
Outdated
| compute *compute.Service | ||
| project string | ||
| region string | ||
| //TODO: add filters |
There was a problem hiding this comment.
If there is a TODO add an issue for it and add the link to the TODO.
google/reader.go
Outdated
| if err != nil { | ||
| return nil, err | ||
| } | ||
| // zones are URL format. Need to split them |
There was a problem hiding this comment.
Can you put an example? I see for the code what it is (it's the last element of the URL that we want) but just to have itmore clear.
google/reader.go
Outdated
| return nil, err | ||
| } | ||
| // zones are URL format. Need to split them | ||
| zones := make([]string, 0) |
There was a problem hiding this comment.
zones := make([]stirng,0, len(region.Zones)) so you already allocate the needed space.
google/reader.go
Outdated
| // ListInstances return a list of instances in the project/region | ||
| func (r *GCPReader) ListInstances() (map[string][]compute.Instance, error) { | ||
| is := compute.NewInstancesService(r.compute) | ||
| //TODO: add filters |
There was a problem hiding this comment.
Same, add issue if this is relevant and link it.
google/reader.go
Outdated
| } | ||
| for _, zone := range zones { | ||
| instances := make([]compute.Instance, 0) | ||
| il, err := is.List(r.project, zone).Do() |
There was a problem hiding this comment.
Dest this return all of them? or it's paginated?
| instances := make([]compute.Instance, 0) | ||
| il, err := is.List(r.project, zone). | ||
| // 500 is the current `maxResults` | ||
| instances := make([]compute.Instance, 0, 500) |
There was a problem hiding this comment.
And if you have more than 500? It's not possible to request them all?
There was a problem hiding this comment.
I use pages. Each page has a default size of 500 elements: see pagination and max result
instances slice will store only the instances returned by the current page (so 500 maximum). Then we append this slice to the final instancesList map.
tormath1
force-pushed
the
google-compute-instance
branch
from
524e0d2 to
755e5c6
Compare
|
ready for a next review :) |
tormath1
force-pushed
the
google-compute-instance
branch
from
755e5c6 to
02fbd01
Compare
|
rebased and squashed ✔️ |
- instance (from PR #56) - network - firewall
* google: pass config as a pointer * google: implement firewall ressource * readme: fix gcp plugin version * google: implement compute network resource * changelog: add google compute resources - instance (from PR #56) - network - firewall
This PR is based on the Google boilerplate. I import Google Compute Instances using this api.
TC been tested against 2 instances: in different zones and different labels.
What I've done:
requiredStringFlagsfunction intoroot.gosince we are using the same function in both command lineszonelist since the API need bothprojectandzoneto list instances. We fetch zones from the givenregionreaderin waiting for RAWS removalhack the ID between read and importI'll squash commits before merging.