openshift / aws-account-operator

Operator to manage pool of AWS accounts for Hive

Apache License 2.0

32 stars 75 forks source link

Initialize only the Regions available in the cluster account #561

Closed ArielLima closed 3 years ago

ArielLima commented 3 years ago

During the region initialization phase, initialize only the regions enabled for the account, instead of using a hardcoded array of covered regions. Card: https://issues.redhat.com/browse/OSD-6825

openshift-ci-robot commented 3 years ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ArielLima To complete the pull request process, please assign iamkirkbater after the PR has been reviewed. You can assign the PR to them by writing /assign @iamkirkbater in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/openshift/aws-account-operator/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment

ArielLima commented 3 years ago

Still missing a few region AMIs since they are taking a while to initialize in my account. I am putting this on hold until I can get those but in the meantime reviews are welcome.

ArielLima commented 3 years ago

/hold

fahlmant commented 3 years ago

Given that we're moving away from this static list, maybe moving the ami IDs into a configmap would help here? In case the ami IDs change. Then a configmap update would be required, rather than a full new deploy of the operator.

ArielLima commented 3 years ago

Given that we're moving away from this static list, maybe moving the ami IDs into a configmap would help here? In case the ami IDs change. Then a configmap update would be required, rather than a full new deploy of the operator.

I think this would be a good change

ArielLima commented 3 years ago

/hold cancel

codecov-commenter commented 3 years ago

Codecov Report

Merging #561 (c2bf644) into master (50eb9ff) will decrease coverage by 0.08%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #561      +/-   ##
==========================================
- Coverage   13.46%   13.37%   -0.09%     
==========================================
  Files          44       45       +1     
  Lines        3774     3850      +76     
==========================================
+ Hits          508      515       +7     
- Misses       3243     3312      +69     
  Partials       23       23

Impacted Files	Coverage Δ
pkg/awsclient/client.go	`0.00% <0.00%> (ø)`
pkg/controller/account/account_controller.go	`0.36% <0.00%> (-0.03%)`	:arrow_down:
pkg/controller/account/ec2.go	`0.00% <0.00%> (ø)`
pkg/controller/account/byoc.go	`34.81% <0.00%> (-0.13%)`	:arrow_down:
pkg/controller/account/iam.go	`0.00% <0.00%> (ø)`
pkg/apis/aws/v1alpha1/accountclaim_types.go	`8.33% <0.00%> (ø)`
pkg/apis/scheme/scheme.go	`63.63% <0.00%> (ø)`

cblecker commented 3 years ago

/label tide/merge-method-squash

iamkirkbater commented 3 years ago

/lgtm

openshift-ci[bot] commented 3 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ArielLima, iamkirkbater

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/openshift/aws-account-operator/blob/master/OWNERS)~~ [iamkirkbater] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment