openshift / aws-account-operator

Operator to manage pool of AWS accounts for Hive
Apache License 2.0
32 stars 75 forks source link

[OSD-7460] check against pre-defined account states #602

Closed dastergon closed 3 years ago

dastergon commented 3 years ago

Instead of checking for a wide range of states in Account Pool controller, I use an already existing func that checks against pre-defined account states and tells us whether an Account operation has failed or not, depending on the state. That way we avoid the case of blocking the controller because a random state doesn't conform with Account operations.

PTAL @iamkirkbater @rogbas @lisa

See https://issues.redhat.com/browse/OSD-7460

codecov-commenter commented 3 years ago

Codecov Report

Merging #602 (825f315) into master (44493ed) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #602   +/-   ##
=======================================
  Coverage   13.16%   13.16%           
=======================================
  Files          45       45           
  Lines        3912     3912           
=======================================
  Hits          515      515           
  Misses       3374     3374           
  Partials       23       23           
Impacted Files Coverage Δ
...g/controller/accountpool/accountpool_controller.go 43.07% <100.00%> (ø)
lisa commented 3 years ago

@dastergon Your PR description reads like code, which is basically the same as looking at the code diff. What would be more helpful to reviewers here is to summarize in human language what we're hoping to achieve and how your PR addresses it. In other words, be a step higher than code changes.

lisa commented 3 years ago

/lgtm /approve

openshift-ci[bot] commented 3 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dastergon, drpaneas, lisa

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)~~ [lisa] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment